Make OAuth client authorizations a Settings panel

Summary: This modernizes and simplifies OAuth client authorizations a bit, moving them to a settings panel similar to the "Sessions" panel.

Test Plan:
  - Viewed authorizations.
  - Revoked an authorization.
  - Created a test authorization.

{F131196}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

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

src/__phutil_library_map__.php

@@ -1706,10 +1706,6 @@ phutil_register_library_map(array(
     'PhabricatorNotificationStatusController' => 'applications/notification/controller/PhabricatorNotificationStatusController.php',
     'PhabricatorNotificationTestController' => 'applications/notification/controller/PhabricatorNotificationTestController.php',
     'PhabricatorOAuthClientAuthorization' => 'applications/oauthserver/storage/PhabricatorOAuthClientAuthorization.php',
-    'PhabricatorOAuthClientAuthorizationBaseController' => 'applications/oauthserver/controller/clientauthorization/PhabricatorOAuthClientAuthorizationBaseController.php',
-    'PhabricatorOAuthClientAuthorizationDeleteController' => 'applications/oauthserver/controller/clientauthorization/PhabricatorOAuthClientAuthorizationDeleteController.php',
-    'PhabricatorOAuthClientAuthorizationEditController' => 'applications/oauthserver/controller/clientauthorization/PhabricatorOAuthClientAuthorizationEditController.php',
-    'PhabricatorOAuthClientAuthorizationListController' => 'applications/oauthserver/controller/clientauthorization/PhabricatorOAuthClientAuthorizationListController.php',
     'PhabricatorOAuthClientAuthorizationQuery' => 'applications/oauthserver/query/PhabricatorOAuthClientAuthorizationQuery.php',
     'PhabricatorOAuthClientBaseController' => 'applications/oauthserver/controller/client/PhabricatorOAuthClientBaseController.php',
     'PhabricatorOAuthClientDeleteController' => 'applications/oauthserver/controller/client/PhabricatorOAuthClientDeleteController.php',
@@ -1721,6 +1717,7 @@ phutil_register_library_map(array(
     'PhabricatorOAuthServerAccessToken' => 'applications/oauthserver/storage/PhabricatorOAuthServerAccessToken.php',
     'PhabricatorOAuthServerAuthController' => 'applications/oauthserver/controller/PhabricatorOAuthServerAuthController.php',
     'PhabricatorOAuthServerAuthorizationCode' => 'applications/oauthserver/storage/PhabricatorOAuthServerAuthorizationCode.php',
+    'PhabricatorOAuthServerAuthorizationsSettingsPanel' => 'applications/oauthserver/panel/PhabricatorOAuthServerAuthorizationsSettingsPanel.php',
     'PhabricatorOAuthServerClient' => 'applications/oauthserver/storage/PhabricatorOAuthServerClient.php',
     'PhabricatorOAuthServerClientQuery' => 'applications/oauthserver/query/PhabricatorOAuthServerClientQuery.php',
     'PhabricatorOAuthServerConsoleController' => 'applications/oauthserver/controller/PhabricatorOAuthServerConsoleController.php',
@@ -4463,10 +4460,6 @@ phutil_register_library_map(array(
       0 => 'PhabricatorOAuthServerDAO',
       1 => 'PhabricatorPolicyInterface',
     ),
-    'PhabricatorOAuthClientAuthorizationBaseController' => 'PhabricatorOAuthServerController',
-    'PhabricatorOAuthClientAuthorizationDeleteController' => 'PhabricatorOAuthClientAuthorizationBaseController',
-    'PhabricatorOAuthClientAuthorizationEditController' => 'PhabricatorOAuthClientAuthorizationBaseController',
-    'PhabricatorOAuthClientAuthorizationListController' => 'PhabricatorOAuthClientAuthorizationBaseController',
     'PhabricatorOAuthClientAuthorizationQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
     'PhabricatorOAuthClientBaseController' => 'PhabricatorOAuthServerController',
     'PhabricatorOAuthClientDeleteController' => 'PhabricatorOAuthClientBaseController',
@@ -4477,6 +4470,7 @@ phutil_register_library_map(array(
     'PhabricatorOAuthServerAccessToken' => 'PhabricatorOAuthServerDAO',
     'PhabricatorOAuthServerAuthController' => 'PhabricatorAuthController',
     'PhabricatorOAuthServerAuthorizationCode' => 'PhabricatorOAuthServerDAO',
+    'PhabricatorOAuthServerAuthorizationsSettingsPanel' => 'PhabricatorSettingsPanel',
     'PhabricatorOAuthServerClient' =>
     array(
       0 => 'PhabricatorOAuthServerDAO',

src/applications/oauthserver/application/PhabricatorApplicationOAuthServer.php

@@ -37,13 +37,6 @@ final class PhabricatorApplicationOAuthServer extends PhabricatorApplication {
         'auth/'          => 'PhabricatorOAuthServerAuthController',
         'test/'          => 'PhabricatorOAuthServerTestController',
         'token/'         => 'PhabricatorOAuthServerTokenController',
-        'clientauthorization/' => array(
-          '' => 'PhabricatorOAuthClientAuthorizationListController',
-          'delete/(?P<phid>[^/]+)/' =>
-            'PhabricatorOAuthClientAuthorizationDeleteController',
-          'edit/(?P<phid>[^/]+)/' =>
-            'PhabricatorOAuthClientAuthorizationEditController',
-        ),
         'client/' => array(
           ''                        => 'PhabricatorOAuthClientListController',
           'create/'                 => 'PhabricatorOAuthClientEditController',

src/applications/oauthserver/controller/PhabricatorOAuthServerConsoleController.php

@@ -10,14 +10,6 @@ final class PhabricatorOAuthServerConsoleController
     $menu = id(new PHUIObjectItemListView())
       ->setUser($viewer);
 
-    $menu->addItem(
-      id(new PHUIObjectItemView())
-        ->setHeader(pht('Authorizations'))
-        ->setHref($this->getApplicationURI('clientauthorization/'))
-        ->addAttribute(
-          pht(
-            'Review your authorizations.')));
-
     $menu->addItem(
       id(new PHUIObjectItemView())
         ->setHeader(pht('Applications'))

src/applications/oauthserver/controller/PhabricatorOAuthServerController.php

@@ -12,9 +12,6 @@ extends PhabricatorController {
 
     $nav = new AphrontSideNavFilterView();
     $nav->setBaseURI(new PhutilURI('/oauthserver/'));
-    $nav->addLabel('Client Authorizations');
-    $nav->addFilter('clientauthorization',
-                    'My Authorizations');
     $nav->addLabel('Clients');
     $nav->addFilter('client/create',
                     'Create Client');

src/applications/oauthserver/controller/PhabricatorOAuthServerTestController.php

@@ -17,11 +17,15 @@ extends PhabricatorOAuthServerController {
     $panels       = array();
     $results      = array();
 
+    if (!$request->isFormPost()) {
+      return new Aphront400Response();
+    }
 
-    if ($request->isFormPost()) {
     $action = $request->getStr('action');
-      switch ($action) {
+    if ($action !== 'testclientauthorization') {
-        case 'testclientauthorization':
+      return new Aphront404Response();
+    }
+
     $user_phid   = $current_user->getPHID();
     $client_phid = $request->getStr('client_phid');
     $client      = id(new PhabricatorOAuthServerClient)
@@ -36,18 +40,16 @@ extends PhabricatorOAuthServerController {
           'for clients you own!'
         );
     }
+
     // blankclientauthorizations don't get scope
     $scope = array();
     $server->setUser($current_user);
     $server->setClient($client);
     $authorization = $server->authorizeClient($scope);
-          return id(new AphrontRedirectResponse())
+
-            ->setURI('/oauthserver/clientauthorization/?edited='.
+    $id = $authorization->getID();
-                     $authorization->getPHID());
+    $panel_uri = '/settings/panel/oauthorizations/?id='.$id;
-          break;
+
-        default:
+    return id(new AphrontRedirectResponse())->setURI($panel_uri);
-          break;
-      }
-    }
   }
 }

src/applications/oauthserver/controller/clientauthorization/PhabricatorOAuthClientAuthorizationBaseController.php

@@ -1,25 +0,0 @@
-<?php
-
-/**
- * @group oauthserver
- */
-abstract class PhabricatorOAuthClientAuthorizationBaseController
-extends PhabricatorOAuthServerController {
-
-  private $authorizationPHID;
-  protected function getAuthorizationPHID() {
-    return $this->authorizationPHID;
-  }
-  private function setAuthorizationPHID($phid) {
-    $this->authorizationPHID = $phid;
-    return $this;
-  }
-
-  public function shouldRequireLogin() {
-    return true;
-  }
-
-  public function willProcessRequest(array $data) {
-    $this->setAuthorizationPHID(idx($data, 'phid'));
-  }
-}

src/applications/oauthserver/controller/clientauthorization/PhabricatorOAuthClientAuthorizationDeleteController.php

@@ -1,56 +0,0 @@
-<?php
-
-/**
- * @group oauthserver
- */
-final class PhabricatorOAuthClientAuthorizationDeleteController
-extends PhabricatorOAuthClientAuthorizationBaseController {
-
-  public function processRequest() {
-    $phid          = $this->getAuthorizationPHID();
-    $title         = 'Delete OAuth Client Authorization';
-    $request       = $this->getRequest();
-    $current_user  = $request->getUser();
-    $authorization = id(new PhabricatorOAuthClientAuthorization())
-      ->loadOneWhere('phid = %s',
-                     $phid);
-
-    if (empty($authorization)) {
-      return new Aphront404Response();
-    }
-    if ($authorization->getUserPHID() != $current_user->getPHID()) {
-      $message = 'Access denied to client authorization with phid '.$phid.'. '.
-                 'Only the user who authorized the client has permission to '.
-                 'delete the authorization.';
-      return id(new Aphront403Response())
-        ->setForbiddenText($message);
-    }
-
-    if ($request->isFormPost()) {
-      $authorization->delete();
-      return id(new AphrontRedirectResponse())
-        ->setURI('/oauthserver/clientauthorization/?notice=deleted');
-    }
-
-    $client_phid = $authorization->getClientPHID();
-    $client      = id(new PhabricatorOAuthServerClient())
-      ->loadOneWhere('phid = %s',
-                     $client_phid);
-    if ($client) {
-      $title .= ' for '.$client->getName();
-    } else {
-      // the client does not exist so token is dead already (but
-      // let's let the user clean this up anyway in that case)
-    }
-
-    $dialog = new AphrontDialogView();
-    $dialog->setUser($current_user);
-    $dialog->setTitle($title);
-    $dialog->appendChild(phutil_tag('p', array(), pht(
-      'Are you sure you want to delete this client authorization?')));
-    $dialog->addSubmitButton();
-    $dialog->addCancelButton($authorization->getEditURI());
-    return id(new AphrontDialogResponse())->setDialog($dialog);
-
-  }
-}

src/applications/oauthserver/controller/clientauthorization/PhabricatorOAuthClientAuthorizationEditController.php

@@ -1,91 +0,0 @@
-<?php
-
-/**
- * @group oauthserver
- */
-final class PhabricatorOAuthClientAuthorizationEditController
-extends PhabricatorOAuthClientAuthorizationBaseController {
-
-  public function processRequest() {
-    $phid          = $this->getAuthorizationPHID();
-    $title         = 'Edit OAuth Client Authorization';
-    $request       = $this->getRequest();
-    $current_user  = $request->getUser();
-    $authorization = id(new PhabricatorOAuthClientAuthorization())
-      ->loadOneWhere('phid = %s',
-                     $phid);
-
-    if (empty($authorization)) {
-      return new Aphront404Response();
-    }
-    if ($authorization->getUserPHID() != $current_user->getPHID()) {
-      $message = 'Access denied to client authorization with phid '.$phid.'. '.
-                 'Only the user who authorized the client has permission to '.
-                 'edit the authorization.';
-      return id(new Aphront403Response())
-        ->setForbiddenText($message);
-    }
-
-    if ($request->isFormPost()) {
-      $scopes = PhabricatorOAuthServerScope::getScopesFromRequest($request);
-      $authorization->setScope($scopes);
-      $authorization->save();
-      return id(new AphrontRedirectResponse())
-        ->setURI('/oauthserver/clientauthorization/?edited='.$phid);
-    }
-
-    $client_phid = $authorization->getClientPHID();
-    $client      = id(new PhabricatorOAuthServerClient())
-      ->loadOneWhere('phid = %s',
-                     $client_phid);
-
-    $created = phabricator_datetime($authorization->getDateCreated(),
-                                    $current_user);
-    $updated = phabricator_datetime($authorization->getDateModified(),
-                                    $current_user);
-
-    $panel = new AphrontPanelView();
-    $delete_button = phutil_tag(
-      'a',
-      array(
-        'href' => $authorization->getDeleteURI(),
-        'class' => 'grey button',
-      ),
-      'Delete OAuth Client Authorization');
-    $panel->addButton($delete_button);
-    $panel->setHeader($title);
-
-    $form = id(new AphrontFormView())
-      ->setUser($current_user)
-      ->appendChild(
-        id(new AphrontFormMarkupControl())
-        ->setLabel('Client')
-        ->setValue(
-          phutil_tag(
-            'a',
-            array(
-              'href' => $client->getViewURI(),
-            ),
-            $client->getName())))
-      ->appendChild(
-        id(new AphrontFormStaticControl())
-        ->setLabel('Created')
-        ->setValue($created))
-      ->appendChild(
-        id(new AphrontFormStaticControl())
-        ->setLabel('Last Updated')
-        ->setValue($updated))
-      ->appendChild(
-        PhabricatorOAuthServerScope::getCheckboxControl(
-          $authorization->getScope()))
-      ->appendChild(
-        id(new AphrontFormSubmitControl())
-        ->setValue('Save OAuth Client Authorization')
-        ->addCancelButton('/oauthserver/clientauthorization/'));
-
-    $panel->appendChild($form);
-    return $this->buildStandardPageResponse(
-      $panel,
-      array('title' => $title));
-  }
-}

src/applications/oauthserver/controller/clientauthorization/PhabricatorOAuthClientAuthorizationListController.php

@@ -1,150 +0,0 @@
-<?php
-
-/**
- * @group oauthserver
- */
-final class PhabricatorOAuthClientAuthorizationListController
-extends PhabricatorOAuthClientAuthorizationBaseController {
-
-  protected function getFilter() {
-    return 'clientauthorization';
-  }
-
-  public function processRequest() {
-    $title        = 'OAuth Client Authorizations';
-    $request      = $this->getRequest();
-    $current_user = $request->getUser();
-    $offset       = $request->getInt('offset', 0);
-    $page_size    = 100;
-    $pager        = new AphrontPagerView();
-    $request_uri  = $request->getRequestURI();
-    $pager->setURI($request_uri, 'offset');
-    $pager->setPageSize($page_size);
-    $pager->setOffset($offset);
-
-    $query = id(new PhabricatorOAuthClientAuthorizationQuery())
-      ->setViewer($current_user)
-      ->withUserPHIDs(array($current_user->getPHID()));
-    $authorizations = $query->executeWithOffsetPager($pager);
-
-    $client_authorizations = mpull($authorizations, null, 'getClientPHID');
-    $client_phids          = array_keys($client_authorizations);
-    if ($client_phids) {
-      $clients = id(new PhabricatorOAuthServerClient())
-        ->loadAllWhere('phid in (%Ls)',
-                       $client_phids);
-    } else {
-      $clients = array();
-    }
-    $client_dict = mpull($clients, null, 'getPHID');
-
-    $rows      = array();
-    $rowc      = array();
-    $highlight = $this->getHighlightPHIDs();
-    foreach ($client_authorizations as $client_phid => $authorization) {
-      $client  = $client_dict[$client_phid];
-      $created = phabricator_datetime($authorization->getDateCreated(),
-                                      $current_user);
-      $updated = phabricator_datetime($authorization->getDateModified(),
-        $current_user);
-      $scope_doc_href = PhabricatorEnv::getDoclink(
-        'Using the Phabricator OAuth Server');
-      $row = array(
-        phutil_tag(
-          'a',
-          array(
-            'href' => $client->getViewURI(),
-          ),
-          $client->getName()),
-        phutil_tag(
-          'a',
-          array(
-            'href' => $scope_doc_href,
-          ),
-          $authorization->getScopeString()),
-        phabricator_datetime(
-          $authorization->getDateCreated(),
-          $current_user),
-        phabricator_datetime(
-          $authorization->getDateModified(),
-          $current_user),
-        phutil_tag(
-          'a',
-          array(
-            'class' => 'small button grey',
-            'href'  => $authorization->getEditURI(),
-          ),
-          'Edit'),
-      );
-
-      $rows[] = $row;
-      if (isset($highlight[$authorization->getPHID()])) {
-        $rowc[] = 'highlighted';
-      } else {
-        $rowc[] = '';
-      }
-    }
-
-    $panel = $this->buildClientAuthorizationList($rows, $rowc, $title);
-
-    return $this->buildStandardPageResponse(
-      array(
-        $this->getNoticeView(),
-        $panel->appendChild($pager),
-      ),
-      array('title' => $title));
-  }
-
-  private function buildClientAuthorizationList($rows, $rowc, $title) {
-    $table = new AphrontTableView($rows);
-    $table->setRowClasses($rowc);
-    $table->setHeaders(
-      array(
-        'Client',
-        'Scope',
-        'Created',
-        'Updated',
-        '',
-      ));
-    $table->setColumnClasses(
-      array(
-        'wide pri',
-        '',
-        '',
-        '',
-        'action',
-      ));
-    if (empty($rows)) {
-      $table->setNoDataString(
-        'You have not authorized any clients for this OAuthServer.');
-    }
-
-    $panel = new AphrontPanelView();
-    $panel->appendChild($table);
-    $panel->setHeader($title);
-
-    return $panel;
-  }
-
-  private function getNoticeView() {
-    $edited  = $this->getRequest()->getStr('edited');
-    $deleted = $this->getRequest()->getBool('deleted');
-    if ($edited) {
-      $title  = 'Successfully edited client authorization.';
-    } else if ($deleted) {
-      $title = 'Successfully deleted client authorization.';
-    } else {
-      $title = null;
-    }
-
-    if ($title) {
-      $view   = new AphrontErrorView();
-      $view->setTitle($title);
-      $view->setSeverity(AphrontErrorView::SEVERITY_NOTICE);
-    } else {
-      $view = null;
-    }
-
-    return $view;
-  }
-}

src/applications/oauthserver/panel/PhabricatorOAuthServerAuthorizationsSettingsPanel.php

@@ -0,0 +1,143 @@
+<?php
+
+final class PhabricatorOAuthServerAuthorizationsSettingsPanel
+  extends PhabricatorSettingsPanel {
+
+  public function getPanelKey() {
+    return 'oauthorizations';
+  }
+
+  public function getPanelName() {
+    return pht('OAuth Authorizations');
+  }
+
+  public function getPanelGroup() {
+    return pht('Authentication');
+  }
+
+  public function isEnabled() {
+    $app_name = 'PhabricatorApplicationOAuthServer';
+    return PhabricatorApplication::isClassInstalled($app_name);
+  }
+
+  public function processRequest(AphrontRequest $request) {
+    $viewer = $request->getUser();
+
+    // TODO: It would be nice to simply disable this panel, but we can't do
+    // viewer-based checks for enabled panels right now.
+
+    $app_class = 'PhabricatorApplicationOAuthServer';
+    $installed = PhabricatorApplication::isClassInstalledForViewer(
+      $app_class,
+      $viewer);
+    if (!$installed) {
+      $dialog = id(new AphrontDialogView())
+        ->setUser($viewer)
+        ->setTitle(pht('OAuth Not Available'))
+        ->appendParagraph(
+          pht('You do not have access to OAuth authorizations.'))
+        ->addCancelButton('/settings/');
+      return id(new AphrontDialogResponse())->setDialog($dialog);
+    }
+
+    $authorizations = id(new PhabricatorOAuthClientAuthorizationQuery())
+      ->setViewer($viewer)
+      ->withUserPHIDs(array($viewer->getPHID()))
+      ->execute();
+    $authorizations = mpull($authorizations, null, 'getID');
+
+    $panel_uri = $this->getPanelURI();
+
+    $revoke = $request->getInt('revoke');
+    if ($revoke) {
+      if (empty($authorizations[$revoke])) {
+        return new Aphront404Response();
+      }
+
+      if ($request->isFormPost()) {
+        $authorizations[$revoke]->delete();
+        return id(new AphrontRedirectResponse())->setURI($panel_uri);
+      }
+
+      $dialog = id(new AphrontDialogView())
+        ->setUser($viewer)
+        ->setTitle(pht('Revoke Authorization?'))
+        ->appendParagraph(
+          pht(
+            'This application will no longer be able to access Phabricator '.
+            'on your behalf.'))
+        ->addSubmitButton(pht('Revoke Authorization'))
+        ->addCancelButton($panel_uri);
+
+      return id(new AphrontDialogResponse())->setDialog($dialog);
+    }
+
+    $highlight = $request->getInt('id');
+
+    $rows = array();
+    $rowc = array();
+    foreach ($authorizations as $authorization) {
+      if ($highlight == $authorization->getID()) {
+        $rowc[] = 'highlighted';
+      } else {
+        $rowc[] = null;
+      }
+
+      $button = javelin_tag(
+        'a',
+        array(
+          'href' => $this->getPanelURI('?revoke='.$authorization->getID()),
+          'class' => 'small grey button',
+          'sigil' => 'workflow',
+        ),
+        pht('Revoke'));
+
+      $rows[] = array(
+        phutil_tag(
+          'a',
+          array(
+            'href' => $authorization->getClient()->getViewURI(),
+          ),
+          $authorization->getClient()->getName()),
+        $authorization->getScopeString(),
+        phabricator_datetime($authorization->getDateCreated(), $viewer),
+        phabricator_datetime($authorization->getDateModified(), $viewer),
+        $button,
+      );
+    }
+
+    $table = new AphrontTableView($rows);
+    $table->setNoDataString(
+      pht(
+        "You haven't authorized any OAuth applications."));
+
+    $table->setRowClasses($rowc);
+    $table->setHeaders(
+      array(
+        pht('Application'),
+        pht('Scope'),
+        pht('Created'),
+        pht('Updated'),
+        null,
+      ));
+
+    $table->setColumnClasses(
+      array(
+        'pri',
+        'wide',
+        'right',
+        'right',
+        'action',
+      ));
+
+    $header = id(new PHUIHeaderView())
+      ->setHeader(pht('OAuth Application Authorizations'));
+
+    $panel = id(new PHUIObjectBoxView())
+      ->setHeader($header)
+      ->appendChild($table);
+
+    return $panel;
+  }
+
+}

src/applications/oauthserver/query/PhabricatorOAuthClientAuthorizationQuery.php

@@ -33,6 +33,27 @@ final class PhabricatorOAuthClientAuthorizationQuery
     return $table->loadAllFromArray($data);
   }
 
+  public function willFilterPage(array $authorizations) {
+    $client_phids = mpull($authorizations, 'getClientPHID');
+
+    $clients = id(new PhabricatorOAuthServerClientQuery())
+      ->setViewer($this->getViewer())
+      ->setParentQuery($this)
+      ->withPHIDs($client_phids)
+      ->execute();
+    $clients = mpull($clients, null, 'getPHID');
+
+    foreach ($authorizations as $key => $authorization) {
+      $client = idx($clients, $authorization->getClientPHID());
+      if (!$client) {
+        unset($authorizations[$key]);
+      }
+      $authorization->attachClient($client);
+    }
+
+    return $authorizations;
+  }
+
   private function buildWhereClause($conn_r) {
     $where = array();
 

src/applications/oauthserver/storage/PhabricatorOAuthClientAuthorization.php

@@ -8,13 +8,7 @@ final class PhabricatorOAuthClientAuthorization
   protected $clientPHID;
   protected $scope;
 
-  public function getEditURI() {
+  private $client = self::ATTACHABLE;
-    return '/oauthserver/clientauthorization/edit/'.$this->getPHID().'/';
-  }
-
-  public function getDeleteURI() {
-    return '/oauthserver/clientauthorization/delete/'.$this->getPHID().'/';
-  }
 
   public function getScopeString() {
     $scope = $this->getScope();
@@ -37,6 +31,14 @@ final class PhabricatorOAuthClientAuthorization
       PhabricatorOAuthServerPHIDTypeClientAuthorization::TYPECONST);
   }
 
+  public function getClient() {
+    return $this->assertAttached($this->client);
+  }
+
+  public function attachClient(PhabricatorOAuthServerClient $client) {
+    $this->client = $client;
+    return $this;
+  }
 
 /* -(  PhabricatorPolicyInterface  )----------------------------------------- */