Make the new Build Plan "Runnable" behavior work

Summary:
Ref T13258. Fixes T11415. This makes "Runnable" actually do something:

  - With "Runnable" set to "If Editable" (default): to manually run, pause, resume, abort, or restart a build, you must normally be able to edit the associated build plan.
  - If you toggle "Runnable" to "If Viewable", anyone who can view the build plan may take these actions.

This is pretty straightforward since T9614 already got us pretty close to this ruleset a while ago.

Test Plan:
  - Created a Build Plan, set "Can Edit" to just me, toggled "Runnable" to "If Viewable"/"If Editable", tried to take actions as another user.
  - With "If Editable", unable to run, pause, resume, abort, or restart as another user.
  - With "If Viewable", those actions work.

Reviewers: amckinley

Reviewed By: amckinley

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13258, T11415

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

src/__phutil_library_map__.php

@@ -1340,6 +1340,7 @@ phutil_register_library_map(array(
     'HarbormasterBuildPlanNameNgrams' => 'applications/harbormaster/storage/configuration/HarbormasterBuildPlanNameNgrams.php',
     'HarbormasterBuildPlanNameTransaction' => 'applications/harbormaster/xaction/plan/HarbormasterBuildPlanNameTransaction.php',
     'HarbormasterBuildPlanPHIDType' => 'applications/harbormaster/phid/HarbormasterBuildPlanPHIDType.php',
+    'HarbormasterBuildPlanPolicyCodex' => 'applications/harbormaster/codex/HarbormasterBuildPlanPolicyCodex.php',
     'HarbormasterBuildPlanQuery' => 'applications/harbormaster/query/HarbormasterBuildPlanQuery.php',
     'HarbormasterBuildPlanSearchAPIMethod' => 'applications/harbormaster/conduit/HarbormasterBuildPlanSearchAPIMethod.php',
     'HarbormasterBuildPlanSearchEngine' => 'applications/harbormaster/query/HarbormasterBuildPlanSearchEngine.php',
@@ -6945,6 +6946,7 @@ phutil_register_library_map(array(
       'PhabricatorNgramsInterface',
       'PhabricatorConduitResultInterface',
       'PhabricatorProjectInterface',
+      'PhabricatorPolicyCodexInterface',
     ),
     'HarbormasterBuildPlanBehavior' => 'Phobject',
     'HarbormasterBuildPlanBehaviorOption' => 'Phobject',
@@ -6958,6 +6960,7 @@ phutil_register_library_map(array(
     'HarbormasterBuildPlanNameNgrams' => 'PhabricatorSearchNgrams',
     'HarbormasterBuildPlanNameTransaction' => 'HarbormasterBuildPlanTransactionType',
     'HarbormasterBuildPlanPHIDType' => 'PhabricatorPHIDType',
+    'HarbormasterBuildPlanPolicyCodex' => 'PhabricatorPolicyCodex',
     'HarbormasterBuildPlanQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
     'HarbormasterBuildPlanSearchAPIMethod' => 'PhabricatorSearchEngineAPIMethod',
     'HarbormasterBuildPlanSearchEngine' => 'PhabricatorApplicationSearchEngine',

src/applications/harbormaster/codex/HarbormasterBuildPlanPolicyCodex.php

@@ -0,0 +1,38 @@
+<?php
+
+final class HarbormasterBuildPlanPolicyCodex
+  extends PhabricatorPolicyCodex {
+
+  public function getPolicySpecialRuleDescriptions() {
+    $object = $this->getObject();
+    $run_with_view = $object->canRunWithoutEditCapability();
+
+    $rules = array();
+
+    $rules[] = $this->newRule()
+      ->setCapabilities(
+        array(
+          PhabricatorPolicyCapability::CAN_EDIT,
+        ))
+      ->setIsActive(!$run_with_view)
+      ->setDescription(
+        pht(
+          'You must have edit permission on this build plan to pause, '.
+          'abort, resume, or restart it.'));
+
+    $rules[] = $this->newRule()
+      ->setCapabilities(
+        array(
+          PhabricatorPolicyCapability::CAN_EDIT,
+        ))
+      ->setIsActive(!$run_with_view)
+      ->setDescription(
+        pht(
+          'You must have edit permission on this build plan to run it '.
+          'manually.'));
+
+    return $rules;
+  }
+
+
+}

src/applications/harbormaster/controller/HarbormasterPlanRunController.php

@@ -9,16 +9,13 @@ final class HarbormasterPlanRunController extends HarbormasterPlanController {
     $plan = id(new HarbormasterBuildPlanQuery())
       ->setViewer($viewer)
       ->withIDs(array($plan_id))
-      ->requireCapabilities(
-        array(
-          PhabricatorPolicyCapability::CAN_VIEW,
-          PhabricatorPolicyCapability::CAN_EDIT,
-        ))
       ->executeOne();
     if (!$plan) {
       return new Aphront404Response();
     }
 
+    $plan->assertHasRunCapability($viewer);
+
     $cancel_uri = $this->getApplicationURI("plan/{$plan_id}/");
 
     if (!$plan->canRunManually()) {

src/applications/harbormaster/controller/HarbormasterPlanViewController.php

@@ -266,7 +266,7 @@ final class HarbormasterPlanViewController extends HarbormasterPlanController {
           ->setIcon('fa-ban'));
     }
 
-    $can_run = ($can_edit && $plan->canRunManually());
+    $can_run = ($plan->hasRunCapability($viewer) && $plan->canRunManually());
 
     $curtain->addAction(
       id(new PhabricatorActionView())

src/applications/harbormaster/plan/HarbormasterBuildPlanBehavior.php

@@ -9,6 +9,10 @@ final class HarbormasterBuildPlanBehavior
   private $defaultKey;
   private $editInstructions;
 
+  const BEHAVIOR_RUNNABLE = 'runnable';
+  const RUNNABLE_IF_VIEWABLE = 'view';
+  const RUNNABLE_IF_EDITABLE = 'edit';
+
   public function setKey($key) {
     $this->key = $key;
     return $this;
@@ -114,6 +118,19 @@ final class HarbormasterBuildPlanBehavior
     return sprintf('behavior.%s', $behavior_key);
   }
 
+  public static function getBehavior($key) {
+    $behaviors = self::newPlanBehaviors();
+
+    if (!isset($behaviors[$key])) {
+      throw new Exception(
+        pht(
+          'No build plan behavior with key "%s" exists.',
+          $key));
+    }
+
+    return $behaviors[$key];
+  }
+
   public static function newPlanBehaviors() {
     $draft_options = array(
       id(new HarbormasterBuildPlanBehaviorOption())
@@ -224,14 +241,14 @@ final class HarbormasterBuildPlanBehavior
 
     $run_options = array(
       id(new HarbormasterBuildPlanBehaviorOption())
-        ->setKey('edit')
+        ->setKey(self::RUNNABLE_IF_EDITABLE)
         ->setIcon('fa-pencil green')
         ->setName(pht('If Editable'))
         ->setIsDefault(true)
         ->setDescription(
           pht('Only users who can edit the plan can run it manually.')),
       id(new HarbormasterBuildPlanBehaviorOption())
-        ->setKey('view')
+        ->setKey(self::RUNNABLE_IF_VIEWABLE)
         ->setIcon('fa-exclamation-triangle yellow')
         ->setName(pht('If Viewable'))
         ->setDescription(
@@ -315,7 +332,7 @@ final class HarbormasterBuildPlanBehavior
         ->setName(pht('Restartable'))
         ->setOptions($restart_options),
       id(new self())
-        ->setKey('runnable')
+        ->setKey(self::BEHAVIOR_RUNNABLE)
         ->setEditInstructions(
           pht(
             'To run a build manually, you normally must have permission to '.
@@ -323,9 +340,8 @@ final class HarbormasterBuildPlanBehavior
             'can see the build plan be able to run and restart the build, you '.
             'can change the behavior here.'.
             "\n\n".
-            'Note that this affects both {nav Run Plan Manually} and '.
+            'Note that this controls access to all build management actions: '.
-            '{nav Restart Build}, since the two actions are largely '.
+            '"Run Plan Manually", "Restart", "Abort", "Pause", and "Resume".'.
-            'equivalent.'.
             "\n\n".
             'WARNING: This may be unsafe, particularly if the build has '.
             'side effects like deployment.'.

src/applications/harbormaster/storage/build/HarbormasterBuild.php

@@ -334,14 +334,17 @@ final class HarbormasterBuild extends HarbormasterDAO
   }
 
   public function assertCanIssueCommand(PhabricatorUser $viewer, $command) {
-    $need_edit = false;
+    $plan = $this->getBuildPlan();
+
+    $need_edit = true;
     switch ($command) {
       case HarbormasterBuildCommand::COMMAND_RESTART:
-        break;
       case HarbormasterBuildCommand::COMMAND_PAUSE:
       case HarbormasterBuildCommand::COMMAND_RESUME:
       case HarbormasterBuildCommand::COMMAND_ABORT:
-        $need_edit = true;
+        if ($plan->canRunWithoutEditCapability()) {
+          $need_edit = false;
+        }
         break;
       default:
         throw new Exception(
@@ -355,7 +358,7 @@ final class HarbormasterBuild extends HarbormasterDAO
     if ($need_edit) {
       PhabricatorPolicyFilter::requireCapability(
         $viewer,
-        $this->getBuildPlan(),
+        $plan,
         PhabricatorPolicyCapability::CAN_EDIT);
     }
   }

src/applications/harbormaster/storage/configuration/HarbormasterBuildPlan.php

@@ -10,7 +10,8 @@ final class HarbormasterBuildPlan extends HarbormasterDAO
     PhabricatorSubscribableInterface,
     PhabricatorNgramsInterface,
     PhabricatorConduitResultInterface,
-    PhabricatorProjectInterface {
+    PhabricatorProjectInterface,
+    PhabricatorPolicyCodexInterface {
 
   protected $name;
   protected $planStatus;
@@ -133,7 +134,6 @@ final class HarbormasterBuildPlan extends HarbormasterDAO
     return true;
   }
 
-
   public function getName() {
     $autoplan = $this->getAutoplan();
     if ($autoplan) {
@@ -143,6 +143,38 @@ final class HarbormasterBuildPlan extends HarbormasterDAO
     return parent::getName();
   }
 
+  public function hasRunCapability(PhabricatorUser $viewer) {
+    try {
+      $this->assertHasRunCapability($viewer);
+      return true;
+    } catch (PhabricatorPolicyException $ex) {
+      return false;
+    }
+  }
+
+  public function canRunWithoutEditCapability() {
+    $runnable = HarbormasterBuildPlanBehavior::BEHAVIOR_RUNNABLE;
+    $if_viewable = HarbormasterBuildPlanBehavior::RUNNABLE_IF_VIEWABLE;
+
+    $option = HarbormasterBuildPlanBehavior::getBehavior($runnable)
+      ->getPlanOption($this);
+
+    return ($option->getKey() === $if_viewable);
+  }
+
+  public function assertHasRunCapability(PhabricatorUser $viewer) {
+    if ($this->canRunWithoutEditCapability()) {
+      $capability = PhabricatorPolicyCapability::CAN_VIEW;
+    } else {
+      $capability = PhabricatorPolicyCapability::CAN_EDIT;
+    }
+
+    PhabricatorPolicyFilter::requireCapability(
+      $viewer,
+      $this,
+      $capability);
+  }
+
 
 /* -(  PhabricatorSubscribableInterface  )----------------------------------- */
 
@@ -265,4 +297,12 @@ final class HarbormasterBuildPlan extends HarbormasterDAO
     return array();
   }
 
+
+/* -(  PhabricatorPolicyCodexInterface  )------------------------------------ */
+
+
+  public function newPolicyCodex() {
+    return new HarbormasterBuildPlanPolicyCodex();
+  }
+
 }