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 )------------------------- */