You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@guacamole.apache.org by GitBox <gi...@apache.org> on 2021/11/25 16:44:21 UTC

[GitHub] [guacamole-client] alubbock opened a new pull request #656: GUACAMOLE-1465: Allow different bind passwords for multi-LDAP

alubbock opened a new pull request #656:
URL: https://github.com/apache/guacamole-client/pull/656


   PR #648 recently added support for querying multiple LDAP servers, but as things stand all servers are required to use the same bind password. This PR enables per-server bind passwords in the new ldap-servers.yml file.


-- 
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-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [guacamole-client] mike-jumper closed pull request #656: GUACAMOLE-1465: Allow different bind passwords for multi-LDAP

Posted by GitBox <gi...@apache.org>.
mike-jumper closed pull request #656:
URL: https://github.com/apache/guacamole-client/pull/656


   


-- 
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-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [guacamole-client] mike-jumper commented on pull request #656: GUACAMOLE-1465: Allow different bind passwords for multi-LDAP

Posted by GitBox <gi...@apache.org>.
mike-jumper commented on pull request #656:
URL: https://github.com/apache/guacamole-client/pull/656#issuecomment-992760841


   Closing as (1) the motivation behind these changes shouldn't apply (though please reopen the JIRA issue if not the case) and (2) the changes themselves introduce an issue where the search bind password may incorrectly be applied as if the user entered it themselves during login.


-- 
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-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [guacamole-client] mike-jumper commented on a change in pull request #656: GUACAMOLE-1465: Allow different bind passwords for multi-LDAP

Posted by GitBox <gi...@apache.org>.
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-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [guacamole-client] alubbock commented on pull request #656: GUACAMOLE-1465: Allow different bind passwords for multi-LDAP

Posted by GitBox <gi...@apache.org>.
alubbock commented on pull request #656:
URL: https://github.com/apache/guacamole-client/pull/656#issuecomment-992839416


   As you stated above, this PR is not needed and was introduced based on an LDAP configuration issue on my end. Apologies for wasting your time.


-- 
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-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org