diff --git a/src/applications/paste/query/PhabricatorPasteQuery.php b/src/applications/paste/query/PhabricatorPasteQuery.php --- a/src/applications/paste/query/PhabricatorPasteQuery.php +++ b/src/applications/paste/query/PhabricatorPasteQuery.php @@ -67,6 +67,10 @@ return $this; } + protected function newResultObject() { + return new PhabricatorPaste(); + } + protected function loadPage() { $table = new PhabricatorPaste(); $conn_r = $table->establishConnection('r'); diff --git a/src/applications/policy/filter/PhabricatorPolicyFilter.php b/src/applications/policy/filter/PhabricatorPolicyFilter.php --- a/src/applications/policy/filter/PhabricatorPolicyFilter.php +++ b/src/applications/policy/filter/PhabricatorPolicyFilter.php @@ -453,6 +453,14 @@ return true; } + if ($object instanceof PhabricatorSpacesInterface) { + $space_phid = $object->getSpacePHID(); + if (!$this->canViewerSeeObjectsInSpace($viewer, $space_phid)) { + $this->rejectObjectFromSpace($object, $space_phid); + return false; + } + } + if ($object->hasAutomaticCapability($capability, $viewer)) { return true; } @@ -676,4 +684,64 @@ return $access_denied; } + + private function canViewerSeeObjectsInSpace( + PhabricatorUser $viewer, + $space_phid) { + + $spaces = PhabricatorSpacesNamespaceQuery::getAllSpaces(); + + // If there are no spaces, everything exists in an implicit default space + // with no policy controls. This is the default state. + if (!$spaces) { + if ($space_phid !== null) { + return false; + } else { + return true; + } + } + + if ($space_phid === null) { + $space = PhabricatorSpacesNamespaceQuery::getDefaultSpace(); + } else { + $space = idx($spaces, $space_phid); + } + + if (!$space) { + return false; + } + + // This may be more involved later, but for now being able to see the + // space is equivalent to being able to see everything in it. + return self::hasCapability( + $viewer, + $space, + PhabricatorPolicyCapability::CAN_VIEW); + } + + private function rejectObjectFromSpace( + PhabricatorPolicyInterface $object, + $space_phid) { + + if (!$this->raisePolicyExceptions) { + return; + } + + if ($this->viewer->isOmnipotent()) { + return; + } + + $access_denied = $this->renderAccessDenied($object); + + $rejection = pht( + 'This object is in a space you do not have permission to access.'); + $full_message = pht('[%s] %s', $access_denied, $rejection); + + $exception = id(new PhabricatorPolicyException($full_message)) + ->setTitle($access_denied) + ->setRejection($rejection); + + throw $exception; + } + } diff --git a/src/applications/spaces/__tests__/PhabricatorSpacesTestCase.php b/src/applications/spaces/__tests__/PhabricatorSpacesTestCase.php --- a/src/applications/spaces/__tests__/PhabricatorSpacesTestCase.php +++ b/src/applications/spaces/__tests__/PhabricatorSpacesTestCase.php @@ -14,10 +14,23 @@ // Test that our helper methods work correctly. $actor = $this->generateNewTestUser(); - $this->newSpace($actor, pht('Test Space'), true); + + $default = $this->newSpace($actor, pht('Test Space'), true); $this->assertEqual(1, count($this->loadAllSpaces())); + $this->assertEqual( + 1, + count(PhabricatorSpacesNamespaceQuery::getAllSpaces())); + $cache_default = PhabricatorSpacesNamespaceQuery::getDefaultSpace(); + $this->assertEqual($default->getPHID(), $cache_default->getPHID()); + $this->destroyAllSpaces(); $this->assertEqual(0, count($this->loadAllSpaces())); + $this->assertEqual( + 0, + count(PhabricatorSpacesNamespaceQuery::getAllSpaces())); + $this->assertEqual( + null, + PhabricatorSpacesNamespaceQuery::getDefaultSpace()); } public function testSpacesSeveralSpaces() { @@ -27,9 +40,15 @@ // work fine. $actor = $this->generateNewTestUser(); - $this->newSpace($actor, pht('Default Space'), true); + $default = $this->newSpace($actor, pht('Default Space'), true); $this->newSpace($actor, pht('Alternate Space'), false); $this->assertEqual(2, count($this->loadAllSpaces())); + $this->assertEqual( + 2, + count(PhabricatorSpacesNamespaceQuery::getAllSpaces())); + + $cache_default = PhabricatorSpacesNamespaceQuery::getDefaultSpace(); + $this->assertEqual($default->getPHID(), $cache_default->getPHID()); } public function testSpacesRequireNames() { @@ -70,6 +89,84 @@ $this->assertTrue(($caught instanceof Exception)); } + public function testSpacesPolicyFiltering() { + $this->destroyAllSpaces(); + + $creator = $this->generateNewTestUser(); + $viewer = $this->generateNewTestUser(); + + // Create a new paste. + $paste = PhabricatorPaste::initializeNewPaste($creator) + ->setViewPolicy(PhabricatorPolicies::POLICY_USER) + ->setFilePHID('') + ->setLanguage('') + ->save(); + + // It should be visible. + $this->assertTrue( + PhabricatorPolicyFilter::hasCapability( + $viewer, + $paste, + PhabricatorPolicyCapability::CAN_VIEW)); + + // Create a default space with an open view policy. + $default = $this->newSpace($creator, pht('Default Space'), true) + ->setViewPolicy(PhabricatorPolicies::POLICY_USER) + ->save(); + PhabricatorSpacesNamespaceQuery::destroySpacesCache(); + + // The paste should now be in the space implicitly, but still visible + // because the space view policy is open. + $this->assertTrue( + PhabricatorPolicyFilter::hasCapability( + $viewer, + $paste, + PhabricatorPolicyCapability::CAN_VIEW)); + + // Make the space view policy restrictive. + $default + ->setViewPolicy(PhabricatorPolicies::POLICY_NOONE) + ->save(); + PhabricatorSpacesNamespaceQuery::destroySpacesCache(); + + // The paste should be in the space implicitly, and no longer visible. + $this->assertFalse( + PhabricatorPolicyFilter::hasCapability( + $viewer, + $paste, + PhabricatorPolicyCapability::CAN_VIEW)); + + // Put the paste in the space explicitly. + $paste + ->setSpacePHID($default->getPHID()) + ->save(); + PhabricatorSpacesNamespaceQuery::destroySpacesCache(); + + // This should still fail, we're just in the space explicitly now. + $this->assertFalse( + PhabricatorPolicyFilter::hasCapability( + $viewer, + $paste, + PhabricatorPolicyCapability::CAN_VIEW)); + + // Create an alternate space with more permissive policies, then move the + // paste to that space. + $alternate = $this->newSpace($creator, pht('Alternate Space'), false) + ->setViewPolicy(PhabricatorPolicies::POLICY_USER) + ->save(); + $paste + ->setSpacePHID($alternate->getPHID()) + ->save(); + PhabricatorSpacesNamespaceQuery::destroySpacesCache(); + + // Now the paste should be visible again. + $this->assertTrue( + PhabricatorPolicyFilter::hasCapability( + $viewer, + $paste, + PhabricatorPolicyCapability::CAN_VIEW)); + } + private function loadAllSpaces() { return id(new PhabricatorSpacesNamespaceQuery()) ->setViewer(PhabricatorUser::getOmnipotentUser()) @@ -77,6 +174,7 @@ } private function destroyAllSpaces() { + PhabricatorSpacesNamespaceQuery::destroySpacesCache(); $spaces = $this->loadAllSpaces(); foreach ($spaces as $space) { $engine = new PhabricatorDestructionEngine(); diff --git a/src/applications/spaces/query/PhabricatorSpacesNamespaceQuery.php b/src/applications/spaces/query/PhabricatorSpacesNamespaceQuery.php --- a/src/applications/spaces/query/PhabricatorSpacesNamespaceQuery.php +++ b/src/applications/spaces/query/PhabricatorSpacesNamespaceQuery.php @@ -3,6 +3,9 @@ final class PhabricatorSpacesNamespaceQuery extends PhabricatorCursorPagedPolicyAwareQuery { + const KEY_ALL = 'spaces.all'; + const KEY_DEFAULT = 'spaces.default'; + private $ids; private $phids; private $isDefaultNamespace; @@ -74,4 +77,68 @@ return $this->formatWhereClause($where); } + public static function destroySpacesCache() { + $cache = PhabricatorCaches::getRequestCache(); + $cache->deleteKeys( + array( + self::KEY_ALL, + self::KEY_DEFAULT, + )); + } + + public static function getAllSpaces() { + $cache = PhabricatorCaches::getRequestCache(); + $cache_key = self::KEY_ALL; + + $spaces = $cache->getKey($cache_key); + if ($spaces === null) { + $spaces = id(new PhabricatorSpacesNamespaceQuery()) + ->setViewer(PhabricatorUser::getOmnipotentUser()) + ->execute(); + $spaces = mpull($spaces, null, 'getPHID'); + $cache->setKey($cache_key, $spaces); + } + + return $spaces; + } + + public static function getDefaultSpace() { + $cache = PhabricatorCaches::getRequestCache(); + $cache_key = self::KEY_DEFAULT; + + $default_space = $cache->getKey($cache_key, false); + if ($default_space === false) { + $default_space = null; + + $spaces = self::getAllSpaces(); + foreach ($spaces as $space) { + if ($space->getIsDefaultNamespace()) { + $default_space = $space; + break; + } + } + + $cache->setKey($cache_key, $default_space); + } + + return $default_space; + } + + public static function getViewerSpaces(PhabricatorUser $viewer) { + $spaces = self::getAllSpaces(); + + $result = array(); + foreach ($spaces as $key => $space) { + $can_see = PhabricatorPolicyFilter::hasCapability( + $viewer, + $space, + PhabricatorPolicyCapability::CAN_VIEW); + if ($can_see) { + $result[$key] = $space; + } + } + + return $result; + } + } diff --git a/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php b/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php --- a/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php +++ b/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php @@ -10,6 +10,7 @@ * @task paging Paging * @task order Result Ordering * @task edgelogic Working with Edge Logic + * @task spaces Working with Spaces */ abstract class PhabricatorCursorPagedPolicyAwareQuery extends PhabricatorPolicyAwareQuery { @@ -23,6 +24,7 @@ private $builtinOrder; private $edgeLogicConstraints = array(); private $edgeLogicConstraintsAreValid = false; + private $spacePHIDs; protected function getPageCursors(array $page) { return array( @@ -247,6 +249,7 @@ $where = array(); $where[] = $this->buildPagingClause($conn); $where[] = $this->buildEdgeLogicWhereClause($conn); + $where[] = $this->buildSpacesWhereClause($conn); return $where; } @@ -1611,4 +1614,163 @@ } +/* -( Spaces )------------------------------------------------------------- */ + + + /** + * Constrain the query to return results from only specific Spaces. + * + * Pass a list of Space PHIDs, or `null` to represent the default space. Only + * results in those Spaces will be returned. + * + * Queries are always constrained to include only results from spaces the + * viewer has access to. + * + * @param list + * @task spaces + */ + public function withSpacePHIDs(array $space_phids) { + $object = $this->newResultObject(); + + if (!$object) { + throw new Exception( + pht( + 'This query (of class "%s") does not implement newResultObject(), '. + 'but must implement this method to enable support for Spaces.', + get_class($this))); + } + + if (!($object instanceof PhabricatorSpacesInterface)) { + throw new Exception( + pht( + 'This query (of class "%s") returned an object of class "%s" from '. + 'getNewResultObject(), but it does not implement the required '. + 'interface ("%s"). Objects must implement this interface to enable '. + 'Spaces support.', + get_class($this), + get_class($object), + 'PhabricatorSpacesInterface')); + } + + $this->spacePHIDs = $space_phids; + + return $this; + } + + + /** + * Constrain the query to include only results in valid Spaces. + * + * This method builds part of a WHERE clause which considers the spaces the + * viewer has access to see with any explicit constraint on spaces added by + * @{method:withSpacePHIDs}. + * + * @param AphrontDatabaseConnection Database connection. + * @return string Part of a WHERE clause. + * @task spaces + */ + private function buildSpacesWhereClause(AphrontDatabaseConnection $conn) { + $object = $this->newResultObject(); + if (!$object) { + return null; + } + + if (!($object instanceof PhabricatorSpacesInterface)) { + return null; + } + + $viewer = $this->getViewer(); + + $space_phids = array(); + $include_null = false; + + $all = PhabricatorSpacesNamespaceQuery::getAllSpaces(); + if (!$all) { + // If there are no spaces at all, implicitly give the viewer access to + // the default space. + $include_null = true; + } else { + // Otherwise, give them access to the spaces they have permission to + // see. + $viewer_spaces = PhabricatorSpacesNamespaceQuery::getViewerSpaces( + $viewer); + foreach ($viewer_spaces as $viewer_space) { + $phid = $viewer_space->getPHID(); + $space_phids[$phid] = $phid; + if ($viewer_space->getIsDefaultNamespace()) { + $include_null = true; + } + } + } + + // If we have additional explicit constraints, evaluate them now. + if ($this->spacePHIDs !== null) { + $explicit = array(); + $explicit_null = false; + foreach ($this->spacePHIDs as $phid) { + if ($phid === null) { + $space = PhabricatorSpacesNamespaceQuery::getDefaultSpace(); + } else { + $space = idx($all, $phid); + } + + if ($space) { + $phid = $space->getPHID(); + $explicit[$phid] = $phid; + if ($space->getIsDefaultNamespace()) { + $explicit_null = true; + } + } + } + + // If the viewer can see the default space but it isn't on the explicit + // list of spaces to query, don't match it. + if ($include_null && !$explicit_null) { + $include_null = false; + } + + // Include only the spaces common to the viewer and the constraints. + $space_phids = array_intersect_key($space_phids, $explicit); + } + + if (!$space_phids && !$include_null) { + if ($this->spacePHIDs === null) { + throw new PhabricatorEmptyQueryException( + pht('You do not have access to any spaces.')); + } else { + throw new PhabricatorEmptyQueryException( + pht( + 'You do not have access to any of the spaces this query '. + 'is constrained to.')); + } + } + + $alias = $this->getPrimaryTableAlias(); + if ($alias) { + $col = qsprintf($conn, '%T.spacePHID', $alias); + } else { + $col = 'spacePHID'; + } + + if ($space_phids && $include_null) { + return qsprintf( + $conn, + '(%Q IN (%Ls) OR %Q IS NULL)', + $col, + $space_phids, + $col); + } else if ($space_phids) { + return qsprintf( + $conn, + '%Q IN (%Ls)', + $col, + $space_phids); + } else { + return qsprintf( + $conn, + '%Q IS NULL', + $col); + } + } + }