Added support for specifying LDAP UPN in properties file #683
Conversation
for any user in LDAP (with the corresponding UPN) to authenticate. Removes requirement of users being within same OU for large LDAP deployments.
|
@agreenbhm : A couple of notes:
|
Thanks for the info. Looks like that PR is nearly 2 years old. @necouchman: Given that you are the author of that PR, is there anything more that is needed to be done to get that merged? It looks like the changes I made are pretty similar to the ones you did so I'm not sure why your PR wouldn't be merged yet since it seems functional. |
| String searchBindLogon; | ||
| String searchBindPassword; | ||
|
|
||
| if(confService.getUPNDomain() != "" && confService.getUPNDomain() != null){ |
There was a problem hiding this comment.
The test confService.getUPNDomain() != "" performs a by-reference comparison against the String object for "", not a comparison of the content of those strings. The proper test for whether a String is empty is isEmpty(), however the null check shown here would have to occur first for that call to not throw a NullPointerException in the event the string is indeed null.
| searchBindLogon = username + "@" + confService.getUPNDomain(); | ||
| searchBindPassword = password; |
There was a problem hiding this comment.
I don't think this should be done. The search bind DN is a very specific thing - it's the service account that Guacamole uses to resolve the DN of a user. It shouldn't be replaced with the user's own account.
If the idea here is to allow logins via UPN and avoid the search bind DN entirely, then the solution would be to allow user authentication to proceed without a user search by directly binding with the provided UPN - essentially an alternative to the direct user mapping already supported.
|
I'm not sure whether these changes (in spirit) nor GUACAMOLE-536 are actually necessary:
I know that Active Directory allows for binding with UPNs, and so adding a feature that would tell Guacamole to accept usernames matching that pattern and attempt to bind with those usernames directly would make configuration easier, but I'm not sure that's what these changes or GUACAMOLE-536 are intended to be. EDIT: Sorry - that should be GUACAMOLE-536*. Copy-pasta of other things I also happen to have open. |
Added support for specifying LDAP UPN in properties file. Allows for any user in LDAP (with the corresponding UPN) to authenticate. Removes requirement of users being within same OU for large LDAP deployments.