Apply application visibility checks during normal object filtering

Summary:
Fixes T9058. Normally, "Query" classes apply an application check and just don't load anything if it fails.

However, in some cases (like email recipient filtering) we run policy checks without having run a Query check first. In that case, one user (the actor) loads the object, then we filter it against other users (the recipeints).

Explicitly apply the application check during normal filtering.

Test Plan: Added a failing test case and made it pass.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9058

Differential Revision: https://secure.phabricator.com/D17127

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) {

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();
+  }
+
 }

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();