diff --git a/changelog.html b/changelog.html index a4e3424559a3e90c738d90d92d782c8416e120e5..83dedd98c997e7549f5bb02c8905a41000b7f30a 100644 --- a/changelog.html +++ b/changelog.html @@ -55,6 +55,9 @@ Upcoming changes
,R extends A public FormValidation doCheckLabel(@AncestorInPath AbstractProject,?> project, @QueryParameter String value) { + return validateLabelExpression(value, project); + } + + /** + * Validate label expression string. + * + * @param project May be specified to perform project specific validation. + * @since 1.590 + */ + public static @Nonnull FormValidation validateLabelExpression(String value, @CheckForNull AbstractProject, ?> project) { if (Util.fixEmpty(value)==null) return FormValidation.ok(); // nothing typed yet try { diff --git a/core/src/main/java/jenkins/security/BasicHeaderProcessor.java b/core/src/main/java/jenkins/security/BasicHeaderProcessor.java index 60d217a67e42ff575e9e3340a2c877949152dd62..6bec1a2e4a2a660565113aaf7d54b77dc13f333a 100644 --- a/core/src/main/java/jenkins/security/BasicHeaderProcessor.java +++ b/core/src/main/java/jenkins/security/BasicHeaderProcessor.java @@ -1,12 +1,14 @@ package jenkins.security; import hudson.security.ACL; +import hudson.security.SecurityRealm; import hudson.util.Scrambler; import org.acegisecurity.Authentication; -import org.acegisecurity.AuthenticationManager; import org.acegisecurity.BadCredentialsException; import org.acegisecurity.context.SecurityContext; import org.acegisecurity.context.SecurityContextHolder; +import org.acegisecurity.providers.UsernamePasswordAuthenticationToken; +import org.acegisecurity.providers.anonymous.AnonymousAuthenticationToken; import org.acegisecurity.ui.AuthenticationEntryPoint; import org.acegisecurity.ui.rememberme.NullRememberMeServices; import org.acegisecurity.ui.rememberme.RememberMeServices; @@ -67,6 +69,11 @@ public class BasicHeaderProcessor implements Filter { String username = uidpassword.substring(0, idx); String password = uidpassword.substring(idx+1); + if (!authenticationIsRequired(username)) { + chain.doFilter(request, response); + return; + } + for (BasicHeaderAuthenticator a : all()) { LOGGER.log(FINER, "Attempting to authenticate with {0}", a); Authentication auth = a.authenticate(req, rsp, username, password); @@ -87,6 +94,44 @@ public class BasicHeaderProcessor implements Filter { } } + /** + * If the request is already authenticated to the same user that the Authorization header claims, + * for example through the HTTP session, then there's no need to re-authenticate the Authorization header, + * so we skip that. This avoids stressing {@link SecurityRealm}. + * + * This method returns false if we can take this short-cut. + */ + // taken from BasicProcessingFilter.java + protected boolean authenticationIsRequired(String username) { + // Only reauthenticate if username doesn't match SecurityContextHolder and user isn't authenticated + // (see SEC-53) + Authentication existingAuth = SecurityContextHolder.getContext().getAuthentication(); + + if(existingAuth == null || !existingAuth.isAuthenticated()) { + return true; + } + + // Limit username comparison to providers which use usernames (ie UsernamePasswordAuthenticationToken) + // (see SEC-348) + + if (existingAuth instanceof UsernamePasswordAuthenticationToken && !existingAuth.getName().equals(username)) { + return true; + } + + // Handle unusual condition where an AnonymousAuthenticationToken is already present + // This shouldn't happen very often, as BasicProcessingFitler is meant to be earlier in the filter + // chain than AnonymousProcessingFilter. Nevertheless, presence of both an AnonymousAuthenticationToken + // together with a BASIC authentication request header should indicate reauthentication using the + // BASIC protocol is desirable. This behaviour is also consistent with that provided by form and digest, + // both of which force re-authentication if the respective header is detected (and in doing so replace + // any existing AnonymousAuthenticationToken). See SEC-610. + if (existingAuth instanceof AnonymousAuthenticationToken) { + return true; + } + + return false; + } + protected void success(HttpServletRequest req, HttpServletResponse rsp, FilterChain chain, Authentication auth) throws IOException, ServletException { rememberMeServices.loginSuccess(req, rsp, auth); diff --git a/core/src/main/java/jenkins/security/BasicHeaderRealPasswordAuthenticator.java b/core/src/main/java/jenkins/security/BasicHeaderRealPasswordAuthenticator.java index 4d3e5554e06ca577f9692b194ea1e20d7e15e6f0..3b41c006ddcc9f89cac44f042ca425bdc523c011 100644 --- a/core/src/main/java/jenkins/security/BasicHeaderRealPasswordAuthenticator.java +++ b/core/src/main/java/jenkins/security/BasicHeaderRealPasswordAuthenticator.java @@ -19,9 +19,7 @@ import jenkins.ExtensionFilter; import jenkins.model.Jenkins; import org.acegisecurity.Authentication; import org.acegisecurity.AuthenticationException; -import org.acegisecurity.context.SecurityContextHolder; import org.acegisecurity.providers.UsernamePasswordAuthenticationToken; -import org.acegisecurity.providers.anonymous.AnonymousAuthenticationToken; import org.acegisecurity.ui.AuthenticationDetailsSource; import org.acegisecurity.ui.AuthenticationDetailsSourceImpl; @@ -49,9 +47,6 @@ public class BasicHeaderRealPasswordAuthenticator extends BasicHeaderAuthenticat if (DISABLE) return null; - if (!authenticationIsRequired(username)) - return null; - UsernamePasswordAuthenticationToken authRequest = new UsernamePasswordAuthenticationToken(username, password); authRequest.setDetails(authenticationDetailsSource.buildDetails(req)); @@ -68,37 +63,6 @@ public class BasicHeaderRealPasswordAuthenticator extends BasicHeaderAuthenticat } } - // taken from BasicProcessingFilter.java - protected boolean authenticationIsRequired(String username) { - // Only reauthenticate if username doesn't match SecurityContextHolder and user isn't authenticated - // (see SEC-53) - Authentication existingAuth = SecurityContextHolder.getContext().getAuthentication(); - - if(existingAuth == null || !existingAuth.isAuthenticated()) { - return true; - } - - // Limit username comparison to providers which use usernames (ie UsernamePasswordAuthenticationToken) - // (see SEC-348) - - if (existingAuth instanceof UsernamePasswordAuthenticationToken && !existingAuth.getName().equals(username)) { - return true; - } - - // Handle unusual condition where an AnonymousAuthenticationToken is already present - // This shouldn't happen very often, as BasicProcessingFitler is meant to be earlier in the filter - // chain than AnonymousProcessingFilter. Nevertheless, presence of both an AnonymousAuthenticationToken - // together with a BASIC authentication request header should indicate reauthentication using the - // BASIC protocol is desirable. This behaviour is also consistent with that provided by form and digest, - // both of which force re-authentication if the respective header is detected (and in doing so replace - // any existing AnonymousAuthenticationToken). See SEC-610. - if (existingAuth instanceof AnonymousAuthenticationToken) { - return true; - } - - return false; - } - private static final Logger LOGGER = Logger.getLogger(BasicHeaderRealPasswordAuthenticator.class.getName()); /** diff --git a/test/src/test/java/jenkins/security/BasicHeaderProcessorTest.java b/test/src/test/java/jenkins/security/BasicHeaderProcessorTest.java index 803767f9a43fd24ebe8def35fd0d5555dc56de90..c11726fd9327acf3c3eed424c422e449930b04a9 100644 --- a/test/src/test/java/jenkins/security/BasicHeaderProcessorTest.java +++ b/test/src/test/java/jenkins/security/BasicHeaderProcessorTest.java @@ -56,10 +56,15 @@ public class BasicHeaderProcessorTest extends Assert { // call with incorrect password makeRequestAndFail("foo:bar"); - // if the session cookie is valid, then basic header won't be needed + wc.login("bar"); + + // if the session cookie is valid, then basic header won't be needed makeRequestWithAuthAndVerify(null, "bar"); + // if the session cookie is valid, and basic header is set anyway login should not fail either + makeRequestWithAuthAndVerify("bar:bar", "bar"); + // but if the password is incorrect, it should fail, instead of silently logging in as the user indicated by session makeRequestAndFail("foo:bar"); }