From 83c99be4234862069db660c4076b0d67ef38b685 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Mon, 21 Oct 2013 16:58:21 -0700
Subject: [PATCH] Clean up supplemental capabilitiy checks in transaction edits

Summary:
We have this commented-out chunk of code now which was originally buggy and is now just nonfunctional.

For now, the core edit types don't always require CAN_EDIT (e.g., subscribe, comment, add edges), except for editing the edit policy itself, which always does. Add a supplemental capability check there and let everything else go through with CAN_VIEW. We can buff the policy checks on application editors over time, they all require appropriate capabilities to get to in the first place anyway.

Test Plan: Created and edited some tasks without getting overwhelmed with policy exceptions.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7351
---
 ...habricatorApplicationTransactionEditor.php | 28 ++++++++++++-------
 1 file changed, 18 insertions(+), 10 deletions(-)

diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
index f1bb0db57c..15241aff2d 100644
--- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
+++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
@@ -697,19 +697,27 @@ abstract class PhabricatorApplicationTransactionEditor
       $object,
       PhabricatorPolicyCapability::CAN_VIEW);
 
-    // TODO: This should be "$object", not "$xaction", but probably breaks a
-    // lot of stuff if fixed -- you don't need to be able to edit in order to
-    // comment. Instead, transactions should specify the capabilities they
-    // require.
+    foreach ($xactions as $xaction) {
+      $this->requireCapabilities($object, $xaction);
+    }
+  }
 
-    /*
+  protected function requireCapabilities(
+    PhabricatorLiskDAO $object,
+    PhabricatorApplicationTransaction $xaction) {
 
-    PhabricatorPolicyFilter::requireCapability(
-      $actor,
-      $xaction,
-      PhabricatorPolicyCapability::CAN_EDIT);
+    switch ($xaction->getTransactionType()) {
+      case PhabricatorTransactions::TYPE_EDIT_POLICY:
+        // You must have the edit capability to alter the edit policy of an
+        // object. For other default transaction types, we don't enforce
+        // anything for the moment.
 
-    */
+        PhabricatorPolicyFilter::requireCapability(
+          $this->requireActor(),
+          $object,
+          PhabricatorPolicyCapability::CAN_EDIT);
+        break;
+    }
   }
 
   private function buildMentionTransaction(