From 12dd9ec3ff34d0037dacf43ce912b9eaa4a9faff Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 16 Nov 2015 09:24:50 -0800 Subject: [PATCH] Have EditEngine API methods provide the correct application to Conduit Summary: Fixes T9799. Currently, if you can't see an application like Paste, we fatal when trying to generate a result for `conduit.query`, because the new EditEngine-based `paste.edit` method doesn't "know" that it's a "Paste" method. Straighten this out, and use policies and queries a little more correctly/consistently. Test Plan: - Called `conduit.query` as a user who does not have permission to use Paste. - Before change: fatal. - After change: results, excluding "paste.*" methods. Reviewers: chad Reviewed By: chad Subscribers: cburroughs Maniphest Tasks: T9799 Differential Revision: https://secure.phabricator.com/D14492 --- .../method/ConduitQueryConduitAPIMethod.php | 18 ++++----- .../query/PhabricatorConduitMethodQuery.php | 37 +++++++++++++++++++ .../editor/PhabricatorPasteEditEngine.php | 4 ++ .../editengine/PhabricatorEditEngine.php | 1 + .../PhabricatorEditEngineAPIMethod.php | 6 +++ ...catorEditEngineConfigurationEditEngine.php | 4 ++ 6 files changed, 61 insertions(+), 9 deletions(-) diff --git a/src/applications/conduit/method/ConduitQueryConduitAPIMethod.php b/src/applications/conduit/method/ConduitQueryConduitAPIMethod.php index 9a982f49b6..04e8b6d05b 100644 --- a/src/applications/conduit/method/ConduitQueryConduitAPIMethod.php +++ b/src/applications/conduit/method/ConduitQueryConduitAPIMethod.php @@ -19,20 +19,20 @@ final class ConduitQueryConduitAPIMethod extends ConduitAPIMethod { } protected function execute(ConduitAPIRequest $request) { - $classes = id(new PhutilClassMapQuery()) - ->setAncestorClass('ConduitAPIMethod') + $methods = id(new PhabricatorConduitMethodQuery()) + ->setViewer($request->getUser()) ->execute(); - $names_to_params = array(); - foreach ($classes as $class) { - $names_to_params[$class->getAPIMethodName()] = array( - 'description' => $class->getMethodDescription(), - 'params' => $class->getParamTypes(), - 'return' => $class->getReturnType(), + $map = array(); + foreach ($methods as $method) { + $map[$method->getAPIMethodName()] = array( + 'description' => $method->getMethodDescription(), + 'params' => $method->getParamTypes(), + 'return' => $method->getReturnType(), ); } - return $names_to_params; + return $map; } } diff --git a/src/applications/conduit/query/PhabricatorConduitMethodQuery.php b/src/applications/conduit/query/PhabricatorConduitMethodQuery.php index e1e7e1914b..cb30bf5e55 100644 --- a/src/applications/conduit/query/PhabricatorConduitMethodQuery.php +++ b/src/applications/conduit/query/PhabricatorConduitMethodQuery.php @@ -115,6 +115,43 @@ final class PhabricatorConduitMethodQuery return $methods; } + protected function willFilterPage(array $methods) { + $application_phids = array(); + foreach ($methods as $method) { + $application = $method->getApplication(); + if ($application === null) { + continue; + } + $application_phids[] = $application->getPHID(); + } + + if ($application_phids) { + $applications = id(new PhabricatorApplicationQuery()) + ->setParentQuery($this) + ->setViewer($this->getViewer()) + ->withPHIDs($application_phids) + ->execute(); + $applications = mpull($applications, null, 'getPHID'); + } else { + $applications = array(); + } + + // Remove methods which belong to an application the viewer can not see. + foreach ($methods as $key => $method) { + $application = $method->getApplication(); + if ($application === null) { + continue; + } + + if (empty($applications[$application->getPHID()])) { + $this->didRejectResult($method); + unset($methods[$key]); + } + } + + return $methods; + } + public function getQueryApplicationClass() { return 'PhabricatorConduitApplication'; } diff --git a/src/applications/paste/editor/PhabricatorPasteEditEngine.php b/src/applications/paste/editor/PhabricatorPasteEditEngine.php index 0198507fa0..ce03a7ff06 100644 --- a/src/applications/paste/editor/PhabricatorPasteEditEngine.php +++ b/src/applications/paste/editor/PhabricatorPasteEditEngine.php @@ -9,6 +9,10 @@ final class PhabricatorPasteEditEngine return pht('Pastes'); } + public function getEngineApplicationClass() { + return 'PhabricatorPasteApplication'; + } + protected function newEditableObject() { return PhabricatorPaste::initializeNewPaste($this->getViewer()); } diff --git a/src/applications/transactions/editengine/PhabricatorEditEngine.php b/src/applications/transactions/editengine/PhabricatorEditEngine.php index 3137a7d8bc..13c2b3e017 100644 --- a/src/applications/transactions/editengine/PhabricatorEditEngine.php +++ b/src/applications/transactions/editengine/PhabricatorEditEngine.php @@ -50,6 +50,7 @@ abstract class PhabricatorEditEngine /* -( Managing Fields )---------------------------------------------------- */ + abstract public function getEngineApplicationClass(); abstract protected function buildCustomEditFields($object); final protected function buildEditFields($object) { diff --git a/src/applications/transactions/editengine/PhabricatorEditEngineAPIMethod.php b/src/applications/transactions/editengine/PhabricatorEditEngineAPIMethod.php index dafc56c825..cb7662c7ba 100644 --- a/src/applications/transactions/editengine/PhabricatorEditEngineAPIMethod.php +++ b/src/applications/transactions/editengine/PhabricatorEditEngineAPIMethod.php @@ -5,6 +5,12 @@ abstract class PhabricatorEditEngineAPIMethod abstract public function newEditEngine(); + public function getApplication() { + $engine = $this->newEditEngine(); + $class = $engine->getEngineApplicationClass(); + return PhabricatorApplication::getByClass($class); + } + public function getMethodStatus() { return self::METHOD_STATUS_UNSTABLE; } diff --git a/src/applications/transactions/editor/PhabricatorEditEngineConfigurationEditEngine.php b/src/applications/transactions/editor/PhabricatorEditEngineConfigurationEditEngine.php index 17b604b7b2..e47447c351 100644 --- a/src/applications/transactions/editor/PhabricatorEditEngineConfigurationEditEngine.php +++ b/src/applications/transactions/editor/PhabricatorEditEngineConfigurationEditEngine.php @@ -20,6 +20,10 @@ final class PhabricatorEditEngineConfigurationEditEngine return pht('Edit Configurations'); } + public function getEngineApplicationClass() { + return 'PhabricatorTransactionsApplication'; + } + protected function newEditableObject() { return PhabricatorEditEngineConfiguration::initializeNewConfiguration( $this->getViewer(),