Skip to content

Commit

Permalink
[JENKINS-43388] Improve validation messages
Browse files Browse the repository at this point in the history
  • Loading branch information
@stephenc
stephenc committed Apr 19, 2017
1 parent ff919c6 commit a258fee
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 19 deletions.
50 changes: 38 additions & 12 deletions src/main/java/hudson/security/LDAPSecurityRealm.java
View file
Expand Up @@ -1195,6 +1195,7 @@ public FormValidation validate(LDAPSecurityRealm realm, String user, String pass
response.append("<div>")
.append(jenkins.security.plugins.ldap.Messages.LDAPSecurityRealm_LoginHeader())
.append("</div>");
boolean potentialLockout = false;

// can we login?
LdapUserDetails loginDetails = null;
Expand All @@ -1205,8 +1206,14 @@ public FormValidation validate(LDAPSecurityRealm realm, String user, String pass
ok(response, "authentication",
jenkins.security.plugins.ldap.Messages.LDAPSecurityRealm_AuthenticationSuccessful());
} catch (AuthenticationException e) {
rsp(response, StringUtils.isBlank(password)? "warning" : "error", "authentication",
jenkins.security.plugins.ldap.Messages.LDAPSecurityRealm_AuthenticationFailed());
if (StringUtils.isBlank(password)) {
warning(response, "authentication",
jenkins.security.plugins.ldap.Messages.LDAPSecurityRealm_AuthenticationFailedEmptyPass(user));
} else {
error(response, "authentication",
jenkins.security.plugins.ldap.Messages.LDAPSecurityRealm_AuthenticationFailed(user));
potentialLockout = true;
}
}
Set<String> loginAuthorities = new HashSet<>();
if (loginDetails != null) {
Expand All @@ -1225,6 +1232,7 @@ public FormValidation validate(LDAPSecurityRealm realm, String user, String pass
if (loginDetails.getAuthorities().length < 1) {
error(response, "authentication-groups",
jenkins.security.plugins.ldap.Messages.LDAPSecurityRealm_NoGroupMembership());
// we do not flag this error as may be legitimate config
} else if (loginDetails.getAuthorities().length == 1) {
warning(response, "authentication-groups",
jenkins.security.plugins.ldap.Messages.LDAPSecurityRealm_BasicGroupMembership(),
Expand Down Expand Up @@ -1257,20 +1265,22 @@ public FormValidation validate(LDAPSecurityRealm realm, String user, String pass
jenkins.security.plugins.ldap.Messages.LDAPSecurityRealm_UserLookupSuccessful());
} catch (UserMayOrMayNotExistException e1) {
rsp(response, loginDetails == null ? "warning" : "error", "lookup",
jenkins.security.plugins.ldap.Messages.LDAPSecurityRealm_UserLookupInconclusive(),
jenkins.security.plugins.ldap.Messages.LDAPSecurityRealm_UserLookupInconclusive(user),
StringUtils.isBlank(realm.managerDN)
? jenkins.security.plugins.ldap.Messages.LDAPSecurityRealm_UserLookupManagerDnRequired()
: jenkins.security.plugins.ldap.Messages
.LDAPSecurityRealm_UserLookupManagerDnPermissions()
);
// we do not flag these errors as could be probing user accounts
} catch (UsernameNotFoundException e1) {
rsp(response, loginDetails == null ? "warning" : "error", "lookup",
jenkins.security.plugins.ldap.Messages.LDAPSecurityRealm_UserLookupDoesNotExist(),
jenkins.security.plugins.ldap.Messages.LDAPSecurityRealm_UserLookupDoesNotExist(user),
StringUtils.isBlank(realm.managerDN)
? jenkins.security.plugins.ldap.Messages.LDAPSecurityRealm_UserLookupManagerDnRequired()
: jenkins.security.plugins.ldap.Messages
.LDAPSecurityRealm_UserLookupManagerDnPermissions(),
jenkins.security.plugins.ldap.Messages.LDAPSecurityRealm_UserLookupSettingsCorrect());
// we do not flag these errors as could be probing user accounts
} catch (LdapDataAccessException e) {
Throwable cause = e.getCause();
while (cause != null && !(cause instanceof BadCredentialsException)) {
Expand All @@ -1285,10 +1295,12 @@ public FormValidation validate(LDAPSecurityRealm realm, String user, String pass
: jenkins.security.plugins.ldap.Messages
.LDAPSecurityRealm_UserLookupManagerDnPermissions()
);
potentialLockout = true;
} else {
error(response, "lookup",
jenkins.security.plugins.ldap.Messages.LDAPSecurityRealm_UserLookupFailed(),
jenkins.security.plugins.ldap.Messages.LDAPSecurityRealm_UserLookupFailed(user),
Util.escape(e.getLocalizedMessage()));
potentialLockout = true;
}
}
if (loginDetails == null && lookUpDetails != null) {
Expand Down Expand Up @@ -1316,6 +1328,7 @@ public FormValidation validate(LDAPSecurityRealm realm, String user, String pass
if (lookUpDetails.getAuthorities().length < 1) {
error(response, "lookup-groups",
jenkins.security.plugins.ldap.Messages.LDAPSecurityRealm_NoGroupMembership());
// we do not flag this error as may be legitimate
} else if (lookUpDetails.getAuthorities().length == 1) {
warning(response, "lookup-groups",
jenkins.security.plugins.ldap.Messages.LDAPSecurityRealm_BasicGroupMembership(),
Expand All @@ -1341,12 +1354,14 @@ public FormValidation validate(LDAPSecurityRealm realm, String user, String pass
error(response, "consistency-username",
jenkins.security.plugins.ldap.Messages.LDAPSecurityRealm_UsernameMismatch(
loginDetails.getUsername(), lookUpDetails.getUsername()));
potentialLockout = true; // consistency is important
}
// dn
if (!StringUtils.equals(loginDetails.getDn(), lookUpDetails.getDn())) {
error(response, "consistency-dn",
jenkins.security.plugins.ldap.Messages.LDAPSecurityRealm_DnMismatch(
loginDetails.getDn(), lookUpDetails.getDn()));
potentialLockout = true; // consistency is important
}
// display name
if (StringUtils.isNotBlank(realm.getDisplayNameAttributeName())) {
Expand All @@ -1368,6 +1383,7 @@ public FormValidation validate(LDAPSecurityRealm realm, String user, String pass
error(response, "consistency-displayname",
jenkins.security.plugins.ldap.Messages.LDAPSecurityRealm_DisplayNameMismatch(
loginValue, lookUpValue));
potentialLockout = true; // consistency is important
}
}
// email address
Expand All @@ -1391,6 +1407,7 @@ public FormValidation validate(LDAPSecurityRealm realm, String user, String pass
error(response, "consistency-email",
jenkins.security.plugins.ldap.Messages.LDAPSecurityRealm_EmailAddressMismatch(
loginValue, lookUpValue));
potentialLockout = true; // consistency is important
}
}
// groups
Expand All @@ -1400,6 +1417,7 @@ public FormValidation validate(LDAPSecurityRealm realm, String user, String pass
} else {
error(response, "consistency",
jenkins.security.plugins.ldap.Messages.LDAPSecurityRealm_GroupMembershipMismatch());
potentialLockout = true; // consistency is important
}
}
// lets check group lookup if we can
Expand All @@ -1424,7 +1442,7 @@ public FormValidation validate(LDAPSecurityRealm realm, String user, String pass
} else {
List<String> escaped = new ArrayList<>(badGroups.size());
for (String group : badGroups) {
escaped.add("<li>"+Util.escape(group)+"</li>");
escaped.add("<code>"+Util.escape(group)+"</code>");
}
warning(response, "resolve-groups",
jenkins.security.plugins.ldap.Messages.LDAPSecurityRealm_GroupLookupFailed(badGroups.size()),
Expand All @@ -1436,6 +1454,14 @@ public FormValidation validate(LDAPSecurityRealm realm, String user, String pass
.LDAPSecurityRealm_GroupLookupManagerDnPermissions(),
jenkins.security.plugins.ldap.Messages.LDAPSecurityRealm_GroupLookupSettingsCorrect());
}
if (potentialLockout) {
response.append("<div>")
.append(jenkins.security.plugins.ldap.Messages.LDAPSecurityRealm_LockoutHeader())
.append("</div>");
error(response, "lockout",
jenkins.security.plugins.ldap.Messages.LDAPSecurityRealm_PotentialLockout(user)
);
}
// and we are done, report the results
return FormValidation.okWithMarkup(response.toString());
}
Expand All @@ -1446,7 +1472,7 @@ private void validateEmailAddress(LDAPSecurityRealm realm, StringBuilder respons
if (attribute == null) {
List<String> alternatives = new ArrayList<>();
for (Attribute attr : Collections.list(details.getAttributes().getAll())) {
alternatives.add("<li>"+Util.escape(attr.getID())+"</li>");
alternatives.add("<code>"+Util.escape(attr.getID())+"</code>");
}
warning(response, testId,
jenkins.security.plugins.ldap.Messages.LDAPSecurityRealm_NoEmailAddress(),
Expand All @@ -1466,7 +1492,7 @@ private void validateEmailAddress(LDAPSecurityRealm realm, StringBuilder respons
} else {
List<String> alternatives = new ArrayList<>();
for (Attribute attr : Collections.list(details.getAttributes().getAll())) {
alternatives.add("<li>" + Util.escape(attr.getID()) + "</li>");
alternatives.add("<code>" + Util.escape(attr.getID()) + "</code>");
}
warning(response, testId,
jenkins.security.plugins.ldap.Messages.LDAPSecurityRealm_EmptyEmailAddress(),
Expand All @@ -1479,7 +1505,7 @@ private void validateEmailAddress(LDAPSecurityRealm realm, StringBuilder respons
} catch (NamingException e) {
List<String> alternatives = new ArrayList<>();
for (Attribute attr : Collections.list(details.getAttributes().getAll())) {
alternatives.add("<li>" + Util.escape(attr.getID()) + "</li>");
alternatives.add("<code>" + Util.escape(attr.getID()) + "</code>");
}
error(response, testId,
jenkins.security.plugins.ldap.Messages.LDAPSecurityRealm_CouldNotRetrieveEmailAddress(),
Expand All @@ -1498,7 +1524,7 @@ private void validateDisplayName(LDAPSecurityRealm realm, StringBuilder response
if (attribute == null) {
List<String> alternatives = new ArrayList<>();
for (Attribute attr : Collections.list(details.getAttributes().getAll())) {
alternatives.add("<li>" + Util.escape(attr.getID()) + "</li>");
alternatives.add("<code>" + Util.escape(attr.getID()) + "</code>");
}
warning(response, testId,
jenkins.security.plugins.ldap.Messages.LDAPSecurityRealm_NoDisplayName(),
Expand All @@ -1518,7 +1544,7 @@ private void validateDisplayName(LDAPSecurityRealm realm, StringBuilder response
} else {
List<String> alternatives = new ArrayList<>();
for (Attribute attr : Collections.list(details.getAttributes().getAll())) {
alternatives.add("<li>" + Util.escape(attr.getID()) + "</li>");
alternatives.add("<code>" + Util.escape(attr.getID()) + "</code>");
}
warning(response, testId,
jenkins.security.plugins.ldap.Messages.LDAPSecurityRealm_EmptyDisplayName(),
Expand All @@ -1531,7 +1557,7 @@ private void validateDisplayName(LDAPSecurityRealm realm, StringBuilder response
} catch (NamingException e) {
List<String> alternatives = new ArrayList<>();
for (Attribute attr : Collections.list(details.getAttributes().getAll())) {
alternatives.add("<li>" + Util.escape(attr.getID()) + "</li>");
alternatives.add("<code>" + Util.escape(attr.getID()) + "</code>");
}
error(response, testId,
jenkins.security.plugins.ldap.Messages.LDAPSecurityRealm_CouldNotRetrieveDisplayName(),
Expand Down
19 changes: 12 additions & 7 deletions src/main/resources/jenkins/security/plugins/ldap/Messages.properties
View file
Expand Up @@ -2,17 +2,18 @@ FromUserRecordLDAPGroupMembershipStrategy.DisplayName=Parse user attribute for l
FromGroupSearchLDAPGroupMembershipStrategy.DisplayName=Search for groups containing user
LDAPSecurityRealm.LoginHeader=Login
LDAPSecurityRealm.AuthenticationSuccessful=Authentication: successful
LDAPSecurityRealm.AuthenticationFailed=Authentication: failed
LDAPSecurityRealm.AuthenticationFailed=Authentication: failed for user "{0}"
LDAPSecurityRealm.AuthenticationFailedEmptyPass=Authentication: failed for user "{0}" with empty password
LDAPSecurityRealm.UserId=User ID: {0}
LDAPSecurityRealm.UserDn=User DN: {0}
LDAPSecurityRealm.UserDn=User Dn: {0}
LDAPSecurityRealm.UserDisplayName=User Display Name: {0}
LDAPSecurityRealm.UserEmail=User email: {0}
LDAPSecurityRealm.GroupMembership=Group membership:
LDAPSecurityRealm.LookupHeader=Lookup
LDAPSecurityRealm.UserLookupSuccessful=User lookup: successful
LDAPSecurityRealm.UserLookupFailed=User lookup: failed
LDAPSecurityRealm.UserLookupInconclusive=User lookup: user may or may not exist.
LDAPSecurityRealm.UserLookupDoesNotExist=User lookup: user does not exist.
LDAPSecurityRealm.UserLookupFailed=User lookup: failed for user "{0}"
LDAPSecurityRealm.UserLookupInconclusive=User lookup: user "{0}" may or may not exist.
LDAPSecurityRealm.UserLookupDoesNotExist=User lookup: user "{0}" does not exist.
LDAPSecurityRealm.UserLookupBadCredentials=User lookup: bad credentials for user lookup.
LDAPSecurityRealm.GroupMembershipMatch=User groups consistent (login and lookup)
LDAPSecurityRealm.GroupMembershipMismatch=User groups inconsistent (login versus lookup)
Expand All @@ -28,12 +29,12 @@ LDAPSecurityRealm.AvailableAttributes=Available attributes are:
LDAPSecurityRealm.UserLookupManagerDnRequired=Is a Manager DN and password required to lookup user details?
LDAPSecurityRealm.UserLookupManagerDnCorrect=Is a Manager DN and password correct?
LDAPSecurityRealm.UserLookupManagerDnPermissions=Does the Manager DN have permissions to perform user lookup?
LDAPSecurityRealm.GroupLookupManagerDnPermissions=Does the Manager DN have permissions to perform user lookup?
LDAPSecurityRealm.GroupLookupManagerDnPermissions=Does the Manager DN have permissions to perform group lookup?
LDAPSecurityRealm.GroupLookupManagerDnRequired=Is a Manager DN and password required to lookup group details?
LDAPSecurityRealm.UserLookupSettingsCorrect=Are the user search base and user search filter settings correct?
LDAPSecurityRealm.GroupLookupSettingsCorrect=Are the group search base and group search filter settings correct?
LDAPSecurityRealm.NoGroupMembership=No group membership reported
LDAPSecurityRealm.BasicGroupMembership=Only basic group membership reported.
LDAPSecurityRealm.BasicGroupMembership=No LDAP group membership reported.
LDAPSecurityRealm.BasicGroupMembershipDetail=If the user is a member of some LDAP groups then the group membership \
settings are probably configured incorrectly.
LDAPSecurityRealm.GroupLookupNotPossible=Group lookup: could not verify.
Expand All @@ -43,3 +44,7 @@ LDAPSecurityRealm.UsernameMismatch=Username inconsistent (login {0} versus looku
LDAPSecurityRealm.DnMismatch=Dn inconsistent (login {0} versus lookup {1})
LDAPSecurityRealm.DisplayNameMismatch=Display name inconsistent (login {0} versus lookup {1})
LDAPSecurityRealm.EmailAddressMismatch=Email address inconsistent (login {0} versus lookup {1})
LDAPSecurityRealm.LockoutHeader=Lockout
LDAPSecurityRealm.PotentialLockout=The user "{0}" will be unable to login with the supplied password.<br/>\
If this is your own account this would mean you would be locked out!<br/>\
Are you sure you want to save this configuration?

0 comments on commit a258fee

Please sign in to comment.