From 650dc0cc302b4e284d2c366ca700788e5e6c0b68 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 9 Oct 2013 13:55:44 -0700
Subject: [PATCH] Remove the "create rules" Herald capability

Summary:
Ref T603. In thinking about this, I think I went mad with power in creating this capability. I can't imagine any reason to give users access to Herald but not let them create rules.

We can restore this later if some install comes up with a good reason to have it, but in the interest of keeping policies as simple as possible, I think we're better off without it. In particular, if you don't want a group of users creating rules, just lock them out of the application entirely.

The "Manage Global Rules" capability is still around, I think that one's super good.

Test Plan: Edited Herald policies, created a rule.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D7268
---
 src/__phutil_library_map__.php                |  2 --
 .../PhabricatorApplicationHerald.php          |  2 --
 .../HeraldCapabilityCreateRules.php           | 20 -------------------
 .../herald/controller/HeraldController.php    |  6 +-----
 .../herald/controller/HeraldNewController.php |  3 ---
 .../controller/HeraldRuleController.php       |  3 ---
 6 files changed, 1 insertion(+), 35 deletions(-)
 delete mode 100644 src/applications/herald/capability/HeraldCapabilityCreateRules.php

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index 28aaa63c27..deb788f089 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -625,7 +625,6 @@ phutil_register_library_map(array(
     'HeraldAction' => 'applications/herald/storage/HeraldAction.php',
     'HeraldAdapter' => 'applications/herald/adapter/HeraldAdapter.php',
     'HeraldApplyTranscript' => 'applications/herald/storage/transcript/HeraldApplyTranscript.php',
-    'HeraldCapabilityCreateRules' => 'applications/herald/capability/HeraldCapabilityCreateRules.php',
     'HeraldCapabilityManageGlobalRules' => 'applications/herald/capability/HeraldCapabilityManageGlobalRules.php',
     'HeraldCommitAdapter' => 'applications/herald/adapter/HeraldCommitAdapter.php',
     'HeraldCondition' => 'applications/herald/storage/HeraldCondition.php',
@@ -2727,7 +2726,6 @@ phutil_register_library_map(array(
     'HarbormasterScratchTable' => 'HarbormasterDAO',
     'HeraldAction' => 'HeraldDAO',
     'HeraldApplyTranscript' => 'HeraldDAO',
-    'HeraldCapabilityCreateRules' => 'PhabricatorPolicyCapability',
     'HeraldCapabilityManageGlobalRules' => 'PhabricatorPolicyCapability',
     'HeraldCommitAdapter' => 'HeraldAdapter',
     'HeraldCondition' => 'HeraldDAO',
diff --git a/src/applications/herald/application/PhabricatorApplicationHerald.php b/src/applications/herald/application/PhabricatorApplicationHerald.php
index b3b7049785..1e0da3354f 100644
--- a/src/applications/herald/application/PhabricatorApplicationHerald.php
+++ b/src/applications/herald/application/PhabricatorApplicationHerald.php
@@ -51,8 +51,6 @@ final class PhabricatorApplicationHerald extends PhabricatorApplication {
 
   protected function getCustomCapabilities() {
     return array(
-      HeraldCapabilityCreateRules::CAPABILITY => array(
-      ),
       HeraldCapabilityManageGlobalRules::CAPABILITY => array(
         'caption' => pht('Global rules can bypass access controls.'),
         'default' => PhabricatorPolicies::POLICY_ADMIN,
diff --git a/src/applications/herald/capability/HeraldCapabilityCreateRules.php b/src/applications/herald/capability/HeraldCapabilityCreateRules.php
deleted file mode 100644
index 8f8026e60d..0000000000
--- a/src/applications/herald/capability/HeraldCapabilityCreateRules.php
+++ /dev/null
@@ -1,20 +0,0 @@
-<?php
-
-final class HeraldCapabilityCreateRules
-  extends PhabricatorPolicyCapability {
-
-  const CAPABILITY = 'herald.create';
-
-  public function getCapabilityKey() {
-    return self::CAPABILITY;
-  }
-
-  public function getCapabilityName() {
-    return pht('Can Create Rules');
-  }
-
-  public function describeCapabilityRejection() {
-    return pht('You do not have permission to create new Herald rules.');
-  }
-
-}
diff --git a/src/applications/herald/controller/HeraldController.php b/src/applications/herald/controller/HeraldController.php
index ff45bc0879..a4f4eab1e0 100644
--- a/src/applications/herald/controller/HeraldController.php
+++ b/src/applications/herald/controller/HeraldController.php
@@ -23,15 +23,11 @@ abstract class HeraldController extends PhabricatorController {
   public function buildApplicationCrumbs() {
     $crumbs = parent::buildApplicationCrumbs();
 
-    $can_create = $this->hasApplicationCapability(
-      HeraldCapabilityCreateRules::CAPABILITY);
-
     $crumbs->addAction(
       id(new PHUIListItemView())
         ->setName(pht('Create Herald Rule'))
         ->setHref($this->getApplicationURI('new/'))
-        ->setIcon('create')
-        ->setDisabled(!$can_create));
+        ->setIcon('create'));
 
     return $crumbs;
   }
diff --git a/src/applications/herald/controller/HeraldNewController.php b/src/applications/herald/controller/HeraldNewController.php
index 7251ca6570..bb866d66ec 100644
--- a/src/applications/herald/controller/HeraldNewController.php
+++ b/src/applications/herald/controller/HeraldNewController.php
@@ -14,9 +14,6 @@ final class HeraldNewController extends HeraldController {
     $request = $this->getRequest();
     $user = $request->getUser();
 
-    $this->requireApplicationCapability(
-      HeraldCapabilityCreateRules::CAPABILITY);
-
     $content_type_map = HeraldAdapter::getEnabledAdapterMap($user);
     if (empty($content_type_map[$this->contentType])) {
       $this->contentType = head_key($content_type_map);
diff --git a/src/applications/herald/controller/HeraldRuleController.php b/src/applications/herald/controller/HeraldRuleController.php
index 7060391bf9..758972fdd2 100644
--- a/src/applications/herald/controller/HeraldRuleController.php
+++ b/src/applications/herald/controller/HeraldRuleController.php
@@ -47,9 +47,6 @@ final class HeraldRuleController extends HeraldController {
       $rule->setRuleType($rule_type);
 
       $cancel_uri = $this->getApplicationURI();
-
-      $this->requireApplicationCapability(
-        HeraldCapabilityCreateRules::CAPABILITY);
     }
 
     if ($rule->getRuleType() == HeraldRuleTypeConfig::RULE_TYPE_GLOBAL) {