-
Notifications
You must be signed in to change notification settings - Fork 100
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[FIXED JENKINS-32033] Support for multiple domain controllers #41
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small comments
@@ -518,7 +518,8 @@ public DirContext bind(String principalName, String password, List<SocketInfo> l | |||
newProps.put("java.naming.ldap.attributes.binary","tokenGroups objectSid"); | |||
newProps.put("java.naming.ldap.factory.socket",TrustAllSocketFactory.class.getName()); | |||
newProps.putAll(props); | |||
NamingException error = null; | |||
NamingException namingException = null; | |||
javax.naming.AuthenticationException authenticationException = null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use BadCredentialsException
here, as it seems the only subtype used.
// if all the attempts failed | ||
throw new BadCredentialsException("Either no such user '"+principalName+"' or incorrect password", error); | ||
if (authenticationException !=null ) { | ||
throw new BadCredentialsException("Either no such user '" + principalName + "' or incorrect password", authenticationException); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can throw the previously created one if you want.
throw new BadCredentialsException("Either no such user '" + principalName + "' or incorrect password", authenticationException); | ||
} else { | ||
throw new BadCredentialsException("Either no such user '" + principalName + "' or incorrect password", namingException); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto
I do not think that this was a good idea as implemented - it can lead to a user locking out their account with a single typos in the password. |
I guess that because if you fail, you will be failling consecutively on all the domain controllers, what can make the user to be locked. Is this right? |
This PR allows to use multiple domain controllers - even with multiple domains.
@jtnord @andresrc