From bded790f4651459b2ac41d9f96f2393204b41012 Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Tue, 11 Mar 2014 15:36:46 -0700 Subject: [PATCH] User.impersonate() now does the loadUserDetailsByName call. So ApiTokenFilter no longer needs to do that. --- core/src/main/java/hudson/model/User.java | 11 ++++-- .../java/jenkins/security/ApiTokenFilter.java | 35 ++++++++----------- 2 files changed, 24 insertions(+), 22 deletions(-) diff --git a/core/src/main/java/hudson/model/User.java b/core/src/main/java/hudson/model/User.java index 7cd3cc635e..257b8cd1ec 100644 --- a/core/src/main/java/hudson/model/User.java +++ b/core/src/main/java/hudson/model/User.java @@ -250,10 +250,17 @@ public class User extends AbstractModelObject implements AccessControlled, Descr /** * Creates an {@link Authentication} object that represents this user. - * + * + * This method checks with {@link SecurityRealm} if the user is a valid user that can login to the security realm. + * If {@link SecurityRealm} is a kind that does not support querying information about other users, this will + * use {@link LastGrantedAuthoritiesProperty} to pick up the granted authorities as of the last time the user has + * logged in. + * + * @throws UsernameNotFoundException + * If this user is not a valid user in the backend {@link SecurityRealm}. * @since 1.419 */ - public Authentication impersonate() { + public Authentication impersonate() throws UsernameNotFoundException { try { UserDetails u = Jenkins.getInstance().getSecurityRealm().loadUserByUsername(id); return new UsernamePasswordAuthenticationToken(u.getUsername(), "", u.getAuthorities()); diff --git a/core/src/main/java/jenkins/security/ApiTokenFilter.java b/core/src/main/java/jenkins/security/ApiTokenFilter.java index b1078e6240..88eb248a1c 100644 --- a/core/src/main/java/jenkins/security/ApiTokenFilter.java +++ b/core/src/main/java/jenkins/security/ApiTokenFilter.java @@ -2,9 +2,7 @@ package jenkins.security; import hudson.model.User; import hudson.security.ACL; -import hudson.security.UserMayOrMayNotExistException; import hudson.util.Scrambler; -import jenkins.model.Jenkins; import org.acegisecurity.context.SecurityContext; import org.acegisecurity.context.SecurityContextHolder; import org.acegisecurity.userdetails.UsernameNotFoundException; @@ -45,32 +43,29 @@ public class ApiTokenFilter implements Filter { int idx = uidpassword.indexOf(':'); if (idx >= 0) { String username = uidpassword.substring(0, idx); - try { - Jenkins.getInstance().getSecurityRealm().loadUserByUsername(username); - } catch (UserMayOrMayNotExistException x) { - // OK, give them the benefit of the doubt. - } catch (UsernameNotFoundException x) { - // Not/no longer a user; deny the API token. (But do not leak the information that this happened.) - chain.doFilter(request, response); - return; - } catch (DataAccessException x) { - throw new ServletException(x); - } String password = uidpassword.substring(idx+1); // attempt to authenticate as API token User u = User.get(username); ApiTokenProperty t = u.getProperty(ApiTokenProperty.class); if (t!=null && t.matchesPassword(password)) { - // even if we fail to match the password, we aren't rejecting it. - // as the user might be passing in a real password. - SecurityContext oldContext = ACL.impersonate(u.impersonate()); try { - request.setAttribute(ApiTokenProperty.class.getName(), u); - chain.doFilter(request,response); + // even if we fail to match the password, we aren't rejecting it. + // as the user might be passing in a real password. + SecurityContext oldContext = ACL.impersonate(u.impersonate()); + try { + request.setAttribute(ApiTokenProperty.class.getName(), u); + chain.doFilter(request,response); + return; + } finally { + SecurityContextHolder.setContext(oldContext); + } + } catch (UsernameNotFoundException x) { + // Not/no longer a user; deny the API token. (But do not leak the information that this happened.) + chain.doFilter(request, response); return; - } finally { - SecurityContextHolder.setContext(oldContext); + } catch (DataAccessException x) { + throw new ServletException(x); } } } -- GitLab