Page MenuHomePhabricator

D15353.id.diff
No OneTemporary

D15353.id.diff

diff --git a/src/applications/harbormaster/controller/HarbormasterBuildActionController.php b/src/applications/harbormaster/controller/HarbormasterBuildActionController.php
--- a/src/applications/harbormaster/controller/HarbormasterBuildActionController.php
+++ b/src/applications/harbormaster/controller/HarbormasterBuildActionController.php
@@ -39,6 +39,8 @@
return new Aphront400Response();
}
+ $build->assertCanIssueCommand($viewer, $action);
+
switch ($via) {
case 'buildable':
$return_uri = '/'.$build->getBuildable()->getMonogram();
diff --git a/src/applications/harbormaster/controller/HarbormasterBuildViewController.php b/src/applications/harbormaster/controller/HarbormasterBuildViewController.php
--- a/src/applications/harbormaster/controller/HarbormasterBuildViewController.php
+++ b/src/applications/harbormaster/controller/HarbormasterBuildViewController.php
@@ -441,10 +441,29 @@
->setUser($viewer)
->setObject($build);
- $can_restart = $build->canRestartBuild();
- $can_pause = $build->canPauseBuild();
- $can_resume = $build->canResumeBuild();
- $can_abort = $build->canAbortBuild();
+ $can_restart =
+ $build->canRestartBuild() &&
+ $build->canIssueCommand(
+ $viewer,
+ HarbormasterBuildCommand::COMMAND_RESTART);
+
+ $can_pause =
+ $build->canPauseBuild() &&
+ $build->canIssueCommand(
+ $viewer,
+ HarbormasterBuildCommand::COMMAND_PAUSE);
+
+ $can_resume =
+ $build->canResumeBuild() &&
+ $build->canIssueCommand(
+ $viewer,
+ HarbormasterBuildCommand::COMMAND_RESUME);
+
+ $can_abort =
+ $build->canAbortBuild() &&
+ $build->canIssueCommand(
+ $viewer,
+ HarbormasterBuildCommand::COMMAND_ABORT);
$list->addAction(
id(new PhabricatorActionView())
diff --git a/src/applications/harbormaster/controller/HarbormasterBuildableActionController.php b/src/applications/harbormaster/controller/HarbormasterBuildableActionController.php
--- a/src/applications/harbormaster/controller/HarbormasterBuildableActionController.php
+++ b/src/applications/harbormaster/controller/HarbormasterBuildableActionController.php
@@ -51,6 +51,14 @@
}
}
+ $restricted = false;
+ foreach ($issuable as $key => $build) {
+ if (!$build->canIssueCommand($viewer, $action)) {
+ $restricted = true;
+ unset($issuable[$key]);
+ }
+ }
+
$return_uri = '/'.$buildable->getMonogram();
if ($request->isDialogFormPost() && $issuable) {
$editor = id(new HarbormasterBuildableTransactionEditor())
@@ -84,49 +92,101 @@
switch ($action) {
case HarbormasterBuildCommand::COMMAND_RESTART:
if ($issuable) {
- $title = pht('Really restart all builds?');
- $body = pht(
- 'Progress on all builds will be discarded, and all builds will '.
- 'restart. Side effects of the builds will occur again. Really '.
- 'restart all builds?');
- $submit = pht('Restart All Builds');
+ $title = pht('Really restart builds?');
+
+ if ($restricted) {
+ $body = pht(
+ 'You only have permission to restart some builds. Progress '.
+ 'on builds you have permission to restart will be discarded '.
+ 'and they will restart. Side effects of these builds will '.
+ 'occur again. Really restart all builds?');
+ } else {
+ $body = pht(
+ 'Progress on all builds will be discarded, and all builds will '.
+ 'restart. Side effects of the builds will occur again. Really '.
+ 'restart all builds?');
+ }
+
+ $submit = pht('Restart Builds');
} else {
$title = pht('Unable to Restart Builds');
- $body = pht('No builds can be restarted.');
+
+ if ($restricted) {
+ $body = pht('You do not have permission to restart any builds.');
+ } else {
+ $body = pht('No builds can be restarted.');
+ }
}
break;
case HarbormasterBuildCommand::COMMAND_PAUSE:
if ($issuable) {
- $title = pht('Really pause all builds?');
- $body = pht(
- 'If you pause all builds, work will halt once the current steps '.
- 'complete. You can resume the builds later.');
- $submit = pht('Pause All Builds');
+ $title = pht('Really pause builds?');
+
+ if ($restricted) {
+ $body = pht(
+ 'You only have permission to pause some builds. Once the '.
+ 'current steps complete, work will halt on builds you have '.
+ 'permission to pause. You can resume the builds later.');
+ } else {
+ $body = pht(
+ 'If you pause all builds, work will halt once the current steps '.
+ 'complete. You can resume the builds later.');
+ }
+ $submit = pht('Pause Builds');
} else {
$title = pht('Unable to Pause Builds');
- $body = pht('No builds can be paused.');
+
+ if ($restricted) {
+ $body = pht('You do not have permission to pause any builds.');
+ } else {
+ $body = pht('No builds can be paused.');
+ }
}
break;
case HarbormasterBuildCommand::COMMAND_ABORT:
if ($issuable) {
- $title = pht('Really abort all builds?');
- $body = pht(
- 'If you abort all builds, work will halt immediately. Work '.
- 'will be discarded, and builds must be completely restarted.');
- $submit = pht('Abort All Builds');
+ $title = pht('Really abort builds?');
+ if ($restricted) {
+ $body = pht(
+ 'You only have permission to abort some builds. Work will '.
+ 'halt immediately on builds you have permission to abort. '.
+ 'Progress will be discarded, and builds must be completely '.
+ 'restarted if you want them to complete.');
+ } else {
+ $body = pht(
+ 'If you abort all builds, work will halt immediately. Work '.
+ 'will be discarded, and builds must be completely restarted.');
+ }
+ $submit = pht('Abort Builds');
} else {
$title = pht('Unable to Abort Builds');
- $body = pht('No builds can be aborted.');
+
+ if ($restricted) {
+ $body = pht('You do not have permission to abort any builds.');
+ } else {
+ $body = pht('No builds can be aborted.');
+ }
}
break;
case HarbormasterBuildCommand::COMMAND_RESUME:
if ($issuable) {
- $title = pht('Really resume all builds?');
- $body = pht('Work will continue on all builds. Really resume?');
- $submit = pht('Resume All Builds');
+ $title = pht('Really resume builds?');
+ if ($restricted) {
+ $body = pht(
+ 'You only have permission to resume some builds. Work will '.
+ 'continue on builds you have permission to resume.');
+ } else {
+ $body = pht('Work will continue on all builds. Really resume?');
+ }
+
+ $submit = pht('Resume Builds');
} else {
$title = pht('Unable to Resume Builds');
- $body = pht('No builds can be resumed.');
+ if ($restricted) {
+ $body = pht('You do not have permission to resume any builds.');
+ } else {
+ $body = pht('No builds can be resumed.');
+ }
}
break;
}
diff --git a/src/applications/harbormaster/controller/HarbormasterBuildableViewController.php b/src/applications/harbormaster/controller/HarbormasterBuildableViewController.php
--- a/src/applications/harbormaster/controller/HarbormasterBuildableViewController.php
+++ b/src/applications/harbormaster/controller/HarbormasterBuildableViewController.php
@@ -86,18 +86,31 @@
$can_pause = false;
$can_abort = false;
+ $command_restart = HarbormasterBuildCommand::COMMAND_RESTART;
+ $command_resume = HarbormasterBuildCommand::COMMAND_RESUME;
+ $command_pause = HarbormasterBuildCommand::COMMAND_PAUSE;
+ $command_abort = HarbormasterBuildCommand::COMMAND_ABORT;
+
foreach ($buildable->getBuilds() as $build) {
if ($build->canRestartBuild()) {
- $can_restart = true;
+ if ($build->canIssueCommand($viewer, $command_restart)) {
+ $can_restart = true;
+ }
}
if ($build->canResumeBuild()) {
- $can_resume = true;
+ if ($build->canIssueCommand($viewer, $command_resume)) {
+ $can_resume = true;
+ }
}
if ($build->canPauseBuild()) {
- $can_pause = true;
+ if ($build->canIssueCommand($viewer, $command_pause)) {
+ $can_pause = true;
+ }
}
if ($build->canAbortBuild()) {
- $can_abort = true;
+ if ($build->canIssueCommand($viewer, $command_abort)) {
+ $can_abort = true;
+ }
}
}
diff --git a/src/applications/harbormaster/editor/HarbormasterBuildTransactionEditor.php b/src/applications/harbormaster/editor/HarbormasterBuildTransactionEditor.php
--- a/src/applications/harbormaster/editor/HarbormasterBuildTransactionEditor.php
+++ b/src/applications/harbormaster/editor/HarbormasterBuildTransactionEditor.php
@@ -88,6 +88,11 @@
return;
}
+ $actor = $this->getActor();
+ if (!$build->canIssueCommand($actor, $command)) {
+ return;
+ }
+
id(new HarbormasterBuildCommand())
->setAuthorPHID($xaction->getAuthorPHID())
->setTargetPHID($build->getPHID())
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
@@ -450,6 +450,42 @@
return $this;
}
+ public function canIssueCommand(PhabricatorUser $viewer, $command) {
+ try {
+ $this->assertCanIssueCommand($viewer, $command);
+ return true;
+ } catch (Exception $ex) {
+ return false;
+ }
+ }
+
+ public function assertCanIssueCommand(PhabricatorUser $viewer, $command) {
+ $need_edit = false;
+ switch ($command) {
+ case HarbormasterBuildCommand::COMMAND_RESTART:
+ break;
+ case HarbormasterBuildCommand::COMMAND_PAUSE:
+ case HarbormasterBuildCommand::COMMAND_RESUME:
+ case HarbormasterBuildCommand::COMMAND_ABORT:
+ $need_edit = true;
+ break;
+ default:
+ throw new Exception(
+ pht(
+ 'Invalid Harbormaster build command "%s".',
+ $command));
+ }
+
+ // Issuing these commands requires that you be able to edit the build, to
+ // prevent enemy engineers from sabotaging your builds. See T9614.
+ if ($need_edit) {
+ PhabricatorPolicyFilter::requireCapability(
+ $viewer,
+ $this->getBuildPlan(),
+ PhabricatorPolicyCapability::CAN_EDIT);
+ }
+ }
+
/* -( PhabricatorApplicationTransactionInterface )------------------------- */

File Metadata

Mime Type
text/plain
Expires
Fri, Jan 31, 4:19 PM (5 h, 23 m)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7079073
Default Alt Text
D15353.id.diff (11 KB)

Event Timeline