diff --git a/src/applications/base/PhabricatorApplication.php b/src/applications/base/PhabricatorApplication.php index 23a759d2aa..95045623d0 100644 --- a/src/applications/base/PhabricatorApplication.php +++ b/src/applications/base/PhabricatorApplication.php @@ -429,8 +429,8 @@ abstract class PhabricatorApplication } $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 index 1466d4e569..855149f6b3 100644 --- a/src/applications/policy/filter/PhabricatorPolicyFilter.php +++ b/src/applications/policy/filter/PhabricatorPolicyFilter.php @@ -123,6 +123,12 @@ final class PhabricatorPolicyFilter extends Phobject { 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 @@ final class PhabricatorPolicyFilter extends Phobject { 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 index b6d3aeebcf..cb977dd681 100644 --- a/src/applications/project/__tests__/PhabricatorProjectCoreTestCase.php +++ b/src/applications/project/__tests__/PhabricatorProjectCoreTestCase.php @@ -39,6 +39,46 @@ final class PhabricatorProjectCoreTestCase extends PhabricatorTestCase { $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();