Make temporary token storage/schema more flexible

Summary: Ref T10603. This makes minor updates to temporary tokens: - Rename `objectPHID` (which is sometimes used to store some other kind of identifier instead of a PHID) to `tokenResource` (i.e., which resource does this token permit access to?). - Add a `userPHID` column. For LFS tokens and some other types of tokens, I want to bind the token to both a resource (like a repository) and a user. - Add a `properties` column. This makes tokens more flexible and supports custom behavior (like scoping LFS tokens even more tightly). Test Plan: - Ran `bin/storage upgrade -f`, got a clean upgrade. - Viewed one-time tokens. - Revoked one token. - Revoked all tokens. - Performed a one-time login. - Performed a password reset. - Added an MFA token. - Removed an MFA token. - Used a file token to view a file. - Verified file token was removed after viewing file. - Linked my account to an OAuth1 account (Twitter). Reviewers: chad Reviewed By: chad Maniphest Tasks: T10603 Differential Revision: https://secure.phabricator.com/D15478
2024-11-18 12:52:42 +01:00 · 2016-03-16 05:17:47 -07:00 · 2016-03-16 05:17:47 -07:00 · a837c3d73e
commit a837c3d73e
parent 8e3ea4e034
14 changed files with 79 additions and 49 deletions
--- a/resources/sql/autopatches/20160316.lfs.01.token.resource.sql
+++ b/resources/sql/autopatches/20160316.lfs.01.token.resource.sql
@ -0,0 +1,2 @@
+ALTER TABLE {$NAMESPACE}_auth.auth_temporarytoken
+  CHANGE objectPHID tokenResource VARBINARY(64) NOT NULL;
--- a/resources/sql/autopatches/20160316.lfs.02.token.user.sql
+++ b/resources/sql/autopatches/20160316.lfs.02.token.user.sql
@ -0,0 +1,2 @@
+ALTER TABLE {$NAMESPACE}_auth.auth_temporarytoken
+  ADD userPHID VARBINARY(64);
--- a/resources/sql/autopatches/20160316.lfs.03.token.properties.sql
+++ b/resources/sql/autopatches/20160316.lfs.03.token.properties.sql
@ -0,0 +1,2 @@
+ALTER TABLE {$NAMESPACE}_auth.auth_temporarytoken
+  ADD properties LONGTEXT NOT NULL COLLATE {$COLLATE_TEXT};
--- a/resources/sql/autopatches/20160316.lfs.04.token.default.sql
+++ b/resources/sql/autopatches/20160316.lfs.04.token.default.sql
@ -0,0 +1,2 @@
+UPDATE {$NAMESPACE}_auth.auth_temporarytoken
+  SET properties = '{}' WHERE properties = '';
--- a/src/applications/auth/controller/PhabricatorAuthOneTimeLoginController.php
+++ b/src/applications/auth/controller/PhabricatorAuthOneTimeLoginController.php
@ -132,7 +132,7 @@ final class PhabricatorAuthOneTimeLoginController

        $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
          id(new PhabricatorAuthTemporaryToken())
-            ->setObjectPHID($target_user->getPHID())
+            ->setTokenResource($target_user->getPHID())
            ->setTokenType($password_type)
            ->setTokenExpires(time() + phutil_units('1 hour in seconds'))
            ->setTokenCode(PhabricatorHash::digest($key))
--- a/src/applications/auth/controller/PhabricatorAuthRevokeTokenController.php
+++ b/src/applications/auth/controller/PhabricatorAuthRevokeTokenController.php
@ -11,7 +11,7 @@ final class PhabricatorAuthRevokeTokenController

    $query = id(new PhabricatorAuthTemporaryTokenQuery())
      ->setViewer($viewer)
-      ->withObjectPHIDs(array($viewer->getPHID()));
+      ->withTokenResources(array($viewer->getPHID()));
    if (!$is_all) {
      $query->withIDs(array($id));
    }
