Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Jenkins now remembers the authorities (read group memberships) that the user had carried when he/she last time interactively logged in. This information is exposed via User.impersonate(), which is used when using Jenkins SSH, Jenkins CLI, or access via API tokens. Previously this was impossible for a subset of SecurityRealms that does not allow us to read group membership information without successful login (such as Active Directory, OpenID, etc.) For security reasons, if the backend determines that the user does not exist (as opposed to the backend who cannot tell if the user exists or not), then the impersonation will fail. I need to check AD plugin is reporting a failure correctly in this case, before marking as JENKINS-20064 fixed.
- Loading branch information
Showing
4 changed files
with
238 additions
and
6 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
147 changes: 147 additions & 0 deletions
147
core/src/main/java/jenkins/security/LastGrantedAuthoritiesProperty.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,147 @@ | ||
package jenkins.security; | ||
|
||
import hudson.Extension; | ||
import hudson.model.Descriptor.FormException; | ||
import hudson.model.User; | ||
import hudson.model.UserProperty; | ||
import hudson.security.SecurityRealm; | ||
import jenkins.model.Jenkins; | ||
import net.sf.json.JSONObject; | ||
import org.acegisecurity.Authentication; | ||
import org.acegisecurity.GrantedAuthority; | ||
import org.acegisecurity.GrantedAuthorityImpl; | ||
import org.acegisecurity.userdetails.UserDetails; | ||
import org.kohsuke.stapler.StaplerRequest; | ||
|
||
import javax.annotation.Nonnull; | ||
import java.io.IOException; | ||
import java.util.ArrayList; | ||
import java.util.Arrays; | ||
import java.util.Collections; | ||
import java.util.List; | ||
import java.util.logging.Level; | ||
import java.util.logging.Logger; | ||
|
||
/** | ||
* Remembers the set of {@link GrantedAuthority}s that was obtained the last time the user has logged in. | ||
* | ||
* This allows us to implement {@link User#impersonate()} with proper set of groups. | ||
* | ||
* @author Kohsuke Kawaguchi | ||
* @since 1.556 | ||
*/ | ||
public class LastGrantedAuthoritiesProperty extends UserProperty { | ||
private volatile String[] roles; | ||
private long timestamp; | ||
|
||
/** | ||
* Stick to the same object since there's no UI for this. | ||
*/ | ||
@Override | ||
public UserProperty reconfigure(StaplerRequest req, JSONObject form) throws FormException { | ||
req.bindJSON(this, form); | ||
return this; | ||
} | ||
|
||
public GrantedAuthority[] getAuthorities() { | ||
String[] roles = this.roles; // capture to a variable for immutability | ||
|
||
GrantedAuthority[] r = new GrantedAuthority[roles==null ? 1 : roles.length+1]; | ||
r[0] = SecurityRealm.AUTHENTICATED_AUTHORITY; | ||
for (int i = 1; i < r.length; i++) { | ||
r[i] = new GrantedAuthorityImpl(roles[i-1]); | ||
} | ||
return r; | ||
} | ||
|
||
/** | ||
* Persist the information with the new {@link UserDetails}. | ||
*/ | ||
public void update(Authentication auth) throws IOException { | ||
List<String> roles = new ArrayList<String>(); | ||
for (GrantedAuthority ga : auth.getAuthorities()) { | ||
roles.add(ga.getAuthority()); | ||
} | ||
String[] a = roles.toArray(new String[roles.size()]); | ||
if (!Arrays.equals(this.roles,a)) { | ||
this.roles = a; | ||
this.timestamp = System.currentTimeMillis(); | ||
user.save(); | ||
} | ||
} | ||
|
||
/** | ||
* Removes the recorded information | ||
*/ | ||
public void invalidate() throws IOException { | ||
if (roles!=null) { | ||
roles = null; | ||
timestamp = System.currentTimeMillis(); | ||
user.save(); | ||
} | ||
} | ||
|
||
/** | ||
* Listen to the login success/failure event to persist {@link GrantedAuthority}s properly. | ||
*/ | ||
@Extension | ||
public static class SecurityListenerImpl extends SecurityListener { | ||
@Override | ||
protected void authenticated(@Nonnull UserDetails details) { | ||
} | ||
|
||
@Override | ||
protected void failedToAuthenticate(@Nonnull String username) { | ||
} | ||
|
||
@Override | ||
protected void loggedIn(@Nonnull String username) { | ||
try { | ||
User u = User.get(username); | ||
LastGrantedAuthoritiesProperty o = u.getProperty(LastGrantedAuthoritiesProperty.class); | ||
if (o==null) | ||
u.addProperty(o=new LastGrantedAuthoritiesProperty()); | ||
Authentication a = Jenkins.getAuthentication(); | ||
if (a!=null && a.getName().equals(username)) | ||
o.update(a); // just for defensive sanity checking | ||
} catch (IOException e) { | ||
LOGGER.log(Level.WARNING, "Failed to record granted authorities",e); | ||
} | ||
} | ||
|
||
@Override | ||
protected void failedToLogIn(@Nonnull String username) { | ||
// while this initially seemed like a good idea to avoid allowing wrong impersonation for too long, | ||
// doing this means a malicious user can break the impersonation capability | ||
// just by failing to login. See ApiTokenFilter that does the following, which seems better: | ||
/* | ||
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); | ||
} | ||
*/ | ||
|
||
// try { | ||
// User u = User.get(username,false,Collections.emptyMap()); | ||
// LastGrantedAuthoritiesProperty o = u.getProperty(LastGrantedAuthoritiesProperty.class); | ||
// if (o!=null) | ||
// o.invalidate(); | ||
// } catch (IOException e) { | ||
// LOGGER.log(Level.WARNING, "Failed to record granted authorities",e); | ||
// } | ||
} | ||
|
||
@Override | ||
protected void loggedOut(@Nonnull String username) { | ||
} | ||
} | ||
|
||
private static final Logger LOGGER = Logger.getLogger(LastGrantedAuthoritiesProperty.class.getName()); | ||
} |
74 changes: 74 additions & 0 deletions
74
test/src/test/java/jenkins/security/LastGrantedAuthoritiesPropertyTest.groovy
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,74 @@ | ||
package jenkins.security | ||
|
||
import hudson.security.AbstractPasswordBasedSecurityRealm | ||
import hudson.security.GroupDetails | ||
import hudson.security.UserMayOrMayNotExistException | ||
import org.acegisecurity.AuthenticationException | ||
import org.acegisecurity.BadCredentialsException | ||
import org.acegisecurity.GrantedAuthority | ||
import org.acegisecurity.GrantedAuthorityImpl | ||
import org.acegisecurity.userdetails.User | ||
import org.acegisecurity.userdetails.UserDetails | ||
import org.acegisecurity.userdetails.UsernameNotFoundException | ||
import org.junit.Rule | ||
import org.junit.Test | ||
import org.jvnet.hudson.test.JenkinsRule | ||
import org.springframework.dao.DataAccessException | ||
|
||
/** | ||
* | ||
* | ||
* @author Kohsuke Kawaguchi | ||
*/ | ||
class LastGrantedAuthoritiesPropertyTest { | ||
@Rule | ||
public JenkinsRule j = new JenkinsRule(); | ||
|
||
@Test | ||
public void basicflow() { | ||
j.jenkins.securityRealm = new TestSecurityRealm() | ||
|
||
// login, and make sure it leaves the LastGrantedAuthoritiesProperty object | ||
def wc = j.createWebClient() | ||
wc.login("alice","alice:development:us") | ||
|
||
def u = hudson.model.User.get("alice") | ||
def p = u.getProperty(LastGrantedAuthoritiesProperty.class) | ||
assertAuthorities(p,"authenticated:alice:development:us") | ||
assertAuthorities(u.impersonate(),"authenticated:alice:development:us") | ||
|
||
// change should be reflected right away | ||
wc.login("alice","alice:development:uk") | ||
p = u.getProperty(LastGrantedAuthoritiesProperty.class) | ||
assertAuthorities(p,"authenticated:alice:development:uk") | ||
assertAuthorities(u.impersonate(),"authenticated:alice:development:uk") | ||
} | ||
|
||
void assertAuthorities(p,expected) { | ||
assert p.authorities*.authority.join(":")==expected | ||
} | ||
|
||
/** | ||
* SecurityRealm that cannot load information about other users, such Active Directory. | ||
*/ | ||
private class TestSecurityRealm extends AbstractPasswordBasedSecurityRealm { | ||
@Override | ||
protected UserDetails authenticate(String username, String password) throws AuthenticationException { | ||
if (password=="error") | ||
throw new BadCredentialsException(username); | ||
def authorities = password.split(":").collect { new GrantedAuthorityImpl(it) } | ||
|
||
return new User(username,"",true,authorities.toArray(new GrantedAuthority[0])) | ||
} | ||
|
||
@Override | ||
GroupDetails loadGroupByGroupname(String groupname) throws UsernameNotFoundException, DataAccessException { | ||
throw new UnsupportedOperationException() | ||
} | ||
|
||
@Override | ||
UserDetails loadUserByUsername(String username) throws UsernameNotFoundException, DataAccessException { | ||
throw new UserMayOrMayNotExistException("fallback"); | ||
} | ||
} | ||
} |