You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@guacamole.apache.org by GitBox <gi...@apache.org> on 2019/08/11 01:53:55 UTC

[GitHub] [guacamole-client] mike-jumper commented on a change in pull request #345: GUACAMOLE-234: Migrate to Apache Directory API for LDAP Extension

mike-jumper commented on a change in pull request #345: GUACAMOLE-234: Migrate to Apache Directory API for LDAP Extension
URL: https://github.com/apache/guacamole-client/pull/345#discussion_r312720143
 
 

 ##########
 File path: extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/LDAPConnectionService.java
 ##########
 @@ -106,86 +125,113 @@ private LDAPConnection createLDAPConnection() throws GuacamoleException {
      * @throws GuacamoleException
      *     If an error occurs while binding to the LDAP server.
      */
-    public LDAPConnection bindAs(String userDN, String password)
+    public LdapNetworkConnection bindAs(Dn userDN, String password)
             throws GuacamoleException {
 
-        // 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
-        ldapConstraints.setReferralFollowing(confService.getFollowReferrals());
-
-        // Set referral authentication to use the provided credentials.
-        if (userDN != null && !userDN.isEmpty())
-            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
-        ldapConstraints.setTimeLimit(confService.getOperationTimeout() * 1000);
-
-        // Apply the constraints to the connection
-        ldapConnection.setConstraints(ldapConstraints);
+        // Obtain appropriately-configured LdapNetworkConnection instance
+        LdapNetworkConnection ldapConnection = createLDAPConnection();
 
         try {
 
             // Connect to LDAP server
-            ldapConnection.connect(
-                confService.getServerHostname(),
-                confService.getServerPort()
-            );
+            ldapConnection.connect();
 
             // Explicitly start TLS if requested
             if (confService.getEncryptionMethod() == EncryptionMethod.STARTTLS)
-                ldapConnection.startTLS();
+                ldapConnection.startTls();
 
         }
-        catch (LDAPException e) {
-            logger.error("Unable to connect to LDAP server: {}", e.getMessage());
-            logger.debug("Failed to connect to LDAP server.", e);
-            return null;
+        catch (LdapException e) {
+            throw new GuacamoleServerException("Error connecting to LDAP server.", e);
         }
 
         // 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);
+            BindRequest bindRequest = new BindRequestImpl();
+            bindRequest.setDn(userDN);
+            bindRequest.setCredentials(password);
+            BindResponse bindResponse = ldapConnection.bind(bindRequest);
+            if (bindResponse.getLdapResult().getResultCode() == ResultCodeEnum.SUCCESS)
+                return ldapConnection;
+            
+            else
+                throw new GuacamoleInvalidCredentialsException("Error binding"
+                        + " to server: " + bindResponse.toString(),
+                        CredentialsInfo.USERNAME_PASSWORD);
 
         }
 
         // Disconnect if an error occurs during bind
-        catch (LDAPException e) {
-            logger.debug("LDAP bind failed.", e);
+        catch (LdapException e) {
+            logger.debug("Unable to bind to LDAP server.", e);
             disconnect(ldapConnection);
-            return null;
+            throw new GuacamoleInvalidCredentialsException(
+                    "Unable to bind to the LDAP server.",
+                    CredentialsInfo.USERNAME_PASSWORD);
         }
 
-        return ldapConnection;
-
+    }
+    
+    /**
+     * Generate a new LdapNetworkConnection object for following a referral
+     * with the given LdapUrl, and copy the username and password
+     * from the original connection.
+     * 
+     * @param referralUrl
+     *     The LDAP URL to follow.
+     * 
+     * @param ldapConfig
+     *     The connection configuration to use to retrieve username and
+     *     password.
+     * 
+     * @param hop
+     *     The current hop number of this referral - once the configured
+     *     limit is reached, this method will throw an exception.
+     * 
+     * @return
+     *     A LdapNetworkConnection object that points at the location
+     *     specified in the referralUrl.
+     *     
+     * @throws GuacamoleException
+     *     If an error occurs parsing out the LdapUrl object or the
+     *     maximum number of referral hops is reached.
+     */
+    public LdapNetworkConnection referralConnection(LdapUrl referralUrl,
 
 Review comment:
   To follow convention, this should be named in a manner that reflects what the function does. This feels like naming a function which scrambles eggs `egg()`. ;)

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services