Additions to pull realName, email and potentially URI from LDAP

README.md

@@ -1,48 +1,81 @@
 libphremoteuser
 ===============
 
-This extension to [Phabricator](http://phabricator.org/) performs basic authentication 
-via a web server's REMOTE_USER variable.  It should be able to work with a variety of 
+This extension to [Phabricator](http://phabricator.org/) performs authentication 
+via a web server's `REMOTE_USER` variable.  It should be able to work with a variety of 
 major servers such as Apache and Nginx, but I have only tested it with Apache.
 
+It can be used with Basic authentication, but is most useful when the server is
+configured for single-sign-on, for example, using kerberos.
+
 Installation
 ------------
 
 To install this library, simply clone this repository alongside your phabricator installation:
 
     cd /path/to/install
-    git clone https://github.com/psigen/libphremoteuser.git
-    
+    git clone https://github.com/make-all/libphremoteuser.git
+
 Then, simply add the path to this library to your phabricator configuration:
 
     cd /path/to/install/phabricator
     ./bin/config set load-libraries '["libphremoteuser/src/"]'
-    
+
 When you next log into Phabricator as an Administrator, go to **Auth > Add Authentication Provider**.  
 In the list, you should now see an entry called **Web Server**.  Enabling this provider should add a 
 new button to your login screen.
 
-In order to actually log in, your web server needs to populate the **$REMOTE_USER** variable when the
+In order to actually log in, your web server needs to populate the `$REMOTE_USER` variable when the
 login button is pressed.  You can do this by forcing the login URI that Phabricator uses to be 
 restricted, by adding a directive like the following to your web server configuration (this is Apache2):
 
     <Location "/auth/login/RemoteUser:self/">
-      Authtype Basic
+      AuthType Kerberos
       AuthName "Phabricator at My Server"
-      Require valid-user
-
+	  KrbAuthRealms DOMAIN.EXAMPLE.COM
+	  KrbServiceName HTTP
+	  Krb5Keytab /etc/apache2/kerberos.keytab
+	  KrbMethodNegotiate On
+	  KrbMethodK5Passwd On
+	  KrbLocalUserMapping On
+
+	  # The following two lines can be used when authenticating against
+	  # Microsoft ActiveDirectory to pull extra info from LDAP, and
+	  # limit access to a certain group.  It may also be useful in
+	  # other cases.
+	  # Require ldap-dn can probably be used to allow all users in
+	  # a domain if you don't wish to limit access to a group, or
+	  # multiple Require ldap-group lines can be used to expand the list.
+	  # Require valid-user will bypass the LDAP authorization step,
+	  # defeating the additional info on the first line
+	  AuthLDAPUrl ldap://adserver.domain.example.com/dc=domain,dc=example,dc=com?uid,cn,mail
+      Require ldap-group DEVELOPERS_GROUP
+
       Options None
       Order allow,deny
       Allow from all
     </Location>
 
 
+When a user registers using this auth provider, it will attempt to discover
+the user's full name and email from `AUTHORIZE_CN` and `AUTHORIZE_MAIL` variables
+in the server environment, as well as getting the username from `REMOTE_USER`.
+These variables are available if you configure LDAP authorization with those
+attributes appended to the `AuthLDAPUrl` directive, as explained in the
+[mod_authnz_ldap](http://httpd.apache.org/docs/current/mod/mod_authnz_ldap.html#exposed)
+[documentation](http://httpd.apache.org/docs/current/mod/mod_authnz_ldap.html#authldapurl).
+
+If you use LDAP for authentication rather than kerberos, the
+environment variables will start with `AUTHENTICATE_` instead of `AUTHORIZE_`, but you are probably better off using
+Phabricator's native LDAP auth provider in that case.
+
+
 Security
 --------
 
 I make no guarantees about this library being totally secure.  It's not __obviously__ insecure.  
 However, please make sure to at least 
-**REDIRECT THE LOGIN URI TO SSL, OTHERWISE YOU ARE SENDING PLAIN TEXT PASSWORDS.**
+**REDIRECT THE LOGIN URI TO SSL, OTHERWISE YOU ARE POTENTIALLY SENDING PLAIN TEXT PASSWORDS.**
 
 If you care about security consider:
   * Hosting Phabricator entirely on https/SSL

src/auth/PhabricatorAuthProviderRemoteUser.php

@@ -71,22 +71,17 @@ public function processLoginRequest(
     $account = null;
     $response = null;
 
-    try {
-      $account_id = $adapter->getAccountID();
-    } catch (Exception $ex) {
-      // TODO: Handle this in a more user-friendly way.
-      throw $ex;
-    }
-
-    if (!strlen($account_id)) {
+    $identifiers = $adapter->getAccountIdentifiers();
+    if (!$identifiers) {
       $response = $controller->buildProviderErrorResponse(
         $this,
         pht(
           'The web server failed to provide an account ID.'));
-
-      return array($account, $response);
     }
-
-    return array($this->loadOrCreateAccount($account_id), $response);
+    else {
+        $account = $this->newExternalAccountForIdentifiers($identifiers);
+    }
+
+    return array($account, $response);
   }
-}
+}

src/auth/PhutilAuthAdapterRemoteUser.php

@@ -28,4 +28,36 @@ public function getAccountName() {
     return $this->getAccountID();
   }
 
+  // if LDAP authorization is configured in addition to kerberos
+  // authentication, Apache allows putting other attributes from LDAP
+  // into the environment prefixed by AUTHORIZE_, so use them if present.
+  public function getAccountRealName() {
+    // cn is a standard LDAP attibute
+    if (!empty($_SERVER['AUTHORIZE_CN']))
+      return $_SERVER['AUTHORIZE_CN'];
+    // Some installations may prefer to use displayName
+    else if (!empty($_SERVER['AUTHORIZE_DISPLAYNAME']))
+      return $_SERVER['AUTHORIZE_DISPLAYNAME'];
+    // Some installations may populate the name field with the user's real
+    // name. This seems to be erroneous, based on Microsoft documenting
+    // this attribute as an RDN, so only use it as a last resort.
+    else if (!empty($_SERVER['AUTHORIZE_NAME']))
+      return $_SERVER['AUTHORIZE_NAME'];
+    else
+      return parent::getAccountRealName();
+  }
+
+  public function getAccountEmail() {
+    if (!empty($_SERVER['AUTHORIZE_MAIL']))
+      return $_SERVER['AUTHORIZE_MAIL'];
+    else
+      return parent::getAccountEmail();
+  }
+
+  public function getAccountURI() {
+    if (!empty($_SERVER['AUTHORIZE_URL']))
+      return $_SERVER['AUTHORIZE_URL'];
+    else
+      return parent::getAccountURI();
+  }
 }