mirror of
https://we.phorge.it/source/phorge.git
synced 2024-11-22 06:42:42 +01:00
Lift bulk tests for "many users against one object" capabilities into "PolicyFilterSet"
Summary: Ref T13602. Currently, the policy framework can not execute "test if many users can see one object" particluarly efficiently. This test must be executed more broadly to implement the changes in T13602. To avoid making this any worse than it already is, lift this block into a wrapper class that has a bulk queue + fetch API and could eventually be optimized. Test Plan: Viewed a task with an `@mention` of a user without permission to see it in the summary, saw it rendered in a disabled style. Maniphest Tasks: T13602 Differential Revision: https://secure.phabricator.com/D21546
This commit is contained in:
parent
3e38579fee
commit
f0dc065290
3 changed files with 131 additions and 11 deletions
|
@ -4270,6 +4270,7 @@ phutil_register_library_map(array(
|
|||
'PhabricatorPolicyExplainController' => 'applications/policy/controller/PhabricatorPolicyExplainController.php',
|
||||
'PhabricatorPolicyFavoritesSetting' => 'applications/settings/setting/PhabricatorPolicyFavoritesSetting.php',
|
||||
'PhabricatorPolicyFilter' => 'applications/policy/filter/PhabricatorPolicyFilter.php',
|
||||
'PhabricatorPolicyFilterSet' => 'applications/policy/filter/PhabricatorPolicyFilterSet.php',
|
||||
'PhabricatorPolicyInterface' => 'applications/policy/interface/PhabricatorPolicyInterface.php',
|
||||
'PhabricatorPolicyManagementShowWorkflow' => 'applications/policy/management/PhabricatorPolicyManagementShowWorkflow.php',
|
||||
'PhabricatorPolicyManagementUnlockWorkflow' => 'applications/policy/management/PhabricatorPolicyManagementUnlockWorkflow.php',
|
||||
|
@ -10920,6 +10921,7 @@ phutil_register_library_map(array(
|
|||
'PhabricatorPolicyExplainController' => 'PhabricatorPolicyController',
|
||||
'PhabricatorPolicyFavoritesSetting' => 'PhabricatorInternalSetting',
|
||||
'PhabricatorPolicyFilter' => 'Phobject',
|
||||
'PhabricatorPolicyFilterSet' => 'Phobject',
|
||||
'PhabricatorPolicyInterface' => 'PhabricatorPHIDInterface',
|
||||
'PhabricatorPolicyManagementShowWorkflow' => 'PhabricatorPolicyManagementWorkflow',
|
||||
'PhabricatorPolicyManagementUnlockWorkflow' => 'PhabricatorPolicyManagementWorkflow',
|
||||
|
|
|
@ -87,24 +87,37 @@ final class PhabricatorMentionRemarkupRule extends PhutilRemarkupRule {
|
|||
$engine->setTextMetadata($mentioned_key, $mentioned);
|
||||
$context_object = $engine->getConfig('contextObject');
|
||||
|
||||
$policy_object = null;
|
||||
if ($context_object) {
|
||||
if ($context_object instanceof PhabricatorPolicyInterface) {
|
||||
$policy_object = $context_object;
|
||||
}
|
||||
}
|
||||
|
||||
if ($policy_object) {
|
||||
$policy_set = new PhabricatorPolicyFilterSet();
|
||||
foreach ($actual_users as $user) {
|
||||
$policy_set->addCapability(
|
||||
$user,
|
||||
$policy_object,
|
||||
PhabricatorPolicyCapability::CAN_VIEW);
|
||||
}
|
||||
}
|
||||
|
||||
foreach ($metadata as $username => $tokens) {
|
||||
$exists = isset($actual_users[$username]);
|
||||
$user_has_no_permission = false;
|
||||
$user_can_not_view = false;
|
||||
|
||||
if ($exists) {
|
||||
$user = $actual_users[$username];
|
||||
Javelin::initBehavior('phui-hovercards');
|
||||
|
||||
// Check if the user has view access to the object she was mentioned in
|
||||
if ($context_object
|
||||
&& $context_object instanceof PhabricatorPolicyInterface) {
|
||||
if (!PhabricatorPolicyFilter::hasCapability(
|
||||
if ($policy_object) {
|
||||
$user_can_not_view = !$policy_set->hasCapability(
|
||||
$user,
|
||||
$context_object,
|
||||
PhabricatorPolicyCapability::CAN_VIEW)) {
|
||||
// User mentioned has no permission to this object
|
||||
$user_has_no_permission = true;
|
||||
}
|
||||
$policy_object,
|
||||
PhabricatorPolicyCapability::CAN_VIEW);
|
||||
}
|
||||
|
||||
$user_href = '/p/'.$user->getUserName().'/';
|
||||
|
@ -112,7 +125,7 @@ final class PhabricatorMentionRemarkupRule extends PhutilRemarkupRule {
|
|||
if ($engine->isHTMLMailMode()) {
|
||||
$user_href = PhabricatorEnv::getProductionURI($user_href);
|
||||
|
||||
if ($user_has_no_permission) {
|
||||
if ($user_can_not_view) {
|
||||
$colors = '
|
||||
border-color: #92969D;
|
||||
color: #92969D;
|
||||
|
@ -146,7 +159,7 @@ final class PhabricatorMentionRemarkupRule extends PhutilRemarkupRule {
|
|||
->setName('@'.$user->getUserName())
|
||||
->setHref($user_href);
|
||||
|
||||
if ($user_has_no_permission) {
|
||||
if ($user_can_not_view) {
|
||||
$tag->addClass('phabricator-remarkup-mention-nopermission');
|
||||
}
|
||||
|
||||
|
|
105
src/applications/policy/filter/PhabricatorPolicyFilterSet.php
Normal file
105
src/applications/policy/filter/PhabricatorPolicyFilterSet.php
Normal file
|
@ -0,0 +1,105 @@
|
|||
<?php
|
||||
|
||||
final class PhabricatorPolicyFilterSet
|
||||
extends Phobject {
|
||||
|
||||
private $users = array();
|
||||
private $objects = array();
|
||||
|
||||
private $capabilities = array();
|
||||
private $queue = array();
|
||||
private $results = array();
|
||||
|
||||
public function addCapability(
|
||||
PhabricatorUser $user,
|
||||
PhabricatorPolicyInterface $object,
|
||||
$capability) {
|
||||
|
||||
$user_key = $this->getUserKey($user);
|
||||
$this->users[$user_key] = $user;
|
||||
|
||||
$object_key = $this->getObjectKey($object);
|
||||
$this->objects[$object_key] = $object;
|
||||
|
||||
if (!isset($this->capabilities[$capability][$user_key][$object_key])) {
|
||||
$this->capabilities[$capability][$user_key][$object_key] = true;
|
||||
$this->queue[$capability][$user_key][$object_key] = true;
|
||||
}
|
||||
|
||||
return $this;
|
||||
}
|
||||
|
||||
public function hasCapability(
|
||||
PhabricatorUser $user,
|
||||
PhabricatorPolicyInterface $object,
|
||||
$capability) {
|
||||
|
||||
$user_key = $this->getUserKey($user);
|
||||
$this->users[$user_key] = $user;
|
||||
|
||||
$object_key = $this->getObjectKey($object);
|
||||
$this->objects[$object_key] = $object;
|
||||
|
||||
if (!isset($this->capabilities[$capability][$user_key][$object_key])) {
|
||||
throw new Exception(
|
||||
pht(
|
||||
'Capability "%s" for user "%s" on object "%s" is being resolved, '.
|
||||
'but was never queued with "addCapability()".',
|
||||
$capability,
|
||||
$user_key,
|
||||
$object_key));
|
||||
}
|
||||
|
||||
if (!isset($this->results[$capability][$user_key][$object_key])) {
|
||||
$this->resolveCapabilities();
|
||||
}
|
||||
|
||||
return $this->results[$capability][$user_key][$object_key];
|
||||
}
|
||||
|
||||
private function getUserKey(PhabricatorUser $user) {
|
||||
return $user->getCacheFragment();
|
||||
}
|
||||
|
||||
private function getObjectKey(PhabricatorPolicyInterface $object) {
|
||||
$object_phid = $object->getPHID();
|
||||
|
||||
if (!$object_phid) {
|
||||
throw new Exception(
|
||||
pht(
|
||||
'Unable to perform capability tests on an object (of class "%s") '.
|
||||
'with no PHID.',
|
||||
get_class($object)));
|
||||
}
|
||||
|
||||
return $object_phid;
|
||||
}
|
||||
|
||||
private function resolveCapabilities() {
|
||||
|
||||
// This class is primarily used to test if a list of users (like
|
||||
// subscribers) can see a single object. It is not structured in a way
|
||||
// that makes this particularly efficient, and performance would probably
|
||||
// be improved if filtering supported this use case more narrowly.
|
||||
|
||||
foreach ($this->queue as $capability => $user_map) {
|
||||
foreach ($user_map as $user_key => $object_map) {
|
||||
$user = $this->users[$user_key];
|
||||
$objects = array_select_keys($this->objects, array_keys($object_map));
|
||||
|
||||
$filter = id(new PhabricatorPolicyFilter())
|
||||
->setViewer($user)
|
||||
->requireCapabilities(array($capability));
|
||||
$results = $filter->apply($objects);
|
||||
|
||||
foreach ($object_map as $object_key => $object) {
|
||||
$has_capability = (bool)isset($results[$object_key]);
|
||||
$this->results[$capability][$user_key][$object_key] = $has_capability;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
$this->queue = array();
|
||||
}
|
||||
|
||||
}
|
Loading…
Reference in a new issue