Fix an issue with incorrect authorization handling in Working Copy build steps
Summary: Fixes T9669. Two issues: - We were using `repositoryPHIDs` instead of `blueprintPHIDs` for the list of allowed blueprints. Use the correct value. - We weren't enforcing `allowedBlueprintPHIDs` fully correctly. We //did// require an authorization, so the net effect was correct in nearly all cases, but we could have selected from too large a pool in the case where the application itself was doing the authorization (e.g., from the command line). Test Plan: Ran a build through Drydock/Harbormaster locally. Reviewers: chad, tycho.tatitscheff Reviewed By: chad, tycho.tatitscheff Subscribers: tycho.tatitscheff Maniphest Tasks: T9669 Differential Revision: https://secure.phabricator.com/D14368
This commit is contained in:
@@ -309,17 +309,18 @@ final class DrydockLeaseUpdateWorker extends DrydockWorker {
|
|||||||
return array();
|
return array();
|
||||||
}
|
}
|
||||||
|
|
||||||
$query = id(new DrydockBlueprintQuery())
|
|
||||||
->setViewer($viewer)
|
|
||||||
->withBlueprintClasses(array_keys($impls))
|
|
||||||
->withDisabled(false);
|
|
||||||
|
|
||||||
$blueprint_phids = $lease->getAllowedBlueprintPHIDs();
|
$blueprint_phids = $lease->getAllowedBlueprintPHIDs();
|
||||||
if (!$blueprint_phids) {
|
if (!$blueprint_phids) {
|
||||||
$lease->logEvent(DrydockLeaseNoBlueprintsLogType::LOGCONST);
|
$lease->logEvent(DrydockLeaseNoBlueprintsLogType::LOGCONST);
|
||||||
return array();
|
return array();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
$query = id(new DrydockBlueprintQuery())
|
||||||
|
->setViewer($viewer)
|
||||||
|
->withPHIDs($blueprint_phids)
|
||||||
|
->withBlueprintClasses(array_keys($impls))
|
||||||
|
->withDisabled(false);
|
||||||
|
|
||||||
// The Drydock application itself is allowed to authorize anything. This
|
// The Drydock application itself is allowed to authorize anything. This
|
||||||
// is primarily used for leases generated by CLI administrative tools.
|
// is primarily used for leases generated by CLI administrative tools.
|
||||||
$drydock_phid = id(new PhabricatorDrydockApplication())->getPHID();
|
$drydock_phid = id(new PhabricatorDrydockApplication())->getPHID();
|
||||||
|
|||||||
@@ -41,7 +41,10 @@ final class HarbormasterLeaseWorkingCopyBuildStepImplementation
|
|||||||
$working_copy_type = id(new DrydockWorkingCopyBlueprintImplementation())
|
$working_copy_type = id(new DrydockWorkingCopyBlueprintImplementation())
|
||||||
->getType();
|
->getType();
|
||||||
|
|
||||||
$allowed_phids = $build_target->getFieldValue('repositoryPHIDs');
|
$allowed_phids = $build_target->getFieldValue('blueprintPHIDs');
|
||||||
|
if (!is_array($allowed_phids)) {
|
||||||
|
$allowed_phids = array();
|
||||||
|
}
|
||||||
$authorizing_phid = $build_target->getBuildStep()->getPHID();
|
$authorizing_phid = $build_target->getBuildStep()->getPHID();
|
||||||
|
|
||||||
$lease = DrydockLease::initializeNewLease()
|
$lease = DrydockLease::initializeNewLease()
|
||||||
|
|||||||
Reference in New Issue
Block a user