From f32e0e9330bd5537ea7802ebc3a404e79ce051e4 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Mon, 22 Jul 2013 12:20:31 -0700
Subject: [PATCH] Provide a callback for Queries to filter objects from
 alternate result sets

Summary: Ref T2715. `PhabricatorObjectQuery` can theoretically bypass policies on its side-channel result set. This can't actually happen in practice because all the loading mechanisms are filtered, but provide a general way to implement side channel results safely.

Test Plan: Loaded some pages; see next diff.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2715

Differential Revision: https://secure.phabricator.com/D6514
---
 .../phid/query/PhabricatorObjectQuery.php     |  8 +++++
 .../policy/PhabricatorPolicyAwareQuery.php    | 32 +++++++++++++++++--
 2 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/src/applications/phid/query/PhabricatorObjectQuery.php b/src/applications/phid/query/PhabricatorObjectQuery.php
index d1a53a2d7e..d3ed12584c 100644
--- a/src/applications/phid/query/PhabricatorObjectQuery.php
+++ b/src/applications/phid/query/PhabricatorObjectQuery.php
@@ -110,4 +110,12 @@ final class PhabricatorObjectQuery
     return $results;
   }
 
+  protected function didFilterResults(array $filtered) {
+    foreach ($this->namedResults as $name => $result) {
+      if (isset($filtered[$result->getPHID()])) {
+        unset($this->namedResults[$name]);
+      }
+    }
+  }
+
 }
diff --git a/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php b/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php
index f544401ddc..79a76d2311 100644
--- a/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php
+++ b/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php
@@ -177,12 +177,22 @@ abstract class PhabricatorPolicyAwareQuery extends PhabricatorOffsetPagedQuery {
       }
 
       if ($page) {
-        $visible = $this->willFilterPage($page);
+        $maybe_visible = $this->willFilterPage($page);
       } else {
-        $visible = array();
+        $maybe_visible = array();
       }
 
-      $visible = $filter->apply($visible);
+      $visible = $filter->apply($maybe_visible);
+
+      $removed = array();
+      foreach ($maybe_visible as $key => $object) {
+        if (empty($visible[$key])) {
+          $removed[$key] = $object;
+        }
+      }
+
+      $this->didFilterResults($removed);
+
       foreach ($visible as $key => $result) {
         ++$count;
 
@@ -289,6 +299,22 @@ abstract class PhabricatorPolicyAwareQuery extends PhabricatorOffsetPagedQuery {
   }
 
 
+  /**
+   * Hook for removing filtered results from alternate result sets. This
+   * hook will be called with any objects which were returned by the query but
+   * filtered for policy reasons. The query should remove them from any cached
+   * or partial result sets.
+   *
+   * @param list<wild>  List of objects that should not be returned by alternate
+   *                    result mechanisms.
+   * @return void
+   * @task policyimpl
+   */
+  protected function didFilterResults(array $results) {
+    return;
+  }
+
+
   /**
    * Hook for applying final adjustments before results are returned. This is
    * used by @{class:PhabricatorCursorPagedPolicyAwareQuery} to reverse results