OAuth - add a little notes section for admins to remember details about external accounts

Summary: Fixes T4755. This also includes putting in a note that Google might ToS you to use the Google+ API. Lots of code here as there was some repeated stuff between OAuth1 and OAuth2 so I made a base OAuth with less-base OAuth1 and OAuth2 inheriting from it. The JIRA provider remains an independent mess and didn't get the notes field thing.

Test Plan: looked at providers and read pretty instructions.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T4755

Differential Revision: https://secure.phabricator.com/D8726

src/__phutil_library_map__.php

@@ -1228,6 +1228,7 @@ phutil_register_library_map(array(
     'PhabricatorAuthProviderOAuth1' => 'applications/auth/provider/PhabricatorAuthProviderOAuth1.php',
     'PhabricatorAuthProviderOAuth1JIRA' => 'applications/auth/provider/PhabricatorAuthProviderOAuth1JIRA.php',
     'PhabricatorAuthProviderOAuth1Twitter' => 'applications/auth/provider/PhabricatorAuthProviderOAuth1Twitter.php',
+    'PhabricatorAuthProviderOAuth2' => 'applications/auth/provider/PhabricatorAuthProviderOAuth2.php',
     'PhabricatorAuthProviderOAuthAmazon' => 'applications/auth/provider/PhabricatorAuthProviderOAuthAmazon.php',
     'PhabricatorAuthProviderOAuthAsana' => 'applications/auth/provider/PhabricatorAuthProviderOAuthAsana.php',
     'PhabricatorAuthProviderOAuthDisqus' => 'applications/auth/provider/PhabricatorAuthProviderOAuthDisqus.php',
@@ -3955,16 +3956,17 @@ phutil_register_library_map(array(
     'PhabricatorAuthProviderConfigTransactionQuery' => 'PhabricatorApplicationTransactionQuery',
     'PhabricatorAuthProviderLDAP' => 'PhabricatorAuthProvider',
     'PhabricatorAuthProviderOAuth' => 'PhabricatorAuthProvider',
-    'PhabricatorAuthProviderOAuth1' => 'PhabricatorAuthProvider',
+    'PhabricatorAuthProviderOAuth1' => 'PhabricatorAuthProviderOAuth',
     'PhabricatorAuthProviderOAuth1JIRA' => 'PhabricatorAuthProviderOAuth1',
     'PhabricatorAuthProviderOAuth1Twitter' => 'PhabricatorAuthProviderOAuth1',
-    'PhabricatorAuthProviderOAuthAmazon' => 'PhabricatorAuthProviderOAuth',
+    'PhabricatorAuthProviderOAuth2' => 'PhabricatorAuthProviderOAuth',
-    'PhabricatorAuthProviderOAuthAsana' => 'PhabricatorAuthProviderOAuth',
+    'PhabricatorAuthProviderOAuthAmazon' => 'PhabricatorAuthProviderOAuth2',
-    'PhabricatorAuthProviderOAuthDisqus' => 'PhabricatorAuthProviderOAuth',
+    'PhabricatorAuthProviderOAuthAsana' => 'PhabricatorAuthProviderOAuth2',
-    'PhabricatorAuthProviderOAuthFacebook' => 'PhabricatorAuthProviderOAuth',
+    'PhabricatorAuthProviderOAuthDisqus' => 'PhabricatorAuthProviderOAuth2',
-    'PhabricatorAuthProviderOAuthGitHub' => 'PhabricatorAuthProviderOAuth',
+    'PhabricatorAuthProviderOAuthFacebook' => 'PhabricatorAuthProviderOAuth2',
-    'PhabricatorAuthProviderOAuthGoogle' => 'PhabricatorAuthProviderOAuth',
+    'PhabricatorAuthProviderOAuthGitHub' => 'PhabricatorAuthProviderOAuth2',
-    'PhabricatorAuthProviderOAuthTwitch' => 'PhabricatorAuthProviderOAuth',
+    'PhabricatorAuthProviderOAuthGoogle' => 'PhabricatorAuthProviderOAuth2',
+    'PhabricatorAuthProviderOAuthTwitch' => 'PhabricatorAuthProviderOAuth2',
     'PhabricatorAuthProviderPassword' => 'PhabricatorAuthProvider',
     'PhabricatorAuthProviderPersona' => 'PhabricatorAuthProvider',
     'PhabricatorAuthRegisterController' => 'PhabricatorAuthController',

src/applications/auth/provider/PhabricatorAuthProviderOAuth.php

@@ -2,9 +2,13 @@
 
 abstract class PhabricatorAuthProviderOAuth extends PhabricatorAuthProvider {
 
+  const PROPERTY_NOTE = 'oauth:app:note';
+
   protected $adapter;
 
   abstract protected function newOAuthAdapter();
+  abstract protected function getIDKey();
+  abstract protected function getSecretKey();
 
   public function getDescriptionForCreate() {
     return pht('Configure %s OAuth.', $this->getProviderName());
@@ -19,130 +23,49 @@ abstract class PhabricatorAuthProviderOAuth extends PhabricatorAuthProvider {
     return $this->adapter;
   }
 
-  protected function configureAdapter(PhutilAuthAdapterOAuth $adapter) {
-    $config = $this->getProviderConfig();
-    $adapter->setClientID($config->getProperty(self::PROPERTY_APP_ID));
-    $adapter->setClientSecret(
-      new PhutilOpaqueEnvelope(
-        $config->getProperty(self::PROPERTY_APP_SECRET)));
-    $adapter->setRedirectURI(PhabricatorEnv::getURI($this->getLoginURI()));
-    return $adapter;
-  }
-
   public function isLoginFormAButton() {
     return true;
   }
 
-  protected function renderLoginForm(AphrontRequest $request, $mode) {
-    $adapter = $this->getAdapter();
-    $adapter->setState($this->getAuthCSRFCode($request));
-
-    $scope = $request->getStr('scope');
-    if ($scope) {
-      $adapter->setScope($scope);
-    }
-
-    $attributes = array(
-      'method' => 'GET',
-      'uri' => $adapter->getAuthenticateURI(),
-    );
-
-    return $this->renderStandardLoginButton($request, $mode, $attributes);
-  }
-
-  public function processLoginRequest(
-    PhabricatorAuthLoginController $controller) {
-
-    $request = $controller->getRequest();
-    $adapter = $this->getAdapter();
-    $account = null;
-    $response = null;
-
-    $error = $request->getStr('error');
-    if ($error) {
-      $response = $controller->buildProviderErrorResponse(
-        $this,
-        pht(
-          'The OAuth provider returned an error: %s',
-          $error));
-
-      return array($account, $response);
-    }
-
-    $this->verifyAuthCSRFCode($request, $request->getStr('state'));
-
-    $code = $request->getStr('code');
-    if (!strlen($code)) {
-      $response = $controller->buildProviderErrorResponse(
-        $this,
-        pht(
-          'The OAuth provider did not return a "code" parameter in its '.
-          'response.'));
-
-      return array($account, $response);
-    }
-
-    $adapter->setCode($code);
-
-    // NOTE: As a side effect, this will cause the OAuth adapter to request
-    // an access token.
-
-    try {
-      $account_id = $adapter->getAccountID();
-    } catch (Exception $ex) {
-      // TODO: Handle this in a more user-friendly way.
-      throw $ex;
-    }
-
-    if (!strlen($account_id)) {
-      $response = $controller->buildProviderErrorResponse(
-        $this,
-        pht(
-          'The OAuth provider failed to retrieve an account ID.'));
-
-      return array($account, $response);
-    }
-
-    return array($this->loadOrCreateAccount($account_id), $response);
-  }
-
-  const PROPERTY_APP_ID = 'oauth:app:id';
-  const PROPERTY_APP_SECRET = 'oauth:app:secret';
-
   public function readFormValuesFromProvider() {
     $config = $this->getProviderConfig();
-    $id = $config->getProperty(self::PROPERTY_APP_ID);
+    $id = $config->getProperty($this->getIDKey());
-    $secret = $config->getProperty(self::PROPERTY_APP_SECRET);
+    $secret = $config->getProperty($this->getSecretKey());
+    $note = $config->getProperty(self::PROPERTY_NOTE);
 
     return array(
-      self::PROPERTY_APP_ID     => $id,
+      $this->getIDKey()     => $id,
-      self::PROPERTY_APP_SECRET => $secret,
+      $this->getSecretKey() => $secret,
+      self::PROPERTY_NOTE   => $note,
     );
   }
 
   public function readFormValuesFromRequest(AphrontRequest $request) {
     return array(
-      self::PROPERTY_APP_ID     => $request->getStr(self::PROPERTY_APP_ID),
+      $this->getIDKey()     => $request->getStr($this->getIDKey()),
-      self::PROPERTY_APP_SECRET => $request->getStr(self::PROPERTY_APP_SECRET),
+      $this->getSecretKey() => $request->getStr($this->getSecretKey()),
+      self::PROPERTY_NOTE   => $request->getStr(self::PROPERTY_NOTE),
     );
   }
 
-  public function processEditForm(
+  protected function processOAuthEditForm(
     AphrontRequest $request,
-    array $values) {
+    array $values,
+    $id_error,
+    $secret_error) {
+
     $errors = array();
     $issues = array();
-
+    $key_id = $this->getIDKey();
-    $key_id = self::PROPERTY_APP_ID;
+    $key_secret = $this->getSecretKey();
-    $key_secret = self::PROPERTY_APP_SECRET;
 
     if (!strlen($values[$key_id])) {
-      $errors[] = pht('Application ID is required.');
+      $errors[] = $id_error;
       $issues[$key_id] = pht('Required');
     }
 
     if (!strlen($values[$key_secret])) {
-      $errors[] = pht('Application secret is required.');
+      $errors[] = $secret_error;
       $issues[$key_secret] = pht('Required');
     }
 
@@ -155,20 +78,34 @@ abstract class PhabricatorAuthProviderOAuth extends PhabricatorAuthProvider {
     return array($errors, $issues, $values);
   }
 
-  public function extendEditForm(
+  public function getConfigurationHelp() {
+    $help = $this->getProviderConfigurationHelp();
+
+    return $help . "\n\n" .
+      pht('Use the **OAuth App Notes** field to record details about which '.
+          'account the external application is registered under.');
+  }
+
+  abstract protected function getProviderConfigurationHelp();
+
+  protected function extendOAuthEditForm(
     AphrontRequest $request,
     AphrontFormView $form,
     array $values,
-    array $issues) {
+    array $issues,
+    $id_label,
+    $secret_label) {
 
-    $key_id = self::PROPERTY_APP_ID;
+    $key_id = $this->getIDKey();
-    $key_secret = self::PROPERTY_APP_SECRET;
+    $key_secret = $this->getSecretKey();
+    $key_note = self::PROPERTY_NOTE;
 
     $v_id = $values[$key_id];
     $v_secret = $values[$key_secret];
     if ($v_secret) {
       $v_secret = str_repeat('*', strlen($v_secret));
     }
+    $v_note = $values[$key_note];
 
     $e_id = idx($issues, $key_id, $request->isFormPost() ? null : true);
     $e_secret = idx($issues, $key_secret, $request->isFormPost() ? null : true);
@@ -176,16 +113,22 @@ abstract class PhabricatorAuthProviderOAuth extends PhabricatorAuthProvider {
     $form
       ->appendChild(
         id(new AphrontFormTextControl())
-          ->setLabel(pht('OAuth App ID'))
+        ->setLabel($id_label)
           ->setName($key_id)
           ->setValue($v_id)
           ->setError($e_id))
       ->appendChild(
         id(new AphrontFormPasswordControl())
-          ->setLabel(pht('OAuth App Secret'))
+        ->setLabel($secret_label)
           ->setName($key_secret)
           ->setValue($v_secret)
-          ->setError($e_secret));
+          ->setError($e_secret))
+      ->appendChild(
+        id(new AphrontFormTextAreaControl())
+        ->setLabel(pht('OAuth App Notes'))
+        ->setHeight(AphrontFormTextAreaControl::HEIGHT_VERY_SHORT)
+        ->setName($key_note)
+        ->setValue($v_note));
   }
 
   public function renderConfigPropertyTransactionTitle(
@@ -198,31 +141,17 @@ abstract class PhabricatorAuthProviderOAuth extends PhabricatorAuthProvider {
       PhabricatorAuthProviderConfigTransaction::PROPERTY_KEY);
 
     switch ($key) {
-      case self::PROPERTY_APP_ID:
+      case self::PROPERTY_NOTE:
         if (strlen($old)) {
           return pht(
-            '%s updated the OAuth application ID for this provider from '.
+            '%s updated the OAuth application notes for this provider.',
-            '"%s" to "%s".',
-            $xaction->renderHandleLink($author_phid),
-            $old,
-            $new);
-        } else {
-          return pht(
-            '%s set the OAuth application ID for this provider to '.
-            '"%s".',
-            $xaction->renderHandleLink($author_phid),
-            $new);
-        }
-      case self::PROPERTY_APP_SECRET:
-        if (strlen($old)) {
-          return pht(
-            '%s updated the OAuth application secret for this provider.',
             $xaction->renderHandleLink($author_phid));
         } else {
           return pht(
-            '%s set the OAuth application seceret for this provider.',
+            '%s set the OAuth application notes for this provider.',
             $xaction->renderHandleLink($author_phid));
         }
+
     }
 
     return parent::renderConfigPropertyTransactionTitle($xaction);
@@ -233,93 +162,7 @@ abstract class PhabricatorAuthProviderOAuth extends PhabricatorAuthProvider {
     $this->synchronizeOAuthAccount($account);
   }
 
-  protected function synchronizeOAuthAccount(
+  abstract protected function synchronizeOAuthAccount(
-    PhabricatorExternalAccount $account) {
+    PhabricatorExternalAccount $account);
-    $adapter = $this->getAdapter();
-
-    $oauth_token = $adapter->getAccessToken();
-    $account->setProperty('oauth.token.access', $oauth_token);
-
-    if ($adapter->supportsTokenRefresh()) {
-      $refresh_token = $adapter->getRefreshToken();
-      $account->setProperty('oauth.token.refresh', $refresh_token);
-    } else {
-      $account->setProperty('oauth.token.refresh', null);
-    }
-
-    $expires = $adapter->getAccessTokenExpires();
-    $account->setProperty('oauth.token.access.expires', $expires);
-  }
-
-  public function getOAuthAccessToken(
-    PhabricatorExternalAccount $account,
-    $force_refresh = false) {
-
-    if ($account->getProviderKey() !== $this->getProviderKey()) {
-      throw new Exception("Account does not match provider!");
-    }
-
-    if (!$force_refresh) {
-      $access_expires = $account->getProperty('oauth.token.access.expires');
-      $access_token = $account->getProperty('oauth.token.access');
-
-      // Don't return a token with fewer than this many seconds remaining until
-      // it expires.
-      $shortest_token = 60;
-      if ($access_token) {
-        if ($access_expires === null ||
-            $access_expires > (time() + $shortest_token)) {
-          return $access_token;
-        }
-      }
-    }
-
-    $refresh_token = $account->getProperty('oauth.token.refresh');
-    if ($refresh_token) {
-      $adapter = $this->getAdapter();
-      if ($adapter->supportsTokenRefresh()) {
-        $adapter->refreshAccessToken($refresh_token);
-
-        $this->synchronizeOAuthAccount($account);
-        $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
-          $account->save();
-        unset($unguarded);
-
-        return $account->getProperty('oauth.token.access');
-      }
-    }
-
-    return null;
-  }
-
-  public function willRenderLinkedAccount(
-    PhabricatorUser $viewer,
-    PHUIObjectItemView $item,
-    PhabricatorExternalAccount $account) {
-
-    // Get a valid token, possibly refreshing it.
-    $oauth_token = $this->getOAuthAccessToken($account);
-
-    $item->addAttribute(pht('OAuth2 Account'));
-
-    if ($oauth_token) {
-      $oauth_expires = $account->getProperty('oauth.token.access.expires');
-      if ($oauth_expires) {
-        $item->addAttribute(
-          pht(
-            'Active OAuth Token (Expires: %s)',
-            phabricator_datetime($oauth_expires, $viewer)));
-      } else {
-        $item->addAttribute(
-          pht(
-            'Active OAuth Token'));
-      }
-    } else {
-      $item->addAttribute(pht('No OAuth Access Token'));
-    }
-
-    parent::willRenderLinkedAccount($viewer, $item, $account);
-  }
-
 
 }

src/applications/auth/provider/PhabricatorAuthProviderOAuth1.php

@@ -1,6 +1,7 @@
 <?php
 
-abstract class PhabricatorAuthProviderOAuth1 extends PhabricatorAuthProvider {
+abstract class PhabricatorAuthProviderOAuth1
+  extends PhabricatorAuthProviderOAuth {
 
   protected $adapter;
 
@@ -8,19 +9,12 @@ abstract class PhabricatorAuthProviderOAuth1 extends PhabricatorAuthProvider {
   const PROPERTY_CONSUMER_SECRET = 'oauth1:consumer:secret';
   const PROPERTY_PRIVATE_KEY = 'oauth1:private:key';
 
-  abstract protected function newOAuthAdapter();
+  protected function getIDKey() {
-
+    return self::PROPERTY_CONSUMER_KEY;
-  public function getDescriptionForCreate() {
-    return pht('Configure %s OAuth.', $this->getProviderName());
   }
 
-  public function getAdapter() {
+  protected function getSecretKey() {
-    if (!$this->adapter) {
+    return self::PROPERTY_CONSUMER_SECRET;
-      $adapter = $this->newOAuthAdapter();
-      $this->adapter = $adapter;
-      $this->configureAdapter($adapter);
-    }
-    return $this->adapter;
   }
 
   protected function configureAdapter(PhutilAuthAdapterOAuth1 $adapter) {
@@ -34,10 +28,6 @@ abstract class PhabricatorAuthProviderOAuth1 extends PhabricatorAuthProvider {
     return $adapter;
   }
 
-  public function isLoginFormAButton() {
-    return true;
-  }
-
   protected function renderLoginForm(AphrontRequest $request, $mode) {
     $attributes = array(
       'method' => 'POST',
@@ -117,52 +107,18 @@ abstract class PhabricatorAuthProviderOAuth1 extends PhabricatorAuthProvider {
     return array($this->loadOrCreateAccount($account_id), $response);
   }
 
-  public function readFormValuesFromProvider() {
-    $config = $this->getProviderConfig();
-    $id = $config->getProperty(self::PROPERTY_CONSUMER_KEY);
-    $secret = $config->getProperty(self::PROPERTY_CONSUMER_SECRET);
-
-    return array(
-      self::PROPERTY_CONSUMER_KEY => $id,
-      self::PROPERTY_CONSUMER_SECRET => $secret,
-    );
-  }
-
-  public function readFormValuesFromRequest(AphrontRequest $request) {
-    return array(
-      self::PROPERTY_CONSUMER_KEY
-        => $request->getStr(self::PROPERTY_CONSUMER_KEY),
-      self::PROPERTY_CONSUMER_SECRET
-        => $request->getStr(self::PROPERTY_CONSUMER_SECRET),
-    );
-  }
-
   public function processEditForm(
     AphrontRequest $request,
     array $values) {
-    $errors = array();
-    $issues = array();
 
     $key_ckey = self::PROPERTY_CONSUMER_KEY;
     $key_csecret = self::PROPERTY_CONSUMER_SECRET;
 
-    if (!strlen($values[$key_ckey])) {
+    return $this->processOAuthEditForm(
-      $errors[] = pht('Consumer key is required.');
+      $request,
-      $issues[$key_ckey] = pht('Required');
+      $values,
-    }
+      pht('Consumer key is required.'),
-
+      pht('Consumer secret is required.'));
-    if (!strlen($values[$key_csecret])) {
-      $errors[] = pht('Consumer secret is required.');
-      $issues[$key_csecret] = pht('Required');
-    }
-
-    // If the user has not changed the secret, don't update it (that is,
-    // don't cause a bunch of "****" to be written to the database).
-    if (preg_match('/^[*]+$/', $values[$key_csecret])) {
-      unset($values[$key_csecret]);
-    }
-
-    return array($errors, $issues, $values);
   }
 
   public function extendEditForm(
@@ -171,31 +127,13 @@ abstract class PhabricatorAuthProviderOAuth1 extends PhabricatorAuthProvider {
     array $values,
     array $issues) {
 
-    $key_id = self::PROPERTY_CONSUMER_KEY;
+    return $this->extendOAuthEditForm(
-    $key_secret = self::PROPERTY_CONSUMER_SECRET;
+      $request,
-
+      $form,
-    $v_id = $values[$key_id];
+      $values,
-    $v_secret = $values[$key_secret];
+      $issues,
-    if ($v_secret) {
+      pht('OAuth Consumer Key'),
-      $v_secret = str_repeat('*', strlen($v_secret));
+      pht('OAuth Consumer Secret'));
-    }
-
-    $e_id = idx($issues, $key_id, $request->isFormPost() ? null : true);
-    $e_secret = idx($issues, $key_secret, $request->isFormPost() ? null : true);
-
-    $form
-      ->appendChild(
-        id(new AphrontFormTextControl())
-          ->setLabel(pht('OAuth Consumer Key'))
-          ->setName($key_id)
-          ->setValue($v_id)
-          ->setError($e_id))
-      ->appendChild(
-        id(new AphrontFormPasswordControl())
-          ->setLabel(pht('OAuth Consumer Secret'))
-          ->setName($key_secret)
-          ->setValue($v_secret)
-          ->setError($e_secret));
   }
 
   public function renderConfigPropertyTransactionTitle(
@@ -238,11 +176,6 @@ abstract class PhabricatorAuthProviderOAuth1 extends PhabricatorAuthProvider {
     return parent::renderConfigPropertyTransactionTitle($xaction);
   }
 
-  protected function willSaveAccount(PhabricatorExternalAccount $account) {
-    parent::willSaveAccount($account);
-    $this->synchronizeOAuthAccount($account);
-  }
-
   protected function synchronizeOAuthAccount(
     PhabricatorExternalAccount $account) {
     $adapter = $this->getAdapter();
@@ -264,5 +197,4 @@ abstract class PhabricatorAuthProviderOAuth1 extends PhabricatorAuthProvider {
     parent::willRenderLinkedAccount($viewer, $item, $account);
   }
 
-
 }

src/applications/auth/provider/PhabricatorAuthProviderOAuth1JIRA.php

@@ -16,6 +16,10 @@ final class PhabricatorAuthProviderOAuth1JIRA
   }
 
   public function getConfigurationHelp() {
+    return $this->getProviderConfigurationHelp();
+  }
+
+  protected function getProviderConfigurationHelp() {
     if ($this->isSetup()) {
       return pht(
         "**Step 1 of 2**: Provide the name and URI for your JIRA install.\n\n".

src/applications/auth/provider/PhabricatorAuthProviderOAuth1Twitter.php

@@ -7,7 +7,7 @@ final class PhabricatorAuthProviderOAuth1Twitter
     return pht('Twitter');
   }
 
-  public function getConfigurationHelp() {
+  protected function getProviderConfigurationHelp() {
     $login_uri = PhabricatorEnv::getURI($this->getLoginURI());
 
     return pht(

src/applications/auth/provider/PhabricatorAuthProviderOAuth2.php

@@ -0,0 +1,266 @@
+<?php
+
+abstract class PhabricatorAuthProviderOAuth2
+  extends PhabricatorAuthProviderOAuth {
+
+  const PROPERTY_APP_ID = 'oauth:app:id';
+  const PROPERTY_APP_SECRET = 'oauth:app:secret';
+
+  protected function getIDKey() {
+    return self::PROPERTY_APP_ID;
+  }
+
+  protected function getSecretKey() {
+    return self::PROPERTY_APP_SECRET;
+  }
+
+
+  protected function configureAdapter(PhutilAuthAdapterOAuth $adapter) {
+    $config = $this->getProviderConfig();
+    $adapter->setClientID($config->getProperty(self::PROPERTY_APP_ID));
+    $adapter->setClientSecret(
+      new PhutilOpaqueEnvelope(
+        $config->getProperty(self::PROPERTY_APP_SECRET)));
+    $adapter->setRedirectURI(PhabricatorEnv::getURI($this->getLoginURI()));
+    return $adapter;
+  }
+
+  protected function renderLoginForm(AphrontRequest $request, $mode) {
+    $adapter = $this->getAdapter();
+    $adapter->setState($this->getAuthCSRFCode($request));
+
+    $scope = $request->getStr('scope');
+    if ($scope) {
+      $adapter->setScope($scope);
+    }
+
+    $attributes = array(
+      'method' => 'GET',
+      'uri' => $adapter->getAuthenticateURI(),
+    );
+
+    return $this->renderStandardLoginButton($request, $mode, $attributes);
+  }
+
+  public function processLoginRequest(
+    PhabricatorAuthLoginController $controller) {
+
+    $request = $controller->getRequest();
+    $adapter = $this->getAdapter();
+    $account = null;
+    $response = null;
+
+    $error = $request->getStr('error');
+    if ($error) {
+      $response = $controller->buildProviderErrorResponse(
+        $this,
+        pht(
+          'The OAuth provider returned an error: %s',
+          $error));
+
+      return array($account, $response);
+    }
+
+    $this->verifyAuthCSRFCode($request, $request->getStr('state'));
+
+    $code = $request->getStr('code');
+    if (!strlen($code)) {
+      $response = $controller->buildProviderErrorResponse(
+        $this,
+        pht(
+          'The OAuth provider did not return a "code" parameter in its '.
+          'response.'));
+
+      return array($account, $response);
+    }
+
+    $adapter->setCode($code);
+
+    // NOTE: As a side effect, this will cause the OAuth adapter to request
+    // an access token.
+
+    try {
+      $account_id = $adapter->getAccountID();
+    } catch (Exception $ex) {
+      // TODO: Handle this in a more user-friendly way.
+      throw $ex;
+    }
+
+    if (!strlen($account_id)) {
+      $response = $controller->buildProviderErrorResponse(
+        $this,
+        pht(
+          'The OAuth provider failed to retrieve an account ID.'));
+
+      return array($account, $response);
+    }
+
+    return array($this->loadOrCreateAccount($account_id), $response);
+  }
+
+  public function processEditForm(
+    AphrontRequest $request,
+    array $values) {
+
+    return $this->processOAuthEditForm(
+      $request,
+      $values,
+      pht('Application ID is required.'),
+      pht('Application secret is required.'));
+  }
+
+  public function extendEditForm(
+    AphrontRequest $request,
+    AphrontFormView $form,
+    array $values,
+    array $issues) {
+
+    return $this->extendOAuthEditForm(
+      $request,
+      $form,
+      $values,
+      $issues,
+      pht('OAuth App ID'),
+      pht('OAuth App Secret'));
+  }
+
+  public function renderConfigPropertyTransactionTitle(
+    PhabricatorAuthProviderConfigTransaction $xaction) {
+
+    $author_phid = $xaction->getAuthorPHID();
+    $old = $xaction->getOldValue();
+    $new = $xaction->getNewValue();
+    $key = $xaction->getMetadataValue(
+      PhabricatorAuthProviderConfigTransaction::PROPERTY_KEY);
+
+    switch ($key) {
+      case self::PROPERTY_APP_ID:
+        if (strlen($old)) {
+          return pht(
+            '%s updated the OAuth application ID for this provider from '.
+            '"%s" to "%s".',
+            $xaction->renderHandleLink($author_phid),
+            $old,
+            $new);
+        } else {
+          return pht(
+            '%s set the OAuth application ID for this provider to '.
+            '"%s".',
+            $xaction->renderHandleLink($author_phid),
+            $new);
+        }
+      case self::PROPERTY_APP_SECRET:
+        if (strlen($old)) {
+          return pht(
+            '%s updated the OAuth application secret for this provider.',
+            $xaction->renderHandleLink($author_phid));
+        } else {
+          return pht(
+            '%s set the OAuth application secret for this provider.',
+            $xaction->renderHandleLink($author_phid));
+        }
+      case self::PROPERTY_APP_NOTE:
+        if (strlen($old)) {
+          return pht(
+            '%s updated the OAuth application notes for this provider.',
+            $xaction->renderHandleLink($author_phid));
+        } else {
+          return pht(
+            '%s set the OAuth application notes for this provider.',
+            $xaction->renderHandleLink($author_phid));
+        }
+
+    }
+
+    return parent::renderConfigPropertyTransactionTitle($xaction);
+  }
+
+  protected function synchronizeOAuthAccount(
+    PhabricatorExternalAccount $account) {
+    $adapter = $this->getAdapter();
+
+    $oauth_token = $adapter->getAccessToken();
+    $account->setProperty('oauth.token.access', $oauth_token);
+
+    if ($adapter->supportsTokenRefresh()) {
+      $refresh_token = $adapter->getRefreshToken();
+      $account->setProperty('oauth.token.refresh', $refresh_token);
+    } else {
+      $account->setProperty('oauth.token.refresh', null);
+    }
+
+    $expires = $adapter->getAccessTokenExpires();
+    $account->setProperty('oauth.token.access.expires', $expires);
+  }
+
+  public function getOAuthAccessToken(
+    PhabricatorExternalAccount $account,
+    $force_refresh = false) {
+
+    if ($account->getProviderKey() !== $this->getProviderKey()) {
+      throw new Exception("Account does not match provider!");
+    }
+
+    if (!$force_refresh) {
+      $access_expires = $account->getProperty('oauth.token.access.expires');
+      $access_token = $account->getProperty('oauth.token.access');
+
+      // Don't return a token with fewer than this many seconds remaining until
+      // it expires.
+      $shortest_token = 60;
+      if ($access_token) {
+        if ($access_expires === null ||
+            $access_expires > (time() + $shortest_token)) {
+          return $access_token;
+        }
+      }
+    }
+
+    $refresh_token = $account->getProperty('oauth.token.refresh');
+    if ($refresh_token) {
+      $adapter = $this->getAdapter();
+      if ($adapter->supportsTokenRefresh()) {
+        $adapter->refreshAccessToken($refresh_token);
+
+        $this->synchronizeOAuthAccount($account);
+        $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
+          $account->save();
+        unset($unguarded);
+
+        return $account->getProperty('oauth.token.access');
+      }
+    }
+
+    return null;
+  }
+
+  public function willRenderLinkedAccount(
+    PhabricatorUser $viewer,
+    PHUIObjectItemView $item,
+    PhabricatorExternalAccount $account) {
+
+    // Get a valid token, possibly refreshing it.
+    $oauth_token = $this->getOAuthAccessToken($account);
+
+    $item->addAttribute(pht('OAuth2 Account'));
+
+    if ($oauth_token) {
+      $oauth_expires = $account->getProperty('oauth.token.access.expires');
+      if ($oauth_expires) {
+        $item->addAttribute(
+          pht(
+            'Active OAuth Token (Expires: %s)',
+            phabricator_datetime($oauth_expires, $viewer)));
+      } else {
+        $item->addAttribute(
+          pht(
+            'Active OAuth Token'));
+      }
+    } else {
+      $item->addAttribute(pht('No OAuth Access Token'));
+    }
+
+    parent::willRenderLinkedAccount($viewer, $item, $account);
+  }
+
+}

src/applications/auth/provider/PhabricatorAuthProviderOAuthAmazon.php

@@ -1,13 +1,13 @@
 <?php
 
 final class PhabricatorAuthProviderOAuthAmazon
-  extends PhabricatorAuthProviderOAuth {
+  extends PhabricatorAuthProviderOAuth2 {
 
   public function getProviderName() {
     return pht('Amazon');
   }
 
-  public function getConfigurationHelp() {
+  protected function getProviderConfigurationHelp() {
     $login_uri = PhabricatorEnv::getURI($this->getLoginURI());
 
     $uri = new PhutilURI(PhabricatorEnv::getProductionURI('/'));

src/applications/auth/provider/PhabricatorAuthProviderOAuthAsana.php

@@ -1,13 +1,13 @@
 <?php
 
 final class PhabricatorAuthProviderOAuthAsana
-  extends PhabricatorAuthProviderOAuth {
+  extends PhabricatorAuthProviderOAuth2 {
 
   public function getProviderName() {
     return pht('Asana');
   }
 
-  public function getConfigurationHelp() {
+  protected function getProviderConfigurationHelp() {
     $app_uri = PhabricatorEnv::getProductionURI('/');
     $login_uri = PhabricatorEnv::getURI($this->getLoginURI());
 

src/applications/auth/provider/PhabricatorAuthProviderOAuthDisqus.php

@@ -1,13 +1,13 @@
 <?php
 
 final class PhabricatorAuthProviderOAuthDisqus
-  extends PhabricatorAuthProviderOAuth {
+  extends PhabricatorAuthProviderOAuth2 {
 
   public function getProviderName() {
     return pht('Disqus');
   }
 
-  public function getConfigurationHelp() {
+  protected function getProviderConfigurationHelp() {
     $login_uri = PhabricatorEnv::getURI($this->getLoginURI());
 
     return pht(

src/applications/auth/provider/PhabricatorAuthProviderOAuthFacebook.php

@@ -1,7 +1,7 @@
 <?php
 
 final class PhabricatorAuthProviderOAuthFacebook
-  extends PhabricatorAuthProviderOAuth {
+  extends PhabricatorAuthProviderOAuth2 {
 
   const KEY_REQUIRE_SECURE = 'oauth:facebook:require-secure';
 
@@ -9,7 +9,7 @@ final class PhabricatorAuthProviderOAuthFacebook
     return pht('Facebook');
   }
 
-  public function getConfigurationHelp() {
+  protected function getProviderConfigurationHelp() {
     $uri = PhabricatorEnv::getProductionURI($this->getLoginURI());
     return pht(
       'To configure Facebook OAuth, create a new Facebook Application here:'.

src/applications/auth/provider/PhabricatorAuthProviderOAuthGitHub.php

@@ -1,13 +1,13 @@
 <?php
 
 final class PhabricatorAuthProviderOAuthGitHub
-  extends PhabricatorAuthProviderOAuth {
+  extends PhabricatorAuthProviderOAuth2 {
 
   public function getProviderName() {
     return pht('GitHub');
   }
 
-  public function getConfigurationHelp() {
+  protected function getProviderConfigurationHelp() {
     $uri = PhabricatorEnv::getProductionURI('/');
     $callback_uri = PhabricatorEnv::getURI($this->getLoginURI());
 

src/applications/auth/provider/PhabricatorAuthProviderOAuthGoogle.php

@@ -1,13 +1,13 @@
 <?php
 
 final class PhabricatorAuthProviderOAuthGoogle
-  extends PhabricatorAuthProviderOAuth {
+  extends PhabricatorAuthProviderOAuth2 {
 
   public function getProviderName() {
     return pht('Google');
   }
 
-  public function getConfigurationHelp() {
+  protected function getProviderConfigurationHelp() {
     $login_uri = PhabricatorEnv::getURI($this->getLoginURI());
 
     return pht(
@@ -19,6 +19,8 @@ final class PhabricatorAuthProviderOAuthGoogle
       "\n\n".
       "  - Under **APIs & auth > APIs**, scroll down the list and enable ".
       "    the **Google+ API**.\n".
+      "     - You will need to consent to the **Google+ API** terms if you ".
+      " have not before.\n".
       "  - Under **APIs & auth > Credentials**, click **Create New Client".
       "    ID** in the **OAuth** section. Then use these settings:\n".
       "     - **Application Type**: Web Application\n".

src/applications/auth/provider/PhabricatorAuthProviderOAuthTwitch.php

@@ -1,13 +1,13 @@
 <?php
 
 final class PhabricatorAuthProviderOAuthTwitch
-  extends PhabricatorAuthProviderOAuth {
+  extends PhabricatorAuthProviderOAuth2 {
 
   public function getProviderName() {
     return pht('Twitch.tv');
   }
 
-  public function getConfigurationHelp() {
+  protected function getProviderConfigurationHelp() {
     $login_uri = PhabricatorEnv::getURI($this->getLoginURI());
 
     return pht(