Improve consistency of policy enforcement on new repository edit UI

Summary: Ref T2231. The policy rules are a little murky right now: the "Edit Repository" link requires CAN_EDIT, but the actualy page doesn't. Instead, require CAN_EDIT for the edit page.

Test Plan: As a user without CAN_EDIT, viewed a repository and clicked the edit link.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2231

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

src/applications/diffusion/controller/DiffusionRepositoryCreateController.php

@@ -22,15 +22,10 @@ final class DiffusionRepositoryCreateController extends DiffusionController {
       $repository = $this->getDiffusionRequest()->getRepository();
 
       // Make sure we have CAN_EDIT.
-      id(new PhabricatorRepositoryQuery())
-        ->setViewer($viewer)
-        ->withIDs(array($repository->getID()))
-        ->requireCapabilities(
-          array(
-            PhabricatorPolicyCapability::CAN_VIEW,
-            PhabricatorPolicyCapability::CAN_EDIT,
-          ))
-        ->executeOne();
+      PhabricatorPolicyFilter::requireCapability(
+        $viewer,
+        $repository,
+        PhabricatorPolicyCapability::CAN_EDIT);
 
       $this->setRepository($repository);
 

src/applications/diffusion/controller/DiffusionRepositoryEditController.php

@@ -4,10 +4,15 @@ final class DiffusionRepositoryEditController extends DiffusionController {
 
   public function processRequest() {
     $request = $this->getRequest();
-    $user = $request->getUser();
+    $viewer = $request->getUser();
     $drequest = $this->diffusionRequest;
     $repository = $drequest->getRepository();
 
+    PhabricatorPolicyFilter::requireCapability(
+      $viewer,
+      $repository,
+      PhabricatorPolicyCapability::CAN_EDIT);
+
     $is_svn = false;
     $is_git = false;
     $is_hg = false;
@@ -77,12 +82,12 @@ final class DiffusionRepositoryEditController extends DiffusionController {
       $this->buildActionsActions($repository));
 
     $xactions = id(new PhabricatorRepositoryTransactionQuery())
-      ->setViewer($user)
+      ->setViewer($viewer)
       ->withObjectPHIDs(array($repository->getPHID()))
       ->execute();
 
     $engine = id(new PhabricatorMarkupEngine())
-      ->setViewer($user);
+      ->setViewer($viewer);
     foreach ($xactions as $xaction) {
       if ($xaction->getComment()) {
         $engine->addObject(
@@ -93,7 +98,7 @@ final class DiffusionRepositoryEditController extends DiffusionController {
     $engine->process();
 
     $xaction_view = id(new PhabricatorApplicationTransactionView())
-      ->setUser($user)
+      ->setUser($viewer)
       ->setObjectPHID($repository->getPHID())
       ->setTransactions($xactions)
       ->setMarkupEngine($engine);
@@ -128,29 +133,21 @@ final class DiffusionRepositoryEditController extends DiffusionController {
   }
 
   private function buildBasicActions(PhabricatorRepository $repository) {
-    $user = $this->getRequest()->getUser();
+    $viewer = $this->getRequest()->getUser();
 
     $view = id(new PhabricatorActionListView())
       ->setObjectURI($this->getRequest()->getRequestURI())
-      ->setUser($user);
-
-    $can_edit = PhabricatorPolicyFilter::hasCapability(
-      $user,
-      $repository,
-      PhabricatorPolicyCapability::CAN_EDIT);
+      ->setUser($viewer);
 
     $edit = id(new PhabricatorActionView())
       ->setIcon('edit')
       ->setName(pht('Edit Basic Information'))
-      ->setHref($this->getRepositoryControllerURI($repository, 'edit/basic/'))
-      ->setDisabled(!$can_edit)
-      ->setWorkflow(!$can_edit);
+      ->setHref($this->getRepositoryControllerURI($repository, 'edit/basic/'));
     $view->addAction($edit);
 
     $activate = id(new PhabricatorActionView())
       ->setHref(
         $this->getRepositoryControllerURI($repository, 'edit/activate/'))
-      ->setDisabled(!$can_edit)
       ->setWorkflow(true);
 
     if ($repository->isTracked()) {
@@ -172,10 +169,10 @@ final class DiffusionRepositoryEditController extends DiffusionController {
     PhabricatorRepository $repository,
     PhabricatorActionListView $actions) {
 
-    $user = $this->getRequest()->getUser();
+    $viewer = $this->getRequest()->getUser();
 
     $view = id(new PHUIPropertyListView())
-      ->setUser($user)
+      ->setUser($viewer)
       ->setActionList($actions);
 
     $view->addProperty(pht('Name'), $repository->getName());
@@ -196,7 +193,7 @@ final class DiffusionRepositoryEditController extends DiffusionController {
       $description = PhabricatorMarkupEngine::renderOneObject(
         $repository,
         'description',
-        $user);
+        $viewer);
     }
     $view->addTextContent($description);
 
@@ -204,24 +201,17 @@ final class DiffusionRepositoryEditController extends DiffusionController {
   }
 
   private function buildEncodingActions(PhabricatorRepository $repository) {
-    $user = $this->getRequest()->getUser();
+    $viewer = $this->getRequest()->getUser();
 
     $view = id(new PhabricatorActionListView())
       ->setObjectURI($this->getRequest()->getRequestURI())
-      ->setUser($user);
-
-    $can_edit = PhabricatorPolicyFilter::hasCapability(
-      $user,
-      $repository,
-      PhabricatorPolicyCapability::CAN_EDIT);
+      ->setUser($viewer);
 
     $edit = id(new PhabricatorActionView())
       ->setIcon('edit')
       ->setName(pht('Edit Text Encoding'))
       ->setHref(
-        $this->getRepositoryControllerURI($repository, 'edit/encoding/'))
-      ->setWorkflow(!$can_edit)
-      ->setDisabled(!$can_edit);
+        $this->getRepositoryControllerURI($repository, 'edit/encoding/'));
     $view->addAction($edit);
 
     return $view;
@@ -231,10 +221,10 @@ final class DiffusionRepositoryEditController extends DiffusionController {
     PhabricatorRepository $repository,
     PhabricatorActionListView $actions) {
 
-    $user = $this->getRequest()->getUser();
+    $viewer = $this->getRequest()->getUser();
 
     $view = id(new PHUIPropertyListView())
-      ->setUser($user)
+      ->setUser($viewer)
       ->setActionList($actions)
       ->addSectionHeader(pht('Text Encoding'));
 
@@ -255,18 +245,11 @@ final class DiffusionRepositoryEditController extends DiffusionController {
       ->setObjectURI($this->getRequest()->getRequestURI())
       ->setUser($viewer);
 
-    $can_edit = PhabricatorPolicyFilter::hasCapability(
-      $viewer,
-      $repository,
-      PhabricatorPolicyCapability::CAN_EDIT);
-
     $edit = id(new PhabricatorActionView())
       ->setIcon('edit')
       ->setName(pht('Edit Policies'))
       ->setHref(
-        $this->getRepositoryControllerURI($repository, 'edit/policy/'))
-      ->setWorkflow(!$can_edit)
-      ->setDisabled(!$can_edit);
+        $this->getRepositoryControllerURI($repository, 'edit/policy/'));
     $view->addAction($edit);
 
     return $view;
@@ -306,18 +289,11 @@ final class DiffusionRepositoryEditController extends DiffusionController {
       ->setObjectURI($this->getRequest()->getRequestURI())
       ->setUser($viewer);
 
-    $can_edit = PhabricatorPolicyFilter::hasCapability(
-      $viewer,
-      $repository,
-      PhabricatorPolicyCapability::CAN_EDIT);
-
     $edit = id(new PhabricatorActionView())
       ->setIcon('edit')
       ->setName(pht('Edit Branches'))
       ->setHref(
-        $this->getRepositoryControllerURI($repository, 'edit/branches/'))
-      ->setWorkflow(!$can_edit)
-      ->setDisabled(!$can_edit);
+        $this->getRepositoryControllerURI($repository, 'edit/branches/'));
     $view->addAction($edit);
 
     return $view;
@@ -359,18 +335,11 @@ final class DiffusionRepositoryEditController extends DiffusionController {
       ->setObjectURI($this->getRequest()->getRequestURI())
       ->setUser($viewer);
 
-    $can_edit = PhabricatorPolicyFilter::hasCapability(
-      $viewer,
-      $repository,
-      PhabricatorPolicyCapability::CAN_EDIT);
-
     $edit = id(new PhabricatorActionView())
       ->setIcon('edit')
       ->setName(pht('Edit Subversion Info'))
       ->setHref(
-        $this->getRepositoryControllerURI($repository, 'edit/subversion/'))
-      ->setWorkflow(!$can_edit)
-      ->setDisabled(!$can_edit);
+        $this->getRepositoryControllerURI($repository, 'edit/subversion/'));
     $view->addAction($edit);
 
     return $view;
@@ -407,18 +376,11 @@ final class DiffusionRepositoryEditController extends DiffusionController {
       ->setObjectURI($this->getRequest()->getRequestURI())
       ->setUser($viewer);
 
-    $can_edit = PhabricatorPolicyFilter::hasCapability(
-      $viewer,
-      $repository,
-      PhabricatorPolicyCapability::CAN_EDIT);
-
     $edit = id(new PhabricatorActionView())
       ->setIcon('edit')
       ->setName(pht('Edit Actions'))
       ->setHref(
-        $this->getRepositoryControllerURI($repository, 'edit/actions/'))
-      ->setWorkflow(!$can_edit)
-      ->setDisabled(!$can_edit);
+        $this->getRepositoryControllerURI($repository, 'edit/actions/'));
     $view->addAction($edit);
 
     return $view;
@@ -457,18 +419,11 @@ final class DiffusionRepositoryEditController extends DiffusionController {
       ->setObjectURI($this->getRequest()->getRequestURI())
       ->setUser($viewer);
 
-    $can_edit = PhabricatorPolicyFilter::hasCapability(
-      $viewer,
-      $repository,
-      PhabricatorPolicyCapability::CAN_EDIT);
-
     $edit = id(new PhabricatorActionView())
       ->setIcon('edit')
       ->setName(pht('Edit Remote'))
       ->setHref(
-        $this->getRepositoryControllerURI($repository, 'edit/remote/'))
-      ->setWorkflow(!$can_edit)
-      ->setDisabled(!$can_edit);
+        $this->getRepositoryControllerURI($repository, 'edit/remote/'));
     $view->addAction($edit);
 
     return $view;