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/06/23 23:06:50 UTC

[GitHub] [guacamole-client] mike-jumper commented on a change in pull request #497: GUACAMOLE-996: Add support for configuring group filter.

mike-jumper commented on a change in pull request #497:
URL: https://github.com/apache/guacamole-client/pull/497#discussion_r657516896



##########
File path: extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/conf/ConfigurationService.java
##########
@@ -321,6 +322,26 @@ public ExprNode getUserSearchFilter() throws GuacamoleException {
         );
     }
 
+    /**
+     * Returns the search filter that should be used when querying the
+     * LDAP server for Guacamole groups.  If no filter is specified,
+     * a default of "(objectClass=group)" is returned.
+     *
+     * @return
+     *     The search filter that should be used when querying the
+     *     LDAP server for groups that are valid in Guacamole, or
+     *     "(objectClass=group)" if not specified.
+     *
+     * @throws GuacamoleException
+     *     If guacamole.properties cannot be parsed.
+     */
+    public ExprNode getGroupSearchFilter() throws GuacamoleException {
+        return environment.getProperty(
+            LDAPGuacamoleProperties.LDAP_GROUP_SEARCH_FILTER,
+            new EqualityNode("objectClass","group")

Review comment:
       I think we should lean toward Principle of Least Surprise and maintain the existing `(objectClass=*)` behavior by default. The default behavior is intended to rely on LDAP directory structure to determine which objects should be returned and classified as users or groups, and I think it makes sense to continue to use that as the default behavior.
   
   Filters like `ldap-user-search-filter` and soon-to-be-implemented `ldap-group-search-filter` exist to allow for more complex and intermingled cases, but default filters would involve too many assumptions and could break existing deployments.




-- 
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:
users@infra.apache.org