git-plugin
git-plugin copied to clipboard
jenkinsci
[JENKINS-67981] Do not fail when User.get() throws auth exception
JENKINS-67981 Do not fail when User.get() throws auth exception
PR #1202 resolved https://issues.jenkins.io/browse/JENKINS-67491 by making the change in a single location for git plugin 4.10.2. Since there have been no reports of unexpected failures due to that change, add the same change in more locations.
Checklist
- [x] I have read the CONTRIBUTING doc
- [x] I have referenced the Jira issue related to my changes in one or more commit messages
- [x] I have added tests that verify my changes
- [x] Unit tests pass locally with my changes
- [x] I have added documentation as necessary
- [x] No Javadoc warnings were introduced with my changes
- [x] No spotbugs warnings were introduced with my changes
- [x] Documentation in README has been updated as necessary
- [x] Online help has been added and reviewed for any new or modified fields
- [ ] I have interactively tested my changes
- [x] Any dependent changes have been merged and published in upstream modules (like git-client-plugin)
Types of changes
- [x] Bug fix (non-breaking change which fixes an issue)
Thanks for looking at this pull request @rsandell . I'd love to merge and release it but there are reports in JENKINS-67981 that the change does not resolve the issue.
Could you review those comments and guide me on other changes that might be made to this pull request in order to fix the problem in the cases that are reported in JENKINS-67981?
Hi @MarkEWaite I'd like to test this one, could you please advise where would I be able to download incremental build from?
@arymkus https://ci.jenkins.io/job/Plugins/job/git-plugin/job/PR-1233/lastStableBuild/ is the most recent stable build. It includes the hpi file.
@MarkEWaite I tried the .hpi file from build #25. See results below.
org.springframework.security.authentication.DisabledException: The user "john.doe" is administratively disabled.
at hudson.security.UserAttributesHelper.checkIfUserEnabled(UserAttributesHelper.java:92)
at hudson.security.LDAPSecurityRealm$LDAPUserDetailsService.loadUserByUsername(LDAPSecurityRealm.java:1315)
at hudson.security.LDAPSecurityRealm$DelegateLDAPUserDetailsService.loadUserByUsername(LDAPSecurityRealm.java:1228)
at hudson.security.LDAPSecurityRealm.loadUserByUsername2(LDAPSecurityRealm.java:763)
at jenkins.security.UserDetailsCache$Retriever.call(UserDetailsCache.java:170)
at jenkins.security.UserDetailsCache$Retriever.call(UserDetailsCache.java:159)
at com.google.common.cache.LocalCache$LocalManualCache$1.load(LocalCache.java:4868)
at com.google.common.cache.LocalCache$LoadingValueReference.loadFuture(LocalCache.java:3533)
at com.google.common.cache.LocalCache$Segment.loadSync(LocalCache.java:2282)
at com.google.common.cache.LocalCache$Segment.lockedGetOrLoad(LocalCache.java:2159)
at com.google.common.cache.LocalCache$Segment.get(LocalCache.java:2049)
Caused: com.google.common.util.concurrent.UncheckedExecutionException
at com.google.common.cache.LocalCache$Segment.get(LocalCache.java:2055)
at com.google.common.cache.LocalCache.get(LocalCache.java:3966)
at com.google.common.cache.LocalCache$LocalManualCache.get(LocalCache.java:4863)
at jenkins.security.UserDetailsCache.loadUserByUsername(UserDetailsCache.java:127)
at hudson.model.User$UserIDCanonicalIdResolver.resolveCanonicalId(User.java:1254)
at hudson.model.User$CanonicalIdResolver.resolve(User.java:1195)
at hudson.model.User.get(User.java:524)
at hudson.plugins.git.GitChangeSet.findOrCreateUser(GitChangeSet.java:475)
at hudson.plugins.git.GitChangeSet.getAuthor(GitChangeSet.java:560)
at jenkins.scm.RunWithSCM.calculateCulprits(RunWithSCM.java:141)
at hudson.model.AbstractBuild.calculateCulprits(AbstractBuild.java:351)
at jenkins.scm.RunWithSCM.getCulprits(RunWithSCM.java:96)
at hudson.model.AbstractBuild.getCulprits(AbstractBuild.java:340)
at jenkins.scm.RunWithSCM.calculateCulprits(RunWithSCM.java:136)
at hudson.model.AbstractBuild.calculateCulprits(AbstractBuild.java:351)
at jenkins.scm.RunWithSCM.getCulprits(RunWithSCM.java:96)
at hudson.model.AbstractBuild.getCulprits(AbstractBuild.java:340)
at jenkins.scm.RunWithSCM.calculateCulprits(RunWithSCM.java:136)
at hudson.model.AbstractBuild.calculateCulprits(AbstractBuild.java:351)
at jenkins.scm.RunWithSCM.getCulprits(RunWithSCM.java:96)
at hudson.model.AbstractBuild.getCulprits(AbstractBuild.java:340)
at jenkins.scm.RunWithSCM.calculateCulprits(RunWithSCM.java:136)
at hudson.model.AbstractBuild.calculateCulprits(AbstractBuild.java:351)
at jenkins.scm.RunWithSCM.getCulprits(RunWithSCM.java:96)
at hudson.model.AbstractBuild.getCulprits(AbstractBuild.java:340)
at jenkins.scm.RunWithSCM.calculateCulprits(RunWithSCM.java:136)
at hudson.model.AbstractBuild.calculateCulprits(AbstractBuild.java:351)
at jenkins.scm.RunWithSCM.getCulprits(RunWithSCM.java:96)
at hudson.model.AbstractBuild.getCulprits(AbstractBuild.java:340)
at jenkins.scm.RunWithSCM.calculateCulprits(RunWithSCM.java:136)
at hudson.model.AbstractBuild.calculateCulprits(AbstractBuild.java:351)
at jenkins.scm.RunWithSCM.getCulprits(RunWithSCM.java:96)
at hudson.model.AbstractBuild.getCulprits(AbstractBuild.java:340)
at jenkins.scm.RunWithSCM.calculateCulprits(RunWithSCM.java:136)
at hudson.model.AbstractBuild.calculateCulprits(AbstractBuild.java:351)
at jenkins.scm.RunWithSCM.getCulprits(RunWithSCM.java:96)
at hudson.model.AbstractBuild.getCulprits(AbstractBuild.java:340)
at jenkins.scm.RunWithSCM.calculateCulprits(RunWithSCM.java:136)
at hudson.model.AbstractBuild.calculateCulprits(AbstractBuild.java:351)
at jenkins.scm.RunWithSCM.getCulprits(RunWithSCM.java:96)
at hudson.model.AbstractBuild.getCulprits(AbstractBuild.java:340)
at jenkins.scm.RunWithSCM.calculateCulprits(RunWithSCM.java:136)
at hudson.model.AbstractBuild.calculateCulprits(AbstractBuild.java:351)
at jenkins.scm.RunWithSCM.getCulprits(RunWithSCM.java:96)
at hudson.model.AbstractBuild.getCulprits(AbstractBuild.java:340)
at hudson.model.AbstractBuild$AbstractBuildExecution.post(AbstractBuild.java:713)
at hudson.model.Run.execute(Run.java:1922)
at hudson.model.FreeStyleBuild.run(FreeStyleBuild.java:44)
at hudson.model.ResourceController.execute(ResourceController.java:101)
at hudson.model.Executor.run(Executor.java:442)
@MarkEWaite I tried the .hpi file from build #25.
Thanks! I've pushed a catch of the broader exception mentioned in the stack trace. It should be visible as a build in the next 60 minutes.
Thanks! I've pushed a catch of the broader exception mentioned in the stack trace. It should be visible as a build in the next 60 minutes.
You're welcome.
I gave it a try. Still the result is basically the same.
org.springframework.security.authentication.DisabledException: The user "john.doe" is administratively disabled.
at hudson.security.UserAttributesHelper.checkIfUserEnabled(UserAttributesHelper.java:92)
at hudson.security.LDAPSecurityRealm$LDAPUserDetailsService.loadUserByUsername(LDAPSecurityRealm.java:1315)
at hudson.security.LDAPSecurityRealm$DelegateLDAPUserDetailsService.loadUserByUsername(LDAPSecurityRealm.java:1228)
at hudson.security.LDAPSecurityRealm.loadUserByUsername2(LDAPSecurityRealm.java:763)
at jenkins.security.UserDetailsCache$Retriever.call(UserDetailsCache.java:170)
at jenkins.security.UserDetailsCache$Retriever.call(UserDetailsCache.java:159)
at com.google.common.cache.LocalCache$LocalManualCache$1.load(LocalCache.java:4868)
at com.google.common.cache.LocalCache$LoadingValueReference.loadFuture(LocalCache.java:3533)
at com.google.common.cache.LocalCache$Segment.loadSync(LocalCache.java:2282)
at com.google.common.cache.LocalCache$Segment.lockedGetOrLoad(LocalCache.java:2159)
at com.google.common.cache.LocalCache$Segment.get(LocalCache.java:2049)
Caused: com.google.common.util.concurrent.UncheckedExecutionException
at com.google.common.cache.LocalCache$Segment.get(LocalCache.java:2055)
at com.google.common.cache.LocalCache.get(LocalCache.java:3966)
at com.google.common.cache.LocalCache$LocalManualCache.get(LocalCache.java:4863)
at jenkins.security.UserDetailsCache.loadUserByUsername(UserDetailsCache.java:127)
at hudson.model.User$UserIDCanonicalIdResolver.resolveCanonicalId(User.java:1254)
at hudson.model.User$CanonicalIdResolver.resolve(User.java:1195)
at hudson.model.User.get(User.java:524)
at hudson.plugins.git.GitChangeSet.findOrCreateUser(GitChangeSet.java:476)
at hudson.plugins.git.GitChangeSet.getAuthor(GitChangeSet.java:561)
at jenkins.scm.RunWithSCM.calculateCulprits(RunWithSCM.java:141)
at hudson.model.AbstractBuild.calculateCulprits(AbstractBuild.java:351)
at jenkins.scm.RunWithSCM.getCulprits(RunWithSCM.java:96)
at hudson.model.AbstractBuild.getCulprits(AbstractBuild.java:340)
at jenkins.scm.RunWithSCM.calculateCulprits(RunWithSCM.java:136)
at hudson.model.AbstractBuild.calculateCulprits(AbstractBuild.java:351)
at jenkins.scm.RunWithSCM.getCulprits(RunWithSCM.java:96)
at hudson.model.AbstractBuild.getCulprits(AbstractBuild.java:340)
at jenkins.scm.RunWithSCM.calculateCulprits(RunWithSCM.java:136)
at hudson.model.AbstractBuild.calculateCulprits(AbstractBuild.java:351)
at jenkins.scm.RunWithSCM.getCulprits(RunWithSCM.java:96)
at hudson.model.AbstractBuild.getCulprits(AbstractBuild.java:340)
at jenkins.scm.RunWithSCM.calculateCulprits(RunWithSCM.java:136)
at hudson.model.AbstractBuild.calculateCulprits(AbstractBuild.java:351)
at jenkins.scm.RunWithSCM.getCulprits(RunWithSCM.java:96)
at hudson.model.AbstractBuild.getCulprits(AbstractBuild.java:340)
at jenkins.scm.RunWithSCM.calculateCulprits(RunWithSCM.java:136)
at hudson.model.AbstractBuild.calculateCulprits(AbstractBuild.java:351)
at jenkins.scm.RunWithSCM.getCulprits(RunWithSCM.java:96)
at hudson.model.AbstractBuild.getCulprits(AbstractBuild.java:340)
at jenkins.scm.RunWithSCM.calculateCulprits(RunWithSCM.java:136)
at hudson.model.AbstractBuild.calculateCulprits(AbstractBuild.java:351)
at jenkins.scm.RunWithSCM.getCulprits(RunWithSCM.java:96)
at hudson.model.AbstractBuild.getCulprits(AbstractBuild.java:340)
at jenkins.scm.RunWithSCM.calculateCulprits(RunWithSCM.java:136)
at hudson.model.AbstractBuild.calculateCulprits(AbstractBuild.java:351)
at jenkins.scm.RunWithSCM.getCulprits(RunWithSCM.java:96)
at hudson.model.AbstractBuild.getCulprits(AbstractBuild.java:340)
at jenkins.scm.RunWithSCM.calculateCulprits(RunWithSCM.java:136)
at hudson.model.AbstractBuild.calculateCulprits(AbstractBuild.java:351)
at jenkins.scm.RunWithSCM.getCulprits(RunWithSCM.java:96)
at hudson.model.AbstractBuild.getCulprits(AbstractBuild.java:340)
at jenkins.scm.RunWithSCM.calculateCulprits(RunWithSCM.java:136)
at hudson.model.AbstractBuild.calculateCulprits(AbstractBuild.java:351)
at jenkins.scm.RunWithSCM.getCulprits(RunWithSCM.java:96)
at hudson.model.AbstractBuild.getCulprits(AbstractBuild.java:340)
at jenkins.scm.RunWithSCM.calculateCulprits(RunWithSCM.java:136)
at hudson.model.AbstractBuild.calculateCulprits(AbstractBuild.java:351)
at jenkins.scm.RunWithSCM.getCulprits(RunWithSCM.java:96)
at hudson.model.AbstractBuild.getCulprits(AbstractBuild.java:340)
at hudson.model.AbstractBuild$AbstractBuildExecution.post(AbstractBuild.java:713)
at hudson.model.Run.execute(Run.java:1922)
at hudson.model.FreeStyleBuild.run(FreeStyleBuild.java:44)
at hudson.model.ResourceController.execute(ResourceController.java:101)
at hudson.model.Executor.run(Executor.java:442)
@MarkEWaite As of this morning I cannot reproduce the error anymore. I don't know why, but it's working now (even with build #25).
@MarkEWaite looks like your first pass was fine (catching just AuthenticationException) since build 25 eventually worked for the user.
Looks like you can remove f33342fe16c644409a7e0e4be2fa6dea270f2712 which catches UncheckedExecutionException and I guess rebase this PR?
We will give it a try since we had a build affected by it ( https://phabricator.wikimedia.org/T315897 ).
A bit more investigation. The plugin throws at:
hudson/plugins/git/GitChangeSet.java
try {
user = User.get(csAuthor, false, Collections.emptyMap());
} catch (AuthenticationException authException) {
// JENKINS-67491 - do not fail due to an authentication exception
return User.getUnknown();
}
Which only catches AuthenticationException.
The second trace at jenkins.security.UserDetailsCache.loadUserByUsername(UserDetailsCache.java:127) comes from Jenkins core:
name=core/src/main/java/jenkins/security/UserDetailsCache.java
/**
* Locates the user based on the username, by first looking in the cache and then delegate to
* {@link hudson.security.SecurityRealm#loadUserByUsername2(String)}.
*
* @param idOrFullName the username
* @return the details
*
* @throws UsernameNotFoundException (normally a {@link hudson.security.UserMayOrMayNotExistException2})
* if the user could not be found or the user has no GrantedAuthority
* @throws ExecutionException if anything else went wrong in the cache lookup/retrieval
*/
@NonNull
public UserDetails loadUserByUsername(String idOrFullName) throws UsernameNotFoundException, ExecutionException {
Boolean exists = existenceCache.getIfPresent(idOrFullName);
if (exists != null && !exists) {
throw new UsernameNotFoundException(String.format("\"%s\" does not exist", idOrFullName));
} else {
try {
return detailsCache.get(idOrFullName, new Retriever(idOrFullName));
} catch (ExecutionException | UncheckedExecutionException e) {
if (e.getCause() instanceof UsernameNotFoundException) {
throw (UsernameNotFoundException) e.getCause();
} else {
throw e;
}
}
}
}
Which retrow apparently either an ExecutionException or an UncheckedExecutionException rather than the DisabledUserException. Or to say it otherwise, the Jenkins user cache wraps in ExecutionException the exceptions about the user not being found or disabled. If the wrapped exception happens to be UsernameNotFoundException it throws it, else it throws the wrapper.
The PR catch AuthenticationException, it should instead catch ExecutionException as mentioned in the javadoc for loadUserByUsername:
* @throws UsernameNotFoundException (normally a {@link hudson.security.UserMayOrMayNotExistException2})
* if the user could not be found or the user has no GrantedAuthority
* @throws ExecutionException if anything else went wrong in the cache lookup/retrieval
I think the issue is loadUserByUsername() only throws the AuthenticationException UsernameNotFoundException, otherwise it throws an ExecutionException.
I have tested my theory on our Jenkins instance via the script console and my dummy code worked:
import org.springframework.security.core.AuthenticationException;
import com.google.common.util.concurrent.UncheckedExecutionException;
try {
try {
println("Attempting to retrieve disabled user 'theresnotime'");
println(User.get("theresnotime", false, Collections.emptyMap()));
} catch (AuthenticationException e) {
println("Throwing the AuthenticationException directly");
throw e;
} catch (UncheckedExecutionException e) {
println("Got an UncheckedExecutionException");
if (e.getCause() instanceof AuthenticationException) {
println("Found cause to be an AuthenticationException, throwing");
throw (AuthenticationException) e.getCause();
} else {
println("Unhandled exception");
throw e;
}
}
} catch (AuthenticationException e) {
println(User.getUnknown());
}
I have send #1322 with the above and a test, it supersedes this #1233