Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[JENKINS-27027] Notify the SecurityListener on authentication (#3074)
* [JENKINS-27026] Notify the SecurityListener in case of Token based authentication success
- due the current version of the method, the UserDetails required for the event was not accessible. In order to stay with the same API in SecurityListener, two "protected" methods were created to split the job and let the UserDetails accessible

* - add test to ensure the SecurityListener is called for REST Token but also for regular basic auth

* - remove the comment about the split, will be put in GitHub comment instead

* - add check for anonymous call instead of just putting a comment
- remove the constructor in the dummy
- add link to PR from Daniel to simplify a call

* - separate the before/after to save one clear and be more explicit
- put more meaning in the assertLastEventIs method by explicitly say we will remove the last event

* - add comment about why we do not fire the "failedToAuthenticated" in the case of an invalid token (tips: it's because it could be a valid password)

* - also add the authenticated trigger on legacy filter as pointed by Ivan

* - add support of event on CLI remoting authentication
- adjust tests by moving the helper class used to spy on events

* - as mentioned Yvan, the code had some problems with null checking, so the approach is changed in order to encapsulate all that internal mechanism

* - add javadoc
- open the getUserDetailsForImpersonation from the User (will let the SSHD module to retrieve UserDetails from that)

* - remove single quote in log messages

* - basic corrections requested by Jesse

* - just another typo

* - adjust the javadoc for SecurityListener events

* - add the link to Jenkins#Anonymous

* - add link (not using see)

* - update comment on the isAnonymous as we (me + Oleg) do not find a best place at the moment

* - put the new method isAnonymous in ACL instead of Functions

* - little typo
- add requirement about the SecurityContext authentication
  • Loading branch information
Wadeck authored and oleg-nenashev committed Dec 14, 2017
1 parent a82a47a commit b7f42b2
Show file tree
Hide file tree
Showing 13 changed files with 403 additions and 40 deletions.
9 changes: 3 additions & 6 deletions core/src/main/java/hudson/Functions.java
Expand Up @@ -26,6 +26,7 @@
package hudson;

import hudson.model.Slave;
import hudson.security.*;
import jenkins.util.SystemProperties;
import hudson.cli.CLICommand;
import hudson.console.ConsoleAnnotationDescriptor;
Expand Down Expand Up @@ -56,11 +57,6 @@
import hudson.scm.SCM;
import hudson.scm.SCMDescriptor;
import hudson.search.SearchableModelObject;
import hudson.security.AccessControlled;
import hudson.security.AuthorizationStrategy;
import hudson.security.GlobalSecurityConfiguration;
import hudson.security.Permission;
import hudson.security.SecurityRealm;
import hudson.security.captcha.CaptchaSupport;
import hudson.security.csrf.CrumbIssuer;
import hudson.slaves.Cloud;
Expand Down Expand Up @@ -137,6 +133,7 @@
import jenkins.model.ModelObjectWithChildren;
import jenkins.model.ModelObjectWithContextMenu;

import org.acegisecurity.Authentication;
import org.acegisecurity.providers.anonymous.AnonymousAuthenticationToken;
import org.apache.commons.jelly.JellyContext;
import org.apache.commons.jelly.JellyTagException;
Expand Down Expand Up @@ -1582,7 +1579,7 @@ public static String toCCStatus(Item i) {
* Checks if the current user is anonymous.
*/
public static boolean isAnonymous() {
return Jenkins.getAuthentication() instanceof AnonymousAuthenticationToken;
return ACL.isAnonymous(Jenkins.getAuthentication());
}

/**
Expand Down
28 changes: 26 additions & 2 deletions core/src/main/java/hudson/cli/CLICommand.java
Expand Up @@ -30,6 +30,8 @@
import hudson.cli.declarative.CLIMethod;
import hudson.ExtensionPoint.LegacyInstancesAreScopedToHudson;
import hudson.Functions;
import hudson.security.ACL;
import jenkins.security.SecurityListener;
import jenkins.util.SystemProperties;
import hudson.cli.declarative.OptionHandlerExtension;
import jenkins.model.Jenkins;
Expand All @@ -44,6 +46,8 @@
import org.acegisecurity.BadCredentialsException;
import org.acegisecurity.context.SecurityContext;
import org.acegisecurity.context.SecurityContextHolder;
import org.acegisecurity.userdetails.User;
import org.acegisecurity.userdetails.UserDetails;
import org.apache.commons.discovery.ResourceClassIterator;
import org.apache.commons.discovery.ResourceNameIterator;
import org.apache.commons.discovery.resource.ClassLoaders;
Expand Down Expand Up @@ -344,8 +348,16 @@ public Channel checkChannel() throws AbortException {
@Deprecated
protected Authentication loadStoredAuthentication() throws InterruptedException {
try {
if (channel!=null)
return new ClientAuthenticationCache(channel).get();
if (channel!=null){
Authentication authLoadedFromCache = new ClientAuthenticationCache(channel).get();

if(!ACL.isAnonymous(authLoadedFromCache)){
UserDetails userDetails = new CLIUserDetails(authLoadedFromCache);
SecurityListener.fireAuthenticated(userDetails);
}

return authLoadedFromCache;
}
} catch (IOException e) {
stderr.println("Failed to access the stored credential");
Functions.printStackTrace(e, stderr); // recover
Expand Down Expand Up @@ -642,4 +654,16 @@ public static CLICommand getCurrent() {
}
}
}

/**
* User details loaded from the CLI {@link ClientAuthenticationCache}
* The user is never anonymous since it must be authenticated to be stored in the cache
*/
@Deprecated
@Restricted(NoExternalUse.class)
private static class CLIUserDetails extends User {
private CLIUserDetails(Authentication auth) {
super(auth.getName(), "", true, true, true, true, auth.getAuthorities());
}
}
}
5 changes: 4 additions & 1 deletion core/src/main/java/hudson/cli/ClientAuthenticationCache.java
Expand Up @@ -22,6 +22,8 @@
import java.util.logging.Logger;
import jenkins.security.HMACConfidentialKey;

import javax.annotation.Nonnull;

/**
* Represents the authentication credential store of the CLI client.
*
Expand Down Expand Up @@ -73,7 +75,7 @@ public FilePath call() throws IOException {
*
* @return {@link jenkins.model.Jenkins#ANONYMOUS} if no such credential is found, or if the stored credential is invalid.
*/
public Authentication get() {
public @Nonnull Authentication get() {
Jenkins h = Jenkins.getActiveInstance();
String val = props.getProperty(getPropertyKey());
if (val == null) {
Expand All @@ -100,6 +102,7 @@ public Authentication get() {
LOGGER.log(Level.FINER, "Loaded stored CLI authentication for {0}", username);
return new UsernamePasswordAuthenticationToken(u.getUsername(), "", u.getAuthorities());
} catch (AuthenticationException | DataAccessException e) {
//TODO there is no check to be consistent with User.ALLOW_NON_EXISTENT_USER_TO_LOGIN
LOGGER.log(Level.FINE, "Stored CLI authentication did not correspond to a valid user: " + username, e);
return Jenkins.ANONYMOUS;
}
Expand Down
3 changes: 3 additions & 0 deletions core/src/main/java/hudson/cli/LoginCommand.java
Expand Up @@ -3,6 +3,7 @@
import hudson.Extension;
import java.io.PrintStream;
import jenkins.model.Jenkins;
import jenkins.security.SecurityListener;
import org.acegisecurity.Authentication;
import org.kohsuke.args4j.CmdLineException;

Expand Down Expand Up @@ -45,6 +46,8 @@ protected int run() throws Exception {
ClientAuthenticationCache store = new ClientAuthenticationCache(checkChannel());
store.set(a);

SecurityListener.fireLoggedIn(a.getName());

return 0;
}

Expand Down
9 changes: 9 additions & 0 deletions core/src/main/java/hudson/cli/LogoutCommand.java
@@ -1,6 +1,9 @@
package hudson.cli;

import hudson.Extension;
import jenkins.security.SecurityListener;
import org.acegisecurity.Authentication;

import java.io.PrintStream;

/**
Expand All @@ -27,7 +30,13 @@ protected void printUsageSummary(PrintStream stderr) {
@Override
protected int run() throws Exception {
ClientAuthenticationCache store = new ClientAuthenticationCache(checkChannel());

Authentication auth = store.get();

store.remove();

SecurityListener.fireLoggedOut(auth.getName());

return 0;
}
}
61 changes: 54 additions & 7 deletions core/src/main/java/hudson/model/User.java
Expand Up @@ -326,23 +326,70 @@ public <T extends UserProperty> T getProperty(Class<T> clazz) {
* @since 1.419
*/
public @Nonnull Authentication impersonate() throws UsernameNotFoundException {
return this.impersonate(this.getUserDetailsForImpersonation());
}

/**
* 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.
*
* @return userDetails for the user, in case he's not found but seems legitimate, we provide a userDetails with minimum access
*
* @throws UsernameNotFoundException
* If this user is not a valid user in the backend {@link SecurityRealm}.
*/
public @Nonnull UserDetails getUserDetailsForImpersonation() throws UsernameNotFoundException {
ImpersonatingUserDetailsService userDetailsService = new ImpersonatingUserDetailsService(
Jenkins.getInstance().getSecurityRealm().getSecurityComponents().userDetails
);

try {
UserDetails u = new ImpersonatingUserDetailsService(
Jenkins.getInstance().getSecurityRealm().getSecurityComponents().userDetails).loadUserByUsername(id);
return new UsernamePasswordAuthenticationToken(u.getUsername(), "", u.getAuthorities());
UserDetails userDetails = userDetailsService.loadUserByUsername(id);
LOGGER.log(Level.FINE, "Impersonation of the user {0} was a success", new Object[]{ id });
return userDetails;
} catch (UserMayOrMayNotExistException e) {
LOGGER.log(Level.FINE, "The user {0} may or may not exist in the SecurityRealm, so we provide minimum access", new Object[]{ id });
// backend can't load information about other users. so use the stored information if available
} catch (UsernameNotFoundException e) {
// if the user no longer exists in the backend, we need to refuse impersonating this user
if (!ALLOW_NON_EXISTENT_USER_TO_LOGIN)
if(ALLOW_NON_EXISTENT_USER_TO_LOGIN){
LOGGER.log(Level.FINE, "The user {0} was not found in the SecurityRealm but we are required to let it pass, due to ALLOW_NON_EXISTENT_USER_TO_LOGIN", new Object[]{ id });
}else{
LOGGER.log(Level.FINE, "The user {0} was not found in the SecurityRealm", new Object[]{ id });
throw e;
}
} catch (DataAccessException e) {
// seems like it's in the same boat as UserMayOrMayNotExistException
LOGGER.log(Level.FINE, "The user {0} retrieval just threw a DataAccess exception with msg = {1}, so we provide minimum access", new Object[]{ id, e.getMessage() });
}

return new LegitimateButUnknownUserDetails(id);
}

/**
* Only used for a legitimate user we have no idea about. We give it only minimum access
*/
private static class LegitimateButUnknownUserDetails extends org.acegisecurity.userdetails.User{
private LegitimateButUnknownUserDetails(String username) throws IllegalArgumentException {
super(
username, "",
true, true, true, true,
new GrantedAuthority[]{SecurityRealm.AUTHENTICATED_AUTHORITY}
);
}
}

// seems like a legitimate user we have no idea about. proceed with minimum access
return new UsernamePasswordAuthenticationToken(id, "",
new GrantedAuthority[]{SecurityRealm.AUTHENTICATED_AUTHORITY});
/**
* Creates an {@link Authentication} object that represents this user using the given userDetails
*
* @param userDetails Provided by {@link #getUserDetailsForImpersonation()}.
* @see #getUserDetailsForImpersonation()
*/
@Restricted(NoExternalUse.class)
public @Nonnull Authentication impersonate(@Nonnull UserDetails userDetails) {
return new UsernamePasswordAuthenticationToken(userDetails.getUsername(), "", userDetails.getAuthorities());
}

/**
Expand Down
10 changes: 10 additions & 0 deletions core/src/main/java/hudson/security/ACL.java
Expand Up @@ -44,6 +44,7 @@
import org.acegisecurity.providers.UsernamePasswordAuthenticationToken;
import org.acegisecurity.acls.sid.PrincipalSid;
import org.acegisecurity.acls.sid.Sid;
import org.acegisecurity.providers.anonymous.AnonymousAuthenticationToken;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;

Expand Down Expand Up @@ -319,4 +320,13 @@ public static ACLContext as(@CheckForNull User user) {
return as(user == null ? Jenkins.ANONYMOUS : user.impersonate());
}

/**
* Checks if the given authentication is anonymous by checking its class.
* @see Jenkins#ANONYMOUS
* @see AnonymousAuthenticationToken
*/
public static boolean isAnonymous(@Nonnull Authentication authentication) {
//TODO use AuthenticationTrustResolver instead to be consistent through the application
return authentication instanceof AnonymousAuthenticationToken;
}
}
Expand Up @@ -27,7 +27,10 @@
import jenkins.model.Jenkins;
import hudson.util.Scrambler;
import jenkins.security.ApiTokenProperty;
import jenkins.security.SecurityListener;
import org.acegisecurity.Authentication;
import org.acegisecurity.context.SecurityContextHolder;
import org.acegisecurity.userdetails.UserDetails;

import javax.servlet.Filter;
import javax.servlet.FilterChain;
Expand Down Expand Up @@ -138,7 +141,12 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha
User u = User.getById(username, true);
ApiTokenProperty t = u.getProperty(ApiTokenProperty.class);
if (t!=null && t.matchesPassword(password)) {
SecurityContextHolder.getContext().setAuthentication(u.impersonate());
UserDetails userDetails = u.getUserDetailsForImpersonation();
Authentication auth = u.impersonate(userDetails);

SecurityListener.fireAuthenticated(userDetails);

SecurityContextHolder.getContext().setAuthentication(auth);
try {
chain.doFilter(request,response);
} finally {
Expand Down
Expand Up @@ -3,6 +3,7 @@
import hudson.Extension;
import hudson.model.User;
import org.acegisecurity.Authentication;
import org.acegisecurity.userdetails.UserDetails;
import org.acegisecurity.userdetails.UsernameNotFoundException;
import org.springframework.dao.DataAccessException;

Expand All @@ -21,14 +22,23 @@
*/
@Extension
public class BasicHeaderApiTokenAuthenticator extends BasicHeaderAuthenticator {
/**
* Note: if the token does not exist or does not match, we do not use {@link SecurityListener#fireFailedToAuthenticate(String)}
* because it will be done in the {@link BasicHeaderRealPasswordAuthenticator} in the case the password is not valid either
*/
@Override
public Authentication authenticate(HttpServletRequest req, HttpServletResponse rsp, String username, String password) throws ServletException {
// attempt to authenticate as API token
User u = User.getById(username, true);
ApiTokenProperty t = u.getProperty(ApiTokenProperty.class);
if (t!=null && t.matchesPassword(password)) {
try {
return u.impersonate();
UserDetails userDetails = u.getUserDetailsForImpersonation();
Authentication auth = u.impersonate(userDetails);

SecurityListener.fireAuthenticated(userDetails);

return auth;
} catch (UsernameNotFoundException x) {
// The token was valid, but the impersonation failed. This token is clearly not his real password,
// so there's no point in continuing the request processing. Report this error and abort.
Expand Down
17 changes: 10 additions & 7 deletions core/src/main/java/jenkins/security/SecurityListener.java
Expand Up @@ -45,29 +45,32 @@ public abstract class SecurityListener implements ExtensionPoint {
private static final Logger LOGGER = Logger.getLogger(SecurityListener.class.getName());

/**
* Fired when a user was successfully authenticated by password.
* This might be via the web UI, or via REST (not with an API token) or CLI (not with an SSH key).
* Only {@link AbstractPasswordBasedSecurityRealm}s are considered.
* @param details details of the newly authenticated user, such as name and groups
* Fired when a user was successfully authenticated using credentials. It could be password or any other credentials.
* This might be via the web UI, or via REST (using API token or Basic), or CLI (remoting, auth, ssh)
* or any other way plugins can propose.
* @param details details of the newly authenticated user, such as name and groups.
*/
protected void authenticated(@Nonnull UserDetails details){}

/**
* Fired when a user tried to authenticate by password but failed.
* Fired when a user tried to authenticate but failed.
* In case the authentication method uses multiple layers to validate the credentials,
* we do fire this event only when even the last layer failed to authenticate.
* @param username the user
* @see #authenticated
*/
protected void failedToAuthenticate(@Nonnull String username){}

/**
* Fired when a user has logged in via the web UI.
* Fired when a user has logged in. Compared to authenticated, there is a notion of storage / cache.
* Would be called after {@link #authenticated}.
* It should be called after the {@link org.acegisecurity.context.SecurityContextHolder#getContext()}'s authentication is set.
* @param username the user
*/
protected void loggedIn(@Nonnull String username){}

/**
* Fired when a user has failed to log in via the web UI.
* Fired when a user has failed to log in.
* Would be called after {@link #failedToAuthenticate}.
* @param username the user
*/
Expand Down

0 comments on commit b7f42b2

Please sign in to comment.