Github OAuth

Summary:

Test Plan:

Reviewers:

CC:

conf/default.conf.php

@@ -98,6 +98,7 @@ return array(
     'recaptcha.private-key',
     'phabricator.csrf-key',
     'facebook.application-secret',
+    'github.secret',
   ),
 
 // --  MySQL  --------------------------------------------------------------- //
@@ -187,6 +188,27 @@ return array(
   'facebook.application-secret' => null,
 
 
+// -- Github ---------------------------------------------------------------- //
+
+  // Can users use Github credentials to login to Phabricator?
+  'github.auth-enabled'         => false,
+
+  // The Github "Client ID" to use for Github API access.
+  'github.application-id'       => null,
+
+  // The Github "Secret" to use for Github API access.
+  'github.application-secret'   => null,
+
+
+  // Github Authorize URI. You don't need to change this unless Github changes
+  // its API in the future (this is unlikely).
+  'github.authorize-uri'        => 'https://github.com/login/oauth/authorize',
+
+  // Github Access Token URI. You don't need to change this unless Github
+  // changes its API in the future (this is unlikely).
+  'github.access-token-uri' => 'https://github.com/login/oauth/access_token',
+
+
 // -- Recaptcha ------------------------------------------------------------- //
 
   // Is Recaptcha enabled? If disabled, captchas will not appear.

resources/sql/patches/002.oauth.sql

@@ -0,0 +1,18 @@
+create table phabricator_user.user_oauthinfo (
+  id int unsigned not null auto_increment primary key,
+  userID int unsigned not null,
+  oauthProvider varchar(255) not null,
+  oauthUID varchar(255) not null,
+  unique key (userID, oauthProvider),
+  unique key (oauthProvider, oauthUID),
+  dateCreated int unsigned not null,
+  dateModified int unsigned not null
+);
+
+insert into phabricator_user.user_oauthinfo
+  (userID, oauthProvider, oauthUID, dateCreated, dateModified)
+  SELECT id, 'facebook', facebookUID, UNIX_TIMESTAMP(), UNIX_TIMESTAMP()
+    FROM phabricator_user.user
+    WHERE facebookUID is not null;
+
+alter table phabricator_user.user drop facebookUID;

src/__phutil_library_map__.php

@@ -187,8 +187,6 @@ phutil_register_library_map(array(
     'PhabricatorEmailLoginController' => 'applications/auth/controller/email',
     'PhabricatorEmailTokenController' => 'applications/auth/controller/emailtoken',
     'PhabricatorEnv' => 'infrastructure/env',
-    'PhabricatorFacebookAuthController' => 'applications/auth/controller/facebookauth',
-    'PhabricatorFacebookAuthDiagnosticsController' => 'applications/auth/controller/facebookauth/diagnostics',
     'PhabricatorFile' => 'applications/files/storage/file',
     'PhabricatorFileController' => 'applications/files/controller/base',
     'PhabricatorFileDAO' => 'applications/files/storage/base',
@@ -214,6 +212,12 @@ phutil_register_library_map(array(
     'PhabricatorMetaMTAMailingListsController' => 'applications/metamta/controller/mailinglists',
     'PhabricatorMetaMTASendController' => 'applications/metamta/controller/send',
     'PhabricatorMetaMTAViewController' => 'applications/metamta/controller/view',
+    'PhabricatorOAuthDiagnosticsController' => 'applications/auth/controller/oauthdiagnostics',
+    'PhabricatorOAuthFailureView' => 'applications/auth/view/oauthfailure',
+    'PhabricatorOAuthLoginController' => 'applications/auth/controller/oauth',
+    'PhabricatorOAuthProvider' => 'applications/auth/oauth/provider/base',
+    'PhabricatorOAuthProviderFacebook' => 'applications/auth/oauth/provider/facebook',
+    'PhabricatorOAuthProviderGithub' => 'applications/auth/oauth/provider/github',
     'PhabricatorObjectHandle' => 'applications/phid/handle',
     'PhabricatorObjectHandleData' => 'applications/phid/handle/data',
     'PhabricatorObjectSelectorDialog' => 'view/control/objectselector',
@@ -270,6 +274,7 @@ phutil_register_library_map(array(
     'PhabricatorTypeaheadDatasourceController' => 'applications/typeahead/controller/base',
     'PhabricatorUser' => 'applications/people/storage/user',
     'PhabricatorUserDAO' => 'applications/people/storage/base',
+    'PhabricatorUserOAuthInfo' => 'applications/people/storage/useroauthinfo',
     'PhabricatorUserProfile' => 'applications/people/storage/profile',
     'PhabricatorUserSettingsController' => 'applications/people/controller/settings',
     'PhabricatorXHProfController' => 'applications/xhprof/controller/base',
@@ -436,8 +441,6 @@ phutil_register_library_map(array(
     'PhabricatorDraftDAO' => 'PhabricatorLiskDAO',
     'PhabricatorEmailLoginController' => 'PhabricatorAuthController',
     'PhabricatorEmailTokenController' => 'PhabricatorAuthController',
-    'PhabricatorFacebookAuthController' => 'PhabricatorAuthController',
-    'PhabricatorFacebookAuthDiagnosticsController' => 'PhabricatorAuthController',
     'PhabricatorFile' => 'PhabricatorFileDAO',
     'PhabricatorFileController' => 'PhabricatorController',
     'PhabricatorFileDAO' => 'PhabricatorLiskDAO',
@@ -459,6 +462,11 @@ phutil_register_library_map(array(
     'PhabricatorMetaMTAMailingListsController' => 'PhabricatorMetaMTAController',
     'PhabricatorMetaMTASendController' => 'PhabricatorMetaMTAController',
     'PhabricatorMetaMTAViewController' => 'PhabricatorMetaMTAController',
+    'PhabricatorOAuthDiagnosticsController' => 'PhabricatorAuthController',
+    'PhabricatorOAuthFailureView' => 'AphrontView',
+    'PhabricatorOAuthLoginController' => 'PhabricatorAuthController',
+    'PhabricatorOAuthProviderFacebook' => 'PhabricatorOAuthProvider',
+    'PhabricatorOAuthProviderGithub' => 'PhabricatorOAuthProvider',
     'PhabricatorPHID' => 'PhabricatorPHIDDAO',
     'PhabricatorPHIDAllocateController' => 'PhabricatorPHIDController',
     'PhabricatorPHIDController' => 'PhabricatorController',
@@ -507,6 +515,7 @@ phutil_register_library_map(array(
     'PhabricatorTypeaheadDatasourceController' => 'PhabricatorController',
     'PhabricatorUser' => 'PhabricatorUserDAO',
     'PhabricatorUserDAO' => 'PhabricatorLiskDAO',
+    'PhabricatorUserOAuthInfo' => 'PhabricatorUserDAO',
     'PhabricatorUserProfile' => 'PhabricatorUserDAO',
     'PhabricatorUserSettingsController' => 'PhabricatorPeopleController',
     'PhabricatorXHProfController' => 'PhabricatorController',

src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php

@@ -130,6 +130,13 @@ class AphrontDefaultApplicationConfiguration
         'diagnose/$' => 'PhabricatorFacebookAuthDiagnosticsController',
       ),
 
+      '/oauth/' => array(
+        '(?P<provider>github|facebook)/' => array(
+          'login/$'     => 'PhabricatorOAuthLoginController',
+          'diagnose/$'  => 'PhabricatorOAuthDiagnosticsController',
+        ),
+      ),
+
       '/xhprof/' => array(
         'profile/(?P<phid>[^/]+)/$' => 'PhabricatorXHProfProfileController',
       ),

src/applications/auth/controller/facebookauth/PhabricatorFacebookAuthController.php

@@ -1,293 +0,0 @@
-<?php
-
-/*
- * Copyright 2011 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.
- */
-
-class PhabricatorFacebookAuthController extends PhabricatorAuthController {
-
-  public function shouldRequireLogin() {
-    return false;
-  }
-
-  public function processRequest() {
-    $auth_enabled = PhabricatorEnv::getEnvConfig('facebook.auth-enabled');
-    if (!$auth_enabled) {
-      return new Aphront400Response();
-    }
-
-    $diagnose_auth =
-      '<a href="/facebook-auth/diagnose/" class="button green">'.
-        'Diagnose Facebook Auth Problems'.
-      '</a>';
-
-    $request = $this->getRequest();
-
-    if ($request->getStr('error')) {
-      $view = new AphrontRequestFailureView();
-      $view->setHeader('Facebook Auth Failed');
-      $view->appendChild(
-        '<p>'.
-          '<strong>Description:</strong> '.
-          phutil_escape_html($request->getStr('error_description')).
-        '</p>');
-      $view->appendChild(
-        '<p>'.
-          '<strong>Error:</strong> '.
-          phutil_escape_html($request->getStr('error')).
-        '</p>');
-      $view->appendChild(
-        '<p>'.
-          '<strong>Error Reason:</strong> '.
-          phutil_escape_html($request->getStr('error_reason')).
-        '</p>');
-      $view->appendChild(
-        '<div class="aphront-failure-continue">'.
-          '<a href="/login/" class="button">Continue</a>'.
-        '</div>');
-
-      return $this->buildStandardPageResponse(
-        $view,
-        array(
-          'title' => 'Facebook Auth Failed',
-        ));
-    }
-
-    $token = $request->getStr('token');
-    if (!$token) {
-      $app_id = PhabricatorEnv::getEnvConfig('facebook.application-id');
-      $app_secret = PhabricatorEnv::getEnvConfig('facebook.application-secret');
-      $redirect_uri = PhabricatorEnv::getURI('/facebook-auth/');
-
-      $code = $request->getStr('code');
-      $auth_uri = new PhutilURI(
-        "https://graph.facebook.com/oauth/access_token");
-      $auth_uri->setQueryParams(
-        array(
-          'client_id'     => $app_id,
-          'redirect_uri'  => $redirect_uri,
-          'client_secret' => $app_secret,
-          'code'          => $code,
-        ));
-
-      $response = @file_get_contents($auth_uri);
-      if ($response === false) {
-        $view = new AphrontRequestFailureView();
-        $view->setHeader('Facebook Auth Failed');
-        $view->appendChild(
-          '<p>Unable to authenticate with Facebook. There are several reasons '.
-          'this might happen:</p>'.
-            '<ul>'.
-              '<li>Phabricator may be configured with the wrong Application '.
-              'Secret; or</li>'.
-              '<li>the Facebook OAuth access token may have expired; or</li>'.
-              '<li>Facebook may have revoked authorization for the '.
-              'Application; or</li>'.
-              '<li>Facebook may be having technical problems.</li>'.
-            '</ul>'.
-          '<p>You can try again, or login using another method.</p>');
-        $view->appendChild(
-          '<div class="aphront-failure-continue">'.
-            $diagnose_auth.
-            '<a href="/login/" class="button">Continue</a>'.
-          '</div>');
-
-        return $this->buildStandardPageResponse(
-          $view,
-          array(
-            'title' => 'Facebook Auth Failed',
-          ));
-      }
-
-      $data = array();
-      parse_str($response, $data);
-
-      $token = $data['access_token'];
-    }
-
-    $user_json = @file_get_contents('https://graph.facebook.com/me?access_token='.$token);
-    $user_data = json_decode($user_json, true);
-
-    $user_id = $user_data['id'];
-
-    $known_user = id(new PhabricatorUser())
-      ->loadOneWhere('facebookUID = %d', $user_id);
-    if ($known_user) {
-      $session_key = $known_user->establishSession('web');
-      $request->setCookie('phusr', $known_user->getUsername());
-      $request->setCookie('phsid', $session_key);
-      return id(new AphrontRedirectResponse())
-        ->setURI('/');
-    }
-
-    $known_email = id(new PhabricatorUser())
-      ->loadOneWhere('email = %s', $user_data['email']);
-    if ($known_email) {
-      if ($known_email->getFacebookUID()) {
-        throw new Exception(
-          "The email associated with the Facebook account you just logged in ".
-          "with is already associated with another Phabricator account which ".
-          "is, in turn, associated with a Facebook account different from ".
-          "the one you just logged in with.");
-      }
-      $known_email->setFacebookUID($user_id);
-      $session_key = $known_email->establishSession('web');
-      $request->setCookie('phusr', $known_email->getUsername());
-      $request->setCookie('phsid', $session_key);
-      return id(new AphrontRedirectResponse())
-        ->setURI('/');
-    }
-
-    $current_user = $this->getRequest()->getUser();
-    if ($current_user->getPHID()) {
-      if ($current_user->getFacebookUID() &&
-          $current_user->getFacebookUID() != $user_id) {
-        throw new Exception(
-          "Your account is already associated with a Facebook user ID other ".
-          "than the one you just logged in with...?");
-      }
-
-      if ($request->isFormPost()) {
-        $current_user->setFacebookUID($user_id);
-        $current_user->save();
-
-        // TODO: ship them back to the 'account' page or whatever?
-        return id(new AphrontRedirectResponse())
-          ->setURI('/');
-      }
-
-      $ph_account = $current_user->getUsername();
-      $fb_account = phutil_escape_html($user_data['name']);
-
-      $form = new AphrontFormView();
-      $form
-        ->addHiddenInput('token', $token)
-        ->setUser($request->getUser())
-        ->setAction('/facebook-auth/')
-        ->appendChild(
-          '<p class="aphront-form-view-instructions">Do you want to link your '.
-          "existing Phabricator account (<strong>{$ph_account}</strong>) ".
-          "with your Facebook account (<strong>{$fb_account}</strong>) so ".
-          "you can login with Facebook?")
-        ->appendChild(
-          id(new AphrontFormSubmitControl())
-            ->setValue('Link Accounts')
-            ->addCancelButton('/login/'));
-
-      $panel = new AphrontPanelView();
-      $panel->setHeader('Link Facebook Account');
-      $panel->setWidth(AphrontPanelView::WIDTH_FORM);
-      $panel->appendChild($form);
-
-      return $this->buildStandardPageResponse(
-        $panel,
-        array(
-          'title' => 'Link Facebook Account',
-        ));
-    }
-
-    $errors = array();
-    $e_username = true;
-
-    $user = new PhabricatorUser();
-
-    $matches = null;
-    if (preg_match('@/([a-zA-Z0-9]+)$@', $user_data['link'], $matches)) {
-      $user->setUsername($matches[1]);
-    }
-
-    if ($request->isFormPost()) {
-
-      $username = $request->getStr('username');
-      if (!strlen($username)) {
-        $e_username = 'Required';
-        $errors[] = 'Username is required.';
-      } else if (!preg_match('/^[a-zA-Z0-9]+$/', $username, $matches)) {
-        $e_username = 'Invalid';
-        $errors[] = 'Username may only contain letters and numbers.';
-      }
-
-      $user->setUsername($username);
-      $user->setFacebookUID($user_id);
-      $user->setEmail($user_data['email']);
-
-      if (!$errors) {
-        $image = @file_get_contents('https://graph.facebook.com/me/picture?access_token='.$token);
-        $file = PhabricatorFile::newFromFileData(
-          $image,
-          array(
-            'name' => 'fbprofile.jpg'
-          ));
-
-        $user->setProfileImagePHID($file->getPHID());
-        $user->setRealName($user_data['name']);
-
-        try {
-          $user->save();
-
-          $session_key = $user->establishSession('web');
-          $request->setCookie('phusr', $user->getUsername());
-          $request->setCookie('phsid', $session_key);
-          return id(new AphrontRedirectResponse())->setURI('/');
-        } catch (AphrontQueryDuplicateKeyException $exception) {
-          $key = $exception->getDuplicateKey();
-          if ($key == 'userName') {
-            $e_username = 'Duplicate';
-            $errors[] = 'That username is not unique.';
-          } else {
-            throw $exception;
-          }
-        }
-      }
-    }
-
-    $error_view = null;
-    if ($errors) {
-      $error_view = new AphrontErrorView();
-      $error_view->setTitle('Facebook Auth Failed');
-      $error_view->setErrors($errors);
-    }
-
-    $form = new AphrontFormView();
-    $form
-      ->addHiddenInput('token', $token)
-      ->setUser($request->getUser())
-      ->setAction('/facebook-auth/')
-      ->appendChild(
-        id(new AphrontFormTextControl())
-          ->setLabel('Username')
-          ->setName('username')
-          ->setValue($user->getUsername())
-          ->setError($e_username))
-      ->appendChild(
-        id(new AphrontFormSubmitControl())
-          ->setValue('Create Account'));
-
-    $panel = new AphrontPanelView();
-    $panel->setHeader('Create New Account');
-    $panel->setWidth(AphrontPanelView::WIDTH_FORM);
-    $panel->appendChild($form);
-
-    return $this->buildStandardPageResponse(
-      array(
-        $error_view,
-        $panel,
-      ),
-      array(
-        'title' => 'Create New Account',
-      ));
-  }
-
-}

src/applications/auth/controller/login/PhabricatorLoginController.php

@@ -88,38 +88,47 @@ class PhabricatorLoginController extends PhabricatorAuthController {
 //    $panel->setCreateButton('Register New Account', '/login/register/');
     $panel->appendChild($form);
 
-    $fbauth_enabled = PhabricatorEnv::getEnvConfig('facebook.auth-enabled');
+    $providers = array(
-    if ($fbauth_enabled) {
+      PhabricatorOAuthProvider::PROVIDER_FACEBOOK,
-      $auth_uri = new PhutilURI("https://www.facebook.com/dialog/oauth");
+      PhabricatorOAuthProvider::PROVIDER_GITHUB,
+    );
+    foreach ($providers as $provider_key) {
+      $provider = PhabricatorOAuthProvider::newProvider($provider_key);
 
-      $user = $request->getUser();
+      $enabled = $provider->isProviderEnabled();
+      if (!$enabled) {
+        continue;
+      }
 
-      $redirect_uri = PhabricatorEnv::getURI('/facebook-auth/');
+      $auth_uri       = $provider->getAuthURI();
-      $app_id = PhabricatorEnv::getEnvConfig('facebook.application-id');
+      $redirect_uri   = $provider->getRedirectURI();
+      $client_id      = $provider->getClientID();
+      $provider_name  = $provider->getProviderName();
 
       // TODO: In theory we should use 'state' to prevent CSRF, but the total
       // effect of the CSRF attack is that an attacker can cause a user to login
-      // to Phabricator if they're already logged into Facebook. This does not
+      // to Phabricator if they're already logged into some OAuth provider. This
-      // seem like the most severe threat in the world, and generating CSRF for
+      // does not seem like the most severe threat in the world, and generating
-      // logged-out users is vaugely tricky.
+      // CSRF for logged-out users is vaugely tricky.
 
-      $facebook_auth = new AphrontFormView();
+      $auth_form = new AphrontFormView();
-      $facebook_auth
+      $auth_form
         ->setAction($auth_uri)
-        ->addHiddenInput('client_id', $app_id)
+        ->addHiddenInput('client_id', $client_id)
         ->addHiddenInput('redirect_uri', $redirect_uri)
-        ->addHiddenInput('scope', 'email')
         ->setUser($request->getUser())
         ->setMethod('GET')
         ->appendChild(
           '<p class="aphront-form-instructions">Login or register for '.
-          'Phabricator using your Facebook account.</p>')
+          'Phabricator using your '.$provider_name.' account.</p>')
         ->appendChild(
           id(new AphrontFormSubmitControl())
-            ->setValue("Login with Facebook \xC2\xBB"));
+            ->setValue("Login with {$provider_name} \xC2\xBB"));
 
-      $panel->appendChild('<br /><h1>Login or Register with Facebook</h1>');
+      $panel->appendChild(
-      $panel->appendChild($facebook_auth);
+          '<br /><h1>Login or Register with '.$provider_name.'</h1>');
+
+      $panel->appendChild($auth_form);
     }
 
     return $this->buildStandardPageResponse(

src/applications/auth/controller/login/__init__.php

@@ -8,14 +8,13 @@
 
 phutil_require_module('phabricator', 'aphront/response/redirect');
 phutil_require_module('phabricator', 'applications/auth/controller/base');
+phutil_require_module('phabricator', 'applications/auth/oauth/provider/base');
 phutil_require_module('phabricator', 'applications/people/storage/user');
-phutil_require_module('phabricator', 'infrastructure/env');
 phutil_require_module('phabricator', 'view/form/base');
 phutil_require_module('phabricator', 'view/form/control/submit');
 phutil_require_module('phabricator', 'view/form/error');
 phutil_require_module('phabricator', 'view/layout/panel');
 
-phutil_require_module('phutil', 'parser/uri');
 phutil_require_module('phutil', 'utils');
 
 

src/applications/auth/controller/oauth/PhabricatorOAuthLoginController.php

@@ -0,0 +1,371 @@
+<?php
+
+/*
+ * Copyright 2011 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.
+ */
+
+class PhabricatorOAuthLoginController extends PhabricatorAuthController {
+
+  private $provider;
+  private $userID;
+  private $accessToken;
+
+  public function shouldRequireLogin() {
+    return false;
+  }
+
+  public function willProcessRequest(array $data) {
+    $this->provider = PhabricatorOAuthProvider::newProvider($data['provider']);
+  }
+
+  public function processRequest() {
+    $current_user = $this->getRequest()->getUser();
+    if ($current_user->getPHID()) {
+      // If we're already logged in, ignore everything going on here. TODO:
+      // restore account linking.
+      return id(new AphrontRedirectResponse())->setURI('/');
+    }
+
+    $provider = $this->provider;
+    if (!$provider->isProviderEnabled()) {
+      return new Aphront400Response();
+    }
+
+    $request = $this->getRequest();
+
+    if ($request->getStr('error')) {
+      $error_view = id(new PhabricatorOAuthFailureView())
+        ->setRequest($request);
+      return $this->buildErrorResponse($error_view);
+    }
+
+    $token = $request->getStr('token');
+    if (!$token) {
+      $client_id        = $provider->getClientID();
+      $client_secret    = $provider->getClientSecret();
+      $redirect_uri     = $provider->getRedirectURI();
+      $auth_uri         = $provider->getTokenURI();
+
+      $code = $request->getStr('code');
+      $query_data = array(
+        'client_id'     => $client_id,
+        'client_secret' => $client_secret,
+        'redirect_uri'  => $redirect_uri,
+        'code'          => $code,
+      );
+
+      $stream_context = stream_context_create(
+        array(
+          'http' => array(
+            'method'  => 'POST',
+            'header'  => 'Content-type: application/x-www-form-urlencoded',
+            'content' => http_build_query($query_data),
+          ),
+        ));
+
+      $stream = fopen($auth_uri, 'r', false, $stream_context);
+
+      $meta = stream_get_meta_data($stream);
+      $response = stream_get_contents($stream);
+
+      fclose($stream);
+
+      if ($response === false) {
+        return $this->buildErrorResponse(new PhabricatorOAuthFailureView());
+      }
+
+      $data = array();
+      parse_str($response, $data);
+
+      $token = idx($data, 'access_token');
+      if (!$token) {
+        return $this->buildErrorResponse(new PhabricatorOAuthFailureView());
+      }
+    }
+
+    $userinfo_uri = new PhutilURI($provider->getUserInfoURI());
+    $userinfo_uri->setQueryParams(
+      array(
+        'access_token' => $token,
+      ));
+
+    $user_json = @file_get_contents($userinfo_uri);
+    $user_data = json_decode($user_json, true);
+
+    $this->accessToken = $token;
+
+    switch ($provider->getProviderKey()) {
+      case PhabricatorOAuthProvider::PROVIDER_GITHUB:
+        $user_data = $user_data['user'];
+        break;
+    }
+    $this->userData = $user_data;
+
+    $user_id = $this->retrieveUserID();
+
+    // Login with known auth.
+
+    $known_oauth = id(new PhabricatorUserOAuthInfo())->loadOneWhere(
+      'oauthProvider = %s and oauthUID = %s',
+      $provider->getProviderKey(),
+      $user_id);
+    if ($known_oauth) {
+      $known_user = id(new PhabricatorUser())->load($known_oauth->getUserID());
+      $session_key = $known_user->establishSession('web');
+      $request->setCookie('phusr', $known_user->getUsername());
+      $request->setCookie('phsid', $session_key);
+      return id(new AphrontRedirectResponse())
+        ->setURI('/');
+    }
+
+    // Merge accounts based on shared email. TODO: should probably get rid of
+    // this.
+
+    $oauth_email = $this->retrieveUserEmail();
+    if ($oauth_email) {
+      $known_email = id(new PhabricatorUser())
+        ->loadOneWhere('email = %s', $oauth_email);
+      if ($known_email) {
+        $known_oauth = id(new PhabricatorUserOAuthInfo())->loadOneWhere(
+          'userID = %d AND oauthProvider = %s',
+          $known_email->getID(),
+          $provider->getProviderKey());
+        if ($known_oauth) {
+          $provider_name = $provider->getName();
+          throw new Exception(
+            "The email associated with the ".$provider_name." account you ".
+            "just logged in with is already associated with another ".
+            "Phabricator account which is, in turn, associated with a ".
+            $provider_name." account different from the one you just logged ".
+            "in with.");
+        }
+
+        $oauth_info = new PhabricatorUserOAuthInfo();
+        $oauth_info->setUserID($known_email->getID());
+        $oauth_info->setOAuthProvider($provider->getProviderKey());
+        $oauth_info->setOAuthUID($user_id);
+        $oauth_info->save();
+
+        $session_key = $known_email->establishSession('web');
+        $request->setCookie('phusr', $known_email->getUsername());
+        $request->setCookie('phsid', $session_key);
+        return id(new AphrontRedirectResponse())
+          ->setURI('/');
+      }
+    }
+
+    $errors = array();
+    $e_username = true;
+    $e_email = true;
+    $e_realname = true;
+
+    $user = new PhabricatorUser();
+
+    $suggestion = $this->retrieveUsernameSuggestion();
+    $user->setUsername($suggestion);
+
+    $oauth_realname = $this->retreiveRealNameSuggestion();
+
+    if ($request->isFormPost()) {
+
+      $user->setUsername($request->getStr('username'));
+      $username = $user->getUsername();
+      $matches = null;
+      if (!strlen($user->getUsername())) {
+        $e_username = 'Required';
+        $errors[] = 'Username is required.';
+      } else if (!preg_match('/^[a-zA-Z0-9]+$/', $username, $matches)) {
+        $e_username = 'Invalid';
+        $errors[] = 'Username may only contain letters and numbers.';
+      } else {
+        $e_username = null;
+      }
+
+      if ($oauth_email) {
+        $user->setEmail($oauth_email);
+      } else {
+        $user->setEmail($request->getStr('email'));
+        if (!strlen($user->getEmail())) {
+          $e_email = 'Required';
+          $errors[] = 'Email is required.';
+        } else {
+          $e_email = null;
+        }
+      }
+
+      if ($oauth_realname) {
+        $user->setRealName($oauth_realname);
+      } else {
+        $user->setRealName($request->getStr('realname'));
+        if (!strlen($user->getStr('realname'))) {
+          $e_realname = 'Required';
+          $errors[] = 'Real name is required.';
+        } else {
+          $e_realname = null;
+        }
+      }
+
+      if (!$errors) {
+        $image = $this->retreiveProfileImageSuggestion();
+        if ($image) {
+          $file = PhabricatorFile::newFromFileData(
+            $image,
+            array(
+              'name' => $provider->getProviderKey().'-profile.jpg'
+            ));
+          $user->setProfileImagePHID($file->getPHID());
+        }
+
+        try {
+          $user->save();
+
+          $oauth_info = new PhabricatorUserOAuthInfo();
+          $oauth_info->setUserID($user->getID());
+          $oauth_info->setOAuthProvider($provider->getProviderKey());
+          $oauth_info->setOAuthUID($user_id);
+          $oauth_info->save();
+
+          $session_key = $user->establishSession('web');
+          $request->setCookie('phusr', $user->getUsername());
+          $request->setCookie('phsid', $session_key);
+          return id(new AphrontRedirectResponse())->setURI('/');
+        } catch (AphrontQueryDuplicateKeyException $exception) {
+          $key = $exception->getDuplicateKey();
+          if ($key == 'userName') {
+            $e_username = 'Duplicate';
+            $errors[] = 'That username is not unique.';
+          } else if ($key == 'email') {
+            $e_email = 'Duplicate';
+            $errors[] = 'That email is not unique.';
+          } else {
+            throw $exception;
+          }
+        }
+      }
+    }
+
+    $error_view = null;
+    if ($errors) {
+      $error_view = new AphrontErrorView();
+      $error_view->setTitle('Registration Failed');
+      $error_view->setErrors($errors);
+    }
+
+    $form = new AphrontFormView();
+    $form
+      ->addHiddenInput('token', $token)
+      ->setUser($request->getUser())
+      ->setAction($provider->getRedirectURI())
+      ->appendChild(
+        id(new AphrontFormTextControl())
+          ->setLabel('Username')
+          ->setName('username')
+          ->setValue($user->getUsername())
+          ->setError($e_username));
+
+    if (!$oauth_email) {
+      $form->appendChild(
+        id(new AphrontFormTextControl())
+          ->setLabel('Email')
+          ->setName('email')
+          ->setValue($request->getStr('email'))
+          ->setError($e_email));
+    }
+
+    if (!$oauth_realname) {
+      $form->appendChild(
+        id(new AphrontFormTextControl())
+          ->setLabel('Real Name')
+          ->setName('realname')
+          ->setValue($request->getStr('realname'))
+          ->setError($e_realname));
+    }
+
+    $form
+      ->appendChild(
+        id(new AphrontFormSubmitControl())
+          ->setValue('Create Account'));
+
+    $panel = new AphrontPanelView();
+    $panel->setHeader('Create New Account');
+    $panel->setWidth(AphrontPanelView::WIDTH_FORM);
+    $panel->appendChild($form);
+
+    return $this->buildStandardPageResponse(
+      array(
+        $error_view,
+        $panel,
+      ),
+      array(
+        'title' => 'Create New Account',
+      ));
+  }
+
+  private function buildErrorResponse(PhabricatorOAuthFailureView $view) {
+    $provider = $this->provider;
+
+    $provider_name = $provider->getProviderName();
+    $view->setOAuthProvider($provider);
+
+    return $this->buildStandardPageResponse(
+      $view,
+      array(
+        'title' => $provider_name.' Auth Failed',
+      ));
+  }
+
+  private function retrieveUserID() {
+    return $this->userData['id'];
+  }
+
+  private function retrieveUserEmail() {
+    return $this->userData['email'];
+  }
+
+  private function retrieveUsernameSuggestion() {
+    switch ($this->provider->getProviderKey()) {
+      case PhabricatorOAuthProvider::PROVIDER_FACEBOOK:
+        $matches = null;
+        $link = $this->userData['link'];
+        if (preg_match('@/([a-zA-Z0-9]+)$@', $link, $matches)) {
+          return $matches[1];
+        }
+        break;
+      case PhabricatorOAuthProvider::PROVIDER_GITHUB:
+        return $this->userData['login'];
+    }
+    return null;
+  }
+
+  private function retreiveProfileImageSuggestion() {
+    switch ($this->provider->getProviderKey()) {
+      case PhabricatorOAuthProvider::PROVIDER_FACEBOOK:
+        $uri = 'https://graph.facebook.com/me/picture?access_token=';
+        return @file_get_contents($uri.$this->accessToken);
+      case PhabricatorOAuthProvider::PROVIDER_GITHUB:
+        $id = $this->userData['gravatar_id'];
+        if ($id) {
+          $uri = 'http://www.gravatar.com/avatar/'.$id.'?s=50';
+          return @file_get_contents($uri);
+        }
+    }
+    return null;
+  }
+
+  private function retreiveRealNameSuggestion() {
+    return $this->userData['name'];
+  }
+
+}

src/applications/auth/controller/oauth/__init__.php

@@ -9,17 +9,19 @@
 phutil_require_module('phabricator', 'aphront/response/400');
 phutil_require_module('phabricator', 'aphront/response/redirect');
 phutil_require_module('phabricator', 'applications/auth/controller/base');
+phutil_require_module('phabricator', 'applications/auth/oauth/provider/base');
+phutil_require_module('phabricator', 'applications/auth/view/oauthfailure');
 phutil_require_module('phabricator', 'applications/files/storage/file');
 phutil_require_module('phabricator', 'applications/people/storage/user');
-phutil_require_module('phabricator', 'infrastructure/env');
+phutil_require_module('phabricator', 'applications/people/storage/useroauthinfo');
 phutil_require_module('phabricator', 'view/form/base');
 phutil_require_module('phabricator', 'view/form/control/submit');
+phutil_require_module('phabricator', 'view/form/control/text');
 phutil_require_module('phabricator', 'view/form/error');
 phutil_require_module('phabricator', 'view/layout/panel');
-phutil_require_module('phabricator', 'view/page/failure');
 
-phutil_require_module('phutil', 'markup');
 phutil_require_module('phutil', 'parser/uri');
 phutil_require_module('phutil', 'utils');
 
-phutil_require_source('PhabricatorFacebookAuthController.php');
+
+phutil_require_source('PhabricatorOAuthLoginController.php');

src/applications/auth/controller/oauthdiagnostics/PhabricatorOAuthDiagnosticsController.php

@@ -16,18 +16,26 @@
  * limitations under the License.
  */
 
-class PhabricatorFacebookAuthDiagnosticsController
+class PhabricatorOAuthDiagnosticsController
   extends PhabricatorAuthController {
 
   public function shouldRequireLogin() {
     return false;
   }
 
+  public function willProcessRequest(array $data) {
+    $this->provider = PhabricatorOAuthProvider::newProvider($data['provider']);
+  }
+
   public function processRequest() {
 
-    $auth_enabled = PhabricatorEnv::getEnvConfig('facebook.auth-enabled');
+    $provider = $this->provider;
-    $app_id = PhabricatorEnv::getEnvConfig('facebook.application-id');
+
-    $app_secret = PhabricatorEnv::getEnvConfig('facebook.application-secret');
+
+
+    $auth_enabled   = $provider->isProviderEnabled();
+    $client_id      = $provider->getClientID();
+    $client_secret  = $provider->getClientSecret();
 
     $res_ok = '<strong style="color: #00aa00;">OK</strong>';
     $res_no = '<strong style="color: #aa0000;">NO</strong>';
@@ -48,7 +56,7 @@ class PhabricatorFacebookAuthDiagnosticsController
         'Facebook authentication is enabled.');
     }
 
-    if (!$app_id) {
+    if (!$client_id) {
       $results['facebook.application-id'] = array(
         $res_no,
         null,
@@ -60,11 +68,11 @@ class PhabricatorFacebookAuthDiagnosticsController
     } else {
       $results['facebook.application-id'] = array(
         $res_ok,
-        $app_id,
+        $client_id,
         'Application ID is set.');
     }
 
-    if (!$app_secret) {
+    if (!$client_secret) {
       $results['facebook.application-secret'] = array(
         $res_no,
         null,
@@ -141,8 +149,8 @@ class PhabricatorFacebookAuthDiagnosticsController
     $test_uri = new PhutilURI('https://graph.facebook.com/oauth/access_token');
     $test_uri->setQueryParams(
       array(
-        'client_id'       => $app_id,
+        'client_id'       => $client_id,
-        'client_secret'   => $app_secret,
+        'client_secret'   => $client_secret,
         'grant_type'      => 'client_credentials',
       ));
 

src/applications/auth/controller/oauthdiagnostics/__init__.php

@@ -7,7 +7,7 @@
 
 
 phutil_require_module('phabricator', 'applications/auth/controller/base');
-phutil_require_module('phabricator', 'infrastructure/env');
+phutil_require_module('phabricator', 'applications/auth/oauth/provider/base');
 phutil_require_module('phabricator', 'view/control/table');
 phutil_require_module('phabricator', 'view/layout/panel');
 
@@ -15,4 +15,4 @@ phutil_require_module('phutil', 'markup');
 phutil_require_module('phutil', 'parser/uri');
 
 
-phutil_require_source('PhabricatorFacebookAuthDiagnosticsController.php');
+phutil_require_source('PhabricatorOAuthDiagnosticsController.php');

src/applications/auth/oauth/provider/base/PhabricatorOAuthProvider.php

@@ -0,0 +1,65 @@
+<?php
+
+/*
+ * Copyright 2011 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.
+ */
+
+abstract class PhabricatorOAuthProvider {
+
+  const PROVIDER_FACEBOOK = 'facebook';
+  const PROVIDER_GITHUB   = 'github';
+
+  abstract public function getProviderKey();
+  abstract public function getProviderName();
+  abstract public function isProviderEnabled();
+  abstract public function getRedirectURI();
+  abstract public function getClientID();
+  abstract public function getClientSecret();
+  abstract public function getAuthURI();
+  abstract public function getTokenURI();
+  abstract public function getUserInfoURI();
+
+  public function __construct() {
+
+  }
+
+  public static function newProvider($which) {
+    switch ($which) {
+      case self::PROVIDER_FACEBOOK:
+        $class = 'PhabricatorOAuthProviderFacebook';
+        break;
+      case self::PROVIDER_GITHUB:
+        $class = 'PhabricatorOAuthProviderGithub';
+        break;
+      default:
+        throw new Exception('Unknown OAuth provider.');
+    }
+    PhutilSymbolLoader::loadClass($class);
+    return newv($class, array());
+  }
+
+  public static function getAllProviders() {
+    $all = array(
+      self::PROVIDER_FACEBOOK,
+      self::PROVIDER_GITHUB,
+    );
+    $providers = array();
+    foreach ($all as $provider) {
+      $providers[] = self::newProvider($provider);
+    }
+    return $providers;
+  }
+
+}

src/applications/auth/oauth/provider/base/__init__.php

@@ -0,0 +1,13 @@
+<?php
+/**
+ * This file is automatically generated. Lint this module to rebuild it.
+ * @generated
+ */
+
+
+
+phutil_require_module('phutil', 'symbols');
+phutil_require_module('phutil', 'utils');
+
+
+phutil_require_source('PhabricatorOAuthProvider.php');

src/applications/auth/oauth/provider/facebook/PhabricatorOAuthProviderFacebook.php

@@ -0,0 +1,57 @@
+<?php
+
+/*
+ * Copyright 2011 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.
+ */
+
+class PhabricatorOAuthProviderFacebook extends PhabricatorOAuthProvider {
+
+  public function getProviderKey() {
+    return self::PROVIDER_FACEBOOK;
+  }
+
+  public function getProviderName() {
+    return 'Facebook';
+  }
+
+  public function isProviderEnabled() {
+    return PhabricatorEnv::getEnvConfig('facebook.auth-enabled');
+  }
+
+  public function getRedirectURI() {
+    return PhabricatorEnv::getURI('/oauth/facebook/login/');
+  }
+
+  public function getClientID() {
+    return PhabricatorEnv::getEnvConfig('facebook.application-id');
+  }
+
+  public function getClientSecret() {
+    return PhabricatorEnv::getEnvConfig('facebook.application-secret');
+  }
+
+  public function getAuthURI() {
+    return 'https://www.facebook.com/dialog/oauth';
+  }
+
+  public function getTokenURI() {
+    return 'https://graph.facebook.com/oauth/access_token';
+  }
+
+  public function getUserInfoURI() {
+    return 'https://graph.facebook.com/me';
+  }
+
+}

src/applications/auth/oauth/provider/facebook/__init__.php

@@ -0,0 +1,13 @@
+<?php
+/**
+ * This file is automatically generated. Lint this module to rebuild it.
+ * @generated
+ */
+
+
+
+phutil_require_module('phabricator', 'applications/auth/oauth/provider/base');
+phutil_require_module('phabricator', 'infrastructure/env');
+
+
+phutil_require_source('PhabricatorOAuthProviderFacebook.php');

src/applications/auth/oauth/provider/github/PhabricatorOAuthProviderGithub.php

@@ -0,0 +1,57 @@
+<?php
+
+/*
+ * Copyright 2011 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.
+ */
+
+class PhabricatorOAuthProviderGithub extends PhabricatorOAuthProvider {
+
+  public function getProviderKey() {
+    return self::PROVIDER_GITHUB;
+  }
+
+  public function getProviderName() {
+    return 'Github';
+  }
+
+  public function isProviderEnabled() {
+    return PhabricatorEnv::getEnvConfig('github.auth-enabled');
+  }
+
+  public function getRedirectURI() {
+    return PhabricatorEnv::getURI('/oauth/github/login/');
+  }
+
+  public function getClientID() {
+    return PhabricatorEnv::getEnvConfig('github.application-id');
+  }
+
+  public function getClientSecret() {
+    return PhabricatorEnv::getEnvConfig('github.application-secret');
+  }
+
+  public function getAuthURI() {
+    return 'https://github.com/login/oauth/authorize';
+  }
+
+  public function getTokenURI() {
+    return 'https://github.com/login/oauth/access_token';
+  }
+
+  public function getUserInfoURI() {
+    return 'https://github.com/api/v2/json/user/show';
+  }
+
+}

src/applications/auth/oauth/provider/github/__init__.php

@@ -0,0 +1,13 @@
+<?php
+/**
+ * This file is automatically generated. Lint this module to rebuild it.
+ * @generated
+ */
+
+
+
+phutil_require_module('phabricator', 'applications/auth/oauth/provider/base');
+phutil_require_module('phabricator', 'infrastructure/env');
+
+
+phutil_require_source('PhabricatorOAuthProviderGithub.php');

src/applications/auth/view/oauthfailure/PhabricatorOAuthFailureView.php

@@ -0,0 +1,91 @@
+<?php
+
+/*
+ * Copyright 2011 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.
+ */
+
+class PhabricatorOAuthFailureView extends AphrontView {
+
+  private $request;
+  private $provider;
+
+  public function setRequest(AphrontRequest $request) {
+    $this->request = $request;
+    return $this;
+  }
+
+  public function setOAuthProvider($provider) {
+    $this->provider = $provider;
+    return $this;
+  }
+
+  public function render() {
+    $request = $this->request;
+    $provider = $this->provider;
+    $provider_name = $provider->getProviderName();
+
+    $diagnose = null;
+
+    $view = new AphrontRequestFailureView();
+    $view->setHeader($provider_name.' Auth Failed');
+    if ($this->request) {
+      $view->appendChild(
+        '<p>'.
+          '<strong>Description:</strong> '.
+          phutil_escape_html($request->getStr('error_description')).
+        '</p>');
+      $view->appendChild(
+        '<p>'.
+          '<strong>Error:</strong> '.
+          phutil_escape_html($request->getStr('error')).
+        '</p>');
+      $view->appendChild(
+        '<p>'.
+          '<strong>Error Reason:</strong> '.
+          phutil_escape_html($request->getStr('error_reason')).
+        '</p>');
+    } else {
+      // TODO: We can probably refine this.
+      $view->appendChild(
+        '<p>Unable to authenticate with '.$provider_name.'. '.
+        'There are several reasons this might happen:</p>'.
+          '<ul>'.
+            '<li>Phabricator may be configured with the wrong Application '.
+            'Secret; or</li>'.
+            '<li>the '.$provider_name.' OAuth access token may have expired; '.
+            'or</li>'.
+            '<li>'.$provider_name.' may have revoked authorization for the '.
+            'Application; or</li>'.
+            '<li>'.$provider_name.' may be having technical problems.</li>'.
+          '</ul>'.
+        '<p>You can try again, or login using another method.</p>');
+
+      $provider_key = $provider->getProviderKey();
+      $diagnose =
+        '<a href="/oauth/'.$provider_key.'/diagnose/" class="button green">'.
+          'Diagnose '.$provider_name.' OAuth Problems'.
+        '</a>';
+    }
+
+    $view->appendChild(
+      '<div class="aphront-failure-continue">'.
+        $diagnose.
+        '<a href="/login/" class="button">Continue</a>'.
+      '</div>');
+
+    return $view->render();
+  }
+
+}

src/applications/auth/view/oauthfailure/__init__.php

@@ -0,0 +1,15 @@
+<?php
+/**
+ * This file is automatically generated. Lint this module to rebuild it.
+ * @generated
+ */
+
+
+
+phutil_require_module('phabricator', 'view/base');
+phutil_require_module('phabricator', 'view/page/failure');
+
+phutil_require_module('phutil', 'markup');
+
+
+phutil_require_source('PhabricatorOAuthFailureView.php');

src/applications/people/storage/user/PhabricatorUser.php

@@ -28,7 +28,6 @@ class PhabricatorUser extends PhabricatorUserDAO {
   protected $email;
   protected $passwordSalt;
   protected $passwordHash;
-  protected $facebookUID;
   protected $profileImagePHID;
 
   protected $consoleEnabled = 0;

src/applications/people/storage/useroauthinfo/PhabricatorUserOAuthInfo.php

@@ -0,0 +1,25 @@
+<?php
+
+/*
+ * Copyright 2011 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.
+ */
+
+class PhabricatorUserOAuthInfo extends PhabricatorUserDAO {
+
+  protected $userID;
+  protected $oauthProvider;
+  protected $oauthUID;
+
+}

src/applications/people/storage/useroauthinfo/__init__.php

@@ -0,0 +1,12 @@
+<?php
+/**
+ * This file is automatically generated. Lint this module to rebuild it.
+ * @generated
+ */
+
+
+
+phutil_require_module('phabricator', 'applications/people/storage/base');
+
+
+phutil_require_source('PhabricatorUserOAuthInfo.php');