mirror of
https://we.phorge.it/source/phorge.git
synced 2024-11-10 00:42:41 +01:00
Remove some defunct old-style transaction policy checks
Summary: Ref PHI193. This method of enforcing policy checks is now (mostly) obsolete, and they're generally checked at the Controller/API level instead. Notably, this method does not call `adjustObjectForPolicyChecks(...)` properly, so it can not handle special cases like "creating a project and taking its newly created members into account" for object policies like "Project Members". Just remove these checks, which are redundant with checks elsewhere. Test Plan: - Set Project application default edit policy to "Administrators and Project Members". - Tried to create a project as a non-administrator, adding myself. - Before patch: policy fatal on a VOID object (the project with no PHID generated yet). - After patch: object created properly. Got a sensible policy error if I didn't include myself as a member. - Also verified that other edit rules are still enforced/respected (I can't edit stuff I shouldn't be able to edit). - There's at least a bit of unit test coverage of this, too, which I updated to work via API (which hits the new broad capability checks) instead of via low-level transactions (which enforce only a subset of policy operations now). Reviewers: amckinley Reviewed By: amckinley Differential Revision: https://secure.phabricator.com/D18763
This commit is contained in:
parent
cc865e549b
commit
587faa6f67
2 changed files with 13 additions and 17 deletions
|
@ -146,8 +146,7 @@ final class PhabricatorProjectCoreTestCase extends PhabricatorTestCase {
|
||||||
$user = $this->createUser();
|
$user = $this->createUser();
|
||||||
$user->save();
|
$user->save();
|
||||||
|
|
||||||
$user2 = $this->createUser();
|
$user->setAllowInlineCacheGeneration(true);
|
||||||
$user2->save();
|
|
||||||
|
|
||||||
$proj = $this->createProject($user);
|
$proj = $this->createProject($user);
|
||||||
|
|
||||||
|
@ -1289,12 +1288,19 @@ final class PhabricatorProjectCoreTestCase extends PhabricatorTestCase {
|
||||||
|
|
||||||
$new_name = $proj->getName().' '.mt_rand();
|
$new_name = $proj->getName().' '.mt_rand();
|
||||||
|
|
||||||
$xaction = new PhabricatorProjectTransaction();
|
$params = array(
|
||||||
$xaction->setTransactionType(
|
'objectIdentifier' => $proj->getID(),
|
||||||
PhabricatorProjectNameTransaction::TRANSACTIONTYPE);
|
'transactions' => array(
|
||||||
$xaction->setNewValue($new_name);
|
array(
|
||||||
|
'type' => 'name',
|
||||||
|
'value' => $new_name,
|
||||||
|
),
|
||||||
|
),
|
||||||
|
);
|
||||||
|
|
||||||
$this->applyTransactions($proj, $user, array($xaction));
|
id(new ConduitCall('project.edit', $params))
|
||||||
|
->setUser($user)
|
||||||
|
->execute();
|
||||||
|
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
|
@ -120,16 +120,6 @@ final class PhabricatorProjectTransactionEditor
|
||||||
PhabricatorApplicationTransaction $xaction) {
|
PhabricatorApplicationTransaction $xaction) {
|
||||||
|
|
||||||
switch ($xaction->getTransactionType()) {
|
switch ($xaction->getTransactionType()) {
|
||||||
case PhabricatorProjectNameTransaction::TRANSACTIONTYPE:
|
|
||||||
case PhabricatorProjectStatusTransaction::TRANSACTIONTYPE:
|
|
||||||
case PhabricatorProjectImageTransaction::TRANSACTIONTYPE:
|
|
||||||
case PhabricatorProjectIconTransaction::TRANSACTIONTYPE:
|
|
||||||
case PhabricatorProjectColorTransaction::TRANSACTIONTYPE:
|
|
||||||
PhabricatorPolicyFilter::requireCapability(
|
|
||||||
$this->requireActor(),
|
|
||||||
$object,
|
|
||||||
PhabricatorPolicyCapability::CAN_EDIT);
|
|
||||||
return;
|
|
||||||
case PhabricatorProjectLockTransaction::TRANSACTIONTYPE:
|
case PhabricatorProjectLockTransaction::TRANSACTIONTYPE:
|
||||||
PhabricatorPolicyFilter::requireCapability(
|
PhabricatorPolicyFilter::requireCapability(
|
||||||
$this->requireActor(),
|
$this->requireActor(),
|
||||||
|
|
Loading…
Reference in a new issue