Use modern UI for OAuthServer details page

Summary: Mostly just UI updates and policy enforcement. Improves error message when trying to authorize an already-authorized application.

Test Plan:
{F131584}

{F131585}

{F131586}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

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

src/__phutil_library_map__.php

@@ -4465,7 +4465,11 @@ phutil_register_library_map(array(
     'PhabricatorOAuthClientBaseController' => 'PhabricatorOAuthServerController',
     'PhabricatorOAuthClientDeleteController' => 'PhabricatorOAuthClientBaseController',
     'PhabricatorOAuthClientEditController' => 'PhabricatorOAuthClientBaseController',
-    'PhabricatorOAuthClientListController' => 'PhabricatorOAuthClientBaseController',
+    'PhabricatorOAuthClientListController' =>
+    array(
+      0 => 'PhabricatorOAuthClientBaseController',
+      1 => 'PhabricatorApplicationSearchResultsControllerInterface',
+    ),
     'PhabricatorOAuthClientViewController' => 'PhabricatorOAuthClientBaseController',
     'PhabricatorOAuthResponse' => 'AphrontResponse',
     'PhabricatorOAuthServerAccessToken' => 'PhabricatorOAuthServerDAO',

src/applications/oauthserver/application/PhabricatorApplicationOAuthServer.php

@@ -36,7 +36,7 @@ final class PhabricatorApplicationOAuthServer extends PhabricatorApplication {
         '(?:query/(?P<queryKey>[^/]+)/)?'
           => 'PhabricatorOAuthClientListController',
         'auth/'          => 'PhabricatorOAuthServerAuthController',
-        'test/'          => 'PhabricatorOAuthServerTestController',
+        'test/(?P<id>\d+)/' => 'PhabricatorOAuthServerTestController',
         'token/'         => 'PhabricatorOAuthServerTokenController',
         'client/' => array(
           'create/'                 => 'PhabricatorOAuthClientEditController',

src/applications/oauthserver/controller/PhabricatorOAuthServerTestController.php

@@ -1,55 +1,81 @@
 <?php
 
-/**
- * @group oauthserver
- */
 final class PhabricatorOAuthServerTestController
-extends PhabricatorOAuthServerController {
+  extends PhabricatorOAuthServerController {
+
+  private $id;
 
   public function shouldRequireLogin() {
     return true;
   }
 
+  public function willProcessRequest(array $data) {
+    $this->id = $data['id'];
+  }
+
   public function processRequest() {
-    $request      = $this->getRequest();
+    $request = $this->getRequest();
-    $current_user = $request->getUser();
+    $viewer = $request->getUser();
-    $server       = new PhabricatorOAuthServer();
+
     $panels       = array();
     $results      = array();
 
-    if (!$request->isFormPost()) {
+    $client = id(new PhabricatorOAuthServerClientQuery())
-      return new Aphront400Response();
+      ->setViewer($viewer)
-    }
+      ->withIDs(array($this->id))
-
+      ->executeOne();
-    $action = $request->getStr('action');
+    if (!$client) {
-    if ($action !== 'testclientauthorization') {
       return new Aphront404Response();
     }
 
-    $user_phid   = $current_user->getPHID();
+    $view_uri = $client->getViewURI();
-    $client_phid = $request->getStr('client_phid');
+
-    $client      = id(new PhabricatorOAuthServerClient)
+    // Look for an existing authorization.
-      ->loadOneWhere('phid = %s', $client_phid);
+    $authorization = id(new PhabricatorOAuthClientAuthorizationQuery())
-    if (!$client) {
+      ->setViewer($viewer)
-      throw new Exception('Failed to load client!');
+      ->withUserPHIDs(array($viewer->getPHID()))
-    }
+      ->withClientPHIDs(array($client->getPHID()))
-    if ($client->getCreatorPHID() != $user_phid ||
+      ->executeOne();
-        $current_user->getPHID()  != $user_phid) {
+    if ($authorization) {
-        throw new Exception(
+      $dialog = id(new AphrontDialogView())
-          'Only allowed to make test data for yourself '.
+        ->setUser($viewer)
-          'for clients you own!'
+        ->setTitle(pht('Already Authorized'))
-        );
+        ->appendParagraph(
+          pht(
+            'You have already authorized this application to access your '.
+            'account.'))
+        ->addCancelButton($view_uri, pht('Close'));
+
+      return id(new AphrontDialogResponse())->setDialog($dialog);
     }
 
-    // blankclientauthorizations don't get scope
+    if ($request->isFormPost()) {
-    $scope = array();
+      $server = id(new PhabricatorOAuthServer())
-    $server->setUser($current_user);
+        ->setUser($viewer)
-    $server->setClient($client);
+        ->setClient($client);
-    $authorization = $server->authorizeClient($scope);
 
-    $id = $authorization->getID();
+      $scope = array();
-    $panel_uri = '/settings/panel/oauthorizations/?id='.$id;
+      $authorization = $server->authorizeClient($scope);
 
-    return id(new AphrontRedirectResponse())->setURI($panel_uri);
+      $id = $authorization->getID();
+      $panel_uri = '/settings/panel/oauthorizations/?id='.$id;
+
+      return id(new AphrontRedirectResponse())->setURI($panel_uri);
+    }
+
+    // TODO: It would be nice to put scope options in this dialog, maybe?
+
+    $dialog = id(new AphrontDialogView())
+      ->setUser($viewer)
+      ->setTitle(pht('Authorize Application?'))
+      ->appendParagraph(
+        pht(
+          'This will create an authorization, permitting %s to access '.
+          'your account.',
+          phutil_tag('strong', array(), $client->getName())))
+      ->addCancelButton($view_uri)
+      ->addSubmitButton(pht('Authorize Application'));
+
+    return id(new AphrontDialogResponse())->setDialog($dialog);
   }
 }

src/applications/oauthserver/controller/client/PhabricatorOAuthClientDeleteController.php

@@ -1,47 +1,42 @@
 <?php
 
-/**
- * @group oauthserver
- */
 final class PhabricatorOAuthClientDeleteController
-extends PhabricatorOAuthClientBaseController {
+  extends PhabricatorOAuthClientBaseController {
 
   public function processRequest() {
-    $phid          = $this->getClientPHID();
+    $request = $this->getRequest();
-    $title         = 'Delete OAuth Client';
+    $viewer = $request->getUser();
-    $request       = $this->getRequest();
-    $current_user  = $request->getUser();
-    $client = id(new PhabricatorOAuthServerClient())
-      ->loadOneWhere('phid = %s',
-                     $phid);
 
-    if (empty($client)) {
+    $client = id(new PhabricatorOAuthServerClientQuery())
+      ->setViewer($viewer)
+      ->withPHIDs(array($this->getClientPHID()))
+      ->requireCapabilities(
+        array(
+          PhabricatorPolicyCapability::CAN_VIEW,
+          PhabricatorPolicyCapability::CAN_EDIT,
+        ))
+      ->executeOne();
+    if (!$client) {
       return new Aphront404Response();
     }
-    if ($client->getCreatorPHID() != $current_user->getPHID()) {
-      $message = 'Access denied to client with phid '.$phid.'. '.
-                 'Only the user who created the client has permission to '.
-                 'delete the client.';
-      return id(new Aphront403Response())
-        ->setForbiddenText($message);
-    }
 
     if ($request->isFormPost()) {
       $client->delete();
-      return id(new AphrontRedirectResponse())
+      $app_uri = $this->getApplicationURI();
-        ->setURI('/oauthserver/client/?deleted=1');
+      return id(new AphrontRedirectResponse())->setURI($app_uri);
     }
 
-    $title .= ' '.$client->getName();
+    $dialog = id(new AphrontDialogView())
+      ->setUser($viewer)
+      ->setTitle(pht('Delete OAuth Application?'))
+      ->appendParagraph(
+        pht(
+          'Really delete the OAuth application %s?',
+          phutil_tag('strong', array(), $client->getName())))
+      ->addCancelButton($client->getViewURI())
+      ->addSubmitButton(pht('Delete Application'));
 
-    $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?')));
-    $dialog->addSubmitButton();
-    $dialog->addCancelButton($client->getEditURI());
     return id(new AphrontDialogResponse())->setDialog($dialog);
-
   }
+
 }

src/applications/oauthserver/controller/client/PhabricatorOAuthClientViewController.php

@@ -1,109 +1,121 @@
 <?php
 
-/**
- * @group oauthserver
- */
 final class PhabricatorOAuthClientViewController
-extends PhabricatorOAuthClientBaseController {
+  extends PhabricatorOAuthClientBaseController {
-
-  protected function getFilter() {
-    return 'client/view/'.$this->getClientPHID();
-  }
-
-  protected function getExtraClientFilters() {
-    return array(
-      array('url'   => $this->getFilter(),
-            'label' => 'View Client')
-      );
-  }
 
   public function processRequest() {
-    $request       = $this->getRequest();
+    $request = $this->getRequest();
-    $current_user  = $request->getUser();
+    $viewer = $request->getUser();
-    $error         = null;
-    $phid          = $this->getClientPHID();
 
-    $client = id(new PhabricatorOAuthServerClient())
+    $client = id(new PhabricatorOAuthServerClientQuery())
-      ->loadOneWhere('phid = %s',
+      ->setViewer($viewer)
-                     $phid);
+      ->withPHIDs(array($this->getClientPHID()))
-    $title  = 'View OAuth Client';
+      ->executeOne();
-
+    if (!$client) {
-    // validate the client
+      return new Aphront404Response();
-    if (empty($client)) {
-      $message = 'No client found with id '.$phid.'.';
-      return $this->buildStandardPageResponse(
-        $this->buildErrorView($message),
-        array('title' => $title));
     }
 
-    $panel = new AphrontPanelView();
+    $header = $this->buildHeaderView($client);
-    $panel->setHeader($title);
+    $actions = $this->buildActionView($client);
+    $properties = $this->buildPropertyListView($client);
+    $properties->setActionList($actions);
 
-    $form = id(new AphrontFormView())
+    $crumbs = $this->buildApplicationCrumbs();
-      ->setUser($current_user)
+    $crumbs->addTextCrumb($client->getName());
-      ->appendChild(
-        id(new AphrontFormStaticControl())
-        ->setLabel('Name')
-        ->setValue($client->getName()))
-      ->appendChild(
-        id(new AphrontFormStaticControl())
-        ->setLabel('ID')
-        ->setValue($phid));
-    if ($current_user->getPHID() == $client->getCreatorPHID()) {
-      $form
-        ->appendChild(
-          id(new AphrontFormStaticControl())
-          ->setLabel('Secret')
-          ->setValue($client->getSecret()));
-    }
-    $form
-      ->appendChild(
-        id(new AphrontFormStaticControl())
-        ->setLabel('Redirect URI')
-        ->setValue($client->getRedirectURI()));
-    $created = phabricator_datetime($client->getDateCreated(),
-                                    $current_user);
-    $updated = phabricator_datetime($client->getDateModified(),
-                                    $current_user);
-    $form
-      ->appendChild(
-        id(new AphrontFormStaticControl())
-        ->setLabel('Created')
-        ->setValue($created))
-      ->appendChild(
-        id(new AphrontFormStaticControl())
-        ->setLabel('Last Updated')
-        ->setValue($updated));
-    $panel->appendChild($form);
-    $admin_panel = null;
-    if ($client->getCreatorPHID() == $current_user->getPHID()) {
-      $edit_button = phutil_tag(
-        'a',
-        array(
-          'href'  => $client->getEditURI(),
-          'class' => 'grey button',
-        ),
-        'Edit OAuth Client');
-      $panel->addButton($edit_button);
 
-      $create_authorization_form = id(new AphrontFormView())
+    $box = id(new PHUIObjectBoxView())
-        ->setUser($current_user)
+      ->setHeader($header)
-        ->addHiddenInput('action', 'testclientauthorization')
+      ->addPropertyList($properties);
-        ->addHiddenInput('client_phid', $phid)
-        ->setAction('/oauthserver/test/')
-        ->appendChild(
-          id(new AphrontFormSubmitControl())
-          ->setValue('Create Scopeless Test Authorization'));
-      $admin_panel = id(new AphrontPanelView())
-        ->setHeader('Admin Tools')
-        ->appendChild($create_authorization_form);
-    }
 
-    return $this->buildStandardPageResponse(
+    return $this->buildApplicationPage(
-      array($error,
+      array(
-            $panel,
+        $crumbs,
-            $admin_panel
+        $box,
-    ),
+      ),
-    array('title' => $title));
+      array(
+        'title' => pht('OAuth Application: %s', $client->getName()),
+        'device' => true,
+      ));
+  }
+
+  private function buildHeaderView(PhabricatorOAuthServerClient $client) {
+    $viewer = $this->getRequest()->getUser();
+
+    $header = id(new PHUIHeaderView())
+      ->setUser($viewer)
+      ->setHeader(pht('OAuth Application: %s', $client->getName()))
+      ->setPolicyObject($client);
+
+    return $header;
+  }
+
+  private function buildActionView(PhabricatorOAuthServerClient $client) {
+    $viewer = $this->getRequest()->getUser();
+
+    $can_edit = PhabricatorPolicyFilter::hasCapability(
+      $viewer,
+      $client,
+      PhabricatorPolicyCapability::CAN_EDIT);
+
+    $authorization = id(new PhabricatorOAuthClientAuthorizationQuery())
+      ->setViewer($viewer)
+      ->withUserPHIDs(array($viewer->getPHID()))
+      ->withClientPHIDs(array($client->getPHID()))
+      ->executeOne();
+    $is_authorized = (bool)$authorization;
+
+    $view = id(new PhabricatorActionListView())
+      ->setUser($viewer);
+
+    $view->addAction(
+      id(new PhabricatorActionView())
+        ->setName(pht('Edit Application'))
+        ->setIcon('edit')
+        ->setWorkflow(!$can_edit)
+        ->setDisabled(!$can_edit)
+        ->setHref($client->getEditURI()));
+
+    $view->addAction(
+      id(new PhabricatorActionView())
+        ->setName(pht('Delete Application'))
+        ->setIcon('delete')
+        ->setWorkflow(true)
+        ->setDisabled(!$can_edit)
+        ->setHref($client->getDeleteURI()));
+
+    $view->addAction(
+      id(new PhabricatorActionView())
+        ->setName(pht('Create Test Authorization'))
+        ->setIcon('wrench')
+        ->setWorkflow(true)
+        ->setDisabled($is_authorized)
+        ->setHref($this->getApplicationURI('test/'.$client->getID().'/')));
+
+    return $view;
+  }
+
+  private function buildPropertyListView(PhabricatorOAuthServerClient $client) {
+    $viewer = $this->getRequest()->getUser();
+
+    $view = id(new PHUIPropertyListView())
+      ->setUser($viewer);
+
+    $view->addProperty(
+      pht('Client ID'),
+      $client->getPHID());
+
+    $view->addProperty(
+      pht('Client Secret'),
+      $client->getSecret());
+
+    $view->addProperty(
+      pht('Redirect URI'),
+      $client->getRedirectURI());
+
+    $view->addProperty(
+      pht('Created'),
+      phabricator_datetime($client->getDateCreated(), $viewer));
+
+    return $view;
   }
 }

src/applications/oauthserver/query/PhabricatorOAuthClientAuthorizationQuery.php

@@ -5,6 +5,7 @@ final class PhabricatorOAuthClientAuthorizationQuery
 
   private $phids;
   private $userPHIDs;
+  private $clientPHIDs;
 
   public function witHPHIDs(array $phids) {
     $this->phids = $phids;
@@ -16,6 +17,11 @@ final class PhabricatorOAuthClientAuthorizationQuery
     return $this;
   }
 
+  public function withClientPHIDs(array $phids) {
+    $this->clientPHIDs = $phids;
+    return $this;
+  }
+
   public function loadPage() {
     $table  = new PhabricatorOAuthClientAuthorization();
     $conn_r = $table->establishConnection('r');
@@ -45,6 +51,7 @@ final class PhabricatorOAuthClientAuthorizationQuery
       $client = idx($clients, $authorization->getClientPHID());
       if (!$client) {
         unset($authorizations[$key]);
+        continue;
       }
       $authorization->attachClient($client);
     }
@@ -69,6 +76,13 @@ final class PhabricatorOAuthClientAuthorizationQuery
         $this->userPHIDs);
     }
 
+    if ($this->clientPHIDs) {
+      $where[] = qsprintf(
+        $conn_r,
+        'clientPHID IN (%Ls)',
+        $this->clientPHIDs);
+    }
+
     $where[] = $this->buildPagingClause($conn_r);
 
     return $this->formatWhereClause($where);

src/applications/oauthserver/query/PhabricatorOAuthServerClientQuery.php

@@ -3,9 +3,15 @@
 final class PhabricatorOAuthServerClientQuery
   extends PhabricatorCursorPagedPolicyAwareQuery {
 
+  private $ids;
   private $phids;
   private $creatorPHIDs;
 
+  public function withIDs(array $ids) {
+    $this->ids = $ids;
+    return $this;
+  }
+
   public function withPHIDs(array $phids) {
     $this->phids = $phids;
     return $this;
@@ -35,6 +41,13 @@ final class PhabricatorOAuthServerClientQuery
   private function buildWhereClause($conn_r) {
     $where = array();
 
+    if ($this->ids) {
+      $where[] = qsprintf(
+        $conn_r,
+        'id IN (%Ld)',
+        $this->ids);
+    }
+
     if ($this->phids) {
       $where[] = qsprintf(
         $conn_r,