Page MenuHomePhabricator
Files F14048993

D17127.diff
No OneTemporary
Actions

D17127.diff
View Options

diff --git a/src/applications/base/PhabricatorApplication.php b/src/applications/base/PhabricatorApplication.php
--- a/src/applications/base/PhabricatorApplication.php
+++ b/src/applications/base/PhabricatorApplication.php
@@ -429,8 +429,8 @@
}
$cache = PhabricatorCaches::getRequestCache();
- $viewer_phid = $viewer->getPHID();
- $key = 'app.'.$class.'.installed.'.$viewer_phid;
+ $viewer_fragment = $viewer->getCacheFragment();
+ $key = 'app.'.$class.'.installed.'.$viewer_fragment;
$result = $cache->getKey($key);
if ($result === null) {
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
@@ -123,6 +123,12 @@
return $objects;
}
+ // Before doing any actual object checks, make sure the viewer can see
+ // the applications that these objects belong to. This is normally enforced
+ // in the Query layer before we reach object filtering, but execution
+ // sometimes reaches policy filtering without running application checks.
+ $objects = $this->applyApplicationChecks($objects);
+
$filtered = array();
$viewer_phid = $viewer->getPHID();
@@ -864,4 +870,49 @@
throw $exception;
}
+ private function applyApplicationChecks(array $objects) {
+ $viewer = $this->viewer;
+
+ foreach ($objects as $key => $object) {
+ $phid = $object->getPHID();
+ if (!$phid) {
+ continue;
+ }
+
+ $application_class = $this->getApplicationForPHID($phid);
+ if ($application_class === null) {
+ continue;
+ }
+
+ $can_see = PhabricatorApplication::isClassInstalledForViewer(
+ $application_class,
+ $viewer);
+ if ($can_see) {
+ continue;
+ }
+
+ unset($objects[$key]);
+
+ $application = newv($application_class, array());
+ $this->rejectObject(
+ $application,
+ $application->getPolicy(PhabricatorPolicyCapability::CAN_VIEW),
+ PhabricatorPolicyCapability::CAN_VIEW);
+ }
+
+ return $objects;
+ }
+
+ private function getApplicationForPHID($phid) {
+ $phid_type = phid_get_type($phid);
+
+ $type_objects = PhabricatorPHIDType::getTypes(array($phid_type));
+ $type_object = idx($type_objects, $phid_type);
+ if (!$type_object) {
+ return null;
+ }
+
+ return $type_object->getPHIDTypeApplicationClass();
+ }
+
}
diff --git a/src/applications/project/__tests__/PhabricatorProjectCoreTestCase.php b/src/applications/project/__tests__/PhabricatorProjectCoreTestCase.php
--- a/src/applications/project/__tests__/PhabricatorProjectCoreTestCase.php
+++ b/src/applications/project/__tests__/PhabricatorProjectCoreTestCase.php
@@ -39,6 +39,46 @@
$this->assertFalse((bool)$this->refreshProject($proj, $user2));
}
+ public function testApplicationPolicy() {
+ $user = $this->createUser()
+ ->save();
+
+ $proj = $this->createProject($user);
+
+ $this->assertTrue(
+ PhabricatorPolicyFilter::hasCapability(
+ $user,
+ $proj,
+ PhabricatorPolicyCapability::CAN_VIEW));
+
+ // Change the "Can Use Application" policy for Projecs to "No One". This
+ // should cause filtering checks to fail even when they are executed
+ // directly rather than via a Query.
+ $env = PhabricatorEnv::beginScopedEnv();
+ $env->overrideEnvConfig(
+ 'phabricator.application-settings',
+ array(
+ 'PHID-APPS-PhabricatorProjectApplication' => array(
+ 'policy' => array(
+ 'view' => PhabricatorPolicies::POLICY_NOONE,
+ ),
+ ),
+ ));
+
+ // Application visibility is cached because it does not normally change
+ // over the course of a single request. Drop the cache so the next filter
+ // test uses the new visibility.
+ PhabricatorCaches::destroyRequestCache();
+
+ $this->assertFalse(
+ PhabricatorPolicyFilter::hasCapability(
+ $user,
+ $proj,
+ PhabricatorPolicyCapability::CAN_VIEW));
+
+ unset($env);
+ }
+
public function testIsViewerMemberOrWatcher() {
$user1 = $this->createUser()
->save();

File Metadata

Mime Type
text/plain
Expires
Fri, Nov 15, 10:36 AM (3 d, 13 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6717923
Default Alt Text
D17127.diff (4 KB)

Event Timeline

Phabricator · Built by Phacility