Page MenuHomePhabricator
Files F13175229

D13156.id31810.diff
No OneTemporary
Actions

D13156.id31810.diff
View Options

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<phid|null>
+ * @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);
+ }
+ }
+
}

File Metadata

Mime Type
text/plain
Expires
Thu, May 9, 4:11 AM (3 w, 4 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6269532
Default Alt Text
D13156.id31810.diff (15 KB)

Event Timeline

Phabricator · Built by Phacility