diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -4270,6 +4270,7 @@ '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 @@ 'PhabricatorPolicyExplainController' => 'PhabricatorPolicyController', 'PhabricatorPolicyFavoritesSetting' => 'PhabricatorInternalSetting', 'PhabricatorPolicyFilter' => 'Phobject', + 'PhabricatorPolicyFilterSet' => 'Phobject', 'PhabricatorPolicyInterface' => 'PhabricatorPHIDInterface', 'PhabricatorPolicyManagementShowWorkflow' => 'PhabricatorPolicyManagementWorkflow', 'PhabricatorPolicyManagementUnlockWorkflow' => 'PhabricatorPolicyManagementWorkflow', diff --git a/src/applications/people/markup/PhabricatorMentionRemarkupRule.php b/src/applications/people/markup/PhabricatorMentionRemarkupRule.php --- a/src/applications/people/markup/PhabricatorMentionRemarkupRule.php +++ b/src/applications/people/markup/PhabricatorMentionRemarkupRule.php @@ -87,24 +87,37 @@ $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 @@ 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 @@ ->setName('@'.$user->getUserName()) ->setHref($user_href); - if ($user_has_no_permission) { + if ($user_can_not_view) { $tag->addClass('phabricator-remarkup-mention-nopermission'); } diff --git a/src/applications/policy/filter/PhabricatorPolicyFilterSet.php b/src/applications/policy/filter/PhabricatorPolicyFilterSet.php new file mode 100644 --- /dev/null +++ b/src/applications/policy/filter/PhabricatorPolicyFilterSet.php @@ -0,0 +1,105 @@ +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(); + } + +}