From 058952e72ecf032735ea019e33014c9981229023 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Tue, 14 Aug 2018 08:50:32 -0700
Subject: [PATCH] Add a "Can Disable Users" capability to the "People"
 application

Summary:
Depends on D19605. Ref T13189. See PHI642. This adds a separate "Can Disable Users" capability, and makes the underlying transaction use it.

This doesn't actually let you weaken the permission, since all pathways need more permissions:

  - `user.edit` needs CAN_EDIT.
  - `user.disable/enable` need admin.
  - Web UI workflow needs admin.

Upcoming changes will update these pathways.

Without additional changes, this does let you //strengthen// the permission.

This also fixes the inability to disable non-bot users via the web UI.

Test Plan:
  - Set permission to "No One", tried to disable users. Got a tailored policy error.
  - Set permission to "All Users", disabled/enabled a non-bot user.

Reviewers: amckinley

Maniphest Tasks: T13189

Differential Revision: https://secure.phabricator.com/D19606
---
 src/__phutil_library_map__.php                   |  2 ++
 .../application/PhabricatorPeopleApplication.php |  3 +++
 .../capability/PeopleDisableUsersCapability.php  | 16 ++++++++++++++++
 .../PhabricatorUserDisableTransaction.php        | 14 ++++++++++++++
 4 files changed, 35 insertions(+)
 create mode 100644 src/applications/people/capability/PeopleDisableUsersCapability.php

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index af6fa96ba3..05ef3c96fe 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -2042,6 +2042,7 @@ phutil_register_library_map(array(
     'PasteSearchConduitAPIMethod' => 'applications/paste/conduit/PasteSearchConduitAPIMethod.php',
     'PeopleBrowseUserDirectoryCapability' => 'applications/people/capability/PeopleBrowseUserDirectoryCapability.php',
     'PeopleCreateUsersCapability' => 'applications/people/capability/PeopleCreateUsersCapability.php',
+    'PeopleDisableUsersCapability' => 'applications/people/capability/PeopleDisableUsersCapability.php',
     'PeopleHovercardEngineExtension' => 'applications/people/engineextension/PeopleHovercardEngineExtension.php',
     'PeopleMainMenuBarExtension' => 'applications/people/engineextension/PeopleMainMenuBarExtension.php',
     'PeopleUserLogGarbageCollector' => 'applications/people/garbagecollector/PeopleUserLogGarbageCollector.php',
@@ -7592,6 +7593,7 @@ phutil_register_library_map(array(
     'PasteSearchConduitAPIMethod' => 'PhabricatorSearchEngineAPIMethod',
     'PeopleBrowseUserDirectoryCapability' => 'PhabricatorPolicyCapability',
     'PeopleCreateUsersCapability' => 'PhabricatorPolicyCapability',
+    'PeopleDisableUsersCapability' => 'PhabricatorPolicyCapability',
     'PeopleHovercardEngineExtension' => 'PhabricatorHovercardEngineExtension',
     'PeopleMainMenuBarExtension' => 'PhabricatorMainMenuBarExtension',
     'PeopleUserLogGarbageCollector' => 'PhabricatorGarbageCollector',
diff --git a/src/applications/people/application/PhabricatorPeopleApplication.php b/src/applications/people/application/PhabricatorPeopleApplication.php
index 6322b29b24..9238d8da3b 100644
--- a/src/applications/people/application/PhabricatorPeopleApplication.php
+++ b/src/applications/people/application/PhabricatorPeopleApplication.php
@@ -97,6 +97,9 @@ final class PhabricatorPeopleApplication extends PhabricatorApplication {
       PeopleCreateUsersCapability::CAPABILITY => array(
         'default' => PhabricatorPolicies::POLICY_ADMIN,
       ),
+      PeopleDisableUsersCapability::CAPABILITY => array(
+        'default' => PhabricatorPolicies::POLICY_ADMIN,
+      ),
       PeopleBrowseUserDirectoryCapability::CAPABILITY => array(),
     );
   }
diff --git a/src/applications/people/capability/PeopleDisableUsersCapability.php b/src/applications/people/capability/PeopleDisableUsersCapability.php
new file mode 100644
index 0000000000..bb58ed2e76
--- /dev/null
+++ b/src/applications/people/capability/PeopleDisableUsersCapability.php
@@ -0,0 +1,16 @@
+<?php
+
+final class PeopleDisableUsersCapability
+  extends PhabricatorPolicyCapability {
+
+  const CAPABILITY = 'people.disable.users';
+
+  public function getCapabilityName() {
+    return pht('Can Disable Users');
+  }
+
+  public function describeCapabilityRejection() {
+    return pht('You do not have permission to disable or enable users.');
+  }
+
+}
diff --git a/src/applications/people/xaction/PhabricatorUserDisableTransaction.php b/src/applications/people/xaction/PhabricatorUserDisableTransaction.php
index 0ea6ef36c1..ab399a28e0 100644
--- a/src/applications/people/xaction/PhabricatorUserDisableTransaction.php
+++ b/src/applications/people/xaction/PhabricatorUserDisableTransaction.php
@@ -60,6 +60,10 @@ final class PhabricatorUserDisableTransaction
         continue;
       }
 
+      // You must have the "Can Disable Users" permission to disable a user.
+      $this->requireApplicationCapability(
+        PeopleDisableUsersCapability::CAPABILITY);
+
       if ($this->getActingAsPHID() === $object->getPHID()) {
         $errors[] = $this->newInvalidError(
           pht('You can not enable or disable your own account.'));
@@ -69,4 +73,14 @@ final class PhabricatorUserDisableTransaction
     return $errors;
   }
 
+  public function getRequiredCapabilities(
+    $object,
+    PhabricatorApplicationTransaction $xaction) {
+
+    // You do not need to be able to edit users to disable them. Instead, this
+    // requirement is replaced with a requirement that you have the "Can
+    // Disable Users" permission.
+
+    return null;
+  }
 }