From 7e29ec2e2a66087528027b7e80cc548ec30f6fff Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 14 Aug 2018 14:46:59 -0700 Subject: [PATCH] Move the "Can Lock Projects" check from requireCapabilities() to transaction validation Summary: Depends on D19584. Ref T13164. This check is an //extra// check: you need EDIT //and// this capability. Thus, we can do it in validation without issues. Test Plan: - This code isn't reachable today: all methods of applying this transaction do a separate check for "Can Lock" upfront. - Commented out the "Can Lock" check in the LockController, tried to lock as a user without permission. Was rejected with a policy exception. Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13164 Differential Revision: https://secure.phabricator.com/D19585 --- .../editor/PhabricatorProjectTransactionEditor.php | 6 ------ .../xaction/PhabricatorProjectLockTransaction.php | 8 ++++++++ .../storage/PhabricatorModularTransactionType.php | 10 ++++++++++ 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/src/applications/project/editor/PhabricatorProjectTransactionEditor.php b/src/applications/project/editor/PhabricatorProjectTransactionEditor.php index 2a90d6ce29..ef0cc65c8e 100644 --- a/src/applications/project/editor/PhabricatorProjectTransactionEditor.php +++ b/src/applications/project/editor/PhabricatorProjectTransactionEditor.php @@ -120,12 +120,6 @@ final class PhabricatorProjectTransactionEditor PhabricatorApplicationTransaction $xaction) { switch ($xaction->getTransactionType()) { - case PhabricatorProjectLockTransaction::TRANSACTIONTYPE: - PhabricatorPolicyFilter::requireCapability( - $this->requireActor(), - newv($this->getEditorApplicationClass(), array()), - ProjectCanLockProjectsCapability::CAPABILITY); - return; case PhabricatorTransactions::TYPE_EDGE: switch ($xaction->getMetadataValue('edge:type')) { case PhabricatorProjectProjectHasMemberEdgeType::EDGECONST: diff --git a/src/applications/project/xaction/PhabricatorProjectLockTransaction.php b/src/applications/project/xaction/PhabricatorProjectLockTransaction.php index 42551dfb2c..c54c9284ce 100644 --- a/src/applications/project/xaction/PhabricatorProjectLockTransaction.php +++ b/src/applications/project/xaction/PhabricatorProjectLockTransaction.php @@ -53,4 +53,12 @@ final class PhabricatorProjectLockTransaction } } + public function validateTransactions($object, array $xactions) { + if ($xactions) { + $this->requireApplicationCapability( + ProjectCanLockProjectsCapability::CAPABILITY); + } + return array(); + } + } diff --git a/src/applications/transactions/storage/PhabricatorModularTransactionType.php b/src/applications/transactions/storage/PhabricatorModularTransactionType.php index eaf7d029ce..b0714aeccf 100644 --- a/src/applications/transactions/storage/PhabricatorModularTransactionType.php +++ b/src/applications/transactions/storage/PhabricatorModularTransactionType.php @@ -356,4 +356,14 @@ abstract class PhabricatorModularTransactionType return array(); } + protected function requireApplicationCapability($capability) { + $application_class = $this->getEditor()->getEditorApplicationClass(); + $application = newv($application_class, array()); + + PhabricatorPolicyFilter::requireCapability( + $this->getActor(), + $application, + $capability); + } + }