--- a/src/applications/auth/engine/PhabricatorAuthSessionEngine.php
+++ b/src/applications/auth/engine/PhabricatorAuthSessionEngine.php
@ -635,7 +635,7 @@ final class PhabricatorAuthSessionEngine extends Phobject {

    $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
      id(new PhabricatorAuthTemporaryToken())
-        ->setObjectPHID($user->getPHID())
+        ->setTokenResource($user->getPHID())
        ->setTokenType($onetime_type)
        ->setTokenExpires(time() + phutil_units('1 day in seconds'))
        ->setTokenCode($key_hash)
@ -679,7 +679,7 @@ final class PhabricatorAuthSessionEngine extends Phobject {

    return id(new PhabricatorAuthTemporaryTokenQuery())
      ->setViewer($user)
-      ->withObjectPHIDs(array($user->getPHID()))
+      ->withTokenResources(array($user->getPHID()))
      ->withTokenTypes(array($onetime_type))
      ->withTokenCodes(array($key_hash))
      ->withExpired(false)
--- a/src/applications/auth/factor/PhabricatorTOTPAuthFactor.php
+++ b/src/applications/auth/factor/PhabricatorTOTPAuthFactor.php
@ -36,7 +36,7 @@ final class PhabricatorTOTPAuthFactor extends PhabricatorAuthFactor {

      $temporary_token = id(new PhabricatorAuthTemporaryTokenQuery())
        ->setViewer($user)
-        ->withObjectPHIDs(array($user->getPHID()))
+        ->withTokenResources(array($user->getPHID()))
        ->withTokenTypes(array(self::TEMPORARY_TOKEN_TYPE))
        ->withExpired(false)
        ->withTokenCodes(array(PhabricatorHash::digest($key)))
@ -55,7 +55,7 @@ final class PhabricatorTOTPAuthFactor extends PhabricatorAuthFactor {

      $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
        id(new PhabricatorAuthTemporaryToken())
-          ->setObjectPHID($user->getPHID())
+          ->setTokenResource($user->getPHID())
          ->setTokenType(self::TEMPORARY_TOKEN_TYPE)
          ->setTokenExpires(time() + phutil_units('1 hour in seconds'))
          ->setTokenCode(PhabricatorHash::digest($key))
--- a/src/applications/auth/provider/PhabricatorOAuth1AuthProvider.php
+++ b/src/applications/auth/provider/PhabricatorOAuth1AuthProvider.php
@ -221,7 +221,7 @@ abstract class PhabricatorOAuth1AuthProvider
    // Wipe out an existing token, if one exists.
    $token = id(new PhabricatorAuthTemporaryTokenQuery())
      ->setViewer(PhabricatorUser::getOmnipotentUser())
-      ->withObjectPHIDs(array($key))
+      ->withTokenResources(array($key))
      ->withTokenTypes(array($type))
      ->executeOne();
    if ($token) {
@ -230,7 +230,7 @@ abstract class PhabricatorOAuth1AuthProvider

    // Save the new secret.
    id(new PhabricatorAuthTemporaryToken())
-      ->setObjectPHID($key)
+      ->setTokenResource($key)
      ->setTokenType($type)
      ->setTokenExpires(time() + phutil_units('1 hour in seconds'))
      ->setTokenCode($secret)
@ -243,7 +243,7 @@ abstract class PhabricatorOAuth1AuthProvider

    $token = id(new PhabricatorAuthTemporaryTokenQuery())
      ->setViewer(PhabricatorUser::getOmnipotentUser())
-      ->withObjectPHIDs(array($key))
+      ->withTokenResources(array($key))
      ->withTokenTypes(array($type))
      ->withExpired(false)
      ->executeOne();
--- a/src/applications/auth/query/PhabricatorAuthTemporaryTokenQuery.php
+++ b/src/applications/auth/query/PhabricatorAuthTemporaryTokenQuery.php
@ -4,8 +4,9 @@ final class PhabricatorAuthTemporaryTokenQuery
  extends PhabricatorCursorPagedPolicyAwareQuery {

  private $ids;
-  private $objectPHIDs;
+  private $tokenResources;
  private $tokenTypes;
+  private $userPHIDs;
  private $expired;
  private $tokenCodes;

@ -14,8 +15,8 @@ final class PhabricatorAuthTemporaryTokenQuery
    return $this;
  }

-  public function withObjectPHIDs(array $object_phids) {
-    $this->objectPHIDs = $object_phids;
+  public function withTokenResources(array $resources) {
+    $this->tokenResources = $resources;
    return $this;
  }

@ -34,41 +35,39 @@ final class PhabricatorAuthTemporaryTokenQuery
    return $this;
  }

-  protected function loadPage() {
-    $table = new PhabricatorAuthTemporaryToken();
-    $conn_r = $table->establishConnection('r');
-
-    $data = queryfx_all(
-      $conn_r,
-      'SELECT * FROM %T %Q %Q %Q',
-      $table->getTableName(),
-      $this->buildWhereClause($conn_r),
-      $this->buildOrderClause($conn_r),
-      $this->buildLimitClause($conn_r));
-
-    return $table->loadAllFromArray($data);
+  public function withUserPHIDs(array $phids) {
+    $this->userPHIDs = $phids;
+    return $this;
  }

-  protected function buildWhereClause(AphrontDatabaseConnection $conn_r) {
-    $where = array();
+  public function newResultObject() {
+    return new PhabricatorAuthTemporaryToken();
+  }
+
+  protected function loadPage() {
+    return $this->loadStandardPage($this->newResultObject());
+  }
+
+  protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) {
+    $where = parent::buildWhereClauseParts($conn);

    if ($this->ids !== null) {
      $where[] = qsprintf(
-        $conn_r,
+        $conn,
        'id IN (%Ld)',
        $this->ids);
    }

-    if ($this->objectPHIDs !== null) {
+    if ($this->tokenResources !== null) {
      $where[] = qsprintf(
-        $conn_r,
-        'objectPHID IN (%Ls)',
-        $this->objectPHIDs);
+        $conn,
+        'tokenResource IN (%Ls)',
+        $this->tokenResources);
    }

    if ($this->tokenTypes !== null) {
      $where[] = qsprintf(
-        $conn_r,
+        $conn,
        'tokenType IN (%Ls)',
        $this->tokenTypes);
    }
@ -76,12 +75,12 @@ final class PhabricatorAuthTemporaryTokenQuery
    if ($this->expired !== null) {
      if ($this->expired) {
        $where[] = qsprintf(
-          $conn_r,
+          $conn,
          'tokenExpires <= %d',
          time());
      } else {
        $where[] = qsprintf(
-          $conn_r,
+          $conn,
          'tokenExpires > %d',
          time());
      }
@ -89,14 +88,19 @@ final class PhabricatorAuthTemporaryTokenQuery

    if ($this->tokenCodes !== null) {
      $where[] = qsprintf(
-        $conn_r,
+        $conn,
        'tokenCode IN (%Ls)',
        $this->tokenCodes);
    }

-    $where[] = $this->buildPagingClause($conn_r);
+    if ($this->userPHIDs !== null) {
+      $where[] = qsprintf(
+        $conn,
+        'userPHID IN (%Ls)',
+        $this->userPHIDs);
+    }

-    return $this->formatWhereClause($where);
+    return $where;
  }

  public function getQueryApplicationClass() {
--- a/src/applications/auth/storage/PhabricatorAuthTemporaryToken.php
+++ b/src/applications/auth/storage/PhabricatorAuthTemporaryToken.php
@ -3,30 +3,39 @@
 final class PhabricatorAuthTemporaryToken extends PhabricatorAuthDAO
  implements PhabricatorPolicyInterface {

-  // TODO: OAuth1 stores a client identifier here, which is not a real PHID.
-  // At some point, we should rename this column to be a little more generic.
-  protected $objectPHID;
-
+  // NOTE: This is usually a PHID, but may be some other kind of resource
+  // identifier for some token types.
+  protected $tokenResource;
  protected $tokenType;
  protected $tokenExpires;
  protected $tokenCode;
+  protected $userPHID;
+  protected $properties;

  protected function getConfiguration() {
    return array(
      self::CONFIG_TIMESTAMPS => false,
+      self::CONFIG_SERIALIZATION => array(
+        'properties' => self::SERIALIZATION_JSON,
+      ),
      self::CONFIG_COLUMN_SCHEMA => array(
+        'tokenResource' => 'phid',
        'tokenType' => 'text64',
        'tokenExpires' => 'epoch',
        'tokenCode' => 'text64',
+        'userPHID' => 'phid?',
      ),
      self::CONFIG_KEY_SCHEMA => array(
        'key_token' => array(
-          'columns' => array('objectPHID', 'tokenType', 'tokenCode'),
+          'columns' => array('tokenResource', 'tokenType', 'tokenCode'),
          'unique' => true,
        ),
        'key_expires' => array(
          'columns' => array('tokenExpires'),
        ),
+        'key_user' => array(
+          'columns' => array('userPHID'),
+        ),
      ),
    ) + parent::getConfiguration();
  }
@ -73,12 +82,12 @@ final class PhabricatorAuthTemporaryToken extends PhabricatorAuthDAO

  public static function revokeTokens(
    PhabricatorUser $viewer,
-    array $object_phids,
+    array $token_resources,
    array $token_types) {

    $tokens = id(new PhabricatorAuthTemporaryTokenQuery())
      ->setViewer($viewer)
-      ->withObjectPHIDs($object_phids)
+      ->withTokenResources($token_resources)
      ->withTokenTypes($token_types)
      ->withExpired(false)
      ->execute();
@ -88,6 +97,15 @@ final class PhabricatorAuthTemporaryToken extends PhabricatorAuthDAO
    }
  }

+  public function getTemporaryTokenProperty($key, $default = null) {
+    return idx($this->properties, $key, $default);
+  }
+
+  public function setTemporaryTokenProperty($key, $value) {
+    $this->properties[$key] = $value;
+    return $this;
+  }
+
 /* -(  PhabricatorPolicyInterface  )----------------------------------------- */


--- a/src/applications/files/storage/PhabricatorFile.php
+++ b/src/applications/files/storage/PhabricatorFile.php
@ -1123,7 +1123,7 @@ final class PhabricatorFile extends PhabricatorFileDAO
    // Save the new secret.
    $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
      $token = id(new PhabricatorAuthTemporaryToken())
-        ->setObjectPHID($this->getPHID())
+        ->setTokenResource($this->getPHID())
        ->setTokenType(self::ONETIME_TEMPORARY_TOKEN_TYPE)
        ->setTokenExpires(time() + phutil_units('1 hour in seconds'))
        ->setTokenCode(PhabricatorHash::digest($key))
@ -1136,7 +1136,7 @@ final class PhabricatorFile extends PhabricatorFileDAO
  public function validateOneTimeToken($token_code) {
    $token = id(new PhabricatorAuthTemporaryTokenQuery())
      ->setViewer(PhabricatorUser::getOmnipotentUser())
-      ->withObjectPHIDs(array($this->getPHID()))
+      ->withTokenResources(array($this->getPHID()))
      ->withTokenTypes(array(self::ONETIME_TEMPORARY_TOKEN_TYPE))
      ->withExpired(false)
      ->withTokenCodes(array(PhabricatorHash::digest($token_code)))
--- a/src/applications/settings/panel/PhabricatorPasswordSettingsPanel.php
+++ b/src/applications/settings/panel/PhabricatorPasswordSettingsPanel.php
@ -46,7 +46,7 @@ final class PhabricatorPasswordSettingsPanel extends PhabricatorSettingsPanel {
    if ($key) {
      $token = id(new PhabricatorAuthTemporaryTokenQuery())
        ->setViewer($user)
-        ->withObjectPHIDs(array($user->getPHID()))
+        ->withTokenResources(array($user->getPHID()))
        ->withTokenTypes(array($password_type))
        ->withTokenCodes(array(PhabricatorHash::digest($key)))
        ->withExpired(false)
--- a/src/applications/settings/panel/PhabricatorTokensSettingsPanel.php
+++ b/src/applications/settings/panel/PhabricatorTokensSettingsPanel.php
@ -23,7 +23,7 @@ final class PhabricatorTokensSettingsPanel extends PhabricatorSettingsPanel {

    $tokens = id(new PhabricatorAuthTemporaryTokenQuery())
      ->setViewer($viewer)
-      ->withObjectPHIDs(array($viewer->getPHID()))
+      ->withTokenResources(array($viewer->getPHID()))
      ->execute();

    $rows = array();