diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1340,6 +1340,7 @@ '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 @@ 'PhabricatorNgramsInterface', 'PhabricatorConduitResultInterface', 'PhabricatorProjectInterface', + 'PhabricatorPolicyCodexInterface', ), 'HarbormasterBuildPlanBehavior' => 'Phobject', 'HarbormasterBuildPlanBehaviorOption' => 'Phobject', @@ -6958,6 +6960,7 @@ 'HarbormasterBuildPlanNameNgrams' => 'PhabricatorSearchNgrams', 'HarbormasterBuildPlanNameTransaction' => 'HarbormasterBuildPlanTransactionType', 'HarbormasterBuildPlanPHIDType' => 'PhabricatorPHIDType', + 'HarbormasterBuildPlanPolicyCodex' => 'PhabricatorPolicyCodex', 'HarbormasterBuildPlanQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'HarbormasterBuildPlanSearchAPIMethod' => 'PhabricatorSearchEngineAPIMethod', 'HarbormasterBuildPlanSearchEngine' => 'PhabricatorApplicationSearchEngine', diff --git a/src/applications/harbormaster/codex/HarbormasterBuildPlanPolicyCodex.php b/src/applications/harbormaster/codex/HarbormasterBuildPlanPolicyCodex.php new file mode 100644 --- /dev/null +++ b/src/applications/harbormaster/codex/HarbormasterBuildPlanPolicyCodex.php @@ -0,0 +1,38 @@ +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; + } + + +} diff --git a/src/applications/harbormaster/controller/HarbormasterPlanRunController.php b/src/applications/harbormaster/controller/HarbormasterPlanRunController.php --- a/src/applications/harbormaster/controller/HarbormasterPlanRunController.php +++ b/src/applications/harbormaster/controller/HarbormasterPlanRunController.php @@ -9,16 +9,13 @@ $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()) { diff --git a/src/applications/harbormaster/controller/HarbormasterPlanViewController.php b/src/applications/harbormaster/controller/HarbormasterPlanViewController.php --- a/src/applications/harbormaster/controller/HarbormasterPlanViewController.php +++ b/src/applications/harbormaster/controller/HarbormasterPlanViewController.php @@ -266,7 +266,7 @@ ->setIcon('fa-ban')); } - $can_run = ($can_edit && $plan->canRunManually()); + $can_run = ($plan->hasRunCapability($viewer) && $plan->canRunManually()); $curtain->addAction( id(new PhabricatorActionView()) diff --git a/src/applications/harbormaster/plan/HarbormasterBuildPlanBehavior.php b/src/applications/harbormaster/plan/HarbormasterBuildPlanBehavior.php --- a/src/applications/harbormaster/plan/HarbormasterBuildPlanBehavior.php +++ b/src/applications/harbormaster/plan/HarbormasterBuildPlanBehavior.php @@ -9,6 +9,10 @@ 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 @@ 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 @@ $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 @@ ->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 @@ '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 '. - '{nav Restart Build}, since the two actions are largely '. - 'equivalent.'. + 'Note that this controls access to all build management actions: '. + '"Run Plan Manually", "Restart", "Abort", "Pause", and "Resume".'. "\n\n". 'WARNING: This may be unsafe, particularly if the build has '. 'side effects like deployment.'. diff --git a/src/applications/harbormaster/storage/build/HarbormasterBuild.php b/src/applications/harbormaster/storage/build/HarbormasterBuild.php --- a/src/applications/harbormaster/storage/build/HarbormasterBuild.php +++ b/src/applications/harbormaster/storage/build/HarbormasterBuild.php @@ -334,14 +334,17 @@ } 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 @@ if ($need_edit) { PhabricatorPolicyFilter::requireCapability( $viewer, - $this->getBuildPlan(), + $plan, PhabricatorPolicyCapability::CAN_EDIT); } } diff --git a/src/applications/harbormaster/storage/configuration/HarbormasterBuildPlan.php b/src/applications/harbormaster/storage/configuration/HarbormasterBuildPlan.php --- a/src/applications/harbormaster/storage/configuration/HarbormasterBuildPlan.php +++ b/src/applications/harbormaster/storage/configuration/HarbormasterBuildPlan.php @@ -10,7 +10,8 @@ PhabricatorSubscribableInterface, PhabricatorNgramsInterface, PhabricatorConduitResultInterface, - PhabricatorProjectInterface { + PhabricatorProjectInterface, + PhabricatorPolicyCodexInterface { protected $name; protected $planStatus; @@ -133,7 +134,6 @@ return true; } - public function getName() { $autoplan = $this->getAutoplan(); if ($autoplan) { @@ -143,6 +143,38 @@ 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 @@ return array(); } + +/* -( PhabricatorPolicyCodexInterface )------------------------------------ */ + + + public function newPolicyCodex() { + return new HarbormasterBuildPlanPolicyCodex(); + } + }