echu2013 commented on pull request #550:
URL: https://github.com/apache/guacamole-client/pull/550#issuecomment-657132619
> I currently only see two issues with this, both related to documentation:
>
> 1. The intentional use of `null` to represent all attributes should be
documented. The existing documentation only describes the meaning of a non-null
value.
> 2. The commit message ("GUACAMOLE-1130: Add LDAP attributes filtering
handler") is unclear to me. Only through reading the code do I see the intent
of these changes (to retrieve only the LDAP attributes that are necessary). I
think this should be rephrased such that someone reading through the commit
logs will understand the high-level intent of these changes. This is
particularly important if the code should prove incorrect sometime in the
future.
>
> There were recent changes related to LDAP attribute handling which allow
the LDAP attributes associated with a user to be made available as parameter
tokens when specified with [the `ldap-user-attributes`
property](http://guacamole.apache.org/doc/gug/ldap-auth.html):
>
>
https://github.com/apache/guacamole-client/blob/3c4c81f0b6b9700abccaefcc695058e515b8b20b/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/AuthenticationProviderService.java#L234-L257
>
> I don't think these changes will affect that behavior, but have you
already confirmed this?
`The commit message ("GUACAMOLE-1130: Add LDAP attributes filtering
handler") is unclear to me. Only through reading the code do I see the intent
of these changes (to retrieve only the LDAP attributes that are necessary). I
think this should be rephrased such that someone reading through the commit
logs will understand the high-level intent of these changes. This is
particularly important if the code should prove incorrect sometime in the
future.`
I will definitely rephrase commit log!
`I don't think these changes will affect that behavior, but have you already
confirmed this?`
This is indeed confirmed, I use LDAP Tokens and they keep working OK as
usual.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
