Improve PolicyFilter and PolicyQuery

Summary:
  - Allow PolicyQuery to require specific sets of capabilities other than "CAN_VIEW", like edit, etc. The default set is "view".
  - Add some convenience methods to PolicyFilter to test for capabilities.

Test Plan: Viewed pastes, projects, etc. Used other stuff in future diff.

Reviewers: vrana, btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T603

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

src/applications/policy/filter/PhabricatorPolicyFilter.php

@@ -20,16 +20,52 @@ final class PhabricatorPolicyFilter {
 
   private $viewer;
   private $objects;
-  private $capability;
+  private $capabilities;
   private $raisePolicyExceptions;
 
+  public static function mustRetainCapability(
+    PhabricatorUser $user,
+    PhabricatorPolicyInterface $object,
+    $capability) {
+
+    if (!self::hasCapability($user, $object, $capability)) {
+      throw new Exception(
+        "You can not make that edit, because it would remove your ability ".
+        "to '{$capability}' the object.");
+    }
+  }
+
+  public static function requireCapability(
+    PhabricatorUser $user,
+    PhabricatorPolicyInterface $object,
+    $capability) {
+    $filter = new PhabricatorPolicyFilter();
+    $filter->setViewer($user);
+    $filter->requireCapabilities(array($capability));
+    $filter->raisePolicyExceptions(true);
+    $filter->apply(array($object));
+  }
+
+  public static function hasCapability(
+    PhabricatorUser $user,
+    PhabricatorPolicyInterface $object,
+    $capability) {
+
+    $filter = new PhabricatorPolicyFilter();
+    $filter->setViewer($user);
+    $filter->requireCapabilities(array($capability));
+    $result = $filter->apply(array($object));
+
+    return (count($result) == 1);
+  }
+
   public function setViewer(PhabricatorUser $user) {
     $this->viewer = $user;
     return $this;
   }
 
-  public function setCapability($capability) {
+  public function requireCapabilities(array $capabilities) {
-    $this->capability = $capability;
+    $this->capabilities = $capabilities;
     return $this;
   }
 
@@ -41,91 +77,121 @@ final class PhabricatorPolicyFilter {
   public function apply(array $objects) {
     assert_instances_of($objects, 'PhabricatorPolicyInterface');
 
-    $viewer     = $this->viewer;
+    $viewer       = $this->viewer;
-    $capability = $this->capability;
+    $capabilities = $this->capabilities;
 
-    if (!$viewer || !$capability) {
+    if (!$viewer || !$capabilities) {
       throw new Exception(
-        'Call setViewer() and setCapability() before apply()!');
+        'Call setViewer() and requireCapabilities() before apply()!');
     }
 
     $filtered = array();
 
     foreach ($objects as $key => $object) {
       $object_capabilities = $object->getCapabilities();
+      foreach ($capabilities as $capability) {
+        if (!in_array($capability, $object_capabilities)) {
+          throw new Exception(
+            "Testing for capability '{$capability}' on an object which does ".
+            "not have that capability!");
+        }
 
-      if (!in_array($capability, $object_capabilities)) {
+        if (!$this->checkCapability($object, $capability)) {
-        throw new Exception(
+          // If we're missing any capability, move on to the next object.
-          "Testing for capability '{$capability}' on an object which does not ".
+          continue 2;
-          "have that capability!");
-      }
-
-      if ($object->hasAutomaticCapability($capability, $this->viewer)) {
-        $filtered[$key] = $object;
-        continue;
-      }
-
-      $policy = $object->getPolicy($capability);
-
-      // If the object is set to "public" but that policy is disabled for this
-      // install, restrict the policy to "user".
-      if ($policy == PhabricatorPolicies::POLICY_PUBLIC) {
-        if (!PhabricatorEnv::getEnvConfig('policy.allow-public')) {
-          $policy = PhabricatorPolicies::POLICY_USER;
         }
       }
 
-      switch ($policy) {
+      // If we make it here, we have all of the required capabilities.
-        case PhabricatorPolicies::POLICY_PUBLIC:
+      $filtered[$key] = $object;
-          $filtered[$key] = $object;
-          break;
-        case PhabricatorPolicies::POLICY_USER:
-          if ($viewer->getPHID()) {
-            $filtered[$key] = $object;
-          } else {
-            $this->rejectObject($object, $policy);
-          }
-          break;
-        case PhabricatorPolicies::POLICY_ADMIN:
-          if ($viewer->getIsAdmin()) {
-            $filtered[$key] = $object;
-          } else {
-            $this->rejectObject($object, $policy);
-          }
-          break;
-        case PhabricatorPolicies::POLICY_NOONE:
-          $this->rejectObject($object, $policy);
-          break;
-        default:
-          throw new Exception("Object has unknown policy '{$policy}'!");
-      }
     }
 
     return $filtered;
   }
 
-  private function rejectObject($object, $policy) {
+  private function checkCapability(
+    PhabricatorPolicyInterface $object,
+    $capability) {
+
+    $policy = $object->getPolicy($capability);
+
+    if (!$policy) {
+      // TODO: Formalize this somehow?
+      $policy = PhabricatorPolicies::POLICY_USER;
+    }
+
+    if ($policy == PhabricatorPolicies::POLICY_PUBLIC) {
+      // If the object is set to "public" but that policy is disabled for this
+      // install, restrict the policy to "user".
+      if (!PhabricatorEnv::getEnvConfig('policy.allow-public')) {
+        $policy = PhabricatorPolicies::POLICY_USER;
+      }
+
+      // If the object is set to "public" but the capability is anything other
+      // than "view", restrict the policy to "user".
+      if ($capability != PhabricatorPolicyCapability::CAN_VIEW) {
+        $policy = PhabricatorPolicies::POLICY_USER;
+      }
+    }
+
+    $viewer = $this->viewer;
+
+    if ($object->hasAutomaticCapability($capability, $viewer)) {
+      return true;
+    }
+
+    switch ($policy) {
+      case PhabricatorPolicies::POLICY_PUBLIC:
+        return true;
+      case PhabricatorPolicies::POLICY_USER:
+        if ($viewer->getPHID()) {
+          return true;
+        } else {
+          $this->rejectObject($object, $policy, $capability);
+        }
+        break;
+      case PhabricatorPolicies::POLICY_ADMIN:
+        if ($viewer->getIsAdmin()) {
+          return true;
+        } else {
+          $this->rejectObject($object, $policy, $capability);
+        }
+        break;
+      case PhabricatorPolicies::POLICY_NOONE:
+        $this->rejectObject($object, $policy, $capability);
+        break;
+      default:
+        throw new Exception("Object has unknown policy '{$policy}'!");
+    }
+
+    return false;
+  }
+
+  private function rejectObject($object, $policy, $capability) {
     if (!$this->raisePolicyExceptions) {
       return;
     }
 
-    $message = "You do not have permission to view this object.";
+    // TODO: clean this up
+    $verb = $capability;
+
+    $message = "You do not have permission to {$verb} this object.";
 
     switch ($policy) {
       case PhabricatorPolicies::POLICY_PUBLIC:
-        $who = "This is curious, since anyone can view the object.";
+        $who = "This is curious, since anyone can {$verb} the object.";
         break;
       case PhabricatorPolicies::POLICY_USER:
-        $who = "To view this object, you must be logged in.";
+        $who = "To {$verb} this object, you must be logged in.";
         break;
       case PhabricatorPolicies::POLICY_ADMIN:
-        $who = "To view this object, you must be an administrator.";
+        $who = "To {$verb} this object, you must be an administrator.";
         break;
       case PhabricatorPolicies::POLICY_NOONE:
-        $who = "No one can view this object.";
+        $who = "No one can {$verb} this object.";
         break;
       default:
-        $who = "It is unclear who can view this object.";
+        $who = "It is unclear who can {$verb} this object.";
         break;
     }
 

src/infrastructure/query/policy/PhabricatorPolicyQuery.php

@@ -46,6 +46,7 @@ abstract class PhabricatorPolicyQuery extends PhabricatorOffsetPagedQuery {
   private $viewer;
   private $raisePolicyExceptions;
   private $rawResultLimit;
+  private $capabilities;
 
 
 /* -(  Query Configuration  )------------------------------------------------ */
@@ -77,6 +78,15 @@ abstract class PhabricatorPolicyQuery extends PhabricatorOffsetPagedQuery {
   }
 
 
+  /**
+   * @task config
+   */
+  final public function requireCapabilities(array $capabilities) {
+    $this->capabilities = $capabilities;
+    return $this;
+  }
+
+
 /* -(  Query Execution  )---------------------------------------------------- */
 
 
@@ -145,8 +155,15 @@ abstract class PhabricatorPolicyQuery extends PhabricatorOffsetPagedQuery {
 
     $filter = new PhabricatorPolicyFilter();
     $filter->setViewer($this->viewer);
-    $filter->setCapability(PhabricatorPolicyCapability::CAN_VIEW);
 
+    if (!$this->capabilities) {
+      $capabilities = array(
+        PhabricatorPolicyCapability::CAN_VIEW,
+      );
+    } else {
+      $capabilities = $this->capabilities;
+    }
+    $filter->requireCapabilities($capabilities);
     $filter->raisePolicyExceptions($this->raisePolicyExceptions);
 
     $offset = (int)$this->getOffset();