mirror of
https://we.phorge.it/source/phorge.git
synced 2024-11-20 05:42:40 +01:00
Apply hierarchical policy checks to Phriction
Summary: Ref T4029. When checking the view policy of a document, require the viewer to also be able to see all of the ancestors. Test Plan: - Hard-coded `/x/y/` to "no one". - Checked that `/x/y/` is not visible. - Checked that `/x/y/z/` is not visible. - Checked that `/x/`, `/x/q/`, etc., are still visible. - Tested project pages and sub-pages for project visibility. Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T4029 Differential Revision: https://secure.phabricator.com/D9199
This commit is contained in:
parent
4d7c1026f4
commit
9cb4047134
5 changed files with 126 additions and 20 deletions
|
@ -1,8 +1,5 @@
|
||||||
<?php
|
<?php
|
||||||
|
|
||||||
/**
|
|
||||||
* @group phriction
|
|
||||||
*/
|
|
||||||
final class PhrictionDocumentController
|
final class PhrictionDocumentController
|
||||||
extends PhrictionController {
|
extends PhrictionController {
|
||||||
|
|
||||||
|
@ -118,7 +115,7 @@ final class PhrictionDocumentController
|
||||||
$new_doc = id(new PhrictionDocumentQuery())
|
$new_doc = id(new PhrictionDocumentQuery())
|
||||||
->setViewer($user)
|
->setViewer($user)
|
||||||
->withIDs(array($new_doc_id))
|
->withIDs(array($new_doc_id))
|
||||||
->exectueOne();
|
->executeOne();
|
||||||
|
|
||||||
$slug_uri = PhrictionDocument::getSlugURI($new_doc->getSlug());
|
$slug_uri = PhrictionDocument::getSlugURI($new_doc->getSlug());
|
||||||
|
|
||||||
|
|
|
@ -25,8 +25,8 @@ final class PhrictionPHIDTypeDocument extends PhabricatorPHIDType {
|
||||||
array $phids) {
|
array $phids) {
|
||||||
|
|
||||||
return id(new PhrictionDocumentQuery())
|
return id(new PhrictionDocumentQuery())
|
||||||
->needContent(true)
|
->withPHIDs($phids)
|
||||||
->withPHIDs($phids);
|
->needContent(true);
|
||||||
}
|
}
|
||||||
|
|
||||||
public function loadHandles(
|
public function loadHandles(
|
||||||
|
|
|
@ -49,21 +49,97 @@ final class PhrictionDocumentQuery
|
||||||
}
|
}
|
||||||
|
|
||||||
protected function loadPage() {
|
protected function loadPage() {
|
||||||
$document = new PhrictionDocument();
|
$table = new PhrictionDocument();
|
||||||
$conn_r = $document->establishConnection('r');
|
$conn_r = $table->establishConnection('r');
|
||||||
|
|
||||||
$rows = queryfx_all(
|
$rows = queryfx_all(
|
||||||
$conn_r,
|
$conn_r,
|
||||||
'SELECT * FROM %T %Q %Q %Q',
|
'SELECT * FROM %T %Q %Q %Q',
|
||||||
$document->getTableName(),
|
$table->getTableName(),
|
||||||
$this->buildWhereClause($conn_r),
|
$this->buildWhereClause($conn_r),
|
||||||
$this->buildOrderClause($conn_r),
|
$this->buildOrderClause($conn_r),
|
||||||
$this->buildLimitClause($conn_r));
|
$this->buildLimitClause($conn_r));
|
||||||
|
|
||||||
return $document->loadAllFromArray($rows);
|
$documents = $table->loadAllFromArray($rows);
|
||||||
|
|
||||||
|
if ($documents) {
|
||||||
|
$ancestor_slugs = array();
|
||||||
|
foreach ($documents as $key => $document) {
|
||||||
|
$document_slug = $document->getSlug();
|
||||||
|
foreach (PhabricatorSlug::getAncestry($document_slug) as $ancestor) {
|
||||||
|
$ancestor_slugs[$ancestor][] = $key;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if ($ancestor_slugs) {
|
||||||
|
$ancestors = queryfx_all(
|
||||||
|
$conn_r,
|
||||||
|
'SELECT * FROM %T WHERE slug IN (%Ls)',
|
||||||
|
$document->getTableName(),
|
||||||
|
array_keys($ancestor_slugs));
|
||||||
|
$ancestors = $table->loadAllFromArray($ancestors);
|
||||||
|
$ancestors = mpull($ancestors, null, 'getSlug');
|
||||||
|
|
||||||
|
foreach ($ancestor_slugs as $ancestor_slug => $document_keys) {
|
||||||
|
$ancestor = idx($ancestors, $ancestor_slug);
|
||||||
|
foreach ($document_keys as $document_key) {
|
||||||
|
$documents[$document_key]->attachAncestor(
|
||||||
|
$ancestor_slug,
|
||||||
|
$ancestor);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return $documents;
|
||||||
}
|
}
|
||||||
|
|
||||||
protected function willFilterPage(array $documents) {
|
protected function willFilterPage(array $documents) {
|
||||||
|
// To view a Phriction document, you must also be able to view all of the
|
||||||
|
// ancestor documents. Filter out documents which have ancestors that are
|
||||||
|
// not visible.
|
||||||
|
|
||||||
|
$document_map = array();
|
||||||
|
foreach ($documents as $document) {
|
||||||
|
$document_map[$document->getSlug()] = $document;
|
||||||
|
foreach ($document->getAncestors() as $key => $ancestor) {
|
||||||
|
if ($ancestor) {
|
||||||
|
$document_map[$key] = $ancestor;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
$filtered_map = $this->applyPolicyFilter(
|
||||||
|
$document_map,
|
||||||
|
array(PhabricatorPolicyCapability::CAN_VIEW));
|
||||||
|
|
||||||
|
// Filter all of the documents where a parent is not visible.
|
||||||
|
foreach ($documents as $document_key => $document) {
|
||||||
|
// If the document itself is not visible, filter it.
|
||||||
|
if (!isset($filtered_map[$document->getSlug()])) {
|
||||||
|
$this->didRejectResult($documents[$document_key]);
|
||||||
|
unset($documents[$document_key]);
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
// If an ancestor exists but is not visible, filter the document.
|
||||||
|
foreach ($document->getAncestors() as $ancestor_key => $ancestor) {
|
||||||
|
if (!$ancestor) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (!isset($filtered_map[$ancestor_key])) {
|
||||||
|
$this->didRejectResult($documents[$document_key]);
|
||||||
|
unset($documents[$document_key]);
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if (!$documents) {
|
||||||
|
return $documents;
|
||||||
|
}
|
||||||
|
|
||||||
if ($this->needContent) {
|
if ($this->needContent) {
|
||||||
$contents = id(new PhrictionContent())->loadAllWhere(
|
$contents = id(new PhrictionContent())->loadAllWhere(
|
||||||
'id IN (%Ld)',
|
'id IN (%Ld)',
|
||||||
|
|
|
@ -1,8 +1,5 @@
|
||||||
<?php
|
<?php
|
||||||
|
|
||||||
/**
|
|
||||||
* @group phriction
|
|
||||||
*/
|
|
||||||
final class PhrictionDocument extends PhrictionDAO
|
final class PhrictionDocument extends PhrictionDAO
|
||||||
implements
|
implements
|
||||||
PhabricatorPolicyInterface,
|
PhabricatorPolicyInterface,
|
||||||
|
@ -16,6 +13,7 @@ final class PhrictionDocument extends PhrictionDAO
|
||||||
protected $status;
|
protected $status;
|
||||||
|
|
||||||
private $contentObject = self::ATTACHABLE;
|
private $contentObject = self::ATTACHABLE;
|
||||||
|
private $ancestors = array();
|
||||||
|
|
||||||
// TODO: This should be `self::ATTACHABLE`, but there are still a lot of call
|
// TODO: This should be `self::ATTACHABLE`, but there are still a lot of call
|
||||||
// sites which load PhrictionDocuments directly.
|
// sites which load PhrictionDocuments directly.
|
||||||
|
@ -84,6 +82,19 @@ final class PhrictionDocument extends PhrictionDAO
|
||||||
return (bool)$this->getProject();
|
return (bool)$this->getProject();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function getAncestors() {
|
||||||
|
return $this->ancestors;
|
||||||
|
}
|
||||||
|
|
||||||
|
public function getAncestor($slug) {
|
||||||
|
return $this->assertAttachedKey($this->ancestors, $slug);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function attachAncestor($slug, $ancestor) {
|
||||||
|
$this->ancestors[$slug] = $ancestor;
|
||||||
|
return $this;
|
||||||
|
}
|
||||||
|
|
||||||
public static function isProjectSlug($slug) {
|
public static function isProjectSlug($slug) {
|
||||||
$slug = PhabricatorSlug::normalize($slug);
|
$slug = PhabricatorSlug::normalize($slug);
|
||||||
$prefix = 'projects/';
|
$prefix = 'projects/';
|
||||||
|
@ -119,6 +130,7 @@ final class PhrictionDocument extends PhrictionDAO
|
||||||
if ($this->hasProject()) {
|
if ($this->hasProject()) {
|
||||||
return $this->getProject()->getPolicy($capability);
|
return $this->getProject()->getPolicy($capability);
|
||||||
}
|
}
|
||||||
|
|
||||||
return PhabricatorPolicies::POLICY_USER;
|
return PhabricatorPolicies::POLICY_USER;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -134,6 +146,14 @@ final class PhrictionDocument extends PhrictionDAO
|
||||||
return pht(
|
return pht(
|
||||||
"This is a project wiki page, and inherits the project's policies.");
|
"This is a project wiki page, and inherits the project's policies.");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
switch ($capability) {
|
||||||
|
case PhabricatorPolicyCapability::CAN_VIEW:
|
||||||
|
return pht(
|
||||||
|
'To view a wiki document, you must also be able to view all '.
|
||||||
|
'of its parents.');
|
||||||
|
}
|
||||||
|
|
||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -302,19 +302,32 @@ abstract class PhabricatorPolicyAwareQuery extends PhabricatorOffsetPagedQuery {
|
||||||
private function getPolicyFilter() {
|
private function getPolicyFilter() {
|
||||||
$filter = new PhabricatorPolicyFilter();
|
$filter = new PhabricatorPolicyFilter();
|
||||||
$filter->setViewer($this->viewer);
|
$filter->setViewer($this->viewer);
|
||||||
if (!$this->capabilities) {
|
$capabilities = $this->getRequiredCapabilities();
|
||||||
$capabilities = array(
|
|
||||||
PhabricatorPolicyCapability::CAN_VIEW,
|
|
||||||
);
|
|
||||||
} else {
|
|
||||||
$capabilities = $this->capabilities;
|
|
||||||
}
|
|
||||||
$filter->requireCapabilities($capabilities);
|
$filter->requireCapabilities($capabilities);
|
||||||
$filter->raisePolicyExceptions($this->shouldRaisePolicyExceptions());
|
$filter->raisePolicyExceptions($this->shouldRaisePolicyExceptions());
|
||||||
|
|
||||||
return $filter;
|
return $filter;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
protected function getRequiredCapabilities() {
|
||||||
|
if ($this->capabilities) {
|
||||||
|
return $this->capabilities;
|
||||||
|
}
|
||||||
|
|
||||||
|
return array(
|
||||||
|
PhabricatorPolicyCapability::CAN_VIEW,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
protected function applyPolicyFilter(array $objects, array $capabilities) {
|
||||||
|
if ($this->shouldDisablePolicyFiltering()) {
|
||||||
|
return $objects;
|
||||||
|
}
|
||||||
|
$filter = $this->getPolicyFilter();
|
||||||
|
$filter->requireCapabilities($capabilities);
|
||||||
|
return $filter->apply($objects);
|
||||||
|
}
|
||||||
|
|
||||||
protected function didRejectResult(PhabricatorPolicyInterface $object) {
|
protected function didRejectResult(PhabricatorPolicyInterface $object) {
|
||||||
$this->getPolicyFilter()->rejectObject(
|
$this->getPolicyFilter()->rejectObject(
|
||||||
$object,
|
$object,
|
||||||
|
|
Loading…
Reference in a new issue