Page MenuHomePhabricator

D20229.diff
No OneTemporary

D20229.diff

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 @@
+<?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;
+ }
+
+
+}
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();
+ }
+
}

File Metadata

Mime Type
text/plain
Expires
Thu, Dec 19, 7:49 AM (21 h, 11 m)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6906725
Default Alt Text
D20229.diff (10 KB)

Event Timeline