Skip to content

Commit 93b8f80

Browse files
author
epriestley
committed
Require "Can Edit" on a build plan to abort or pause associated builds
Summary: Fixes T9614. This is kind of silly, but stop users from fighting turf wars over build resources or showing up on an install and just aborting a bunch of builds for the heck of it. Test Plan: - Restarted / paused / aborted / etc builds. - Tried to do the same for builds I didn't have edit permission on the build plan for, got errors. Reviewers: chad Reviewed By: chad Maniphest Tasks: T9614 Differential Revision: https://secure.phabricator.com/D15353
1 parent c64b822 commit 93b8f80

File tree

6 files changed

+166
-31
lines changed

6 files changed

+166
-31
lines changed

‎src/applications/harbormaster/controller/HarbormasterBuildActionController.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ public function handleRequest(AphrontRequest $request) {
3939
return new Aphront400Response();
4040
}
4141

42+
$build->assertCanIssueCommand($viewer, $action);
43+
4244
switch ($via) {
4345
case 'buildable':
4446
$return_uri = '/'.$build->getBuildable()->getMonogram();

‎src/applications/harbormaster/controller/HarbormasterBuildViewController.php‎

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -441,10 +441,29 @@ private function buildActionList(HarbormasterBuild $build) {
441441
->setUser($viewer)
442442
->setObject($build);
443443

444-
$can_restart = $build->canRestartBuild();
445-
$can_pause = $build->canPauseBuild();
446-
$can_resume = $build->canResumeBuild();
447-
$can_abort = $build->canAbortBuild();
444+
$can_restart =
445+
$build->canRestartBuild() &&
446+
$build->canIssueCommand(
447+
$viewer,
448+
HarbormasterBuildCommand::COMMAND_RESTART);
449+
450+
$can_pause =
451+
$build->canPauseBuild() &&
452+
$build->canIssueCommand(
453+
$viewer,
454+
HarbormasterBuildCommand::COMMAND_PAUSE);
455+
456+
$can_resume =
457+
$build->canResumeBuild() &&
458+
$build->canIssueCommand(
459+
$viewer,
460+
HarbormasterBuildCommand::COMMAND_RESUME);
461+
462+
$can_abort =
463+
$build->canAbortBuild() &&
464+
$build->canIssueCommand(
465+
$viewer,
466+
HarbormasterBuildCommand::COMMAND_ABORT);
448467

449468
$list->addAction(
450469
id(new PhabricatorActionView())

‎src/applications/harbormaster/controller/HarbormasterBuildableActionController.php‎

Lines changed: 83 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,14 @@ public function handleRequest(AphrontRequest $request) {
5151
}
5252
}
5353

54+
$restricted = false;
55+
foreach ($issuable as $key => $build) {
56+
if (!$build->canIssueCommand($viewer, $action)) {
57+
$restricted = true;
58+
unset($issuable[$key]);
59+
}
60+
}
61+
5462
$return_uri = '/'.$buildable->getMonogram();
5563
if ($request->isDialogFormPost() && $issuable) {
5664
$editor = id(new HarbormasterBuildableTransactionEditor())
@@ -84,49 +92,101 @@ public function handleRequest(AphrontRequest $request) {
8492
switch ($action) {
8593
case HarbormasterBuildCommand::COMMAND_RESTART:
8694
if ($issuable) {
87-
$title = pht('Really restart all builds?');
88-
$body = pht(
89-
'Progress on all builds will be discarded, and all builds will '.
90-
'restart. Side effects of the builds will occur again. Really '.
91-
'restart all builds?');
92-
$submit = pht('Restart All Builds');
95+
$title = pht('Really restart builds?');
96+
97+
if ($restricted) {
98+
$body = pht(
99+
'You only have permission to restart some builds. Progress '.
100+
'on builds you have permission to restart will be discarded '.
101+
'and they will restart. Side effects of these builds will '.
102+
'occur again. Really restart all builds?');
103+
} else {
104+
$body = pht(
105+
'Progress on all builds will be discarded, and all builds will '.
106+
'restart. Side effects of the builds will occur again. Really '.
107+
'restart all builds?');
108+
}
109+
110+
$submit = pht('Restart Builds');
93111
} else {
94112
$title = pht('Unable to Restart Builds');
95-
$body = pht('No builds can be restarted.');
113+
114+
if ($restricted) {
115+
$body = pht('You do not have permission to restart any builds.');
116+
} else {
117+
$body = pht('No builds can be restarted.');
118+
}
96119
}
97120
break;
98121
case HarbormasterBuildCommand::COMMAND_PAUSE:
99122
if ($issuable) {
100-
$title = pht('Really pause all builds?');
101-
$body = pht(
102-
'If you pause all builds, work will halt once the current steps '.
103-
'complete. You can resume the builds later.');
104-
$submit = pht('Pause All Builds');
123+
$title = pht('Really pause builds?');
124+
125+
if ($restricted) {
126+
$body = pht(
127+
'You only have permission to pause some builds. Once the '.
128+
'current steps complete, work will halt on builds you have '.
129+
'permission to pause. You can resume the builds later.');
130+
} else {
131+
$body = pht(
132+
'If you pause all builds, work will halt once the current steps '.
133+
'complete. You can resume the builds later.');
134+
}
135+
$submit = pht('Pause Builds');
105136
} else {
106137
$title = pht('Unable to Pause Builds');
107-
$body = pht('No builds can be paused.');
138+
139+
if ($restricted) {
140+
$body = pht('You do not have permission to pause any builds.');
141+
} else {
142+
$body = pht('No builds can be paused.');
143+
}
108144
}
109145
break;
110146
case HarbormasterBuildCommand::COMMAND_ABORT:
111147
if ($issuable) {
112-
$title = pht('Really abort all builds?');
113-
$body = pht(
114-
'If you abort all builds, work will halt immediately. Work '.
115-
'will be discarded, and builds must be completely restarted.');
116-
$submit = pht('Abort All Builds');
148+
$title = pht('Really abort builds?');
149+
if ($restricted) {
150+
$body = pht(
151+
'You only have permission to abort some builds. Work will '.
152+
'halt immediately on builds you have permission to abort. '.
153+
'Progress will be discarded, and builds must be completely '.
154+
'restarted if you want them to complete.');
155+
} else {
156+
$body = pht(
157+
'If you abort all builds, work will halt immediately. Work '.
158+
'will be discarded, and builds must be completely restarted.');
159+
}
160+
$submit = pht('Abort Builds');
117161
} else {
118162
$title = pht('Unable to Abort Builds');
119-
$body = pht('No builds can be aborted.');
163+
164+
if ($restricted) {
165+
$body = pht('You do not have permission to abort any builds.');
166+
} else {
167+
$body = pht('No builds can be aborted.');
168+
}
120169
}
121170
break;
122171
case HarbormasterBuildCommand::COMMAND_RESUME:
123172
if ($issuable) {
124-
$title = pht('Really resume all builds?');
125-
$body = pht('Work will continue on all builds. Really resume?');
126-
$submit = pht('Resume All Builds');
173+
$title = pht('Really resume builds?');
174+
if ($restricted) {
175+
$body = pht(
176+
'You only have permission to resume some builds. Work will '.
177+
'continue on builds you have permission to resume.');
178+
} else {
179+
$body = pht('Work will continue on all builds. Really resume?');
180+
}
181+
182+
$submit = pht('Resume Builds');
127183
} else {
128184
$title = pht('Unable to Resume Builds');
129-
$body = pht('No builds can be resumed.');
185+
if ($restricted) {
186+
$body = pht('You do not have permission to resume any builds.');
187+
} else {
188+
$body = pht('No builds can be resumed.');
189+
}
130190
}
131191
break;
132192
}

‎src/applications/harbormaster/controller/HarbormasterBuildableViewController.php‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -86,18 +86,31 @@ private function buildActionList(HarbormasterBuildable $buildable) {
8686
$can_pause = false;
8787
$can_abort = false;
8888

89+
$command_restart = HarbormasterBuildCommand::COMMAND_RESTART;
90+
$command_resume = HarbormasterBuildCommand::COMMAND_RESUME;
91+
$command_pause = HarbormasterBuildCommand::COMMAND_PAUSE;
92+
$command_abort = HarbormasterBuildCommand::COMMAND_ABORT;
93+
8994
foreach ($buildable->getBuilds() as $build) {
9095
if ($build->canRestartBuild()) {
91-
$can_restart = true;
96+
if ($build->canIssueCommand($viewer, $command_restart)) {
97+
$can_restart = true;
98+
}
9299
}
93100
if ($build->canResumeBuild()) {
94-
$can_resume = true;
101+
if ($build->canIssueCommand($viewer, $command_resume)) {
102+
$can_resume = true;
103+
}
95104
}
96105
if ($build->canPauseBuild()) {
97-
$can_pause = true;
106+
if ($build->canIssueCommand($viewer, $command_pause)) {
107+
$can_pause = true;
108+
}
98109
}
99110
if ($build->canAbortBuild()) {
100-
$can_abort = true;
111+
if ($build->canIssueCommand($viewer, $command_abort)) {
112+
$can_abort = true;
113+
}
101114
}
102115
}
103116

‎src/applications/harbormaster/editor/HarbormasterBuildTransactionEditor.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,11 @@ private function executeBuildCommand(
8888
return;
8989
}
9090

91+
$actor = $this->getActor();
92+
if (!$build->canIssueCommand($actor, $command)) {
93+
return;
94+
}
95+
9196
id(new HarbormasterBuildCommand())
9297
->setAuthorPHID($xaction->getAuthorPHID())
9398
->setTargetPHID($build->getPHID())

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

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -450,6 +450,42 @@ public function deleteUnprocessedCommands() {
450450
return $this;
451451
}
452452

453+
public function canIssueCommand(PhabricatorUser $viewer, $command) {
454+
try {
455+
$this->assertCanIssueCommand($viewer, $command);
456+
return true;
457+
} catch (Exception $ex) {
458+
return false;
459+
}
460+
}
461+
462+
public function assertCanIssueCommand(PhabricatorUser $viewer, $command) {
463+
$need_edit = false;
464+
switch ($command) {
465+
case HarbormasterBuildCommand::COMMAND_RESTART:
466+
break;
467+
case HarbormasterBuildCommand::COMMAND_PAUSE:
468+
case HarbormasterBuildCommand::COMMAND_RESUME:
469+
case HarbormasterBuildCommand::COMMAND_ABORT:
470+
$need_edit = true;
471+
break;
472+
default:
473+
throw new Exception(
474+
pht(
475+
'Invalid Harbormaster build command "%s".',
476+
$command));
477+
}
478+
479+
// Issuing these commands requires that you be able to edit the build, to
480+
// prevent enemy engineers from sabotaging your builds. See T9614.
481+
if ($need_edit) {
482+
PhabricatorPolicyFilter::requireCapability(
483+
$viewer,
484+
$this->getBuildPlan(),
485+
PhabricatorPolicyCapability::CAN_EDIT);
486+
}
487+
}
488+
453489

454490
/* -( PhabricatorApplicationTransactionInterface )------------------------- */
455491

0 commit comments

Comments
 (0)