mike-jumper commented on a change in pull request #656:
URL: https://github.com/apache/guacamole-client/pull/656#discussion_r757125086
##########
File path:
extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/AuthenticationProviderService.java
##########
@@ -228,7 +228,7 @@ private UserLDAPConfiguration getLDAPConfiguration(String
username,
}
// Attempt bind (authentication)
- LdapNetworkConnection ldapConnection = ldapService.bindAs(config,
bindDn.getName(), password);
+ LdapNetworkConnection ldapConnection = ldapService.bindAs(config,
bindDn.getName(), (password == null || password.isEmpty()) ?
config.getSearchBindPassword() : password);
Review comment:
In this context, `password` is the password provided by the user during
login, not the search bind password. The search bind password should definitely
not be used as a fallback for a user's login attempt, which would potentially
allow a user to successfully authenticate despite not providing a valid
password.
The only place that the search bind password should be pulled is in the
context of the search bind DN (within `getUserBindDn()`). This should already
be the case:
https://github.com/apache/guacamole-client/blob/262643b2930aad5b6dc31df75cb928577b6a99a8/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/AuthenticationProviderService.java#L131-L133
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
