revi-archive/phorge-phorge
1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-22 23:02:42 +01:00
Code Issues Releases Wiki Activity

Stabilize fatals when a build has a build plan the viewer can't see because of policy restrictions

Browse source 
Summary:
Ref T13526. Currently, if a build plan is restricted, viewers may fatal when trying to view related builds.

The old behavior allowed them to see the build even if they can not see the build plan. This is sort of incoherent, but try to stabilize things before fixing this.

Test Plan:
This is a muddy change.

  - Created a build with a build plan that Alice can't see.
  - As Alice, viewed the build page (restricted before, restricted after); the buildable page (fatal before, works after).
  - Also viewed a revision page (works before and after, but user-reported fatal).

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13526

Differential Revision: https://secure.phabricator.com/D21194
This commit is contained in:
epriestley 2020-04-30 07:41:50 -07:00
parent 186a12ef7f
commit 304467feb2
2 changed files with 28 additions and 3 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

15
src/applications/differential/storage/DifferentialRevision.php
View file

@ -870,10 +870,19 @@ final class DifferentialRevision extends DifferentialDAO
foreach ($builds as $key => $build) { foreach ($builds as $key => $build) {
$plan = $build->getBuildPlan(); $plan = $build->getBuildPlan();
$hold_key = $behavior->getPlanOption($plan)->getKey();
$hold_never = ($hold_key === $key_never); // See T13526. If the viewer can't see the build plan, pretend it has
$hold_building = ($hold_key === $key_building); // generic options. This is often wrong, but "often wrong" is better than
// "fatal".
if ($plan) {
$hold_key = $behavior->getPlanOption($plan)->getKey();
$hold_never = ($hold_key === $key_never);
$hold_building = ($hold_key === $key_building);
} else {
$hold_never = false;
$hold_building = false;
}
// If the build "Never" holds drafts from promoting, we don't care what // If the build "Never" holds drafts from promoting, we don't care what
// the status is. // the status is.

16
src/applications/harbormaster/storage/build/HarbormasterBuild.php
View file

@ -235,6 +235,16 @@ final class HarbormasterBuild extends HarbormasterDAO
$restartable = HarbormasterBuildPlanBehavior::BEHAVIOR_RESTARTABLE; $restartable = HarbormasterBuildPlanBehavior::BEHAVIOR_RESTARTABLE;
$plan = $this->getBuildPlan(); $plan = $this->getBuildPlan();
// See T13526. Users who can't see the "BuildPlan" can end up here with
// no object. This is highly questionable.
if (!$plan) {
throw new HarbormasterRestartException(
pht('No Build Plan Permission'),
pht(
'You can not restart this build because you do not have '.
'permission to access the build plan.'));
}
$option = HarbormasterBuildPlanBehavior::getBehavior($restartable) $option = HarbormasterBuildPlanBehavior::getBehavior($restartable)
->getPlanOption($plan); ->getPlanOption($plan);
$option_key = $option->getKey(); $option_key = $option->getKey();
@ -389,6 +399,12 @@ final class HarbormasterBuild extends HarbormasterDAO
public function assertCanIssueCommand(PhabricatorUser $viewer, $command) { public function assertCanIssueCommand(PhabricatorUser $viewer, $command) {
$plan = $this->getBuildPlan(); $plan = $this->getBuildPlan();
// See T13526. Users without permission to access the build plan can
// currently end up here with no "BuildPlan" object.
if (!$plan) {
return false;
}
$need_edit = true; $need_edit = true;
switch ($command) { switch ($command) {
case HarbormasterBuildCommand::COMMAND_RESTART: case HarbormasterBuildCommand::COMMAND_RESTART: