From 30deacdbaf487afcc74c08aba8564313a7afbe45 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Tue, 17 Jul 2012 14:05:26 -0700
Subject: [PATCH] Fix some more ldap issues

Summary:
  - LDAP import needs to use envelopes.
  - Use ldap_sprintf().

Test Plan: Configured an LDAP server. Added an account. Imported it; logged in with it. Tried to login with accounts like ",", etc., got good errors.

Reviewers: vrana, btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D2995
---
 .../PhabricatorLDAPLoginController.php        |  5 +-
 .../auth/ldap/PhabricatorLDAPProvider.php     | 19 ++++--
 .../PhabricatorPeopleLdapController.php       | 61 ++++++++++---------
 3 files changed, 48 insertions(+), 37 deletions(-)

diff --git a/src/applications/auth/controller/PhabricatorLDAPLoginController.php b/src/applications/auth/controller/PhabricatorLDAPLoginController.php
index 524f52ef13..6b3c1e86da 100644
--- a/src/applications/auth/controller/PhabricatorLDAPLoginController.php
+++ b/src/applications/auth/controller/PhabricatorLDAPLoginController.php
@@ -35,10 +35,12 @@ final class PhabricatorLDAPLoginController extends PhabricatorAuthController {
     $current_user = $this->getRequest()->getUser();
     $request = $this->getRequest();
 
+    $ldap_username = $request->getCookie('phusr');
     if ($request->isFormPost()) {
+      $ldap_username = $request->getStr('username');
       try {
         $envelope = new PhutilOpaqueEnvelope($request->getStr('password'));
-        $this->provider->auth($request->getStr('username'), $envelope);
+        $this->provider->auth($ldap_username, $envelope);
       } catch (Exception $e) {
         $errors[] = $e->getMessage();
       }
@@ -124,7 +126,6 @@ final class PhabricatorLDAPLoginController extends PhabricatorAuthController {
       }
     }
 
-    $ldap_username = $request->getCookie('phusr');
     $ldap_form = new AphrontFormView();
     $ldap_form
       ->setUser($request->getUser())
diff --git a/src/applications/auth/ldap/PhabricatorLDAPProvider.php b/src/applications/auth/ldap/PhabricatorLDAPProvider.php
index 166b312a5d..2fbaa99581 100644
--- a/src/applications/auth/ldap/PhabricatorLDAPProvider.php
+++ b/src/applications/auth/ldap/PhabricatorLDAPProvider.php
@@ -117,8 +117,11 @@ final class PhabricatorLDAPProvider {
     if ($activeDirectoryDomain) {
       $dn = $username . '@' . $activeDirectoryDomain;
     } else {
-      $dn = $this->getSearchAttribute() . '=' . $username . ',' .
-            $this->getBaseDN();
+      $dn = ldap_sprintf(
+        '%Q=%s,%Q',
+        $this->getSearchAttribute(),
+        $username,
+        $this->getBaseDN());
     }
 
     $conn = $this->getConnection();
@@ -139,15 +142,21 @@ final class PhabricatorLDAPProvider {
   }
 
   private function getUser($username) {
-    $result = ldap_search($this->getConnection(), $this->getBaseDN(),
-              $this->getSearchAttribute() . '=' . $username);
+    $conn = $this->getConnection();
+
+    $query = ldap_sprintf(
+      '%Q=%S',
+      $this->getSearchAttribute(),
+      $username);
+
+    $result = ldap_search($conn, $this->getBaseDN(), $query);
 
     if (!$result) {
       throw new Exception('Search failed. Please check your LDAP and HTTP '.
         'logs for more information.');
     }
 
-    $entries = ldap_get_entries($this->getConnection(), $result);
+    $entries = ldap_get_entries($conn, $result);
 
     if ($entries === false) {
       throw new Exception('Could not get entries');
diff --git a/src/applications/people/controller/PhabricatorPeopleLdapController.php b/src/applications/people/controller/PhabricatorPeopleLdapController.php
index 333f8f373d..57d47ae29b 100644
--- a/src/applications/people/controller/PhabricatorPeopleLdapController.php
+++ b/src/applications/people/controller/PhabricatorPeopleLdapController.php
@@ -54,7 +54,7 @@ final class PhabricatorPeopleLdapController
         ->setValue('Search'));
 
     $panel = new AphrontPanelView();
-    $panel->setHeader('Import Ldap Users');
+    $panel->setHeader('Import LDAP Users');
     $panel->appendChild($form);
 
 
@@ -126,7 +126,8 @@ final class PhabricatorPeopleLdapController
 
     try {
       $ldap_provider = new PhabricatorLDAPProvider();
-      $ldap_provider->auth($username, $password);
+      $envelope = new PhutilOpaqueEnvelope($password);
+      $ldap_provider->auth($username, $envelope);
       $results = $ldap_provider->search($search);
       foreach ($results as $key => $result) {
         $results[$key][] = $this->renderUserInputs($result);
@@ -141,7 +142,7 @@ final class PhabricatorPeopleLdapController
           'Username',
           'Email',
           'RealName',
-          '',
+          'Import?',
         ));
       $form->appendChild($table);
       $form->setAction($request->getRequestURI()
@@ -163,35 +164,35 @@ final class PhabricatorPeopleLdapController
   }
 
   private function renderUserInputs($user) {
-        $username = $user[0];
-        $inputs =  phutil_render_tag(
-          'input',
-          array(
-            'type' => 'checkbox',
-            'name' => 'usernames[]',
-            'value' =>$username,
-          ),
-          '');
+    $username = $user[0];
+    $inputs =  phutil_render_tag(
+      'input',
+      array(
+        'type' => 'checkbox',
+        'name' => 'usernames[]',
+        'value' =>$username,
+      ),
+      '');
 
-	$inputs .=  phutil_render_tag(
-          'input',
-          array(
-            'type' => 'hidden',
-            'name' => "email[$username]",
-            'value' =>$user[1],
-          ),
-          '');
+    $inputs .=  phutil_render_tag(
+      'input',
+      array(
+        'type' => 'hidden',
+        'name' => "email[$username]",
+        'value' =>$user[1],
+      ),
+      '');
 
-	$inputs .=  phutil_render_tag(
-          'input',
-          array(
-            'type' => 'hidden',
-            'name' => "name[$username]",
-            'value' =>$user[2],
-          ),
-          '');
-
-        return $inputs;
+    $inputs .=  phutil_render_tag(
+      'input',
+      array(
+        'type' => 'hidden',
+        'name' => "name[$username]",
+        'value' =>$user[2],
+      ),
+      '');
 
+    return $inputs;
   }
+
 }