From 8909f8ec599b3751006f530797ede9061b9fd6ed Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Tue, 18 Mar 2014 13:27:55 -0700
Subject: [PATCH] Modernize OAuthServer PHIDs and Queries

Summary:
  - Modernize PHID types.
  - Implement `PhabricatorPolicyInterface`.
  - Make queries policy aware.

Test Plan: Browsed client and authorization lists.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: chad, epriestley

Differential Revision: https://secure.phabricator.com/D8560
---
 src/__phutil_library_map__.php                | 20 ++++++++--
 .../PhabricatorOAuthClientListController.php  |  5 ++-
 ...OAuthClientAuthorizationListController.php |  5 ++-
 .../PhabricatorOAuthServerPHIDTypeClient.php  | 40 +++++++++++++++++++
 ...OAuthServerPHIDTypeClientAuthorization.php | 39 ++++++++++++++++++
 ...abricatorOAuthClientAuthorizationQuery.php | 32 +++++++++++----
 .../PhabricatorOAuthServerClientQuery.php     | 32 +++++++++++----
 .../PhabricatorOAuthClientAuthorization.php   | 36 +++++++++++++----
 .../storage/PhabricatorOAuthServerClient.php  | 35 ++++++++++++----
 .../phid/PhabricatorPHIDConstants.php         |  2 -
 10 files changed, 208 insertions(+), 38 deletions(-)
 create mode 100644 src/applications/oauthserver/phid/PhabricatorOAuthServerPHIDTypeClient.php
 create mode 100644 src/applications/oauthserver/phid/PhabricatorOAuthServerPHIDTypeClientAuthorization.php

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index c70e073f2c..7437b4535a 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -1726,6 +1726,8 @@ phutil_register_library_map(array(
     'PhabricatorOAuthServerConsoleController' => 'applications/oauthserver/controller/PhabricatorOAuthServerConsoleController.php',
     'PhabricatorOAuthServerController' => 'applications/oauthserver/controller/PhabricatorOAuthServerController.php',
     'PhabricatorOAuthServerDAO' => 'applications/oauthserver/storage/PhabricatorOAuthServerDAO.php',
+    'PhabricatorOAuthServerPHIDTypeClient' => 'applications/oauthserver/phid/PhabricatorOAuthServerPHIDTypeClient.php',
+    'PhabricatorOAuthServerPHIDTypeClientAuthorization' => 'applications/oauthserver/phid/PhabricatorOAuthServerPHIDTypeClientAuthorization.php',
     'PhabricatorOAuthServerScope' => 'applications/oauthserver/PhabricatorOAuthServerScope.php',
     'PhabricatorOAuthServerTestCase' => 'applications/oauthserver/__tests__/PhabricatorOAuthServerTestCase.php',
     'PhabricatorOAuthServerTestController' => 'applications/oauthserver/controller/PhabricatorOAuthServerTestController.php',
@@ -4456,12 +4458,16 @@ phutil_register_library_map(array(
     'PhabricatorNotificationQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
     'PhabricatorNotificationStatusController' => 'PhabricatorNotificationController',
     'PhabricatorNotificationTestController' => 'PhabricatorNotificationController',
-    'PhabricatorOAuthClientAuthorization' => 'PhabricatorOAuthServerDAO',
+    'PhabricatorOAuthClientAuthorization' =>
+    array(
+      0 => 'PhabricatorOAuthServerDAO',
+      1 => 'PhabricatorPolicyInterface',
+    ),
     'PhabricatorOAuthClientAuthorizationBaseController' => 'PhabricatorOAuthServerController',
     'PhabricatorOAuthClientAuthorizationDeleteController' => 'PhabricatorOAuthClientAuthorizationBaseController',
     'PhabricatorOAuthClientAuthorizationEditController' => 'PhabricatorOAuthClientAuthorizationBaseController',
     'PhabricatorOAuthClientAuthorizationListController' => 'PhabricatorOAuthClientAuthorizationBaseController',
-    'PhabricatorOAuthClientAuthorizationQuery' => 'PhabricatorOffsetPagedQuery',
+    'PhabricatorOAuthClientAuthorizationQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
     'PhabricatorOAuthClientBaseController' => 'PhabricatorOAuthServerController',
     'PhabricatorOAuthClientDeleteController' => 'PhabricatorOAuthClientBaseController',
     'PhabricatorOAuthClientEditController' => 'PhabricatorOAuthClientBaseController',
@@ -4471,11 +4477,17 @@ phutil_register_library_map(array(
     'PhabricatorOAuthServerAccessToken' => 'PhabricatorOAuthServerDAO',
     'PhabricatorOAuthServerAuthController' => 'PhabricatorAuthController',
     'PhabricatorOAuthServerAuthorizationCode' => 'PhabricatorOAuthServerDAO',
-    'PhabricatorOAuthServerClient' => 'PhabricatorOAuthServerDAO',
-    'PhabricatorOAuthServerClientQuery' => 'PhabricatorOffsetPagedQuery',
+    'PhabricatorOAuthServerClient' =>
+    array(
+      0 => 'PhabricatorOAuthServerDAO',
+      1 => 'PhabricatorPolicyInterface',
+    ),
+    'PhabricatorOAuthServerClientQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
     'PhabricatorOAuthServerConsoleController' => 'PhabricatorOAuthServerController',
     'PhabricatorOAuthServerController' => 'PhabricatorController',
     'PhabricatorOAuthServerDAO' => 'PhabricatorLiskDAO',
+    'PhabricatorOAuthServerPHIDTypeClient' => 'PhabricatorPHIDType',
+    'PhabricatorOAuthServerPHIDTypeClientAuthorization' => 'PhabricatorPHIDType',
     'PhabricatorOAuthServerTestCase' => 'PhabricatorTestCase',
     'PhabricatorOAuthServerTestController' => 'PhabricatorOAuthServerController',
     'PhabricatorOAuthServerTokenController' => 'PhabricatorAuthController',
diff --git a/src/applications/oauthserver/controller/client/PhabricatorOAuthClientListController.php b/src/applications/oauthserver/controller/client/PhabricatorOAuthClientListController.php
index f1a78110f4..fa1437f3fa 100644
--- a/src/applications/oauthserver/controller/client/PhabricatorOAuthClientListController.php
+++ b/src/applications/oauthserver/controller/client/PhabricatorOAuthClientListController.php
@@ -22,8 +22,9 @@ extends PhabricatorOAuthClientBaseController {
     $pager->setPageSize($page_size);
     $pager->setOffset($offset);
 
-    $query = new PhabricatorOAuthServerClientQuery();
-    $query->withCreatorPHIDs(array($current_user->getPHID()));
+    $query = id(new PhabricatorOAuthServerClientQuery())
+      ->setViewer($current_user)
+      ->withCreatorPHIDs(array($current_user->getPHID()));
     $clients = $query->executeWithOffsetPager($pager);
 
     $rows      = array();
diff --git a/src/applications/oauthserver/controller/clientauthorization/PhabricatorOAuthClientAuthorizationListController.php b/src/applications/oauthserver/controller/clientauthorization/PhabricatorOAuthClientAuthorizationListController.php
index 82c30609ed..5a88188a33 100644
--- a/src/applications/oauthserver/controller/clientauthorization/PhabricatorOAuthClientAuthorizationListController.php
+++ b/src/applications/oauthserver/controller/clientauthorization/PhabricatorOAuthClientAuthorizationListController.php
@@ -22,8 +22,9 @@ extends PhabricatorOAuthClientAuthorizationBaseController {
     $pager->setPageSize($page_size);
     $pager->setOffset($offset);
 
-    $query = new PhabricatorOAuthClientAuthorizationQuery();
-    $query->withUserPHIDs(array($current_user->getPHID()));
+    $query = id(new PhabricatorOAuthClientAuthorizationQuery())
+      ->setViewer($current_user)
+      ->withUserPHIDs(array($current_user->getPHID()));
     $authorizations = $query->executeWithOffsetPager($pager);
 
     $client_authorizations = mpull($authorizations, null, 'getClientPHID');
diff --git a/src/applications/oauthserver/phid/PhabricatorOAuthServerPHIDTypeClient.php b/src/applications/oauthserver/phid/PhabricatorOAuthServerPHIDTypeClient.php
new file mode 100644
index 0000000000..f66101a62f
--- /dev/null
+++ b/src/applications/oauthserver/phid/PhabricatorOAuthServerPHIDTypeClient.php
@@ -0,0 +1,40 @@
+<?php
+
+final class PhabricatorOAuthServerPHIDTypeClient
+  extends PhabricatorPHIDType {
+
+  const TYPECONST = 'OASC';
+
+  public function getTypeConstant() {
+    return self::TYPECONST;
+  }
+
+  public function getTypeName() {
+    return pht('OAuth Application');
+  }
+
+  public function newObject() {
+    return new PhabricatorOAuthServerClient();
+  }
+
+  protected function buildQueryForObjects(
+    PhabricatorObjectQuery $query,
+    array $phids) {
+
+    return id(new PhabricatorOAuthServerClientQuery())
+      ->withPHIDs($phids);
+  }
+
+  public function loadHandles(
+    PhabricatorHandleQuery $query,
+    array $handles,
+    array $objects) {
+
+    foreach ($handles as $phid => $handle) {
+      $client = $objects[$phid];
+
+      $handle->setName($client->getName());
+    }
+  }
+
+}
diff --git a/src/applications/oauthserver/phid/PhabricatorOAuthServerPHIDTypeClientAuthorization.php b/src/applications/oauthserver/phid/PhabricatorOAuthServerPHIDTypeClientAuthorization.php
new file mode 100644
index 0000000000..2784263667
--- /dev/null
+++ b/src/applications/oauthserver/phid/PhabricatorOAuthServerPHIDTypeClientAuthorization.php
@@ -0,0 +1,39 @@
+<?php
+
+final class PhabricatorOAuthServerPHIDTypeClientAuthorization
+  extends PhabricatorPHIDType {
+
+  const TYPECONST = 'OASA';
+
+  public function getTypeConstant() {
+    return self::TYPECONST;
+  }
+
+  public function getTypeName() {
+    return pht('OAuth Authorization');
+  }
+
+  public function newObject() {
+    return new PhabricatorOAuthClientAuthorization();
+  }
+
+  protected function buildQueryForObjects(
+    PhabricatorObjectQuery $query,
+    array $phids) {
+
+    return id(new PhabricatorOAuthClientAuthorizationQuery())
+      ->withPHIDs($phids);
+  }
+
+  public function loadHandles(
+    PhabricatorHandleQuery $query,
+    array $handles,
+    array $objects) {
+
+    foreach ($handles as $phid => $handle) {
+      $authorization = $objects[$phid];
+      $handle->setName(pht('Authorization %d', $authorization->getID()));
+    }
+  }
+
+}
diff --git a/src/applications/oauthserver/query/PhabricatorOAuthClientAuthorizationQuery.php b/src/applications/oauthserver/query/PhabricatorOAuthClientAuthorizationQuery.php
index cb8fe083c7..bd13c69eb5 100644
--- a/src/applications/oauthserver/query/PhabricatorOAuthClientAuthorizationQuery.php
+++ b/src/applications/oauthserver/query/PhabricatorOAuthClientAuthorizationQuery.php
@@ -1,18 +1,22 @@
 <?php
 
 final class PhabricatorOAuthClientAuthorizationQuery
-extends PhabricatorOffsetPagedQuery {
+  extends PhabricatorCursorPagedPolicyAwareQuery {
+
+  private $phids;
   private $userPHIDs;
 
+  public function witHPHIDs(array $phids) {
+    $this->phids = $phids;
+    return $this;
+  }
+
   public function withUserPHIDs(array $phids) {
     $this->userPHIDs = $phids;
     return $this;
   }
-  private function getUserPHIDs() {
-    return $this->userPHIDs;
-  }
 
-  public function execute() {
+  public function loadPage() {
     $table  = new PhabricatorOAuthClientAuthorization();
     $conn_r = $table->establishConnection('r');
 
@@ -32,13 +36,27 @@ extends PhabricatorOffsetPagedQuery {
   private function buildWhereClause($conn_r) {
     $where = array();
 
-    if ($this->getUserPHIDs()) {
+    if ($this->phids) {
+      $where[] = qsprintf(
+        $conn_r,
+        'phid IN (%Ls)',
+        $this->phids);
+    }
+
+    if ($this->userPHIDs) {
       $where[] = qsprintf(
         $conn_r,
         'userPHID IN (%Ls)',
-        $this->getUserPHIDs());
+        $this->userPHIDs);
     }
 
+    $where[] = $this->buildPagingClause($conn_r);
+
     return $this->formatWhereClause($where);
   }
+
+  public function getQueryApplicationClass() {
+    return 'PhabricatorApplicationOAuthServer';
+  }
+
 }
diff --git a/src/applications/oauthserver/query/PhabricatorOAuthServerClientQuery.php b/src/applications/oauthserver/query/PhabricatorOAuthServerClientQuery.php
index 0e2272e6e4..681ca16a1e 100644
--- a/src/applications/oauthserver/query/PhabricatorOAuthServerClientQuery.php
+++ b/src/applications/oauthserver/query/PhabricatorOAuthServerClientQuery.php
@@ -1,18 +1,22 @@
 <?php
 
 final class PhabricatorOAuthServerClientQuery
-extends PhabricatorOffsetPagedQuery {
+  extends PhabricatorCursorPagedPolicyAwareQuery {
+
+  private $phids;
   private $creatorPHIDs;
 
+  public function withPHIDs(array $phids) {
+    $this->phids = $phids;
+    return $this;
+  }
+
   public function withCreatorPHIDs(array $phids) {
     $this->creatorPHIDs = $phids;
     return $this;
   }
-  private function getCreatorPHIDs() {
-    return $this->creatorPHIDs;
-  }
 
-  public function execute() {
+  public function loadPage() {
     $table  = new PhabricatorOAuthServerClient();
     $conn_r = $table->establishConnection('r');
 
@@ -32,13 +36,27 @@ extends PhabricatorOffsetPagedQuery {
   private function buildWhereClause($conn_r) {
     $where = array();
 
-    if ($this->getCreatorPHIDs()) {
+    if ($this->phids) {
+      $where[] = qsprintf(
+        $conn_r,
+        'phid IN (%Ls)',
+        $this->phids);
+    }
+
+    if ($this->creatorPHIDs) {
       $where[] = qsprintf(
         $conn_r,
         'creatorPHID IN (%Ls)',
-        $this->getCreatorPHIDs());
+        $this->creatorPHIDs);
     }
 
+    $where[] = $this->buildPagingClause($conn_r);
+
     return $this->formatWhereClause($where);
   }
+
+  public function getQueryApplicationClass() {
+    return 'PhabricatorApplicationOAuthServer';
+  }
+
 }
diff --git a/src/applications/oauthserver/storage/PhabricatorOAuthClientAuthorization.php b/src/applications/oauthserver/storage/PhabricatorOAuthClientAuthorization.php
index 7a420a8152..3c9137d8d0 100644
--- a/src/applications/oauthserver/storage/PhabricatorOAuthClientAuthorization.php
+++ b/src/applications/oauthserver/storage/PhabricatorOAuthClientAuthorization.php
@@ -1,13 +1,9 @@
 <?php
 
-/**
- * @group oauthserver
- */
 final class PhabricatorOAuthClientAuthorization
-extends PhabricatorOAuthServerDAO {
+  extends PhabricatorOAuthServerDAO
+  implements PhabricatorPolicyInterface {
 
-  protected $id;
-  protected $phid;
   protected $userPHID;
   protected $clientPHID;
   protected $scope;
@@ -38,6 +34,32 @@ extends PhabricatorOAuthServerDAO {
 
   public function generatePHID() {
     return PhabricatorPHID::generateNewPHID(
-      PhabricatorPHIDConstants::PHID_TYPE_OASA);
+      PhabricatorOAuthServerPHIDTypeClientAuthorization::TYPECONST);
   }
+
+
+/* -(  PhabricatorPolicyInterface  )----------------------------------------- */
+
+
+  public function getCapabilities() {
+    return array(
+      PhabricatorPolicyCapability::CAN_VIEW,
+    );
+  }
+
+  public function getPolicy($capability) {
+    switch ($capability) {
+      case PhabricatorPolicyCapability::CAN_VIEW:
+        return PhabricatorPolicies::POLICY_NOONE;
+    }
+  }
+
+  public function hasAutomaticCapability($capability, PhabricatorUser $viewer) {
+    return ($viewer->getPHID() == $this->getUserPHID());
+  }
+
+  public function describeAutomaticCapability($capability) {
+    return pht('Authorizations can only be viewed by the authorizing user.');
+  }
+
 }
diff --git a/src/applications/oauthserver/storage/PhabricatorOAuthServerClient.php b/src/applications/oauthserver/storage/PhabricatorOAuthServerClient.php
index 1c6f4aac17..e8f0541e75 100644
--- a/src/applications/oauthserver/storage/PhabricatorOAuthServerClient.php
+++ b/src/applications/oauthserver/storage/PhabricatorOAuthServerClient.php
@@ -1,13 +1,9 @@
 <?php
 
-/**
- * @group oauthserver
- */
 final class PhabricatorOAuthServerClient
-extends PhabricatorOAuthServerDAO {
+  extends PhabricatorOAuthServerDAO
+  implements PhabricatorPolicyInterface {
 
-  protected $id;
-  protected $phid;
   protected $secret;
   protected $name;
   protected $redirectURI;
@@ -33,7 +29,32 @@ extends PhabricatorOAuthServerDAO {
 
   public function generatePHID() {
     return PhabricatorPHID::generateNewPHID(
-      PhabricatorPHIDConstants::PHID_TYPE_OASC);
+      PhabricatorOAuthServerPHIDTypeClient::TYPECONST);
+  }
+
+
+/* -(  PhabricatorPolicyInterface  )----------------------------------------- */
+
+
+  public function getCapabilities() {
+    return array(
+      PhabricatorPolicyCapability::CAN_VIEW,
+    );
+  }
+
+  public function getPolicy($capability) {
+    switch ($capability) {
+      case PhabricatorPolicyCapability::CAN_VIEW:
+        return PhabricatorPolicies::POLICY_USER;
+    }
+  }
+
+  public function hasAutomaticCapability($capability, PhabricatorUser $viewer) {
+    return false;
+  }
+
+  public function describeAutomaticCapability($capability) {
+    return null;
   }
 
 }
diff --git a/src/applications/phid/PhabricatorPHIDConstants.php b/src/applications/phid/PhabricatorPHIDConstants.php
index ca53e365b9..4591d3608e 100644
--- a/src/applications/phid/PhabricatorPHIDConstants.php
+++ b/src/applications/phid/PhabricatorPHIDConstants.php
@@ -7,8 +7,6 @@ final class PhabricatorPHIDConstants {
   const PHID_TYPE_MAGIC   = '!!!!';
   const PHID_TYPE_STRY    = 'STRY';
   const PHID_TYPE_ACMT    = 'ACMT';
-  const PHID_TYPE_OASC    = 'OASC';
-  const PHID_TYPE_OASA    = 'OASA';
   const PHID_TYPE_TOBJ    = 'TOBJ';
   const PHID_TYPE_ACNT    = 'ACNT';
   const PHID_TYPE_PDCT    = 'PDCT';