From ce1bfc449909cb64bf521ab4f55c2cfb12d699d2 Mon Sep 17 00:00:00 2001 From: The Maker Date: Tue, 11 Nov 2014 23:21:39 +0800 Subject: [PATCH 1/5] Add additional methods to get info from mod_authnz_ldap environment. --- src/auth/PhutilAuthAdapterRemoteUser.php | 32 ++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/src/auth/PhutilAuthAdapterRemoteUser.php b/src/auth/PhutilAuthAdapterRemoteUser.php index 7fe77bd..e224a2f 100644 --- a/src/auth/PhutilAuthAdapterRemoteUser.php +++ b/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(); + } } From 1621708e2552ee2aff9fad09026e1875f590d2b4 Mon Sep 17 00:00:00 2001 From: The Maker Date: Tue, 11 Nov 2014 23:26:50 +0800 Subject: [PATCH 2/5] Add additional info about single-signon configurations. In particular, configuring kerberos authentication alongside LDAP authorization for combined environments like Active Directory, and passing additional info from LDAP in the authorization phase, which we can now use in this extension to provide more than just username. --- README.md | 51 ++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 42 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 8fd6bb3..49728eb 100644 --- a/README.md +++ b/README.md @@ -1,42 +1,75 @@ 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): - 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 +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 authorization 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 -------- From 944307decd9cc48678c9a80b2b60a8d434d6001f Mon Sep 17 00:00:00 2001 From: make-all Date: Wed, 17 Jun 2015 00:20:30 +0800 Subject: [PATCH 3/5] Update README.md --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 49728eb..14e4f18 100644 --- a/README.md +++ b/README.md @@ -65,7 +65,7 @@ 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 authorization rather than kerberos, the +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. @@ -75,7 +75,7 @@ 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 From 8280e3daed2543e9a7caff698badbd133ec47eac Mon Sep 17 00:00:00 2001 From: Jason Rumney Date: Tue, 5 May 2020 15:55:33 +1200 Subject: [PATCH 4/5] Update for upstream API change In https://secure.phabricator.com/D21014 the underlying PhabricatorAuthProvider API was changed, and we need to change the function we call to finish authentication and register a new user if necessary. --- src/auth/PhabricatorAuthProviderRemoteUser.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/auth/PhabricatorAuthProviderRemoteUser.php b/src/auth/PhabricatorAuthProviderRemoteUser.php index f24ff6f..ad14e35 100644 --- a/src/auth/PhabricatorAuthProviderRemoteUser.php +++ b/src/auth/PhabricatorAuthProviderRemoteUser.php @@ -86,7 +86,8 @@ public function processLoginRequest( return array($account, $response); } + $account = $this->newExternalAccountForIdentifiers($account_id); - return array($this->loadOrCreateAccount($account_id), $response); + return array($account, $response); } -} \ No newline at end of file +} From bc8a78060e55ecd207425ce876b3b4e69bfa7fb1 Mon Sep 17 00:00:00 2001 From: Jason Rumney Date: Tue, 5 May 2020 16:14:15 +1200 Subject: [PATCH 5/5] Auth requires an array of auth identifiers. The Adapter base class has a method for this, and the upstream has used it for some time. This should have been failing since earlier, unless multiple changes upstream were all done together. --- src/auth/PhabricatorAuthProviderRemoteUser.php | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/src/auth/PhabricatorAuthProviderRemoteUser.php b/src/auth/PhabricatorAuthProviderRemoteUser.php index ad14e35..d91de7f 100644 --- a/src/auth/PhabricatorAuthProviderRemoteUser.php +++ b/src/auth/PhabricatorAuthProviderRemoteUser.php @@ -71,23 +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); } - $account = $this->newExternalAccountForIdentifiers($account_id); - + else { + $account = $this->newExternalAccountForIdentifiers($identifiers); + } + return array($account, $response); } }