Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[JENKINS-37582] Return null inside the cache block is not allowed
  • Loading branch information
fbelzunc committed Sep 9, 2016
1 parent 52435d4 commit e208127
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 13 deletions.
Expand Up @@ -41,6 +41,7 @@
import com4j.util.ComObjectCollector;
import hudson.security.GroupDetails;
import hudson.security.SecurityRealm;
import hudson.security.UserMayOrMayNotExistException;
import org.acegisecurity.AuthenticationException;
import org.acegisecurity.BadCredentialsException;
import org.acegisecurity.GrantedAuthority;
Expand Down Expand Up @@ -291,17 +292,18 @@ public ActiveDirectoryGroupDetails call() throws Exception {
IADsGroup group = dso.openDSObject(dnToLdapUrl(dn), null, null, ADS_READONLY_SERVER)
.queryInterface(IADsGroup.class);

// If not a group will return null
// If not a group will throw UserMayOrMayNotExistException
if (group == null) {
return null;
throw new UserMayOrMayNotExistException(groupname);
}
return new ActiveDirectoryGroupDetails(groupname);
} catch (UsernameNotFoundException e) {
return null; // failed to convert group name to DN
// failed to convert group name to DN
throw new UsernameNotFoundException("Failed to get the DN of the group " + groupname);
} catch (ComException e) {
// recover gracefully since AD might behave in a way we haven't anticipated
LOGGER.log(Level.WARNING, String.format("Failed to figure out details of AD group: %s", groupname), e);
return null;
throw new UserMayOrMayNotExistException(groupname);
} finally {
col.disposeAll();
COM4J.removeListener(col);
Expand Down
Expand Up @@ -366,7 +366,6 @@ public GroupDetails loadGroupByGroupname(final String groupname) {
try {
return groupCache.get(groupname, new Callable<ActiveDirectoryGroupDetails>() {
public ActiveDirectoryGroupDetails call() {
boolean problem = false;
for (String domainName : domainNames) {
// when we use custom socket factory below, every LDAP operations result
// in a classloading via context classloader, so we need it to resolve.
Expand Down Expand Up @@ -403,18 +402,12 @@ public ActiveDirectoryGroupDetails call() {
} catch (AuthenticationException e) {
// something went wrong talking to the server. This should be reported
LOGGER.log(Level.WARNING, String.format("Failed to find the group %s in %s domain", groupname, domainName), e);
problem = true;
} finally {
Thread.currentThread().setContextClassLoader(ccl);
}
}

if (!problem) {
return null; // group not found anywhere. cache this result
} else {
LOGGER.log(Level.WARNING, "Exhausted all configured domains and could not authenticate against any");
throw new UserMayOrMayNotExistException(groupname);
}
LOGGER.log(Level.WARNING, "Exhausted all configured domains and could not authenticate against any");
throw new UserMayOrMayNotExistException(groupname);
}
});
} catch (UncheckedExecutionException e) {
Expand Down

0 comments on commit e208127

Please sign in to comment.