提交 9e81b8e4 编写于 作者: K Kohsuke Kawaguchi

[FIXED JENKINS-25144] Revisiting the attempted fix in the previous commit.

IIUC, the issue here is that the request in question contains both a
valid session cookie AND basic authentication header, and that path
results in a failure because BasicHeaderProcessor expects one of
BasicHeaderAuthenticators to validate the basic authentication header
(without knowing that there's already a valid Authentication object that
came from the HTTP session, yet no BasicHeaderAuthenticator actually
processes this because BasicHeaderRealPasswordAuthenticator backs away
from doing that.

I think the corect fix is for BasicHeaderRealPasswordAuthenticator to
get rid of authenticationIsRequired check. This check instead belongs to
BasicHeaderProcessor, where it should be used to check if any
BasicHeaderAuthenticator should be consulted or not.

The problem with having this logic in
BasicHeaderRealPasswordAuthenticator is that this is just an
implementation of an extension point, and thus it needs to be removable.
As it stands right now in this fix, if this impl is removed,
JENKINS-25144 will be back again.
上级 0176b6d9
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);
......
......@@ -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 SecurityContextHolder.getContext().getAuthentication();
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());
/**
......
Markdown is supported
0% .
You are about to add 0 people to the discussion. Proceed with caution.
先完成此消息的编辑!
想要评论请 注册