From d98cdd29178b5623032b7d340658221549114361 Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Fri, 17 Mar 2017 16:16:04 -0400 Subject: [PATCH 1/7] GUACAMOLE-243: Implement LDAP referral handling in Guacamole LDAP extension. --- .../auth/ldap/ConfigurationService.java | 84 +++++++++++++++- .../auth/ldap/LDAPConnectionService.java | 22 +++++ .../auth/ldap/LDAPGuacamoleProperties.java | 41 ++++++++ .../auth/ldap/ReferralAuthHandler.java | 97 +++++++++++++++++++ .../guacamole/auth/ldap/user/UserService.java | 39 +++++--- 5 files changed, 271 insertions(+), 12 deletions(-) create mode 100644 extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ReferralAuthHandler.java 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 c7e4819d1..0b6f9e9e9 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 @@ -251,6 +251,24 @@ public class ConfigurationService { ); } + /** + * Returns the boolean value for whether the connection should + * follow referrals or not. By default, it will not. + * + * @return + * The boolean value of whether to follow referrals + * as configured in guacamole.properties + * + * @throws GuacamoleException + * If guacamole.properties cannot be parsed. + */ + public boolean getFollowReferrals() throws GuacamoleException { + return environment.getProperty( + LDAPGuacamoleProperties.LDAP_FOLLOW_REFERRALS, + false + ); + } + /** * Returns a set of LDAPSearchConstraints to apply globally * to all LDAP searches. @@ -272,6 +290,23 @@ public class ConfigurationService { return constraints; } + /** + * Returns the maximum number of referral hops to follow. + * + * @return + * The maximum number of referral hops to follow + * as configured in guacamole.properties + * + * @throws GuacamoleException + * If guacamole.properties cannot be parsed. + */ + public int getMaxReferralHops() throws GuacamoleException { + return environment.getProperty( + LDAPGuacamoleProperties.LDAP_MAX_REFERRAL_HOPS, + 5 + ); + } + /** * Returns the search filter that should be used when querying the * LDAP server for Guacamole users. If no filter is specified, @@ -281,7 +316,6 @@ public class ConfigurationService { * 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. */ @@ -292,4 +326,52 @@ public class ConfigurationService { ); } + /** + * Returns the authentication method to use during referral following. + * + * @return + * The authentication method to use during referral following + * as configured in guacamole.properties or as derived from + * other configuration options. + * + * @throws GuacamoleException + * If guacamole.properties cannot be parsed. + */ + public String getReferralAuthentication() throws GuacamoleException { + String confMethod = environment.getProperty( + LDAPGuacamoleProperties.LDAP_REFERRAL_AUTHENTICATION + ); + + if (confMethod == null) + + if (getSearchBindDN() != null && getSearchBindPassword() != null) + return "bind"; + + else + return "anonymous"; + + else if (confMethod.equals("bind") && (getSearchBindDN() == null || getSearchBindPassword() == null)) + throw new GuacamoleException("Referral is set to bind with credentials, but credentials are not configured."); + + return confMethod; + + } + + /** + * Returns the maximum number of seconds to wait for LDAP operations + * + * @return + * The maximum number of seconds to wait for LDAP operations + * as configured in guacamole.properties + * + * @throws GuacamoleException + * If guacamole.properties cannot be parsed. + */ + public int getOperationTimeout() throws GuacamoleException { + return environment.getProperty( + LDAPGuacamoleProperties.LDAP_OPERATION_TIMEOUT, + 30 + ); + } + } diff --git a/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/LDAPConnectionService.java b/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/LDAPConnectionService.java index bf0534c64..c3b2e12fd 100644 --- a/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/LDAPConnectionService.java +++ b/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/LDAPConnectionService.java @@ -21,12 +21,14 @@ package org.apache.guacamole.auth.ldap; import com.google.inject.Inject; import com.novell.ldap.LDAPConnection; +import com.novell.ldap.LDAPConstraints; import com.novell.ldap.LDAPException; import com.novell.ldap.LDAPJSSESecureSocketFactory; import com.novell.ldap.LDAPJSSEStartTLSFactory; import java.io.UnsupportedEncodingException; import org.apache.guacamole.GuacamoleException; import org.apache.guacamole.GuacamoleUnsupportedException; +import org.apache.guacamole.auth.ldap.ReferralAuthHandler; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -111,6 +113,26 @@ public class LDAPConnectionService { // Obtain appropriately-configured LDAPConnection instance LDAPConnection ldapConnection = createLDAPConnection(); + // Configure LDAP connection constraints + LDAPConstraints ldapConstraints = ldapConnection.getConstraints(); + if (ldapConstraints == null) + ldapConstraints = new LDAPConstraints(); + + // Set whether or not we follow referrals, and max hops + ldapConstraints.setReferralFollowing(confService.getFollowReferrals()); + String refAuthMethod = confService.getReferralAuthentication(); + + if (refAuthMethod != null && refAuthMethod.equals("bind")) + ldapConstraints.setReferralHandler(new ReferralAuthHandler(userDN, password)); + + ldapConstraints.setHopLimit(confService.getMaxReferralHops()); + + // Set timelimit to wait for LDAP operations, converting to ms + ldapConstraints.setTimeLimit(confService.getOperationTimeout() * 1000); + + // Apply the constraints to the connection + ldapConnection.setConstraints(ldapConstraints); + try { // Connect to LDAP server 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 e13264dd8..7a1dcadf6 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 @@ -19,6 +19,7 @@ package org.apache.guacamole.auth.ldap; +import org.apache.guacamole.properties.BooleanGuacamoleProperty; import org.apache.guacamole.properties.IntegerGuacamoleProperty; import org.apache.guacamole.properties.StringGuacamoleProperty; @@ -174,4 +175,44 @@ public class LDAPGuacamoleProperties { }; + /** + * Whether or not we should follow referrals + */ + public static final BooleanGuacamoleProperty LDAP_FOLLOW_REFERRALS = new BooleanGuacamoleProperty() { + + @Override + public String getName() { return "ldap-follow-referrals"; } + + }; + + /** + * Maximum number of referral hops to follow + */ + public static final IntegerGuacamoleProperty LDAP_MAX_REFERRAL_HOPS = new IntegerGuacamoleProperty() { + + @Override + public String getName() { return "ldap-max-referral-hops"; } + + }; + + /** + * Authentication method to use to follow referrals + */ + public static final StringGuacamoleProperty LDAP_REFERRAL_AUTHENTICATION = new StringGuacamoleProperty() { + + @Override + public String getName() { return "ldap-referral-authentication"; } + + }; + + /** + * Number of seconds to wait for LDAP operations to complete + */ + public static final IntegerGuacamoleProperty LDAP_OPERATION_TIMEOUT = new IntegerGuacamoleProperty() { + + @Override + public String getName() { return "ldap-operation-timeout"; } + + }; + } diff --git a/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ReferralAuthHandler.java b/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ReferralAuthHandler.java new file mode 100644 index 000000000..21a7644c3 --- /dev/null +++ b/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ReferralAuthHandler.java @@ -0,0 +1,97 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.guacamole.auth.ldap; + +import com.google.inject.Inject; +import com.novell.ldap.LDAPAuthHandler; +import com.novell.ldap.LDAPAuthProvider; +import com.novell.ldap.LDAPConnection; +import java.io.UnsupportedEncodingException; +import org.apache.guacamole.GuacamoleException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class ReferralAuthHandler implements LDAPAuthHandler { + + /** + * Logger for this class. + */ + private final Logger logger = LoggerFactory.getLogger(ReferralAuthHandler.class); + + /** + * The LDAPAuthProvider object that will be set and returned to the referral handler. + */ + private final LDAPAuthProvider ldapAuth; + + /** + * Service for retrieving LDAP server configuration information. + */ + @Inject + private ConfigurationService confService; + + + public ReferralAuthHandler() throws GuacamoleException { + String binddn = confService.getSearchBindDN(); + String password = confService.getSearchBindPassword(); + 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); + throw new GuacamoleException("Could not set password due to missing support for UTF-8 encoding."); + } + + ldapAuth = new LDAPAuthProvider(binddn, passwordBytes); + + } + + public ReferralAuthHandler(String dn, String password) throws GuacamoleException { + 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); + throw new GuacamoleException("Could not set password due to missing UTF-8 support."); + } + ldapAuth = new LDAPAuthProvider(dn, passwordBytes); + } + + @Override + public LDAPAuthProvider getAuthProvider(String host, int port) { + return ldapAuth; + } + +} 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 91f1636e5..087365f1b 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 @@ -24,6 +24,7 @@ import com.novell.ldap.LDAPAttribute; import com.novell.ldap.LDAPConnection; import com.novell.ldap.LDAPEntry; import com.novell.ldap.LDAPException; +import com.novell.ldap.LDAPReferralException; import com.novell.ldap.LDAPSearchResults; import java.util.ArrayList; import java.util.HashMap; @@ -107,19 +108,35 @@ public class UserService { // Read all visible users while (results.hasMore()) { - LDAPEntry entry = results.next(); + try { + + 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); - // 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 (LDAPReferralException e) { + if (confService.getFollowReferrals()) { + logger.error("Could not follow referral.", e.getMessage()); + logger.debug("Error encountered trying to follow referral.", e); + throw new GuacamoleException("Could not follow LDAP referral."); + } + else { + logger.warn("Encountered a referral, but not following it.", e.getMessage()); + logger.debug("Got a referral, but not configured to follow it.", e); + continue; + } + } } From 242cfbaf852b4380395103b8ca8e574cf30c323f Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Sat, 18 Mar 2017 11:08:56 -0400 Subject: [PATCH 2/7] GUACAMOLE-243: Finish up changes to deal with LDAP referrals, both in UserServer and ConnectionServer classes, along with global changes in LDAPConnectionService class. --- .../auth/ldap/LDAPConnectionService.java | 7 +- .../ldap/connection/ConnectionService.java | 118 +++++++++++------- .../guacamole/auth/ldap/user/UserService.java | 28 ++++- 3 files changed, 104 insertions(+), 49 deletions(-) diff --git a/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/LDAPConnectionService.java b/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/LDAPConnectionService.java index c3b2e12fd..82e6ca5af 100644 --- a/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/LDAPConnectionService.java +++ b/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/LDAPConnectionService.java @@ -118,13 +118,16 @@ public class LDAPConnectionService { if (ldapConstraints == null) ldapConstraints = new LDAPConstraints(); - // Set whether or not we follow referrals, and max hops + // Set whether or not we follow referrals ldapConstraints.setReferralFollowing(confService.getFollowReferrals()); - String refAuthMethod = confService.getReferralAuthentication(); + // If the referral auth method is set to bind, we set it using the existing + // username and password. + String refAuthMethod = confService.getReferralAuthentication(); if (refAuthMethod != null && refAuthMethod.equals("bind")) ldapConstraints.setReferralHandler(new ReferralAuthHandler(userDN, password)); + // Set the maximum number of referrals we follow ldapConstraints.setHopLimit(confService.getMaxReferralHops()); // Set timelimit to wait for LDAP operations, converting to ms 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 eea1a95ac..7f0634c93 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 @@ -24,6 +24,7 @@ import com.novell.ldap.LDAPAttribute; import com.novell.ldap.LDAPConnection; import com.novell.ldap.LDAPEntry; import com.novell.ldap.LDAPException; +import com.novell.ldap.LDAPReferralException; import com.novell.ldap.LDAPSearchResults; import java.util.Collections; import java.util.Enumeration; @@ -129,62 +130,80 @@ public class ConnectionService { Map connections = new HashMap(); while (results.hasMore()) { - LDAPEntry entry = results.next(); + try { - // Get common name (CN) - LDAPAttribute cn = entry.getAttribute("cn"); - if (cn == null) { - logger.warn("guacConfigGroup is missing a cn."); - continue; - } + LDAPEntry entry = results.next(); - // Get associated protocol - LDAPAttribute protocol = entry.getAttribute("guacConfigProtocol"); - if (protocol == null) { - logger.warn("guacConfigGroup \"{}\" is missing the " - + "required \"guacConfigProtocol\" attribute.", - cn.getStringValue()); - continue; - } + // Get common name (CN) + LDAPAttribute cn = entry.getAttribute("cn"); + if (cn == null) { + logger.warn("guacConfigGroup is missing a cn."); + continue; + } - // Set protocol - GuacamoleConfiguration config = new GuacamoleConfiguration(); - config.setProtocol(protocol.getStringValue()); + // Get associated protocol + LDAPAttribute protocol = entry.getAttribute("guacConfigProtocol"); + if (protocol == null) { + logger.warn("guacConfigGroup \"{}\" is missing the " + + "required \"guacConfigProtocol\" attribute.", + cn.getStringValue()); + continue; + } - // Get parameters, if any - LDAPAttribute parameterAttribute = entry.getAttribute("guacConfigParameter"); - if (parameterAttribute != null) { + // Set protocol + GuacamoleConfiguration config = new GuacamoleConfiguration(); + config.setProtocol(protocol.getStringValue()); - // For each parameter - Enumeration parameters = parameterAttribute.getStringValues(); - while (parameters.hasMoreElements()) { + // Get parameters, if any + LDAPAttribute parameterAttribute = entry.getAttribute("guacConfigParameter"); + if (parameterAttribute != null) { - String parameter = (String) parameters.nextElement(); + // For each parameter + Enumeration parameters = parameterAttribute.getStringValues(); + while (parameters.hasMoreElements()) { - // Parse parameter - int equals = parameter.indexOf('='); - if (equals != -1) { + String parameter = (String) parameters.nextElement(); - // Parse name - String name = parameter.substring(0, equals); - String value = parameter.substring(equals+1); + // Parse parameter + int equals = parameter.indexOf('='); + if (equals != -1) { - config.setParameter(name, value); + // Parse name + String name = parameter.substring(0, equals); + String value = parameter.substring(equals+1); + + config.setParameter(name, value); + + } } } + // Filter the configuration, substituting all defined tokens + tokenFilter.filterValues(config.getParameters()); + + // Store connection using cn for both identifier and name + String name = cn.getStringValue(); + Connection connection = new SimpleConnection(name, name, config); + connection.setParentIdentifier(LDAPAuthenticationProvider.ROOT_CONNECTION_GROUP); + connections.put(name, connection); + } - // Filter the configuration, substituting all defined tokens - tokenFilter.filterValues(config.getParameters()); - - // Store connection using cn for both identifier and name - String name = cn.getStringValue(); - Connection connection = new SimpleConnection(name, name, config); - connection.setParentIdentifier(LDAPAuthenticationProvider.ROOT_CONNECTION_GROUP); - connections.put(name, connection); + // Deal with issues following LDAP referrals + catch (LDAPReferralException e) { + if (confService.getFollowReferrals()) { + logger.error("Could not follow referral.", e.getMessage()); + logger.debug("Error encountered trying to follow referral.", e); + throw new GuacamoleServerException("Could not follow LDAP referral.", e); + } + else { + logger.warn("Given a referral, but referrals are disabled.", e.getMessage()); + logger.debug("Got a referral, but configured to not follow them.", e); + continue; + } + } } @@ -251,8 +270,23 @@ public class ConnectionService { // The guacConfig group uses the seeAlso attribute to refer // to these other groups while (userRoleGroupResults.hasMore()) { - LDAPEntry entry = userRoleGroupResults.next(); - connectionSearchFilter.append("(seeAlso=").append(escapingService.escapeLDAPSearchFilter(entry.getDN())).append(")"); + try { + LDAPEntry entry = userRoleGroupResults.next(); + connectionSearchFilter.append("(seeAlso=").append(escapingService.escapeLDAPSearchFilter(entry.getDN())).append(")"); + } + + catch (LDAPReferralException e) { + if (confService.getFollowReferrals()) { + logger.error("Could not follow referral.", e.getMessage()); + logger.debug("Error encountered trying to follow referral.", e); + throw new GuacamoleServerException("Could not follow LDAP referral.", e); + } + else { + logger.warn("Given a referral, but referrals are disabled.", e.getMessage()); + logger.debug("Got a referral, but configured to not follow them.", e); + continue; + } + } } } 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 087365f1b..74d65c418 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 @@ -125,15 +125,17 @@ public class UserService { logger.warn("Possibly ambiguous user account: \"{}\".", identifier); } + + // Deal with errors trying to follow referrals catch (LDAPReferralException e) { if (confService.getFollowReferrals()) { logger.error("Could not follow referral.", e.getMessage()); logger.debug("Error encountered trying to follow referral.", e); - throw new GuacamoleException("Could not follow LDAP referral."); + throw new GuacamoleServerException("Could not follow LDAP referral.", e); } else { - logger.warn("Encountered a referral, but not following it.", e.getMessage()); - logger.debug("Got a referral, but not configured to follow it.", e); + logger.warn("Given a referral, but referrals are disabled.", e.getMessage()); + logger.debug("Got a referral, but configured to not follow them.", e); continue; } } @@ -284,8 +286,24 @@ public class UserService { // Add all DNs for found users while (results.hasMore()) { - LDAPEntry entry = results.next(); - userDNs.add(entry.getDN()); + try { + LDAPEntry entry = results.next(); + userDNs.add(entry.getDN()); + } + + // Deal with errors following referrals + catch (LDAPReferralException e) { + if (confService.getFollowReferrals()) { + logger.error("Error trying to follow a referral.", e.getMessage()); + logger.debug("Encountered an error trying to follow a referral.", e); + throw new GuacamoleServerException("Failed while trying to follow referrals.", e); + } + else { + logger.warn("Given a referral, not following it.", e.getMessage()); + logger.debug("Given a referral, but configured to not follow them.", e); + continue; + } + } } // Return all discovered DNs (if any) From 9c99905a1dd01e7ab6b8b49f23c5623e71df5123 Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Mon, 23 Oct 2017 09:48:20 -0400 Subject: [PATCH 3/7] GUACAMOLE-243: Change warning message to include failed referral. --- .../guacamole/auth/ldap/connection/ConnectionService.java | 2 +- .../java/org/apache/guacamole/auth/ldap/user/UserService.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) 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 7f0634c93..0b03c63c2 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 @@ -194,7 +194,7 @@ public class ConnectionService { // Deal with issues following LDAP referrals catch (LDAPReferralException e) { if (confService.getFollowReferrals()) { - logger.error("Could not follow referral.", e.getMessage()); + logger.error("Could not follow referral.", e.getFailedReferral()); logger.debug("Error encountered trying to follow referral.", e); throw new GuacamoleServerException("Could not follow LDAP referral.", e); } 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 74d65c418..bd8f183f1 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 @@ -129,7 +129,7 @@ public class UserService { // Deal with errors trying to follow referrals catch (LDAPReferralException e) { if (confService.getFollowReferrals()) { - logger.error("Could not follow referral.", e.getMessage()); + logger.error("Could not follow referral.", e.getFailedReferral()); logger.debug("Error encountered trying to follow referral.", e); throw new GuacamoleServerException("Could not follow LDAP referral.", e); } From 72c8308b991ea101f5c3a3d0b93f98b67f1e4dc1 Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Mon, 23 Oct 2017 20:13:17 -0400 Subject: [PATCH 4/7] GUACAMOLE-243: Remove referall authentication parameter and just use search credentials. --- .../auth/ldap/ConfigurationService.java | 32 +------------------ .../auth/ldap/LDAPConnectionService.java | 3 +- .../auth/ldap/LDAPGuacamoleProperties.java | 10 ------ 3 files changed, 2 insertions(+), 43 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 0b6f9e9e9..b7812369e 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 @@ -316,6 +316,7 @@ public class ConfigurationService { * 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. */ @@ -326,37 +327,6 @@ public class ConfigurationService { ); } - /** - * Returns the authentication method to use during referral following. - * - * @return - * The authentication method to use during referral following - * as configured in guacamole.properties or as derived from - * other configuration options. - * - * @throws GuacamoleException - * If guacamole.properties cannot be parsed. - */ - public String getReferralAuthentication() throws GuacamoleException { - String confMethod = environment.getProperty( - LDAPGuacamoleProperties.LDAP_REFERRAL_AUTHENTICATION - ); - - if (confMethod == null) - - if (getSearchBindDN() != null && getSearchBindPassword() != null) - return "bind"; - - else - return "anonymous"; - - else if (confMethod.equals("bind") && (getSearchBindDN() == null || getSearchBindPassword() == null)) - throw new GuacamoleException("Referral is set to bind with credentials, but credentials are not configured."); - - return confMethod; - - } - /** * Returns the maximum number of seconds to wait for LDAP operations * diff --git a/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/LDAPConnectionService.java b/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/LDAPConnectionService.java index 82e6ca5af..a4cb8bb72 100644 --- a/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/LDAPConnectionService.java +++ b/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/LDAPConnectionService.java @@ -123,8 +123,7 @@ public class LDAPConnectionService { // If the referral auth method is set to bind, we set it using the existing // username and password. - String refAuthMethod = confService.getReferralAuthentication(); - if (refAuthMethod != null && refAuthMethod.equals("bind")) + if (userDN != null && !userDN.isEmpty()) ldapConstraints.setReferralHandler(new ReferralAuthHandler(userDN, password)); // Set the maximum number of referrals we follow 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 7a1dcadf6..63f5d0dfa 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 @@ -195,16 +195,6 @@ public class LDAPGuacamoleProperties { }; - /** - * Authentication method to use to follow referrals - */ - public static final StringGuacamoleProperty LDAP_REFERRAL_AUTHENTICATION = new StringGuacamoleProperty() { - - @Override - public String getName() { return "ldap-referral-authentication"; } - - }; - /** * Number of seconds to wait for LDAP operations to complete */ From 1212ba13fac2e4db40afea82ca041eb6e80bef77 Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Tue, 24 Oct 2017 14:52:57 -0400 Subject: [PATCH 5/7] GUACAMOLE-243: Clean up code, remove unnecessary items, add JavaDocs, etc. --- .../auth/ldap/LDAPConnectionService.java | 3 +- .../auth/ldap/ReferralAuthHandler.java | 39 ++++++------------- .../ldap/connection/ConnectionService.java | 3 +- .../guacamole/auth/ldap/user/UserService.java | 3 +- 4 files changed, 14 insertions(+), 34 deletions(-) diff --git a/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/LDAPConnectionService.java b/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/LDAPConnectionService.java index a4cb8bb72..f84912610 100644 --- a/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/LDAPConnectionService.java +++ b/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/LDAPConnectionService.java @@ -121,8 +121,7 @@ public class LDAPConnectionService { // Set whether or not we follow referrals ldapConstraints.setReferralFollowing(confService.getFollowReferrals()); - // If the referral auth method is set to bind, we set it using the existing - // username and password. + // Set referral authentication to use the provided credentials. if (userDN != null && !userDN.isEmpty()) ldapConstraints.setReferralHandler(new ReferralAuthHandler(userDN, password)); diff --git a/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ReferralAuthHandler.java b/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ReferralAuthHandler.java index 21a7644c3..ed0f07af8 100644 --- a/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ReferralAuthHandler.java +++ b/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ReferralAuthHandler.java @@ -28,6 +28,10 @@ import org.apache.guacamole.GuacamoleException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +/** + * Class that implements the necessary authentication handling + * for following referrals in LDAP connections. + */ public class ReferralAuthHandler implements LDAPAuthHandler { /** @@ -41,35 +45,14 @@ public class ReferralAuthHandler implements LDAPAuthHandler { private final LDAPAuthProvider ldapAuth; /** - * Service for retrieving LDAP server configuration information. + * Creates a ReferralAuthHandler object to handle authentication when + * following referrals in a LDAP connection, using the provided dn and + * password. + * + * @throws GuacamoleException + * If exceptions are caught while converting the password from a string + * into a byte array. */ - @Inject - private ConfigurationService confService; - - - public ReferralAuthHandler() throws GuacamoleException { - String binddn = confService.getSearchBindDN(); - String password = confService.getSearchBindPassword(); - 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); - throw new GuacamoleException("Could not set password due to missing support for UTF-8 encoding."); - } - - ldapAuth = new LDAPAuthProvider(binddn, passwordBytes); - - } - public ReferralAuthHandler(String dn, String password) throws GuacamoleException { byte[] passwordBytes; try { 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 0b03c63c2..a091e44ad 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 @@ -277,14 +277,13 @@ public class ConnectionService { catch (LDAPReferralException e) { if (confService.getFollowReferrals()) { - logger.error("Could not follow referral.", e.getMessage()); + logger.error("Could not follow referral: {}", e.getFailedReferral()); logger.debug("Error encountered trying to follow referral.", e); throw new GuacamoleServerException("Could not follow LDAP referral.", e); } else { logger.warn("Given a referral, but referrals are disabled.", e.getMessage()); logger.debug("Got a referral, but configured to not follow them.", e); - continue; } } } 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 bd8f183f1..fc7d5a3c8 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 @@ -129,14 +129,13 @@ public class UserService { // Deal with errors trying to follow referrals catch (LDAPReferralException e) { if (confService.getFollowReferrals()) { - logger.error("Could not follow referral.", e.getFailedReferral()); + logger.error("Could not follow referral: {}", e.getFailedReferral()); logger.debug("Error encountered trying to follow referral.", e); throw new GuacamoleServerException("Could not follow LDAP referral.", e); } else { logger.warn("Given a referral, but referrals are disabled.", e.getMessage()); logger.debug("Got a referral, but configured to not follow them.", e); - continue; } } From 37bfa9e00fde4be4fbf63c4ec9bee54da223c148 Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Tue, 24 Oct 2017 15:01:58 -0400 Subject: [PATCH 6/7] GUACAMOLE-243: Remove more unnecessary continue statements. --- .../apache/guacamole/auth/ldap/connection/ConnectionService.java | 1 - .../java/org/apache/guacamole/auth/ldap/user/UserService.java | 1 - 2 files changed, 2 deletions(-) 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 a091e44ad..6f8bb088e 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 @@ -201,7 +201,6 @@ public class ConnectionService { else { logger.warn("Given a referral, but referrals are disabled.", e.getMessage()); logger.debug("Got a referral, but configured to not follow them.", e); - continue; } } 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 fc7d5a3c8..083df8122 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 @@ -300,7 +300,6 @@ public class UserService { else { logger.warn("Given a referral, not following it.", e.getMessage()); logger.debug("Given a referral, but configured to not follow them.", e); - continue; } } } From 124cf923588579d721604f943d87f505846e5242 Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Mon, 6 Nov 2017 22:11:45 -0500 Subject: [PATCH 7/7] GUACAMOLE-243: Clean up JavaDoc comments, fix error messages and exceptions. --- .../apache/guacamole/auth/ldap/ConfigurationService.java | 8 ++++---- .../guacamole/auth/ldap/LDAPGuacamoleProperties.java | 6 +++--- .../apache/guacamole/auth/ldap/ReferralAuthHandler.java | 8 ++------ .../guacamole/auth/ldap/connection/ConnectionService.java | 6 +++--- .../org/apache/guacamole/auth/ldap/user/UserService.java | 6 +++--- 5 files changed, 15 insertions(+), 19 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 b7812369e..2ab7aadf6 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 @@ -257,7 +257,7 @@ public class ConfigurationService { * * @return * The boolean value of whether to follow referrals - * as configured in guacamole.properties + * as configured in guacamole.properties. * * @throws GuacamoleException * If guacamole.properties cannot be parsed. @@ -295,7 +295,7 @@ public class ConfigurationService { * * @return * The maximum number of referral hops to follow - * as configured in guacamole.properties + * as configured in guacamole.properties. * * @throws GuacamoleException * If guacamole.properties cannot be parsed. @@ -328,11 +328,11 @@ public class ConfigurationService { } /** - * Returns the maximum number of seconds to wait for LDAP operations + * Returns the maximum number of seconds to wait for LDAP operations. * * @return * The maximum number of seconds to wait for LDAP operations - * as configured in guacamole.properties + * as configured in guacamole.properties. * * @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 63f5d0dfa..0d3823fed 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 @@ -176,7 +176,7 @@ public class LDAPGuacamoleProperties { }; /** - * Whether or not we should follow referrals + * Whether or not we should follow referrals. */ public static final BooleanGuacamoleProperty LDAP_FOLLOW_REFERRALS = new BooleanGuacamoleProperty() { @@ -186,7 +186,7 @@ public class LDAPGuacamoleProperties { }; /** - * Maximum number of referral hops to follow + * Maximum number of referral hops to follow. */ public static final IntegerGuacamoleProperty LDAP_MAX_REFERRAL_HOPS = new IntegerGuacamoleProperty() { @@ -196,7 +196,7 @@ public class LDAPGuacamoleProperties { }; /** - * Number of seconds to wait for LDAP operations to complete + * Number of seconds to wait for LDAP operations to complete. */ public static final IntegerGuacamoleProperty LDAP_OPERATION_TIMEOUT = new IntegerGuacamoleProperty() { diff --git a/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ReferralAuthHandler.java b/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ReferralAuthHandler.java index ed0f07af8..e605b3c0b 100644 --- a/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ReferralAuthHandler.java +++ b/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ReferralAuthHandler.java @@ -48,12 +48,8 @@ public class ReferralAuthHandler implements LDAPAuthHandler { * Creates a ReferralAuthHandler object to handle authentication when * following referrals in a LDAP connection, using the provided dn and * password. - * - * @throws GuacamoleException - * If exceptions are caught while converting the password from a string - * into a byte array. */ - public ReferralAuthHandler(String dn, String password) throws GuacamoleException { + public ReferralAuthHandler(String dn, String password) { byte[] passwordBytes; try { @@ -67,7 +63,7 @@ public class ReferralAuthHandler implements LDAPAuthHandler { 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); - throw new GuacamoleException("Could not set password due to missing UTF-8 support."); + throw new UnsupportedOperationException("Unexpected lack of UTF-8 support.", e); } ldapAuth = new LDAPAuthProvider(dn, passwordBytes); } 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 6f8bb088e..3ce00e3f2 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 @@ -194,12 +194,12 @@ public class ConnectionService { // Deal with issues following LDAP referrals catch (LDAPReferralException e) { if (confService.getFollowReferrals()) { - logger.error("Could not follow referral.", e.getFailedReferral()); + logger.error("Could not follow referral: {}", e.getFailedReferral()); logger.debug("Error encountered trying to follow referral.", e); throw new GuacamoleServerException("Could not follow LDAP referral.", e); } else { - logger.warn("Given a referral, but referrals are disabled.", e.getMessage()); + logger.warn("Given a referral, but referrals are disabled. Error was: {}", e.getMessage()); logger.debug("Got a referral, but configured to not follow them.", e); } } @@ -281,7 +281,7 @@ public class ConnectionService { throw new GuacamoleServerException("Could not follow LDAP referral.", e); } else { - logger.warn("Given a referral, but referrals are disabled.", e.getMessage()); + logger.warn("Given a referral, but referrals are disabled. Error was: {}", e.getMessage()); logger.debug("Got a referral, but configured to not follow them.", e); } } 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 083df8122..9d27f1e00 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 @@ -134,7 +134,7 @@ public class UserService { throw new GuacamoleServerException("Could not follow LDAP referral.", e); } else { - logger.warn("Given a referral, but referrals are disabled.", e.getMessage()); + logger.warn("Given a referral, but referrals are disabled. Error was: {}", e.getMessage()); logger.debug("Got a referral, but configured to not follow them.", e); } } @@ -293,12 +293,12 @@ public class UserService { // Deal with errors following referrals catch (LDAPReferralException e) { if (confService.getFollowReferrals()) { - logger.error("Error trying to follow a referral.", e.getMessage()); + logger.error("Error trying to follow a referral: {}", e.getFailedReferral()); logger.debug("Encountered an error trying to follow a referral.", e); throw new GuacamoleServerException("Failed while trying to follow referrals.", e); } else { - logger.warn("Given a referral, not following it.", e.getMessage()); + logger.warn("Given a referral, not following it. Error was: {}", e.getMessage()); logger.debug("Given a referral, but configured to not follow them.", e); } }