From 8c284399b17fc9cb9818f6ed07dd6beb75368a7a Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Tue, 20 Oct 2015 14:57:09 -0700 Subject: [PATCH 01/17] GUAC-1115: Accept multiple username attributes. --- .../ldap/AuthenticationProviderService.java | 86 +++++++++++++-- .../auth/ldap/ConfigurationService.java | 10 +- .../auth/ldap/LDAPGuacamoleProperties.java | 11 +- .../auth/ldap/StringListProperty.java | 63 +++++++++++ .../guacamole/auth/ldap/user/UserService.java | 103 +++++++++++------- 5 files changed, 216 insertions(+), 57 deletions(-) create mode 100644 extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/StringListProperty.java diff --git a/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/AuthenticationProviderService.java b/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/AuthenticationProviderService.java index 63cbeddfe..4d63b5fea 100644 --- a/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/AuthenticationProviderService.java +++ b/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/AuthenticationProviderService.java @@ -27,6 +27,7 @@ import com.google.inject.Provider; import com.novell.ldap.LDAPConnection; import com.novell.ldap.LDAPException; import java.io.UnsupportedEncodingException; +import java.util.List; import org.glyptodon.guacamole.auth.ldap.user.AuthenticatedUser; import org.glyptodon.guacamole.auth.ldap.user.UserContext; import org.glyptodon.guacamole.GuacamoleException; @@ -73,6 +74,42 @@ public class AuthenticationProviderService { @Inject private Provider userContextProvider; + /** + * Determines the DN which corresponds to the user having the given + * username. The DN will either be derived directly from the user base DN, + * or queried from the LDAP server, depending on how LDAP authentication + * has been configured. + * + * @param username + * The username of the user whose corresponding DN should be returned. + * + * @return + * The DN which corresponds to the user having the given username. + * + * @throws GuacamoleException + * If required properties are missing, and thus the user DN cannot be + * determined. + */ + private String getUserBindDN(String username) + throws GuacamoleException { + + // Pull username attributes from properties + List usernameAttributes = confService.getUsernameAttributes(); + if (usernameAttributes.isEmpty()) + return null; + + // We need exactly one base DN to derive the user DN + if (usernameAttributes.size() != 1) + return null; + + // Derive user DN from base DN + return + escapingService.escapeDN(usernameAttributes.get(0)) + + "=" + escapingService.escapeDN(username) + + "," + confService.getUserBaseDN(); + + } + /** * Binds to the LDAP server using the provided Guacamole credentials. The * DN of the user is derived using the LDAP configuration properties @@ -94,15 +131,18 @@ public class AuthenticationProviderService { LDAPConnection ldapConnection; + // Get username and password from credentials + String username = credentials.getUsername(); + String password = credentials.getPassword(); + // Require username - if (credentials.getUsername() == null) { + if (username == null || username.isEmpty()) { logger.debug("Anonymous bind is not currently allowed by the LDAP authentication provider."); return null; } // Require password, and do not allow anonymous binding - if (credentials.getPassword() == null - || credentials.getPassword().length() == 0) { + if (password == null || password.isEmpty()) { logger.debug("Anonymous bind is not currently allowed by the LDAP authentication provider."); return null; } @@ -124,16 +164,17 @@ public class AuthenticationProviderService { // Bind using provided credentials try { - // Construct user DN - String userDN = - escapingService.escapeDN(confService.getUsernameAttribute()) - + "=" + escapingService.escapeDN(credentials.getUsername()) - + "," + confService.getUserBaseDN(); + // Determine user DN + String userDN = getUserBindDN(username); + if (userDN == null) { + logger.error("Unable to determine DN for user \"{}\".", username); + return null; + } // Bind as user try { ldapConnection.bind(LDAPConnection.LDAP_V3, userDN, - credentials.getPassword().getBytes("UTF-8")); + password.getBytes("UTF-8")); } catch (UnsupportedEncodingException e) { logger.error("Unexpected lack of support for UTF-8: {}", e.getMessage()); @@ -157,11 +198,36 @@ public class AuthenticationProviderService { } + /** + * Returns an AuthenticatedUser representing the user authenticated by the + * given credentials. + * + * @param credentials + * The credentials to use for authentication. + * + * @return + * An AuthenticatedUser representing the user authenticated by the + * given credentials. + * + * @throws GuacamoleException + * If an error occurs while authenticating the user, or if access is + * denied. + */ public AuthenticatedUser authenticateUser(Credentials credentials) throws GuacamoleException { // Attempt bind - LDAPConnection ldapConnection = bindAs(credentials); + LDAPConnection ldapConnection; + try { + ldapConnection = bindAs(credentials); + } + catch (GuacamoleException e) { + logger.error("Cannot bind with LDAP server: {}", e.getMessage()); + logger.debug("Error binding with LDAP server.", e); + ldapConnection = null; + } + + // If bind fails, permission to login is denied if (ldapConnection == null) throw new GuacamoleInvalidCredentialsException("Permission denied.", CredentialsInfo.USERNAME_PASSWORD); diff --git a/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/ConfigurationService.java b/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/ConfigurationService.java index 692300499..7d3f77b29 100644 --- a/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/ConfigurationService.java +++ b/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/ConfigurationService.java @@ -23,6 +23,8 @@ package org.glyptodon.guacamole.auth.ldap; import com.google.inject.Inject; +import java.util.Collections; +import java.util.List; import org.glyptodon.guacamole.GuacamoleException; import org.glyptodon.guacamole.environment.Environment; @@ -77,21 +79,21 @@ public class ConfigurationService { } /** - * Returns the username attribute which should be used to query and bind + * Returns all username attributes which should be used to query and bind * users using the LDAP directory. By default, this will be "uid" - a * common attribute used for this purpose. * * @return - * The username attribute which should be used to query and bind users + * The username attributes which should be used to query and bind users * using the LDAP directory. * * @throws GuacamoleException * If guacamole.properties cannot be parsed. */ - public String getUsernameAttribute() throws GuacamoleException { + public List getUsernameAttributes() throws GuacamoleException { return environment.getProperty( LDAPGuacamoleProperties.LDAP_USERNAME_ATTRIBUTE, - "uid" + Collections.singletonList("uid") ); } diff --git a/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/LDAPGuacamoleProperties.java b/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/LDAPGuacamoleProperties.java index 8fabf816e..ff7c76c1e 100644 --- a/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/LDAPGuacamoleProperties.java +++ b/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/LDAPGuacamoleProperties.java @@ -62,11 +62,14 @@ public class LDAPGuacamoleProperties { }; /** - * The attribute which identifies users. This attribute must be part of - * each user's DN such that the concatenation of this attribute and - * LDAP_USER_BASE_DN equals the users full DN. + * The attribute or attributes which identify users. One of these + * attributes must be present within the each Guacamole user's record in + * the LDAP directory. If the LDAP authentication will not be given its own + * credentials for querying other LDAP users, this list may contain only + * one attribute, and the concatenation of that attribute and the value of + * LDAP_USER_BASE_DN must equal the user's full DN. */ - public static final StringGuacamoleProperty LDAP_USERNAME_ATTRIBUTE = new StringGuacamoleProperty() { + public static final StringListProperty LDAP_USERNAME_ATTRIBUTE = new StringListProperty() { @Override public String getName() { return "ldap-username-attribute"; } diff --git a/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/StringListProperty.java b/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/StringListProperty.java new file mode 100644 index 000000000..f5f75cd1c --- /dev/null +++ b/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/StringListProperty.java @@ -0,0 +1,63 @@ +/* + * Copyright (C) 2015 Glyptodon LLC + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +package org.glyptodon.guacamole.auth.ldap; + +import java.util.Arrays; +import java.util.List; +import java.util.regex.Pattern; +import org.glyptodon.guacamole.GuacamoleException; +import org.glyptodon.guacamole.properties.GuacamoleProperty; + +/** + * A GuacamoleProperty whose value is a List of Strings. The string value + * parsed to produce this list is a comma-delimited list. Duplicate values are + * ignored, as is any whitespace following delimiters. To maintain + * compatibility with the behavior of Java properties in general, only + * whitespace at the beginning of each value is ignored; trailing whitespace + * becomes part of the value. + * + * @author Michael Jumper + */ +public abstract class StringListProperty implements GuacamoleProperty> { + + /** + * A pattern which matches against the delimiters between values. This is + * currently simply a comma and any following whitespace. Parts of the + * input string which match this pattern will not be included in the parsed + * result. + */ + private static final Pattern DELIMITER_PATTERN = Pattern.compile(",\\s*"); + + @Override + public List parseValue(String values) throws GuacamoleException { + + // If no property provided, return null. + if (values == null) + return null; + + // Split string into a list of individual values + return Arrays.asList(DELIMITER_PATTERN.split(values)); + + } + +} diff --git a/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/user/UserService.java b/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/user/UserService.java index 9b72c0f44..9e9533dd9 100644 --- a/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/user/UserService.java +++ b/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/user/UserService.java @@ -64,6 +64,64 @@ public class UserService { @Inject private ConfigurationService confService; + /** + * Adds all Guacamole users accessible to the user currently bound under + * the given LDAP connection to the provided map. Only users with the + * specified attribute are added. If the same username is encountered + * multiple times, warnings about possible ambiguity will be logged. + * + * @param ldapConnection + * The current connection to the LDAP server, associated with the + * current user. + * + * @return + * All users accessible to the user currently bound under the given + * LDAP connection, as a map of connection identifier to corresponding + * user object. + * + * @throws GuacamoleException + * If an error occurs preventing retrieval of users. + */ + private void putAllUsers(Map users, LDAPConnection ldapConnection, + String usernameAttribute) throws GuacamoleException { + + try { + + // Find all Guacamole users underneath base DN + LDAPSearchResults results = ldapConnection.search( + confService.getUserBaseDN(), + LDAPConnection.SCOPE_SUB, + "(&(objectClass=*)(" + escapingService.escapeLDAPSearchFilter(usernameAttribute) + "=*))", + null, + false + ); + + // Read all visible users + while (results.hasMore()) { + + LDAPEntry entry = results.next(); + + // Get username from record + LDAPAttribute username = entry.getAttribute(usernameAttribute); + if (username == null) { + logger.warn("Queried user is missing the username attribute \"{}\".", usernameAttribute); + continue; + } + + // Store user using their username as the identifier + String identifier = username.getStringValue(); + if (users.put(identifier, new SimpleUser(identifier)) != null) + logger.warn("Possibly ambiguous user account: \"{}\".", identifier); + + } + + } + catch (LDAPException e) { + throw new GuacamoleServerException("Error while querying users.", e); + } + + } + /** * Returns all Guacamole users accessible to the user currently bound under * the given LDAP connection. @@ -83,46 +141,13 @@ public class UserService { public Map getUsers(LDAPConnection ldapConnection) throws GuacamoleException { - try { + // Build map of users by querying each username attribute separately + Map users = new HashMap(); + for (String usernameAttribute : confService.getUsernameAttributes()) + putAllUsers(users, ldapConnection, usernameAttribute); - // Get username attribute - String usernameAttribute = confService.getUsernameAttribute(); - - // Find all Guacamole users underneath base DN - LDAPSearchResults results = ldapConnection.search( - confService.getUserBaseDN(), - LDAPConnection.SCOPE_ONE, - "(&(objectClass=*)(" + escapingService.escapeLDAPSearchFilter(usernameAttribute) + "=*))", - null, - false - ); - - // Read all visible users - Map users = new HashMap(); - while (results.hasMore()) { - - LDAPEntry entry = results.next(); - - // Get common name (CN) - LDAPAttribute username = entry.getAttribute(usernameAttribute); - if (username == null) { - logger.warn("Queried user is missing the username attribute \"{}\".", usernameAttribute); - continue; - } - - // Store connection using cn for both identifier and name - String identifier = username.getStringValue(); - users.put(identifier, new SimpleUser(identifier)); - - } - - // Return map of all connections - return users; - - } - catch (LDAPException e) { - throw new GuacamoleServerException("Error while querying users.", e); - } + // Return map of all users + return users; } From b87afb9b5483e3d87989bc8218b1140e99f42b4e Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Tue, 20 Oct 2015 15:00:19 -0700 Subject: [PATCH 02/17] GUAC-1115: Correct documented semantics of LDAP_USER_BASE_DN. --- .../guacamole/auth/ldap/LDAPGuacamoleProperties.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/LDAPGuacamoleProperties.java b/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/LDAPGuacamoleProperties.java index ff7c76c1e..724739fdc 100644 --- a/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/LDAPGuacamoleProperties.java +++ b/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/LDAPGuacamoleProperties.java @@ -51,8 +51,10 @@ public class LDAPGuacamoleProperties { }; /** - * The base DN of users. All users must be direct children of this DN, - * varying only by LDAP_USERNAME_ATTRIBUTE. + * The base DN of users. All users must be contained somewhere within the + * subtree of this DN. If the LDAP authentication will not be given its own + * credentials for querying other LDAP users, all users must be direct + * children of this base DN, varying only by LDAP_USERNAME_ATTRIBUTE. */ public static final StringGuacamoleProperty LDAP_USER_BASE_DN = new StringGuacamoleProperty() { From abe709a71b808d69633904a2ce130a664bcd3b73 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Tue, 20 Oct 2015 15:18:17 -0700 Subject: [PATCH 03/17] GUAC-1115: Proceed even if an error prevents retrieval of all users in the directory (mitigates GUAC-1353). --- .../guacamole/auth/ldap/user/UserService.java | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/user/UserService.java b/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/user/UserService.java index 9e9533dd9..7599d5f10 100644 --- a/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/user/UserService.java +++ b/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/user/UserService.java @@ -143,8 +143,21 @@ public class UserService { // Build map of users by querying each username attribute separately Map users = new HashMap(); - for (String usernameAttribute : confService.getUsernameAttributes()) - putAllUsers(users, ldapConnection, usernameAttribute); + for (String usernameAttribute : confService.getUsernameAttributes()) { + + // Attempt to pull all users with given attribute + try { + putAllUsers(users, ldapConnection, usernameAttribute); + } + + // Log any errors non-fatally + catch (GuacamoleException e) { + logger.warn("Could not query list of all users for attribute \"{}\": {}", + usernameAttribute, e.getMessage()); + logger.debug("Error querying list of all users.", e); + } + + } // Return map of all users return users; From cbfcd8b1e47dd9f0e255176340c6f20e38e7691b Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Tue, 20 Oct 2015 15:19:04 -0700 Subject: [PATCH 04/17] GUAC-1115: Add and document ldap-search-bind-* properties. --- .../auth/ldap/LDAPGuacamoleProperties.java | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/LDAPGuacamoleProperties.java b/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/LDAPGuacamoleProperties.java index 724739fdc..6e51c6f99 100644 --- a/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/LDAPGuacamoleProperties.java +++ b/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/LDAPGuacamoleProperties.java @@ -98,4 +98,30 @@ public class LDAPGuacamoleProperties { }; + /** + * The DN of the user that the LDAP authentication should bind as when + * searching for the user accounts of users attempting to log in. If not + * specified, the DNs of users attempting to log in will be derived from + * the LDAP_BASE_DN and LDAP_USERNAME_ATTRIBUTE directly. + */ + public static final StringGuacamoleProperty LDAP_SEARCH_BIND_DN = new StringGuacamoleProperty() { + + @Override + public String getName() { return "ldap-search-bind-dn"; } + + }; + + /** + * The password to provide to the LDAP server when binding as + * LDAP_SEARCH_BIND_DN. If LDAP_SEARCH_BIND_DN is not specified, this + * property has no effect. If this property is not specified, no password + * will be provided when attempting to bind as LDAP_SEARCH_BIND_DN. + */ + public static final StringGuacamoleProperty LDAP_SEARCH_BIND_PASSWORD = new StringGuacamoleProperty() { + + @Override + public String getName() { return "ldap-search-bind-password"; } + + }; + } From 1c7794b87066030490d19ead12cbbc90bc87827b Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Tue, 20 Oct 2015 15:21:47 -0700 Subject: [PATCH 05/17] GUAC-1115: Treat empty lists as blank. --- .../guacamole/auth/ldap/AuthenticationProviderService.java | 2 -- .../glyptodon/guacamole/auth/ldap/StringListProperty.java | 6 +++++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/AuthenticationProviderService.java b/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/AuthenticationProviderService.java index 4d63b5fea..cae0f9030 100644 --- a/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/AuthenticationProviderService.java +++ b/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/AuthenticationProviderService.java @@ -95,8 +95,6 @@ public class AuthenticationProviderService { // Pull username attributes from properties List usernameAttributes = confService.getUsernameAttributes(); - if (usernameAttributes.isEmpty()) - return null; // We need exactly one base DN to derive the user DN if (usernameAttributes.size() != 1) diff --git a/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/StringListProperty.java b/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/StringListProperty.java index f5f75cd1c..e545e56ab 100644 --- a/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/StringListProperty.java +++ b/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/StringListProperty.java @@ -56,7 +56,11 @@ public abstract class StringListProperty implements GuacamoleProperty stringValues = Arrays.asList(DELIMITER_PATTERN.split(values)); + if (stringValues.isEmpty()) + return null; + + return stringValues; } From c563fa43b4f2572d6ef78083bb741716e2fea8ec Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Tue, 20 Oct 2015 15:24:26 -0700 Subject: [PATCH 06/17] GUAC-1115: Warn if we need to directly derive the user DN, but can't because multiple username attributes were provided. --- .../guacamole/auth/ldap/AuthenticationProviderService.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/AuthenticationProviderService.java b/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/AuthenticationProviderService.java index cae0f9030..d2b56c080 100644 --- a/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/AuthenticationProviderService.java +++ b/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/AuthenticationProviderService.java @@ -97,8 +97,10 @@ public class AuthenticationProviderService { List usernameAttributes = confService.getUsernameAttributes(); // We need exactly one base DN to derive the user DN - if (usernameAttributes.size() != 1) + if (usernameAttributes.size() != 1) { + logger.warn("Cannot directly derive user DN when multiple username attributes are specified"); return null; + } // Derive user DN from base DN return From eca825c899999f9bfd3ae84328586a4d92e447ef Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Fri, 23 Oct 2015 15:17:57 -0700 Subject: [PATCH 07/17] GUAC-1115: Split bindAs() into LDAP- and Guacamole-specific versions of the same. --- .../ldap/AuthenticationProviderService.java | 87 ++++++++++++------- 1 file changed, 56 insertions(+), 31 deletions(-) diff --git a/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/AuthenticationProviderService.java b/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/AuthenticationProviderService.java index d2b56c080..cde57227e 100644 --- a/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/AuthenticationProviderService.java +++ b/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/AuthenticationProviderService.java @@ -111,13 +111,14 @@ public class AuthenticationProviderService { } /** - * Binds to the LDAP server using the provided Guacamole credentials. The - * DN of the user is derived using the LDAP configuration properties - * provided in guacamole.properties, as is the server hostname and port - * information. + * Binds to the LDAP server using the provided user DN and password. * - * @param credentials - * The credentials to use to bind to the LDAP server. + * @param userDN + * The DN of the user to bind as, or null to bind anonymously. + * + * @param password + * The password to use when binding as the specified user, or null to + * attempt to bind without a password. * * @return * A bound LDAP connection, or null if the connection could not be @@ -126,27 +127,11 @@ public class AuthenticationProviderService { * @throws GuacamoleException * If an error occurs while binding to the LDAP server. */ - private LDAPConnection bindAs(Credentials credentials) - throws GuacamoleException { + private LDAPConnection bindAs(String userDN, String password) + throws GuacamoleException { LDAPConnection ldapConnection; - // Get username and password from credentials - String username = credentials.getUsername(); - String password = credentials.getPassword(); - - // Require username - if (username == null || username.isEmpty()) { - logger.debug("Anonymous bind is not currently allowed by the LDAP authentication provider."); - return null; - } - - // Require password, and do not allow anonymous binding - if (password == null || password.isEmpty()) { - logger.debug("Anonymous bind is not currently allowed by the LDAP authentication provider."); - return null; - } - // Connect to LDAP server try { ldapConnection = new LDAPConnection(); @@ -164,13 +149,6 @@ public class AuthenticationProviderService { // Bind using provided credentials try { - // Determine user DN - String userDN = getUserBindDN(username); - if (userDN == null) { - logger.error("Unable to determine DN for user \"{}\".", username); - return null; - } - // Bind as user try { ldapConnection.bind(LDAPConnection.LDAP_V3, userDN, @@ -195,6 +173,53 @@ public class AuthenticationProviderService { } return ldapConnection; + + } + + /** + * Binds to the LDAP server using the provided Guacamole credentials. The + * DN of the user is derived using the LDAP configuration properties + * provided in guacamole.properties, as is the server hostname and port + * information. + * + * @param credentials + * The credentials to use to bind to the LDAP server. + * + * @return + * A bound LDAP connection, or null if the connection could not be + * bound. + * + * @throws GuacamoleException + * If an error occurs while binding to the LDAP server. + */ + private LDAPConnection bindAs(Credentials credentials) + throws GuacamoleException { + + // Get username and password from credentials + String username = credentials.getUsername(); + String password = credentials.getPassword(); + + // Require username + if (username == null || username.isEmpty()) { + logger.debug("Anonymous bind is not currently allowed by the LDAP authentication provider."); + return null; + } + + // Require password, and do not allow anonymous binding + if (password == null || password.isEmpty()) { + logger.debug("Anonymous bind is not currently allowed by the LDAP authentication provider."); + return null; + } + + // Determine user DN + String userDN = getUserBindDN(username); + if (userDN == null) { + logger.error("Unable to determine DN for user \"{}\".", username); + return null; + } + + // Bind using user's DN + return bindAs(userDN, password); } From 947e7b100411581017fa012697674d43495a8904 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Fri, 23 Oct 2015 15:18:33 -0700 Subject: [PATCH 08/17] GUAC-1115: Add LDAP query for retrieving the DNs which correspond to a particular user account. --- .../guacamole/auth/ldap/user/UserService.java | 105 ++++++++++++++++++ 1 file changed, 105 insertions(+) diff --git a/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/user/UserService.java b/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/user/UserService.java index 7599d5f10..a7af395ef 100644 --- a/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/user/UserService.java +++ b/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/user/UserService.java @@ -28,7 +28,9 @@ import com.novell.ldap.LDAPConnection; import com.novell.ldap.LDAPEntry; import com.novell.ldap.LDAPException; import com.novell.ldap.LDAPSearchResults; +import java.util.ArrayList; import java.util.HashMap; +import java.util.List; import java.util.Map; import org.glyptodon.guacamole.auth.ldap.ConfigurationService; import org.glyptodon.guacamole.auth.ldap.EscapingService; @@ -164,4 +166,107 @@ public class UserService { } + /** + * Generates a properly-escaped LDAP query which finds all objects having + * at least one username attribute set to the specified username, where + * the possible username attributes are defined within + * guacamole.properties. + * + * @param username + * The username that the resulting LDAP query should search for within + * objects within the LDAP directory. + * + * @return + * An LDAP query which will search for arbitrary LDAP objects + * containing at least one username attribute set to the specified + * username. + * + * @throws GuacamoleException + * If the LDAP query cannot be generated because the list of username + * attributes cannot be parsed from guacamole.properties. + */ + private String generateLDAPQuery(String username) + throws GuacamoleException { + + List usernameAttributes = confService.getUsernameAttributes(); + + // Build LDAP query for users having at least one username attribute + // with the specified username as its value + StringBuilder ldapQuery = new StringBuilder("(&(objectClass=*)"); + + // Include all attributes within OR clause if there are more than one + if (usernameAttributes.size() > 1) + ldapQuery.append("(|"); + + // Add equality comparison for each possible username attribute + for (String usernameAttribute : usernameAttributes) { + ldapQuery.append("("); + ldapQuery.append(escapingService.escapeLDAPSearchFilter(usernameAttribute)); + ldapQuery.append("="); + ldapQuery.append(escapingService.escapeLDAPSearchFilter(username)); + ldapQuery.append(")"); + } + + // Close OR clause, if any + if (usernameAttributes.size() > 1) + ldapQuery.append(")"); + + return ldapQuery.toString(); + + } + + /** + * Returns a list of all DNs corresponding to the users having the given + * username. If multiple username attributes are defined, or if uniqueness + * is not enforced across the username attribute, it is possible that this + * will return multiple DNs. + * + * @param ldapConnection + * The connection to the LDAP server to use when querying user DNs. + * + * @param username + * The username of the user whose corresponding user account DNs are + * to be retrieved. + * + * @return + * A list of all DNs corresponding to the users having the given + * username. If no such DNs exist, this list will be empty. + * + * @throws GuacamoleException + * If an error occurs while querying the user DNs, or if the username + * attribute property cannot be parsed within guacamole.properties. + */ + public List getUserDNs(LDAPConnection ldapConnection, + String username) throws GuacamoleException { + + try { + + List userDNs = new ArrayList(); + + // Find all Guacamole users underneath base DN and matching the + // specified username + LDAPSearchResults results = ldapConnection.search( + confService.getUserBaseDN(), + LDAPConnection.SCOPE_SUB, + generateLDAPQuery(username), + null, + false + ); + + // Add all DNs for found users + while (results.hasMore()) { + LDAPEntry entry = results.next(); + userDNs.add(entry.getDN()); + } + + // Return all discovered DNs (if any) + return userDNs; + + } + catch (LDAPException e) { + throw new GuacamoleServerException("Error while query user DNs.", e); + } + + } + } From bf53b5515dd64f7d8779a21fd8f4a2f9761a42f5 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Fri, 23 Oct 2015 15:22:31 -0700 Subject: [PATCH 09/17] GUAC-1115: Move DN derivation into UserService. --- .../ldap/AuthenticationProviderService.java | 28 +++++---------- .../guacamole/auth/ldap/user/UserService.java | 36 +++++++++++++++++++ 2 files changed, 44 insertions(+), 20 deletions(-) diff --git a/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/AuthenticationProviderService.java b/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/AuthenticationProviderService.java index cde57227e..a9098fbfe 100644 --- a/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/AuthenticationProviderService.java +++ b/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/AuthenticationProviderService.java @@ -27,10 +27,10 @@ import com.google.inject.Provider; import com.novell.ldap.LDAPConnection; import com.novell.ldap.LDAPException; import java.io.UnsupportedEncodingException; -import java.util.List; import org.glyptodon.guacamole.auth.ldap.user.AuthenticatedUser; import org.glyptodon.guacamole.auth.ldap.user.UserContext; import org.glyptodon.guacamole.GuacamoleException; +import org.glyptodon.guacamole.auth.ldap.user.UserService; import org.glyptodon.guacamole.net.auth.Credentials; import org.glyptodon.guacamole.net.auth.credentials.CredentialsInfo; import org.glyptodon.guacamole.net.auth.credentials.GuacamoleInvalidCredentialsException; @@ -50,18 +50,18 @@ public class AuthenticationProviderService { */ private final Logger logger = LoggerFactory.getLogger(AuthenticationProviderService.class); - /** - * Service for escaping parts of LDAP queries. - */ - @Inject - private EscapingService escapingService; - /** * Service for retrieving LDAP server configuration information. */ @Inject private ConfigurationService confService; + /** + * Service for retrieving users and their corresponding LDAP DNs. + */ + @Inject + private UserService userService; + /** * Provider for AuthenticatedUser objects. */ @@ -93,20 +93,8 @@ public class AuthenticationProviderService { private String getUserBindDN(String username) throws GuacamoleException { - // Pull username attributes from properties - List usernameAttributes = confService.getUsernameAttributes(); - - // We need exactly one base DN to derive the user DN - if (usernameAttributes.size() != 1) { - logger.warn("Cannot directly derive user DN when multiple username attributes are specified"); - return null; - } - // Derive user DN from base DN - return - escapingService.escapeDN(usernameAttributes.get(0)) - + "=" + escapingService.escapeDN(username) - + "," + confService.getUserBaseDN(); + return userService.deriveUserDN(username); } diff --git a/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/user/UserService.java b/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/user/UserService.java index a7af395ef..0f01bde36 100644 --- a/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/user/UserService.java +++ b/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/user/UserService.java @@ -269,4 +269,40 @@ public class UserService { } + /** + * Determines the DN which corresponds to the user having the given + * username. The DN will either be derived directly from the user base DN, + * or queried from the LDAP server, depending on how LDAP authentication + * has been configured. + * + * @param username + * The username of the user whose corresponding DN should be returned. + * + * @return + * The DN which corresponds to the user having the given username. + * + * @throws GuacamoleException + * If required properties are missing, and thus the user DN cannot be + * determined. + */ + public String deriveUserDN(String username) + throws GuacamoleException { + + // Pull username attributes from properties + List usernameAttributes = confService.getUsernameAttributes(); + + // We need exactly one base DN to derive the user DN + if (usernameAttributes.size() != 1) { + logger.warn("Cannot directly derive user DN when multiple username attributes are specified"); + return null; + } + + // Derive user DN from base DN + return + escapingService.escapeDN(usernameAttributes.get(0)) + + "=" + escapingService.escapeDN(username) + + "," + confService.getUserBaseDN(); + + } + } From 725e7d553cf4b41d317536675d23e1b8b8d64c3b Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Fri, 23 Oct 2015 15:38:44 -0700 Subject: [PATCH 10/17] GUAC-1115: Add ConfigurationService functions for retrieving search DN and password. --- .../auth/ldap/ConfigurationService.java | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/ConfigurationService.java b/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/ConfigurationService.java index 7d3f77b29..3550d8a4f 100644 --- a/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/ConfigurationService.java +++ b/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/ConfigurationService.java @@ -133,4 +133,42 @@ public class ConfigurationService { ); } + /** + * Returns the DN that should be used when searching for the DNs of users + * attempting to authenticate. If no such search should be performed, null + * is returned. + * + * @return + * The DN that should be used when searching for the DNs of users + * attempting to authenticate, or null if no such search should be + * performed. + * + * @throws GuacamoleException + * If guacamole.properties cannot be parsed. + */ + public String getSearchBindDN() throws GuacamoleException { + return environment.getProperty( + LDAPGuacamoleProperties.LDAP_SEARCH_BIND_DN + ); + } + + /** + * Returns the password that should be used when binding to the LDAP server + * using the DN returned by getSearchBindDN(). If no password should be + * used, null is returned. + * + * @return + * The password that should be used when binding to the LDAP server + * using the DN returned by getSearchBindDN(), or null if no password + * should be used. + * + * @throws GuacamoleException + * If guacamole.properties cannot be parsed. + */ + public String getSearchBindPassword() throws GuacamoleException { + return environment.getProperty( + LDAPGuacamoleProperties.LDAP_SEARCH_BIND_PASSWORD + ); + } + } From 529dccf675cbdf696d433a7dadf72ec9049305c5 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Fri, 23 Oct 2015 15:51:22 -0700 Subject: [PATCH 11/17] GUAC-1115: Fix NPE in password conversion. --- .../ldap/AuthenticationProviderService.java | 110 ++++++++++++------ 1 file changed, 76 insertions(+), 34 deletions(-) diff --git a/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/AuthenticationProviderService.java b/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/AuthenticationProviderService.java index a9098fbfe..1b3f59afd 100644 --- a/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/AuthenticationProviderService.java +++ b/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/AuthenticationProviderService.java @@ -27,6 +27,7 @@ import com.google.inject.Provider; import com.novell.ldap.LDAPConnection; import com.novell.ldap.LDAPException; import java.io.UnsupportedEncodingException; +import java.util.List; import org.glyptodon.guacamole.auth.ldap.user.AuthenticatedUser; import org.glyptodon.guacamole.auth.ldap.user.UserContext; import org.glyptodon.guacamole.GuacamoleException; @@ -74,6 +75,28 @@ public class AuthenticationProviderService { @Inject private Provider userContextProvider; + /** + * Disconnects the given LDAP connection, logging any failure to do so + * appropriately. + * + * @param ldapConnection + * The LDAP connection to disconnect. + */ + private void disconnect(LDAPConnection ldapConnection) { + + // Attempt disconnect + try { + ldapConnection.disconnect(); + } + + // Warn if disconnect unexpectedly fails + catch (LDAPException e) { + logger.warn("Unable to disconnect from LDAP server: {}", e.getMessage()); + logger.debug("LDAP disconnect failed.", e); + } + + } + /** * Determines the DN which corresponds to the user having the given * username. The DN will either be derived directly from the user base DN, @@ -93,7 +116,43 @@ public class AuthenticationProviderService { private String getUserBindDN(String username) throws GuacamoleException { - // Derive user DN from base DN + // If a search DN is provided, search the LDAP directory for the DN + // corresponding to the given username + String searchBindDN = confService.getSearchBindDN(); + if (searchBindDN != null) { + + // Create an LDAP connection using the search account + LDAPConnection searchConnection = bindAs( + searchBindDN, + confService.getSearchBindPassword() + ); + + try { + + // Retrieve all DNs associated with the given username + List userDNs = userService.getUserDNs(searchConnection, username); + if (userDNs.isEmpty()) + return null; + + // Warn if multiple DNs exist for the same user + if (userDNs.size() != 1) { + logger.warn("Multiple DNs possible for user \"{}\": {}", username, userDNs); + return null; + } + + // Return the single possible DN + return userDNs.get(0); + + } + + // Always disconnect + finally { + disconnect(searchConnection); + } + + } + + // Otherwise, derive user DN from base DN return userService.deriveUserDN(username); } @@ -137,10 +196,15 @@ public class AuthenticationProviderService { // Bind using provided credentials try { - // Bind as user + byte[] passwordBytes; try { - ldapConnection.bind(LDAPConnection.LDAP_V3, userDN, - password.getBytes("UTF-8")); + + // Convert password into corresponding byte array + if (password != null) + passwordBytes = password.getBytes("UTF-8"); + else + passwordBytes = null; + } catch (UnsupportedEncodingException e) { logger.error("Unexpected lack of support for UTF-8: {}", e.getMessage()); @@ -148,15 +212,15 @@ public class AuthenticationProviderService { return null; } - // Disconnect if an error occurs during bind - catch (LDAPException e) { - ldapConnection.disconnect(); - throw e; - } + // Bind as user + ldapConnection.bind(LDAPConnection.LDAP_V3, userDN, passwordBytes); } + + // Disconnect if an error occurs during bind catch (LDAPException e) { logger.debug("LDAP bind failed.", e); + disconnect(ldapConnection); return null; } @@ -202,7 +266,7 @@ public class AuthenticationProviderService { // Determine user DN String userDN = getUserBindDN(username); if (userDN == null) { - logger.error("Unable to determine DN for user \"{}\".", username); + logger.debug("Unable to determine DN for user \"{}\".", username); return null; } @@ -255,18 +319,7 @@ public class AuthenticationProviderService { // Always disconnect finally { - - // Attempt disconnect - try { - ldapConnection.disconnect(); - } - - // Warn if disconnect unexpectedly fails - catch (LDAPException e) { - logger.warn("Unable to disconnect from LDAP server: {}", e.getMessage()); - logger.debug("LDAP disconnect failed.", e); - } - + disconnect(ldapConnection); } } @@ -305,18 +358,7 @@ public class AuthenticationProviderService { // Always disconnect finally { - - // Attempt disconnect - try { - ldapConnection.disconnect(); - } - - // Warn if disconnect unexpectedly fails - catch (LDAPException e) { - logger.warn("Unable to disconnect from LDAP server: {}", e.getMessage()); - logger.debug("LDAP disconnect failed.", e); - } - + disconnect(ldapConnection); } } From 80a6e4cac6a96323b5cb46a9fd1d41694fce633f Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Fri, 23 Oct 2015 15:51:40 -0700 Subject: [PATCH 12/17] GUAC-1115: Fix formatting of user query. --- .../org/glyptodon/guacamole/auth/ldap/user/UserService.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/user/UserService.java b/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/user/UserService.java index 0f01bde36..5761b6184 100644 --- a/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/user/UserService.java +++ b/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/user/UserService.java @@ -211,6 +211,9 @@ public class UserService { if (usernameAttributes.size() > 1) ldapQuery.append(")"); + // Close overall query (AND clause) + ldapQuery.append(")"); + return ldapQuery.toString(); } From 00bf24791fb53e369affea77f13eef685b7200af Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Fri, 23 Oct 2015 15:53:04 -0700 Subject: [PATCH 13/17] GUAC-1115: Ensure LDAP connection is always cleaned up. --- .../guacamole/auth/ldap/AuthenticationProviderService.java | 1 + 1 file changed, 1 insertion(+) diff --git a/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/AuthenticationProviderService.java b/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/AuthenticationProviderService.java index 1b3f59afd..541906986 100644 --- a/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/AuthenticationProviderService.java +++ b/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/AuthenticationProviderService.java @@ -209,6 +209,7 @@ public class AuthenticationProviderService { catch (UnsupportedEncodingException e) { logger.error("Unexpected lack of support for UTF-8: {}", e.getMessage()); logger.debug("Support for UTF-8 (as required by Java spec) not found.", e); + disconnect(ldapConnection); return null; } From bd497c40b12c21afd5be0b70ea424daa85fe79ce Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Fri, 23 Oct 2015 16:03:53 -0700 Subject: [PATCH 14/17] GUAC-1115: Move LDAP connection management into own service. --- .../ldap/AuthenticationProviderService.java | 112 ++------------ .../LDAPAuthenticationProviderModule.java | 1 + .../auth/ldap/LDAPConnectionService.java | 145 ++++++++++++++++++ 3 files changed, 157 insertions(+), 101 deletions(-) create mode 100644 extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/LDAPConnectionService.java diff --git a/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/AuthenticationProviderService.java b/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/AuthenticationProviderService.java index 541906986..d10095bd2 100644 --- a/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/AuthenticationProviderService.java +++ b/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/AuthenticationProviderService.java @@ -25,8 +25,6 @@ package org.glyptodon.guacamole.auth.ldap; import com.google.inject.Inject; import com.google.inject.Provider; import com.novell.ldap.LDAPConnection; -import com.novell.ldap.LDAPException; -import java.io.UnsupportedEncodingException; import java.util.List; import org.glyptodon.guacamole.auth.ldap.user.AuthenticatedUser; import org.glyptodon.guacamole.auth.ldap.user.UserContext; @@ -51,6 +49,12 @@ public class AuthenticationProviderService { */ private final Logger logger = LoggerFactory.getLogger(AuthenticationProviderService.class); + /** + * Service for creating and managing connections to LDAP servers. + */ + @Inject + private LDAPConnectionService ldapService; + /** * Service for retrieving LDAP server configuration information. */ @@ -75,28 +79,6 @@ public class AuthenticationProviderService { @Inject private Provider userContextProvider; - /** - * Disconnects the given LDAP connection, logging any failure to do so - * appropriately. - * - * @param ldapConnection - * The LDAP connection to disconnect. - */ - private void disconnect(LDAPConnection ldapConnection) { - - // Attempt disconnect - try { - ldapConnection.disconnect(); - } - - // Warn if disconnect unexpectedly fails - catch (LDAPException e) { - logger.warn("Unable to disconnect from LDAP server: {}", e.getMessage()); - logger.debug("LDAP disconnect failed.", e); - } - - } - /** * Determines the DN which corresponds to the user having the given * username. The DN will either be derived directly from the user base DN, @@ -122,7 +104,7 @@ public class AuthenticationProviderService { if (searchBindDN != null) { // Create an LDAP connection using the search account - LDAPConnection searchConnection = bindAs( + LDAPConnection searchConnection = ldapService.bindAs( searchBindDN, confService.getSearchBindPassword() ); @@ -147,7 +129,7 @@ public class AuthenticationProviderService { // Always disconnect finally { - disconnect(searchConnection); + ldapService.disconnect(searchConnection); } } @@ -157,78 +139,6 @@ public class AuthenticationProviderService { } - /** - * Binds to the LDAP server using the provided user DN and password. - * - * @param userDN - * The DN of the user to bind as, or null to bind anonymously. - * - * @param password - * The password to use when binding as the specified user, or null to - * attempt to bind without a password. - * - * @return - * A bound LDAP connection, or null if the connection could not be - * bound. - * - * @throws GuacamoleException - * If an error occurs while binding to the LDAP server. - */ - private LDAPConnection bindAs(String userDN, String password) - throws GuacamoleException { - - LDAPConnection ldapConnection; - - // Connect to LDAP server - try { - ldapConnection = new LDAPConnection(); - ldapConnection.connect( - confService.getServerHostname(), - confService.getServerPort() - ); - } - catch (LDAPException e) { - logger.error("Unable to connect to LDAP server: {}", e.getMessage()); - logger.debug("Failed to connect to LDAP server.", e); - return null; - } - - // Bind using provided credentials - try { - - byte[] passwordBytes; - try { - - // Convert password into corresponding byte array - if (password != null) - passwordBytes = password.getBytes("UTF-8"); - else - passwordBytes = null; - - } - catch (UnsupportedEncodingException e) { - logger.error("Unexpected lack of support for UTF-8: {}", e.getMessage()); - logger.debug("Support for UTF-8 (as required by Java spec) not found.", e); - disconnect(ldapConnection); - return null; - } - - // Bind as user - ldapConnection.bind(LDAPConnection.LDAP_V3, userDN, passwordBytes); - - } - - // Disconnect if an error occurs during bind - catch (LDAPException e) { - logger.debug("LDAP bind failed.", e); - disconnect(ldapConnection); - return null; - } - - return ldapConnection; - - } - /** * Binds to the LDAP server using the provided Guacamole credentials. The * DN of the user is derived using the LDAP configuration properties @@ -272,7 +182,7 @@ public class AuthenticationProviderService { } // Bind using user's DN - return bindAs(userDN, password); + return ldapService.bindAs(userDN, password); } @@ -320,7 +230,7 @@ public class AuthenticationProviderService { // Always disconnect finally { - disconnect(ldapConnection); + ldapService.disconnect(ldapConnection); } } @@ -359,7 +269,7 @@ public class AuthenticationProviderService { // Always disconnect finally { - disconnect(ldapConnection); + ldapService.disconnect(ldapConnection); } } diff --git a/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/LDAPAuthenticationProviderModule.java b/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/LDAPAuthenticationProviderModule.java index 4dbc1eddc..c0133baea 100644 --- a/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/LDAPAuthenticationProviderModule.java +++ b/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/LDAPAuthenticationProviderModule.java @@ -81,6 +81,7 @@ public class LDAPAuthenticationProviderModule extends AbstractModule { bind(ConfigurationService.class); bind(ConnectionService.class); bind(EscapingService.class); + bind(LDAPConnectionService.class); bind(UserService.class); } diff --git a/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/LDAPConnectionService.java b/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/LDAPConnectionService.java new file mode 100644 index 000000000..cc0140a86 --- /dev/null +++ b/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/LDAPConnectionService.java @@ -0,0 +1,145 @@ +/* + * Copyright (C) 2015 Glyptodon LLC + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +package org.glyptodon.guacamole.auth.ldap; + +import com.google.inject.Inject; +import com.novell.ldap.LDAPConnection; +import com.novell.ldap.LDAPException; +import java.io.UnsupportedEncodingException; +import org.glyptodon.guacamole.GuacamoleException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Service for creating and managing connections to LDAP servers. + * + * @author Michael Jumper + */ +public class LDAPConnectionService { + + /** + * Logger for this class. + */ + private final Logger logger = LoggerFactory.getLogger(LDAPConnectionService.class); + + /** + * Service for retrieving LDAP server configuration information. + */ + @Inject + private ConfigurationService confService; + + /** + * Binds to the LDAP server using the provided user DN and password. + * + * @param userDN + * The DN of the user to bind as, or null to bind anonymously. + * + * @param password + * The password to use when binding as the specified user, or null to + * attempt to bind without a password. + * + * @return + * A bound LDAP connection, or null if the connection could not be + * bound. + * + * @throws GuacamoleException + * If an error occurs while binding to the LDAP server. + */ + public LDAPConnection bindAs(String userDN, String password) + throws GuacamoleException { + + LDAPConnection ldapConnection; + + // Connect to LDAP server + try { + ldapConnection = new LDAPConnection(); + ldapConnection.connect( + confService.getServerHostname(), + confService.getServerPort() + ); + } + catch (LDAPException e) { + logger.error("Unable to connect to LDAP server: {}", e.getMessage()); + logger.debug("Failed to connect to LDAP server.", e); + return null; + } + + // Bind using provided credentials + try { + + byte[] passwordBytes; + try { + + // Convert password into corresponding byte array + if (password != null) + passwordBytes = password.getBytes("UTF-8"); + else + passwordBytes = null; + + } + catch (UnsupportedEncodingException e) { + logger.error("Unexpected lack of support for UTF-8: {}", e.getMessage()); + logger.debug("Support for UTF-8 (as required by Java spec) not found.", e); + disconnect(ldapConnection); + return null; + } + + // Bind as user + ldapConnection.bind(LDAPConnection.LDAP_V3, userDN, passwordBytes); + + } + + // Disconnect if an error occurs during bind + catch (LDAPException e) { + logger.debug("LDAP bind failed.", e); + disconnect(ldapConnection); + return null; + } + + return ldapConnection; + + } + + /** + * Disconnects the given LDAP connection, logging any failure to do so + * appropriately. + * + * @param ldapConnection + * The LDAP connection to disconnect. + */ + public void disconnect(LDAPConnection ldapConnection) { + + // Attempt disconnect + try { + ldapConnection.disconnect(); + } + + // Warn if disconnect unexpectedly fails + catch (LDAPException e) { + logger.warn("Unable to disconnect from LDAP server: {}", e.getMessage()); + logger.debug("LDAP disconnect failed.", e); + } + + } + +} From c1739290022a4dbc369e067c1ea5a2c41df82f9e Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Fri, 23 Oct 2015 16:09:54 -0700 Subject: [PATCH 15/17] GUAC-1115: Log failures to bind with search DN. --- .../guacamole/auth/ldap/AuthenticationProviderService.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/AuthenticationProviderService.java b/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/AuthenticationProviderService.java index d10095bd2..24dd17f13 100644 --- a/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/AuthenticationProviderService.java +++ b/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/AuthenticationProviderService.java @@ -109,6 +109,12 @@ public class AuthenticationProviderService { confService.getSearchBindPassword() ); + // Warn of failure to find + if (searchConnection == null) { + logger.error("Unable to bind using search DN \"{}\"", searchBindDN); + return null; + } + try { // Retrieve all DNs associated with the given username From 1b0961bee51b7d44e23fa1584530b13c0b57bbe9 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Fri, 23 Oct 2015 16:29:44 -0700 Subject: [PATCH 16/17] GUAC-1115: Do not require config base DN if not storing connections. --- .../guacamole/auth/ldap/ConfigurationService.java | 11 ++++++----- .../auth/ldap/connection/ConnectionService.java | 8 +++++++- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/ConfigurationService.java b/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/ConfigurationService.java index 3550d8a4f..886e405e4 100644 --- a/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/ConfigurationService.java +++ b/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/ConfigurationService.java @@ -117,18 +117,19 @@ public class ConfigurationService { /** * Returns the base DN under which all Guacamole configurations - * (connections) will be stored within the LDAP directory. + * (connections) will be stored within the LDAP directory. If Guacamole + * configurations will not be stored within LDAP, null is returned. * * @return * The base DN under which all Guacamole configurations will be stored - * within the LDAP directory. + * within the LDAP directory, or null if no Guacamole configurations + * will be stored within the LDAP directory. * * @throws GuacamoleException - * If guacamole.properties cannot be parsed, or if the configuration - * base DN property is not specified. + * If guacamole.properties cannot be parsed. */ public String getConfigurationBaseDN() throws GuacamoleException { - return environment.getRequiredProperty( + return environment.getProperty( LDAPGuacamoleProperties.LDAP_CONFIG_BASE_DN ); } diff --git a/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/connection/ConnectionService.java b/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/connection/ConnectionService.java index 9a065fda2..199e91d46 100644 --- a/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/connection/ConnectionService.java +++ b/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/connection/ConnectionService.java @@ -28,6 +28,7 @@ import com.novell.ldap.LDAPConnection; import com.novell.ldap.LDAPEntry; import com.novell.ldap.LDAPException; import com.novell.ldap.LDAPSearchResults; +import java.util.Collections; import java.util.Enumeration; import java.util.HashMap; import java.util.Map; @@ -86,6 +87,11 @@ public class ConnectionService { public Map getConnections(LDAPConnection ldapConnection) throws GuacamoleException { + // Do not return any connections if base DN is not specified + String configurationBaseDN = confService.getConfigurationBaseDN(); + if (configurationBaseDN == null) + return Collections.emptyMap(); + try { // Pull the current user DN from the LDAP connection @@ -98,7 +104,7 @@ public class ConnectionService { // Find all Guacamole connections for the given user LDAPSearchResults results = ldapConnection.search( - confService.getConfigurationBaseDN(), + configurationBaseDN, LDAPConnection.SCOPE_SUB, "(&(objectClass=guacConfigGroup)(member=" + escapingService.escapeLDAPSearchFilter(userDN) + "))", null, From 70785697f08dd0a319dd4357d5d3cd736404a0c5 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Fri, 23 Oct 2015 16:42:01 -0700 Subject: [PATCH 17/17] GUAC-1115: Fix typo in comment regarding ldap-username-attribute. --- .../guacamole/auth/ldap/LDAPGuacamoleProperties.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/LDAPGuacamoleProperties.java b/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/LDAPGuacamoleProperties.java index 6e51c6f99..efd69e6fd 100644 --- a/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/LDAPGuacamoleProperties.java +++ b/extensions/guacamole-auth-ldap/src/main/java/org/glyptodon/guacamole/auth/ldap/LDAPGuacamoleProperties.java @@ -65,8 +65,8 @@ public class LDAPGuacamoleProperties { /** * The attribute or attributes which identify users. One of these - * attributes must be present within the each Guacamole user's record in - * the LDAP directory. If the LDAP authentication will not be given its own + * attributes must be present within each Guacamole user's record in the + * LDAP directory. If the LDAP authentication will not be given its own * credentials for querying other LDAP users, this list may contain only * one attribute, and the concatenation of that attribute and the value of * LDAP_USER_BASE_DN must equal the user's full DN.