Merge pull request #168 from ossareh/support_ldap_search_for_users

Support ldap search for users

conf/default.conf.php

@@ -642,6 +642,14 @@ return array(
   // The attribute to be regarded as 'username'. Has to be unique
   'ldap.search_attribute' => '',
 
+  // Perform a search to find a user
+  // Many LDAP installations do not have the username in the dn, if this is
+  // true for you set this to true and configure the username_attribute below
+  'ldap.search-first'         => false,
+
+  // The attribute to search for if you have to search for a user
+  'ldap.username-attribute' => '',
+
   // The attribute(s) to be regarded as 'real name'.
   // If more then one attribute is supplied the values of the attributes in
   // the array will be joined

src/__phutil_library_map__.php

@@ -713,6 +713,7 @@ phutil_register_library_map(array(
     'PhabricatorLDAPLoginController' => 'applications/auth/controller/PhabricatorLDAPLoginController.php',
     'PhabricatorLDAPProvider' => 'applications/auth/ldap/PhabricatorLDAPProvider.php',
     'PhabricatorLDAPRegistrationController' => 'applications/auth/controller/PhabricatorLDAPRegistrationController.php',
+    'PhabricatorLDAPUnknownUserException' => 'applications/auth/ldap/PhabricatorLDAPUnknownUserException.php',
     'PhabricatorLDAPUnlinkController' => 'applications/auth/controller/PhabricatorLDAPUnlinkController.php',
     'PhabricatorLintEngine' => 'infrastructure/lint/PhabricatorLintEngine.php',
     'PhabricatorLiskDAO' => 'infrastructure/storage/lisk/PhabricatorLiskDAO.php',
@@ -1722,6 +1723,7 @@ phutil_register_library_map(array(
     'PhabricatorJavelinLinter' => 'ArcanistLinter',
     'PhabricatorLDAPLoginController' => 'PhabricatorAuthController',
     'PhabricatorLDAPRegistrationController' => 'PhabricatorAuthController',
+    'PhabricatorLDAPUnknownUserException' => 'Exception',
     'PhabricatorLDAPUnlinkController' => 'PhabricatorAuthController',
     'PhabricatorLintEngine' => 'PhutilLintEngine',
     'PhabricatorLiskDAO' => 'LiskDAO',

src/applications/auth/ldap/PhabricatorLDAPProvider.php

@@ -17,6 +17,10 @@
  */
 
 final class PhabricatorLDAPProvider {
+  // http://www.php.net/manual/en/function.ldap-errno.php#20665 states
+  // that the number could be 31 or 49, in testing it has always been 49
+  const LDAP_INVALID_CREDENTIALS = 49;
+
   private $userData;
   private $connection;
 
@@ -46,6 +50,10 @@ final class PhabricatorLDAPProvider {
     return PhabricatorEnv::getEnvConfig('ldap.search_attribute');
   }
 
+  public function getUsernameAttribute() {
+    return PhabricatorEnv::getEnvConfig('ldap.username-attribute');
+  }
+
   public function getLDAPVersion() {
     return PhabricatorEnv::getEnvConfig('ldap.version');
   }
@@ -112,11 +120,32 @@ final class PhabricatorLDAPProvider {
     return $this->userData;
   }
 
+  private function invalidLDAPUserErrorMessage($errno, $errmsg) {
+    return "LDAP Error #".$errno.": ".$errmsg;
+  }
+
   public function auth($username, PhutilOpaqueEnvelope $password) {
     if (strlen(trim($username)) == 0) {
       throw new Exception('Username can not be empty');
     }
 
+    if (PhabricatorEnv::getEnvConfig('ldap.search-first')) {
+      // To protect against people phishing for accounts we catch the
+      // exception and present the default exception that would be presented
+      // in the case of a failed bind.
+      try {
+        $user = $this->getUser($this->getUsernameAttribute(), $username);
+        $username = $user[$this->getSearchAttribute()][0];
+      } catch (PhabricatorLDAPUnknownUserException $e) {
+        throw new Exception(
+          $this->invalidLDAPUserErrorMessage(
+            self::LDAP_INVALID_CREDENTIALS,
+            ldap_err2str(self::LDAP_INVALID_CREDENTIALS)));
+      }
+    }
+
+    $conn = $this->getConnection();
+
     $activeDirectoryDomain =
       PhabricatorEnv::getEnvConfig('ldap.activedirectory_domain');
 
@@ -130,8 +159,6 @@ final class PhabricatorLDAPProvider {
         $this->getBaseDN());
     }
 
-    $conn = $this->getConnection();
-
     // NOTE: It is very important we suppress any messages that occur here,
     // because it logs passwords if it reaches an error log of any sort.
     DarkConsoleErrorLogPluginAPI::enableDiscardMode();
@@ -140,19 +167,21 @@ final class PhabricatorLDAPProvider {
 
     if (!$result) {
       throw new Exception(
-        "LDAP Error #".ldap_errno($conn).": ".ldap_error($conn));
+        $this->invalidLDAPUserErrorMessage(
+          ldap_errno($conn),
+          ldap_error($conn)));
     }
 
-    $this->userData = $this->getUser($username);
+    $this->userData = $this->getUser($this->getSearchAttribute(), $username);
     return $this->userData;
   }
 
-  private function getUser($username) {
+  private function getUser($attribute, $username) {
     $conn = $this->getConnection();
 
     $query = ldap_sprintf(
       '%Q=%S',
-      $this->getSearchAttribute(),
+      $attribute,
       $username);
 
     $result = ldap_search($conn, $this->getBaseDN(), $query);
@@ -170,11 +199,11 @@ final class PhabricatorLDAPProvider {
 
     if ($entries['count'] > 1) {
       throw new Exception('Found more then one user with this ' .
-        $this->getSearchAttribute());
+        $attribute);
     }
 
     if ($entries['count'] == 0) {
-      throw new Exception('Could not find user');
+      throw new PhabricatorLDAPUnknownUserException('Could not find user');
     }
 
     return $entries[0];

src/applications/auth/ldap/PhabricatorLDAPUnknownUserException.php

@@ -0,0 +1,20 @@
+<?php
+
+/*
+ * Copyright 2012 Facebook, Inc.
+ *
+ * Licensed 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.
+ */
+
+final class PhabricatorLDAPUnknownUserException extends Exception{
+}