From a1df1f2b704188ebfdfe788fbdc0fa5b8c443861 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 13 Sep 2012 10:15:08 -0700 Subject: [PATCH] Allow projects to be set as policies Summary: - Renames `PhabricatorPolicyQuery` to `PhabricatorPolicyAwareQuery` (a query which respects policy settings). - Introduces `PhabricatorPolicyQuery`, which loads available policies (e.g., "member of project X"). - Introduces `PhabricatorPolicy`, which describes a policy. - Allows projects to be set as policies. - Allows Paste policies to be edited. - Covers crazy cases where you make projects depend on themselves or each other because you are a dastardly villan. Test Plan: Set paste and project policies, including crazy policies like A -> B -> A, A -> A, etc. Reviewers: vrana, btrahan Reviewed By: vrana CC: aran Maniphest Tasks: T603 Differential Revision: https://secure.phabricator.com/D3476 --- src/__phutil_library_map__.php | 27 +-- .../chatlog/PhabricatorChatLogQuery.php | 3 +- .../storage/PhabricatorChatLogEvent.php | 2 +- .../feed/PhabricatorFeedQuery.php | 3 +- .../query/PhabricatorOwnersPackageQuery.php | 2 +- .../PhabricatorPasteEditController.php | 27 +-- .../PhabricatorPasteViewController.php | 8 + .../paste/query/PhabricatorPasteQuery.php | 3 +- .../handle/PhabricatorObjectHandleData.php | 10 +- ...hp => PhabricatorPolicyAwareTestQuery.php} | 4 +- .../__tests__/PhabricatorPolicyTestCase.php | 6 +- .../constants/PhabricatorPolicyType.php | 46 +++++ .../policy/filter/PhabricatorPolicy.php | 119 ++++++++++++ .../policy/filter/PhabricatorPolicyFilter.php | 97 +++++++++- .../policy/query/PhabricatorPolicyQuery.php | 169 ++++++++++++++++++ ...habricatorProjectProfileEditController.php | 8 + .../project/query/PhabricatorProjectQuery.php | 3 +- ...habricatorCursorPagedPolicyAwareQuery.php} | 4 +- ...ry.php => PhabricatorPolicyAwareQuery.php} | 13 +- .../form/control/AphrontFormPolicyControl.php | 42 ++--- 20 files changed, 524 insertions(+), 72 deletions(-) rename src/applications/policy/__tests__/{PhabricatorPolicyTestQuery.php => PhabricatorPolicyAwareTestQuery.php} (93%) create mode 100644 src/applications/policy/constants/PhabricatorPolicyType.php create mode 100644 src/applications/policy/filter/PhabricatorPolicy.php create mode 100644 src/applications/policy/query/PhabricatorPolicyQuery.php rename src/infrastructure/query/policy/{PhabricatorCursorPagedPolicyQuery.php => PhabricatorCursorPagedPolicyAwareQuery.php} (97%) rename src/infrastructure/query/policy/{PhabricatorPolicyQuery.php => PhabricatorPolicyAwareQuery.php} (95%) diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 2f0449d89e..4bf0ad4b7d 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -626,7 +626,7 @@ phutil_register_library_map(array( 'PhabricatorCountdownEditController' => 'applications/countdown/controller/PhabricatorCountdownEditController.php', 'PhabricatorCountdownListController' => 'applications/countdown/controller/PhabricatorCountdownListController.php', 'PhabricatorCountdownViewController' => 'applications/countdown/controller/PhabricatorCountdownViewController.php', - 'PhabricatorCursorPagedPolicyQuery' => 'infrastructure/query/policy/PhabricatorCursorPagedPolicyQuery.php', + 'PhabricatorCursorPagedPolicyAwareQuery' => 'infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php', 'PhabricatorDaemon' => 'infrastructure/daemon/PhabricatorDaemon.php', 'PhabricatorDaemonCombinedLogController' => 'applications/daemon/controller/PhabricatorDaemonCombinedLogController.php', 'PhabricatorDaemonConsoleController' => 'applications/daemon/controller/PhabricatorDaemonConsoleController.php', @@ -909,15 +909,18 @@ phutil_register_library_map(array( 'PhabricatorPeopleProfileController' => 'applications/people/controller/PhabricatorPeopleProfileController.php', 'PhabricatorPeopleQuery' => 'applications/people/PhabricatorPeopleQuery.php', 'PhabricatorPolicies' => 'applications/policy/constants/PhabricatorPolicies.php', + 'PhabricatorPolicy' => 'applications/policy/filter/PhabricatorPolicy.php', + 'PhabricatorPolicyAwareQuery' => 'infrastructure/query/policy/PhabricatorPolicyAwareQuery.php', + 'PhabricatorPolicyAwareTestQuery' => 'applications/policy/__tests__/PhabricatorPolicyAwareTestQuery.php', 'PhabricatorPolicyCapability' => 'applications/policy/constants/PhabricatorPolicyCapability.php', 'PhabricatorPolicyConstants' => 'applications/policy/constants/PhabricatorPolicyConstants.php', 'PhabricatorPolicyException' => 'applications/policy/exception/PhabricatorPolicyException.php', 'PhabricatorPolicyFilter' => 'applications/policy/filter/PhabricatorPolicyFilter.php', 'PhabricatorPolicyInterface' => 'applications/policy/interface/PhabricatorPolicyInterface.php', - 'PhabricatorPolicyQuery' => 'infrastructure/query/policy/PhabricatorPolicyQuery.php', + 'PhabricatorPolicyQuery' => 'applications/policy/query/PhabricatorPolicyQuery.php', 'PhabricatorPolicyTestCase' => 'applications/policy/__tests__/PhabricatorPolicyTestCase.php', 'PhabricatorPolicyTestObject' => 'applications/policy/__tests__/PhabricatorPolicyTestObject.php', - 'PhabricatorPolicyTestQuery' => 'applications/policy/__tests__/PhabricatorPolicyTestQuery.php', + 'PhabricatorPolicyType' => 'applications/policy/constants/PhabricatorPolicyType.php', 'PhabricatorProfileHeaderView' => 'view/layout/PhabricatorProfileHeaderView.php', 'PhabricatorProject' => 'applications/project/storage/PhabricatorProject.php', 'PhabricatorProjectConstants' => 'applications/project/constants/PhabricatorProjectConstants.php', @@ -1753,7 +1756,7 @@ phutil_register_library_map(array( 1 => 'PhabricatorPolicyInterface', ), 'PhabricatorChatLogEventType' => 'PhabricatorChatLogConstants', - 'PhabricatorChatLogQuery' => 'PhabricatorCursorPagedPolicyQuery', + 'PhabricatorChatLogQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PhabricatorConduitAPIController' => 'PhabricatorConduitController', 'PhabricatorConduitCertificateToken' => 'PhabricatorConduitDAO', 'PhabricatorConduitConnectionLog' => 'PhabricatorConduitDAO', @@ -1772,7 +1775,7 @@ phutil_register_library_map(array( 'PhabricatorCountdownEditController' => 'PhabricatorCountdownController', 'PhabricatorCountdownListController' => 'PhabricatorCountdownController', 'PhabricatorCountdownViewController' => 'PhabricatorCountdownController', - 'PhabricatorCursorPagedPolicyQuery' => 'PhabricatorPolicyQuery', + 'PhabricatorCursorPagedPolicyAwareQuery' => 'PhabricatorPolicyAwareQuery', 'PhabricatorDaemon' => 'PhutilDaemon', 'PhabricatorDaemonCombinedLogController' => 'PhabricatorDaemonController', 'PhabricatorDaemonConsoleController' => 'PhabricatorDaemonController', @@ -1828,7 +1831,7 @@ phutil_register_library_map(array( 'PhabricatorFeedController' => 'PhabricatorController', 'PhabricatorFeedDAO' => 'PhabricatorLiskDAO', 'PhabricatorFeedPublicStreamController' => 'PhabricatorFeedController', - 'PhabricatorFeedQuery' => 'PhabricatorCursorPagedPolicyQuery', + 'PhabricatorFeedQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PhabricatorFeedStory' => 'PhabricatorPolicyInterface', 'PhabricatorFeedStoryAggregate' => 'PhabricatorFeedStory', 'PhabricatorFeedStoryAudit' => 'PhabricatorFeedStory', @@ -1996,7 +1999,7 @@ phutil_register_library_map(array( 0 => 'PhabricatorOwnersDAO', 1 => 'PhabricatorPolicyInterface', ), - 'PhabricatorOwnersPackageQuery' => 'PhabricatorCursorPagedPolicyQuery', + 'PhabricatorOwnersPackageQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PhabricatorOwnersPath' => 'PhabricatorOwnersDAO', 'PhabricatorPHIDController' => 'PhabricatorController', 'PhabricatorPHIDLookupController' => 'PhabricatorPHIDController', @@ -2009,7 +2012,7 @@ phutil_register_library_map(array( 'PhabricatorPasteDAO' => 'PhabricatorLiskDAO', 'PhabricatorPasteEditController' => 'PhabricatorPasteController', 'PhabricatorPasteListController' => 'PhabricatorPasteController', - 'PhabricatorPasteQuery' => 'PhabricatorCursorPagedPolicyQuery', + 'PhabricatorPasteQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PhabricatorPasteViewController' => 'PhabricatorPasteController', 'PhabricatorPeopleController' => 'PhabricatorController', 'PhabricatorPeopleEditController' => 'PhabricatorPeopleController', @@ -2019,12 +2022,14 @@ phutil_register_library_map(array( 'PhabricatorPeopleProfileController' => 'PhabricatorPeopleController', 'PhabricatorPeopleQuery' => 'PhabricatorOffsetPagedQuery', 'PhabricatorPolicies' => 'PhabricatorPolicyConstants', + 'PhabricatorPolicyAwareQuery' => 'PhabricatorOffsetPagedQuery', + 'PhabricatorPolicyAwareTestQuery' => 'PhabricatorPolicyAwareQuery', 'PhabricatorPolicyCapability' => 'PhabricatorPolicyConstants', 'PhabricatorPolicyException' => 'Exception', - 'PhabricatorPolicyQuery' => 'PhabricatorOffsetPagedQuery', + 'PhabricatorPolicyQuery' => 'PhabricatorQuery', 'PhabricatorPolicyTestCase' => 'PhabricatorTestCase', 'PhabricatorPolicyTestObject' => 'PhabricatorPolicyInterface', - 'PhabricatorPolicyTestQuery' => 'PhabricatorPolicyQuery', + 'PhabricatorPolicyType' => 'PhabricatorPolicyConstants', 'PhabricatorProfileHeaderView' => 'AphrontView', 'PhabricatorProject' => array( @@ -2041,7 +2046,7 @@ phutil_register_library_map(array( 'PhabricatorProjectProfile' => 'PhabricatorProjectDAO', 'PhabricatorProjectProfileController' => 'PhabricatorProjectController', 'PhabricatorProjectProfileEditController' => 'PhabricatorProjectController', - 'PhabricatorProjectQuery' => 'PhabricatorCursorPagedPolicyQuery', + 'PhabricatorProjectQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PhabricatorProjectTransaction' => 'PhabricatorProjectDAO', 'PhabricatorProjectTransactionType' => 'PhabricatorProjectConstants', 'PhabricatorProjectUpdateController' => 'PhabricatorProjectController', diff --git a/src/applications/chatlog/PhabricatorChatLogQuery.php b/src/applications/chatlog/PhabricatorChatLogQuery.php index 6b453cb3a9..02b1e1e56c 100644 --- a/src/applications/chatlog/PhabricatorChatLogQuery.php +++ b/src/applications/chatlog/PhabricatorChatLogQuery.php @@ -16,7 +16,8 @@ * limitations under the License. */ -final class PhabricatorChatLogQuery extends PhabricatorCursorPagedPolicyQuery { +final class PhabricatorChatLogQuery + extends PhabricatorCursorPagedPolicyAwareQuery { private $channels; private $maximumEpoch; diff --git a/src/applications/chatlog/storage/PhabricatorChatLogEvent.php b/src/applications/chatlog/storage/PhabricatorChatLogEvent.php index bd869821f6..5b44aff2f6 100644 --- a/src/applications/chatlog/storage/PhabricatorChatLogEvent.php +++ b/src/applications/chatlog/storage/PhabricatorChatLogEvent.php @@ -35,7 +35,7 @@ final class PhabricatorChatLogEvent public function getPolicy($capability) { // TODO: This is sort of silly and mostly just so that we can use - // CursorPagedPolicyQuery; once we implement Channel objects we should + // CursorPagedPolicyAwareQuery; once we implement Channel objects we should // just delegate policy to them. return PhabricatorPolicies::POLICY_PUBLIC; } diff --git a/src/applications/feed/PhabricatorFeedQuery.php b/src/applications/feed/PhabricatorFeedQuery.php index 6e7c38020e..25a58a8eda 100644 --- a/src/applications/feed/PhabricatorFeedQuery.php +++ b/src/applications/feed/PhabricatorFeedQuery.php @@ -16,7 +16,8 @@ * limitations under the License. */ -final class PhabricatorFeedQuery extends PhabricatorCursorPagedPolicyQuery { +final class PhabricatorFeedQuery + extends PhabricatorCursorPagedPolicyAwareQuery { private $filterPHIDs; diff --git a/src/applications/owners/query/PhabricatorOwnersPackageQuery.php b/src/applications/owners/query/PhabricatorOwnersPackageQuery.php index 0467fe68f0..0c0d7555c1 100644 --- a/src/applications/owners/query/PhabricatorOwnersPackageQuery.php +++ b/src/applications/owners/query/PhabricatorOwnersPackageQuery.php @@ -17,7 +17,7 @@ */ final class PhabricatorOwnersPackageQuery - extends PhabricatorCursorPagedPolicyQuery { + extends PhabricatorCursorPagedPolicyAwareQuery { private $ownerPHIDs; diff --git a/src/applications/paste/controller/PhabricatorPasteEditController.php b/src/applications/paste/controller/PhabricatorPasteEditController.php index c09167a4ff..9d1488a72c 100644 --- a/src/applications/paste/controller/PhabricatorPasteEditController.php +++ b/src/applications/paste/controller/PhabricatorPasteEditController.php @@ -87,6 +87,10 @@ final class PhabricatorPasteEditController extends PhabricatorPasteController { $paste->setTitle($request->getStr('title')); $paste->setLanguage($request->getStr('language')); + $paste->setViewPolicy($request->getStr('can_view')); + + // NOTE: The author is the only editor and can always view the paste, + // so it's impossible for them to choose an invalid policy. if (!$errors) { if ($is_create) { @@ -139,6 +143,19 @@ final class PhabricatorPasteEditController extends PhabricatorPasteController { ->setValue($paste->getLanguage()) ->setOptions($langs)); + $policies = id(new PhabricatorPolicyQuery()) + ->setViewer($user) + ->setObject($paste) + ->execute(); + + $form->appendChild( + id(new AphrontFormPolicyControl()) + ->setUser($user) + ->setCapability(PhabricatorPolicyCapability::CAN_VIEW) + ->setPolicyObject($paste) + ->setPolicies($policies) + ->setName('can_view')); + if ($is_create) { $form ->appendChild( @@ -151,16 +168,6 @@ final class PhabricatorPasteEditController extends PhabricatorPasteController { ->setName('text')); } - /* TODO: Doesn't have any useful options yet. - ->appendChild( - id(new AphrontFormPolicyControl()) - ->setLabel('Visible To') - ->setUser($user) - ->setValue( - $new_paste->getPolicy(PhabricatorPolicyCapability::CAN_VIEW)) - ->setName('policy')) - */ - $submit = new AphrontFormSubmitControl(); if (!$is_create) { diff --git a/src/applications/paste/controller/PhabricatorPasteViewController.php b/src/applications/paste/controller/PhabricatorPasteViewController.php index ddd559ce74..83308d52b7 100644 --- a/src/applications/paste/controller/PhabricatorPasteViewController.php +++ b/src/applications/paste/controller/PhabricatorPasteViewController.php @@ -147,6 +147,14 @@ final class PhabricatorPasteViewController extends PhabricatorPasteController { $this->renderHandlesForPHIDs($child_phids)); } + $descriptions = PhabricatorPolicyQuery::renderPolicyDescriptions( + $user, + $paste); + + $properties->addProperty( + pht('Visible To'), + $descriptions[PhabricatorPolicyCapability::CAN_VIEW]); + return $properties; } diff --git a/src/applications/paste/query/PhabricatorPasteQuery.php b/src/applications/paste/query/PhabricatorPasteQuery.php index 2ac2bb7d16..fdbb14251a 100644 --- a/src/applications/paste/query/PhabricatorPasteQuery.php +++ b/src/applications/paste/query/PhabricatorPasteQuery.php @@ -16,7 +16,8 @@ * limitations under the License. */ -final class PhabricatorPasteQuery extends PhabricatorCursorPagedPolicyQuery { +final class PhabricatorPasteQuery + extends PhabricatorCursorPagedPolicyAwareQuery { private $ids; private $phids; diff --git a/src/applications/phid/handle/PhabricatorObjectHandleData.php b/src/applications/phid/handle/PhabricatorObjectHandleData.php index 54c7f0d68b..8d8767df9a 100644 --- a/src/applications/phid/handle/PhabricatorObjectHandleData.php +++ b/src/applications/phid/handle/PhabricatorObjectHandleData.php @@ -345,7 +345,15 @@ final class PhabricatorObjectHandleData { case PhabricatorPHIDConstants::PHID_TYPE_PROJ: $object = new PhabricatorProject(); - $projects = $object->loadAllWhere('phid IN (%Ls)', $phids); + if ($this->viewer) { + $projects = id(new PhabricatorProjectQuery()) + ->setViewer($this->viewer) + ->withPHIDs($phids) + ->execute(); + } else { + $projects = $object->loadAllWhere('phid IN (%Ls)', $phids); + } + $projects = mpull($projects, null, 'getPHID'); foreach ($phids as $phid) { diff --git a/src/applications/policy/__tests__/PhabricatorPolicyTestQuery.php b/src/applications/policy/__tests__/PhabricatorPolicyAwareTestQuery.php similarity index 93% rename from src/applications/policy/__tests__/PhabricatorPolicyTestQuery.php rename to src/applications/policy/__tests__/PhabricatorPolicyAwareTestQuery.php index 5129e5bacc..e1d869d09b 100644 --- a/src/applications/policy/__tests__/PhabricatorPolicyTestQuery.php +++ b/src/applications/policy/__tests__/PhabricatorPolicyAwareTestQuery.php @@ -19,8 +19,8 @@ /** * Configurable test query for implementing Policy unit tests. */ -final class PhabricatorPolicyTestQuery - extends PhabricatorPolicyQuery { +final class PhabricatorPolicyAwareTestQuery + extends PhabricatorPolicyAwareQuery { private $results; private $offset = 0; diff --git a/src/applications/policy/__tests__/PhabricatorPolicyTestCase.php b/src/applications/policy/__tests__/PhabricatorPolicyTestCase.php index 2f305ff866..2eb49697a6 100644 --- a/src/applications/policy/__tests__/PhabricatorPolicyTestCase.php +++ b/src/applications/policy/__tests__/PhabricatorPolicyTestCase.php @@ -115,7 +115,7 @@ final class PhabricatorPolicyTestCase extends PhabricatorTestCase { $this->buildObject(PhabricatorPolicies::POLICY_USER), ); - $query = new PhabricatorPolicyTestQuery(); + $query = new PhabricatorPolicyAwareTestQuery(); $query->setResults($results); $query->setViewer($this->buildUser('user')); @@ -154,7 +154,7 @@ final class PhabricatorPolicyTestCase extends PhabricatorTestCase { $this->buildObject(PhabricatorPolicies::POLICY_USER), ); - $query = new PhabricatorPolicyTestQuery(); + $query = new PhabricatorPolicyAwareTestQuery(); $query->setResults($results); $query->setViewer($this->buildUser('user')); @@ -181,7 +181,7 @@ final class PhabricatorPolicyTestCase extends PhabricatorTestCase { foreach ($map as $spec => $expect) { $viewer = $this->buildUser($spec); - $query = new PhabricatorPolicyTestQuery(); + $query = new PhabricatorPolicyAwareTestQuery(); $query->setResults(array($object)); $query->setViewer($viewer); diff --git a/src/applications/policy/constants/PhabricatorPolicyType.php b/src/applications/policy/constants/PhabricatorPolicyType.php new file mode 100644 index 0000000000..819e9fc588 --- /dev/null +++ b/src/applications/policy/constants/PhabricatorPolicyType.php @@ -0,0 +1,46 @@ + 0, + self::TYPE_PROJECT => 1, + self::TYPE_MASKED => 9, + ); + return idx($map, $type, 9); + } + + public static function getPolicyTypeName($type) { + switch ($type) { + case self::TYPE_GLOBAL: + return pht('Global Policies'); + case self::TYPE_PROJECT: + return pht('Members of Project'); + case self::TYPE_MASKED: + default: + return pht('Other Policies'); + } + } + +} diff --git a/src/applications/policy/filter/PhabricatorPolicy.php b/src/applications/policy/filter/PhabricatorPolicy.php new file mode 100644 index 0000000000..e917720969 --- /dev/null +++ b/src/applications/policy/filter/PhabricatorPolicy.php @@ -0,0 +1,119 @@ +type = $type; + return $this; + } + + public function getType() { + return $this->type; + } + + public function setName($name) { + $this->name = $name; + return $this; + } + + public function getName() { + return $this->name; + } + + public function setPHID($phid) { + $this->phid = $phid; + return $this; + } + + public function getPHID() { + return $this->phid; + } + + public function setHref($href) { + $this->href = $href; + return $this; + } + + public function getHref() { + return $this->href; + } + + public function getSortKey() { + return sprintf( + '%02d%s', + PhabricatorPolicyType::getPolicyTypeOrder($this->getType()), + $this->getSortName()); + } + + private function getSortName() { + if ($this->getType() == PhabricatorPolicyType::TYPE_GLOBAL) { + static $map = array( + PhabricatorPolicies::POLICY_PUBLIC => 0, + PhabricatorPolicies::POLICY_USER => 1, + PhabricatorPolicies::POLICY_ADMIN => 2, + PhabricatorPolicies::POLICY_NOONE => 3, + ); + return idx($map, $this->getPHID()); + } + return $this->getName(); + } + + public function getFullName() { + switch ($this->getType()) { + case PhabricatorPolicyType::TYPE_PROJECT: + return pht('Project: %s', $this->getName()); + case PhabricatorPolicyType::TYPE_MASKED: + return pht('Other: %s', $this->getName()); + default: + return $this->getName(); + } + } + + public function renderDescription() { + if ($this->getHref()) { + $desc = phutil_render_tag( + 'a', + array( + 'href' => $this->getHref(), + ), + phutil_escape_html($this->getName())); + } else { + $desc = phutil_escape_html($this->getName()); + } + + switch ($this->getType()) { + case PhabricatorPolicyType::TYPE_PROJECT: + return pht('%s (Project)', $desc); + case PhabricatorPolicyType::TYPE_MASKED: + return pht( + '%s (You do not have permission to view policy details.)', + $desc); + default: + return $desc; + } + } + + +} diff --git a/src/applications/policy/filter/PhabricatorPolicyFilter.php b/src/applications/policy/filter/PhabricatorPolicyFilter.php index 30608d741d..541ea76264 100644 --- a/src/applications/policy/filter/PhabricatorPolicyFilter.php +++ b/src/applications/policy/filter/PhabricatorPolicyFilter.php @@ -22,6 +22,7 @@ final class PhabricatorPolicyFilter { private $objects; private $capabilities; private $raisePolicyExceptions; + private $userProjects; public static function mustRetainCapability( PhabricatorUser $user, @@ -87,6 +88,7 @@ final class PhabricatorPolicyFilter { $filtered = array(); + $need_projects = array(); foreach ($objects as $key => $object) { $object_capabilities = $object->getCapabilities(); foreach ($capabilities as $capability) { @@ -96,14 +98,65 @@ final class PhabricatorPolicyFilter { "not have that capability!"); } + $policy = $object->getPolicy($capability); + $type = phid_get_type($policy); + if ($type == PhabricatorPHIDConstants::PHID_TYPE_PROJ) { + $need_projects[] = $policy; + } + } + } + + if ($need_projects) { + $need_projects = array_unique($need_projects); + + // If projects have recursive policies, automatically fail them rather + // than looping. This will fall back to automatic capabilities and + // resolve the policies in a sensible way. + static $querying_projects = array(); + foreach ($need_projects as $key => $project) { + if (empty($querying_projects[$project])) { + $querying_projects[$project] = true; + continue; + } + unset($need_projects[$key]); + } + + if ($need_projects) { + $caught = null; + try { + $projects = id(new PhabricatorProjectQuery()) + ->setViewer($viewer) + ->withMemberPHIDs(array($viewer->getPHID())) + ->withPHIDs($need_projects) + ->execute(); + } catch (Exception $ex) { + $caught = $ex; + } + + foreach ($need_projects as $key => $project) { + unset($querying_projects[$project]); + } + + if ($caught) { + throw $caught; + } + + $projects = mpull($projects, null, 'getPHID'); + $this->userProjects[$viewer->getPHID()] = $projects; + } + } + + foreach ($objects as $key => $object) { + $object_capabilities = $object->getCapabilities(); + foreach ($capabilities as $capability) { if (!$this->checkCapability($object, $capability)) { // If we're missing any capability, move on to the next object. continue 2; } - } - // If we make it here, we have all of the required capabilities. - $filtered[$key] = $object; + // If we make it here, we have all of the required capabilities. + $filtered[$key] = $object; + } } return $filtered; @@ -161,12 +214,37 @@ final class PhabricatorPolicyFilter { $this->rejectObject($object, $policy, $capability); break; default: - throw new Exception("Object has unknown policy '{$policy}'!"); + $type = phid_get_type($policy); + if ($type == PhabricatorPHIDConstants::PHID_TYPE_PROJ) { + if (isset($this->userProjects[$viewer->getPHID()][$policy])) { + return true; + } else { + $this->rejectObject($object, $policy, $capability); + } + } else { + throw new Exception("Object has unknown policy '{$policy}'!"); + } } return false; } + private function rejectImpossiblePolicy( + PhabricatorPolicyInterface $object, + $policy, + $capability) { + + if (!$this->raisePolicyExceptions) { + return; + } + + // TODO: clean this up + $verb = $capability; + + throw new PhabricatorPolicyException( + "This object has an impossible {$verb} policy."); + } + private function rejectObject($object, $policy, $capability) { if (!$this->raisePolicyExceptions) { return; @@ -191,7 +269,16 @@ final class PhabricatorPolicyFilter { $who = "No one can {$verb} this object."; break; default: - $who = "It is unclear who can {$verb} this object."; + $type = phid_get_type($policy); + if ($type == PhabricatorPHIDConstants::PHID_TYPE_PROJ) { + $handle = PhabricatorObjectHandleData::loadOneHandle( + $policy, + $this->viewer); + $who = "To {$verb} this object, you must be a member of project ". + "'".$handle->getFullName()."'."; + } else { + $who = "It is unclear who can {$verb} this object."; + } break; } diff --git a/src/applications/policy/query/PhabricatorPolicyQuery.php b/src/applications/policy/query/PhabricatorPolicyQuery.php new file mode 100644 index 0000000000..67cc497582 --- /dev/null +++ b/src/applications/policy/query/PhabricatorPolicyQuery.php @@ -0,0 +1,169 @@ +viewer = $viewer; + return $this; + } + + public function setObject(PhabricatorPolicyInterface $object) { + $this->object = $object; + return $this; + } + + public static function renderPolicyDescriptions( + PhabricatorUser $viewer, + PhabricatorPolicyInterface $object) { + + $results = array(); + $policies = null; + $global = self::getGlobalPolicies(); + + $capabilities = $object->getCapabilities(); + foreach ($capabilities as $capability) { + $policy = $object->getPolicy($capability); + + if (isset($global[$policy])) { + $results[$capability] = $global[$policy]->renderDescription(); + continue; + } + + if ($policies === null) { + // This slightly overfetches data, but it shouldn't generally + // be a problem. + $policies = id(new PhabricatorPolicyQuery()) + ->setViewer($viewer) + ->setObject($object) + ->execute(); + } + + $results[$capability] = $policies[$policy]->renderDescription(); + } + + return $results; + } + + public function execute() { + if (!$this->viewer) { + throw new Exception('Call setViewer() before execute()!'); + } + if (!$this->object) { + throw new Exception('Call setObject() before execute()!'); + } + + $results = $this->getGlobalPolicies(); + + if ($this->viewer->getPHID()) { + $projects = id(new PhabricatorProjectQuery()) + ->setViewer($this->viewer) + ->withMemberPHIDs(array($this->viewer->getPHID())) + ->execute(); + if ($projects) { + foreach ($projects as $project) { + $results[] = id(new PhabricatorPolicy()) + ->setType(PhabricatorPolicyType::TYPE_PROJECT) + ->setPHID($project->getPHID()) + ->setHref('/project/view/'.$project->getID().'/') + ->setName($project->getName()); + } + } + } + + $results = mpull($results, null, 'getPHID'); + + $other_policies = array(); + $capabilities = $this->object->getCapabilities(); + foreach ($capabilities as $capability) { + $policy = $this->object->getPolicy($capability); + if (!$policy) { + continue; + } + $other_policies[$policy] = $policy; + } + + // If this install doesn't have "Public" enabled, remove it as an option + // unless the object already has a "Public" policy. In this case we retain + // the policy but enforce it as thought it was "All Users". + $show_public = PhabricatorEnv::getEnvConfig('policy.allow-public'); + if (!$show_public && + empty($other_policies[PhabricatorPolicies::POLICY_PUBLIC])) { + unset($results[PhabricatorPolicies::POLICY_PUBLIC]); + } + + $other_policies = array_diff_key($other_policies, $results); + + if ($other_policies) { + $handles = id(new PhabricatorObjectHandleData($other_policies)) + ->setViewer($this->viewer) + ->loadHandles(); + foreach ($other_policies as $phid) { + $handle = $handles[$phid]; + $results[$phid] = id(new PhabricatorPolicy()) + ->setType(PhabricatorPolicyType::TYPE_MASKED) + ->setPHID($handle->getPHID()) + ->setHref($handle->getLink()) + ->setName($handle->getFullName()); + } + } + + $results = msort($results, 'getSortKey'); + + return $results; + } + + private static function getGlobalPolicies() { + static $constants = array( + PhabricatorPolicies::POLICY_PUBLIC, + PhabricatorPolicies::POLICY_USER, + PhabricatorPolicies::POLICY_ADMIN, + PhabricatorPolicies::POLICY_NOONE, + ); + + $results = array(); + foreach ($constants as $constant) { + $results[$constant] = id(new PhabricatorPolicy()) + ->setType(PhabricatorPolicyType::TYPE_GLOBAL) + ->setPHID($constant) + ->setName(self::getGlobalPolicyName($constant)); + } + + return $results; + } + + private static function getGlobalPolicyName($policy) { + switch ($policy) { + case PhabricatorPolicies::POLICY_PUBLIC: + return pht('Public'); + case PhabricatorPolicies::POLICY_USER: + return pht('All Users'); + case PhabricatorPolicies::POLICY_ADMIN: + return pht('Administrators'); + case PhabricatorPolicies::POLICY_NOONE: + return pht('No One'); + default: + return pht('Unknown Policy'); + } + } + +} + diff --git a/src/applications/project/controller/PhabricatorProjectProfileEditController.php b/src/applications/project/controller/PhabricatorProjectProfileEditController.php index b2839fa9b8..8da592b910 100644 --- a/src/applications/project/controller/PhabricatorProjectProfileEditController.php +++ b/src/applications/project/controller/PhabricatorProjectProfileEditController.php @@ -154,6 +154,11 @@ final class PhabricatorProjectProfileEditController $title = 'Edit Project'; $action = '/project/edit/'.$project->getID().'/'; + $policies = id(new PhabricatorPolicyQuery()) + ->setViewer($user) + ->setObject($project) + ->execute(); + $form = new AphrontFormView(); $form ->setID('project-edit-form') @@ -187,12 +192,14 @@ final class PhabricatorProjectProfileEditController ->setName('can_view') ->setCaption('Members can always view a project.') ->setPolicyObject($project) + ->setPolicies($policies) ->setCapability(PhabricatorPolicyCapability::CAN_VIEW)) ->appendChild( id(new AphrontFormPolicyControl()) ->setUser($user) ->setName('can_edit') ->setPolicyObject($project) + ->setPolicies($policies) ->setCapability(PhabricatorPolicyCapability::CAN_EDIT)) ->appendChild( id(new AphrontFormPolicyControl()) @@ -201,6 +208,7 @@ final class PhabricatorProjectProfileEditController ->setCaption( 'Users who can edit a project can always join a project.') ->setPolicyObject($project) + ->setPolicies($policies) ->setCapability(PhabricatorPolicyCapability::CAN_JOIN)) ->appendChild( id(new AphrontFormMarkupControl()) diff --git a/src/applications/project/query/PhabricatorProjectQuery.php b/src/applications/project/query/PhabricatorProjectQuery.php index 74fd7e5adc..68f3af2db1 100644 --- a/src/applications/project/query/PhabricatorProjectQuery.php +++ b/src/applications/project/query/PhabricatorProjectQuery.php @@ -16,7 +16,8 @@ * limitations under the License. */ -final class PhabricatorProjectQuery extends PhabricatorCursorPagedPolicyQuery { +final class PhabricatorProjectQuery + extends PhabricatorCursorPagedPolicyAwareQuery { private $ids; private $phids; diff --git a/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyQuery.php b/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php similarity index 97% rename from src/infrastructure/query/policy/PhabricatorCursorPagedPolicyQuery.php rename to src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php index a3059437ae..9c070455bb 100644 --- a/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyQuery.php +++ b/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php @@ -20,8 +20,8 @@ * A query class which uses cursor-based paging. This paging is much more * performant than offset-based paging in the presence of policy filtering. */ -abstract class PhabricatorCursorPagedPolicyQuery - extends PhabricatorPolicyQuery { +abstract class PhabricatorCursorPagedPolicyAwareQuery + extends PhabricatorPolicyAwareQuery { private $afterID; private $beforeID; diff --git a/src/infrastructure/query/policy/PhabricatorPolicyQuery.php b/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php similarity index 95% rename from src/infrastructure/query/policy/PhabricatorPolicyQuery.php rename to src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php index 941005a251..fb204dead1 100644 --- a/src/infrastructure/query/policy/PhabricatorPolicyQuery.php +++ b/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php @@ -26,22 +26,23 @@ * ->withConstraint($example) * ->execute(); * - * Normally, you should extend @{class:PhabricatorCursorPagedPolicyQuery}, not - * this class. @{class:PhabricatorCursorPagedPolicyQuery} provides a more - * practical interface for building usable queries against most object types. + * Normally, you should extend @{class:PhabricatorCursorPagedPolicyAwareQuery}, + * not this class. @{class:PhabricatorCursorPagedPolicyAwareQuery} provides a + * more practical interface for building usable queries against most object + * types. * * NOTE: Although this class extends @{class:PhabricatorOffsetPagedQuery}, * offset paging with policy filtering is not efficient. All results must be * loaded into the application and filtered here: skipping `N` rows via offset * is an `O(N)` operation with a large constant. Prefer cursor-based paging - * with @{class:PhabricatorCursorPagedPolicyQuery}, which can filter far more - * efficiently in MySQL. + * with @{class:PhabricatorCursorPagedPolicyAwareQuery}, which can filter far + * more efficiently in MySQL. * * @task config Query Configuration * @task exec Executing Queries * @task policyimpl Policy Query Implementation */ -abstract class PhabricatorPolicyQuery extends PhabricatorOffsetPagedQuery { +abstract class PhabricatorPolicyAwareQuery extends PhabricatorOffsetPagedQuery { private $viewer; private $raisePolicyExceptions; diff --git a/src/view/form/control/AphrontFormPolicyControl.php b/src/view/form/control/AphrontFormPolicyControl.php index 40dac998ad..2d85812850 100644 --- a/src/view/form/control/AphrontFormPolicyControl.php +++ b/src/view/form/control/AphrontFormPolicyControl.php @@ -21,6 +21,7 @@ final class AphrontFormPolicyControl extends AphrontFormControl { private $user; private $object; private $capability; + private $policies; public function setUser(PhabricatorUser $user) { $this->user = $user; @@ -36,6 +37,12 @@ final class AphrontFormPolicyControl extends AphrontFormControl { return $this; } + public function setPolicies(array $policies) { + assert_instances_of($policies, 'PhabricatorPolicy'); + $this->policies = $policies; + return $this; + } + public function setCapability($capability) { $this->capability = $capability; @@ -54,35 +61,18 @@ final class AphrontFormPolicyControl extends AphrontFormControl { return 'aphront-form-control-policy'; } - private function getOptions() { - $show_public = PhabricatorEnv::getEnvConfig('policy.allow-public'); - - if ($this->capability != PhabricatorPolicyCapability::CAN_VIEW) { - // We don't generally permit 'public' for anything except viewing. - $show_public = false; - } - - if ($this->getValue() == PhabricatorPolicies::POLICY_PUBLIC) { - // If the object already has a "public" policy, show the option in - // the dropdown even if it will be enforced as "users", so we don't - // change the policy just because the config is changing. - $show_public = true; - } - + protected function getOptions() { $options = array(); + foreach ($this->policies as $policy) { + if (($policy->getPHID() == PhabricatorPolicies::POLICY_PUBLIC) && + ($this->capability != PhabricatorPolicyCapability::CAN_VIEW)) { + // Never expose "Public" for anything except "Can View". + continue; + } - if ($show_public) { - $options[PhabricatorPolicies::POLICY_PUBLIC] = 'Public'; + $type_name = PhabricatorPolicyType::getPolicyTypeName($policy->getType()); + $options[$type_name][$policy->getPHID()] = $policy->getFullName(); } - - $options[PhabricatorPolicies::POLICY_USER] = 'All Users'; - - if ($this->user->getIsAdmin()) { - $options[PhabricatorPolicies::POLICY_ADMIN] = 'Administrators'; - } - - $options[PhabricatorPolicies::POLICY_NOONE] = 'No One'; - return $options; }