From 2aec452aa5d2430295039cb0e087957b5396c9aa Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Mon, 20 Mar 2017 22:15:14 -0400 Subject: [PATCH 1/6] GUACAMOLE-101: Impelement properties for controller user and connection search filters. --- .../auth/ldap/ConfigurationService.java | 39 +++++++++++++++++++ .../auth/ldap/LDAPGuacamoleProperties.java | 20 ++++++++++ .../ldap/connection/ConnectionService.java | 6 ++- .../guacamole/auth/ldap/user/UserService.java | 13 ++++++- 4 files changed, 74 insertions(+), 4 deletions(-) diff --git a/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ConfigurationService.java b/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ConfigurationService.java index f0988a741..19df4839c 100644 --- a/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ConfigurationService.java +++ b/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ConfigurationService.java @@ -270,7 +270,46 @@ public class ConfigurationService { constraints.setDereference(getDereferenceAliases().DEREF_VALUE); return constraints; + } + /** + * Returns the search filter that should be used when querying the + * LDAP server for Guacamole users. If no filter is specified, + * a default of objectClass=* is returned. + * + * @return + * The search filter that should be used when querying the + * LDAP server for users that are valid in Guacamole, or + * objectClass=* if not specified. + * + * @throws GuacamoleException + * If guacamole.properties cannot be parsed. + */ + public String getUserSearchFilter() throws GuacamoleException { + return environment.getProperty( + LDAPGuacamoleProperties.LDAP_USER_SEARCH_FILTER, + "(objectClass=*)" + ); + } + + /** + * Returns the search filter that should be used when querying the + * LDAP server for Guacamole connections. If no filter is specified, + * null is returned. + * + * @return + * The search filter that should be used when querying the + * LDAP server for connections for Guacamole, or + * null if no filter is specified. + * + * @throws GuacamoleException + * If guacamole.properties cannot be parsed. + */ + public String getConnectionSearchFilter() throws GuacamoleException { + return environment.getProperty( + LDAPGuacamoleProperties.LDAP_CONNECTION_SEARCH_FILTER, + "(objectClass=guacConfigGroup)" + ); } } diff --git a/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/LDAPGuacamoleProperties.java b/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/LDAPGuacamoleProperties.java index 266af8e93..691a6fca9 100644 --- a/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/LDAPGuacamoleProperties.java +++ b/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/LDAPGuacamoleProperties.java @@ -164,4 +164,24 @@ public class LDAPGuacamoleProperties { }; + /** + * A search filter to apply to the user LDAP query. + */ + public static final StringGuacamoleProperty LDAP_USER_SEARCH_FILTER = new StringGuacamoleProperty() { + + @Override + public String getName() { return "ldap-user-search-filter"; } + + }; + + /** + * A search filter to apply to the connection LDAP query. + */ + public static final StringGuacamoleProperty LDAP_CONNECTION_SEARCH_FILTER = new StringGuacamoleProperty() { + + @Override + public String getName() { return "ldap-connection-search-filter"; } + + }; + } diff --git a/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/connection/ConnectionService.java b/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/connection/ConnectionService.java index d256ebb3b..04e57f0f2 100644 --- a/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/connection/ConnectionService.java +++ b/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/connection/ConnectionService.java @@ -227,7 +227,9 @@ public class ConnectionService { StringBuilder connectionSearchFilter = new StringBuilder(); // Add the prefix to the search filter, prefix filter searches for guacConfigGroups with the userDN as the member attribute value - connectionSearchFilter.append("(&(objectClass=guacConfigGroup)(|(member="); + connectionSearchFilter.append("(&"); + connectionSearchFilter.append(confService.getConnectionSearchFilter()); + connectionSearchFilter.append("(|(member="); connectionSearchFilter.append(escapingService.escapeLDAPSearchFilter(userDN)); connectionSearchFilter.append(")"); @@ -239,7 +241,7 @@ public class ConnectionService { LDAPSearchResults userRoleGroupResults = ldapConnection.search( groupBaseDN, LDAPConnection.SCOPE_SUB, - "(&(!(objectClass=guacConfigGroup))(member=" + escapingService.escapeLDAPSearchFilter(userDN) + "))", + "(&(!" + confService.getConnectionSearchFilter() + ")(member=" + escapingService.escapeLDAPSearchFilter(userDN) + "))", null, false, confService.getLDAPSearchConstraints() diff --git a/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/user/UserService.java b/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/user/UserService.java index f7c571678..f58b410c4 100644 --- a/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/user/UserService.java +++ b/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/user/UserService.java @@ -85,11 +85,18 @@ public class UserService { try { + StringBuilder userSearchFilter = new StringBuilder(); + userSearchFilter.append("(&"); + userSearchFilter.append(confService.getUserSearchFilter()); + userSearchFilter.append("(" + escapeService.escapeLDAPSearchFilter(usernameAttribute) + "=*)"); + userSearchFilter.append(")"); + + // Find all Guacamole users underneath base DN LDAPSearchResults results = ldapConnection.search( confService.getUserBaseDN(), LDAPConnection.SCOPE_SUB, - "(&(objectClass=*)(" + escapingService.escapeLDAPSearchFilter(usernameAttribute) + "=*))", + userSearchFilter.toString(), null, false, confService.getLDAPSearchConstraints() @@ -189,7 +196,9 @@ public class UserService { // Build LDAP query for users having at least one username attribute // with the specified username as its value - StringBuilder ldapQuery = new StringBuilder("(&(objectClass=*)"); + StringBuilder ldapQuery = new StringBuilder(); + ldapQuery.append("(&"); + ldapQuery.append(confService.getUserSearchFilter()); // Include all attributes within OR clause if there are more than one if (usernameAttributes.size() > 1) From c5fe3d1df35305b5824fea924571bbbc8032a040 Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Mon, 20 Mar 2017 22:29:55 -0400 Subject: [PATCH 2/6] GUACAMOLE-101: Update comments to some of the changes. --- .../apache/guacamole/auth/ldap/ConfigurationService.java | 4 ++-- .../apache/guacamole/auth/ldap/LDAPGuacamoleProperties.java | 4 ++-- .../org/apache/guacamole/auth/ldap/user/UserService.java | 6 ++++-- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ConfigurationService.java b/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ConfigurationService.java index 19df4839c..2131797a2 100644 --- a/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ConfigurationService.java +++ b/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ConfigurationService.java @@ -295,12 +295,12 @@ public class ConfigurationService { /** * Returns the search filter that should be used when querying the * LDAP server for Guacamole connections. If no filter is specified, - * null is returned. + * the default of objectClass=guacConfigGroup is returned. * * @return * The search filter that should be used when querying the * LDAP server for connections for Guacamole, or - * null if no filter is specified. + * objectClass=guacConfigGroup if no filter is specified. * * @throws GuacamoleException * If guacamole.properties cannot be parsed. diff --git a/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/LDAPGuacamoleProperties.java b/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/LDAPGuacamoleProperties.java index 691a6fca9..5f49a8c9c 100644 --- a/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/LDAPGuacamoleProperties.java +++ b/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/LDAPGuacamoleProperties.java @@ -165,7 +165,7 @@ public class LDAPGuacamoleProperties { }; /** - * A search filter to apply to the user LDAP query. + * A search filter to apply to user LDAP queries. */ public static final StringGuacamoleProperty LDAP_USER_SEARCH_FILTER = new StringGuacamoleProperty() { @@ -175,7 +175,7 @@ public class LDAPGuacamoleProperties { }; /** - * A search filter to apply to the connection LDAP query. + * A search filter to apply to connection LDAP queries. */ public static final StringGuacamoleProperty LDAP_CONNECTION_SEARCH_FILTER = new StringGuacamoleProperty() { diff --git a/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/user/UserService.java b/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/user/UserService.java index f58b410c4..94763e6f5 100644 --- a/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/user/UserService.java +++ b/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/user/UserService.java @@ -85,10 +85,12 @@ public class UserService { try { + // Build a filter using the configured or default user search filter + // to find all user objects in the LDAP tree StringBuilder userSearchFilter = new StringBuilder(); userSearchFilter.append("(&"); userSearchFilter.append(confService.getUserSearchFilter()); - userSearchFilter.append("(" + escapeService.escapeLDAPSearchFilter(usernameAttribute) + "=*)"); + userSearchFilter.append("(" + escapingService.escapeLDAPSearchFilter(usernameAttribute) + "=*)"); userSearchFilter.append(")"); @@ -195,7 +197,7 @@ public class UserService { List usernameAttributes = confService.getUsernameAttributes(); // Build LDAP query for users having at least one username attribute - // with the specified username as its value + // and with the configured or default search filter StringBuilder ldapQuery = new StringBuilder(); ldapQuery.append("(&"); ldapQuery.append(confService.getUserSearchFilter()); From 5c768384bc9b2a92b9196aedc930ed0d536b5994 Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Tue, 28 Mar 2017 07:41:52 -0400 Subject: [PATCH 3/6] GUACAMOLE-101: Corrections to comments for proper LDAP filter syntax. --- .../org/apache/guacamole/auth/ldap/ConfigurationService.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ConfigurationService.java b/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ConfigurationService.java index 2131797a2..eec032485 100644 --- a/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ConfigurationService.java +++ b/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ConfigurationService.java @@ -275,12 +275,12 @@ public class ConfigurationService { /** * Returns the search filter that should be used when querying the * LDAP server for Guacamole users. If no filter is specified, - * a default of objectClass=* is returned. + * a default of "(objectClass=*)" is returned. * * @return * The search filter that should be used when querying the * LDAP server for users that are valid in Guacamole, or - * objectClass=* if not specified. + * "(objectClass=*)" if not specified. * * @throws GuacamoleException * If guacamole.properties cannot be parsed. From db9876e736a08a5be538e4b5a45de19e906cd543 Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Tue, 28 Mar 2017 07:50:08 -0400 Subject: [PATCH 4/6] GUACAMOLE-101: Remove connection search filter changes. --- .../auth/ldap/ConfigurationService.java | 20 ------------------- .../auth/ldap/LDAPGuacamoleProperties.java | 10 ---------- .../ldap/connection/ConnectionService.java | 6 ++---- 3 files changed, 2 insertions(+), 34 deletions(-) diff --git a/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ConfigurationService.java b/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ConfigurationService.java index eec032485..c7e4819d1 100644 --- a/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ConfigurationService.java +++ b/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ConfigurationService.java @@ -292,24 +292,4 @@ public class ConfigurationService { ); } - /** - * Returns the search filter that should be used when querying the - * LDAP server for Guacamole connections. If no filter is specified, - * the default of objectClass=guacConfigGroup is returned. - * - * @return - * The search filter that should be used when querying the - * LDAP server for connections for Guacamole, or - * objectClass=guacConfigGroup if no filter is specified. - * - * @throws GuacamoleException - * If guacamole.properties cannot be parsed. - */ - public String getConnectionSearchFilter() throws GuacamoleException { - return environment.getProperty( - LDAPGuacamoleProperties.LDAP_CONNECTION_SEARCH_FILTER, - "(objectClass=guacConfigGroup)" - ); - } - } diff --git a/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/LDAPGuacamoleProperties.java b/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/LDAPGuacamoleProperties.java index 5f49a8c9c..e13264dd8 100644 --- a/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/LDAPGuacamoleProperties.java +++ b/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/LDAPGuacamoleProperties.java @@ -174,14 +174,4 @@ public class LDAPGuacamoleProperties { }; - /** - * A search filter to apply to connection LDAP queries. - */ - public static final StringGuacamoleProperty LDAP_CONNECTION_SEARCH_FILTER = new StringGuacamoleProperty() { - - @Override - public String getName() { return "ldap-connection-search-filter"; } - - }; - } diff --git a/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/connection/ConnectionService.java b/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/connection/ConnectionService.java index 04e57f0f2..d256ebb3b 100644 --- a/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/connection/ConnectionService.java +++ b/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/connection/ConnectionService.java @@ -227,9 +227,7 @@ public class ConnectionService { StringBuilder connectionSearchFilter = new StringBuilder(); // Add the prefix to the search filter, prefix filter searches for guacConfigGroups with the userDN as the member attribute value - connectionSearchFilter.append("(&"); - connectionSearchFilter.append(confService.getConnectionSearchFilter()); - connectionSearchFilter.append("(|(member="); + connectionSearchFilter.append("(&(objectClass=guacConfigGroup)(|(member="); connectionSearchFilter.append(escapingService.escapeLDAPSearchFilter(userDN)); connectionSearchFilter.append(")"); @@ -241,7 +239,7 @@ public class ConnectionService { LDAPSearchResults userRoleGroupResults = ldapConnection.search( groupBaseDN, LDAPConnection.SCOPE_SUB, - "(&(!" + confService.getConnectionSearchFilter() + ")(member=" + escapingService.escapeLDAPSearchFilter(userDN) + "))", + "(&(!(objectClass=guacConfigGroup))(member=" + escapingService.escapeLDAPSearchFilter(userDN) + "))", null, false, confService.getLDAPSearchConstraints() From f54af15f0b89268bdb5541407ad9053e18b33c74 Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Tue, 28 Mar 2017 11:29:46 -0400 Subject: [PATCH 5/6] GUACAMOLE-101: Avoid using both StringBuilder and String concatenation. --- .../java/org/apache/guacamole/auth/ldap/user/UserService.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/user/UserService.java b/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/user/UserService.java index 94763e6f5..d2c0cd725 100644 --- a/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/user/UserService.java +++ b/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/user/UserService.java @@ -90,7 +90,9 @@ public class UserService { StringBuilder userSearchFilter = new StringBuilder(); userSearchFilter.append("(&"); userSearchFilter.append(confService.getUserSearchFilter()); - userSearchFilter.append("(" + escapingService.escapeLDAPSearchFilter(usernameAttribute) + "=*)"); + userSearchFilter.append("("); + userSearchFilter.append(escapingService.escapeLDAPSearchFilter(usernameAttribute)); + userSearchFilter.append("=*)"); userSearchFilter.append(")"); From b91d446f1fa79cdc4c68f80ea68f8a3646505c8d Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Tue, 28 Mar 2017 11:39:37 -0400 Subject: [PATCH 6/6] GUACAMOLE-101: Collapse StringBuilder append lines, remove extra space. --- .../java/org/apache/guacamole/auth/ldap/user/UserService.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/user/UserService.java b/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/user/UserService.java index d2c0cd725..91f1636e5 100644 --- a/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/user/UserService.java +++ b/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/user/UserService.java @@ -92,10 +92,8 @@ public class UserService { userSearchFilter.append(confService.getUserSearchFilter()); userSearchFilter.append("("); userSearchFilter.append(escapingService.escapeLDAPSearchFilter(usernameAttribute)); - userSearchFilter.append("=*)"); - userSearchFilter.append(")"); + userSearchFilter.append("=*))"); - // Find all Guacamole users underneath base DN LDAPSearchResults results = ldapConnection.search( confService.getUserBaseDN(),