Github user necouchman commented on a diff in the pull request:

    https://github.com/apache/guacamole-client/pull/299#discussion_r194164950
  
    --- Diff: 
extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/AuthenticationProviderService.java
 ---
    @@ -236,6 +253,58 @@ public AuthenticatedUser authenticateUser(Credentials 
credentials)
     
         }
     
    +    /**
    +     * Returns all custom LDAP attributes on the user currently bound under
    +     * the given LDAP connection. The custom attributes are specified in
    +     * guacamole.properties.
    +     *
    +     * @param ldapConnection
    +     *     LDAP connection to find the custom LDAP attributes.
    +     * @param username
    +     *     The username of the user whose attributes are queried.
    +     *
    +     * @return
    +     *     All attributes on the user currently bound under the
    +     *     given LDAP connection, as a map of attribute name to
    +     *     corresponding attribute value.
    +     *
    +     * @throws LDAPException
    +     *     If an error occurs while searching for the user attributes.
    +     *
    +     * @throws GuacamoleException
    +     *     If an error occurs retrieving the user DN.
    +     */
    +    private Map<String, String> getLDAPAttributes(LDAPConnection 
ldapConnection,
    +            String username) throws LDAPException, GuacamoleException {
    --- End diff --
    
    Maybe @mike-jumper has opinions about this, but I'd suggest only throwing 
the `GuacamoleException` in this method and catching the `LDAPException`, here, 
and re-throwing it as a `GuacamoleException` (`GuacamoleServerException`, 
prehaps).  This also clears up the nested try above...


---

Reply via email to