Update "bin/policy unlock" to be more surgical, flexible, modular, and modern

Summary:
See PHI1115. Ref T13249. Currently, you can `bin/policy unlock` objects which have become inaccessible through some sort of policy mistake.

This script uses a very blunt mechanism to perform unlocks: just manually calling `setXPolicy()` and then trying to `save()` the object. Improve things a bit:

  - More surgical: allow selection of which policies you want to adjust with "--view", "--edit", and "--owner" (potentially important for some objects like Herald rules which don't have policies, and "edit-locked" tasks which basically ignore the edit policy).
  - More flexible: Instead of unlocking into "All Users" (which could be bad for stuff like Passphrase credentials, since you create a short window where anyone can access them), take a username as a parameter and set the policy to "just that user". Normally, you'd run this as `bin/policy unlock --view myself --edit myself` or similar, now.
  - More modular: We can't do "owner" transactions in a generic way, but lay the groundwork for letting applications support providing an owner reassignment mechanism.
  - More modern: Use transactions, not raw `set()` + `save()`.

This previously had some hard-coded logic around unlocking applications. I've removed it, and the new generic stuff doesn't actually work. It probably should be made to work at some point, but I believe it's exceptionally difficult to lock yourself out of applications, and you can unlock them with `bin/config set phabricator.application-settings ...` anyway so I'm not too worried about this. It's also hard to figure out the PHID of an application and no one has ever asked about this so I'd guess the reasonable use rate of `bin/policy unlock` to unlock applications in the wild may be zero.

Test Plan:
  - Used `bin/policy unlock` to unlock some objects, saw sensible transactions.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13249

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

src/__phutil_library_map__.php

@@ -2974,6 +2974,7 @@ phutil_register_library_map(array(
     'PhabricatorDebugController' => 'applications/system/controller/PhabricatorDebugController.php',
     'PhabricatorDefaultRequestExceptionHandler' => 'aphront/handler/PhabricatorDefaultRequestExceptionHandler.php',
     'PhabricatorDefaultSyntaxStyle' => 'infrastructure/syntax/PhabricatorDefaultSyntaxStyle.php',
+    'PhabricatorDefaultUnlockEngine' => 'applications/system/engine/PhabricatorDefaultUnlockEngine.php',
     'PhabricatorDestructibleCodex' => 'applications/system/codex/PhabricatorDestructibleCodex.php',
     'PhabricatorDestructibleCodexInterface' => 'applications/system/interface/PhabricatorDestructibleCodexInterface.php',
     'PhabricatorDestructibleInterface' => 'applications/system/interface/PhabricatorDestructibleInterface.php',
@@ -4701,6 +4702,7 @@ phutil_register_library_map(array(
     'PhabricatorUnitTestContentSource' => 'infrastructure/contentsource/PhabricatorUnitTestContentSource.php',
     'PhabricatorUnitsTestCase' => 'view/__tests__/PhabricatorUnitsTestCase.php',
     'PhabricatorUnknownContentSource' => 'infrastructure/contentsource/PhabricatorUnknownContentSource.php',
+    'PhabricatorUnlockEngine' => 'applications/system/engine/PhabricatorUnlockEngine.php',
     'PhabricatorUnsubscribedFromObjectEdgeType' => 'applications/transactions/edges/PhabricatorUnsubscribedFromObjectEdgeType.php',
     'PhabricatorUser' => 'applications/people/storage/PhabricatorUser.php',
     'PhabricatorUserApproveTransaction' => 'applications/people/xaction/PhabricatorUserApproveTransaction.php',
@@ -8871,6 +8873,7 @@ phutil_register_library_map(array(
     'PhabricatorDebugController' => 'PhabricatorController',
     'PhabricatorDefaultRequestExceptionHandler' => 'PhabricatorRequestExceptionHandler',
     'PhabricatorDefaultSyntaxStyle' => 'PhabricatorSyntaxStyle',
+    'PhabricatorDefaultUnlockEngine' => 'PhabricatorUnlockEngine',
     'PhabricatorDestructibleCodex' => 'Phobject',
     'PhabricatorDestructionEngine' => 'Phobject',
     'PhabricatorDestructionEngineExtension' => 'Phobject',
@@ -10881,6 +10884,7 @@ phutil_register_library_map(array(
     'PhabricatorUnitTestContentSource' => 'PhabricatorContentSource',
     'PhabricatorUnitsTestCase' => 'PhabricatorTestCase',
     'PhabricatorUnknownContentSource' => 'PhabricatorContentSource',
+    'PhabricatorUnlockEngine' => 'Phobject',
     'PhabricatorUnsubscribedFromObjectEdgeType' => 'PhabricatorEdgeType',
     'PhabricatorUser' => array(
       'PhabricatorUserDAO',

src/applications/policy/management/PhabricatorPolicyManagementUnlockWorkflow.php

@@ -8,40 +8,72 @@ final class PhabricatorPolicyManagementUnlockWorkflow
       ->setName('unlock')
       ->setSynopsis(
         pht(
-          'Unlock an object by setting its policies to allow anyone to view '.
-          'and edit it.'))
-      ->setExamples('**unlock** D123')
+          'Unlock an object which has policies that prevent it from being '.
+          'viewed or edited.'))
+      ->setExamples('**unlock** --view __user__ __object__')
       ->setArguments(
         array(
           array(
-            'name'      => 'objects',
-            'wildcard'  => true,
+            'name' => 'view',
+            'param' => 'username',
+            'help' => pht(
+              'Change the view policy of an object so that the specified '.
+              'user may view it.'),
+          ),
+          array(
+            'name' => 'edit',
+            'param' => 'username',
+            'help' => pht(
+              'Change the edit policy of an object so that the specified '.
+              'user may edit it.'),
+          ),
+          array(
+            'name' => 'owner',
+            'param' => 'username',
+            'help' => pht(
+              'Change the owner of an object to the specified user.'),
+          ),
+          array(
+            'name' => 'objects',
+            'wildcard' => true,
           ),
         ));
   }
 
   public function execute(PhutilArgumentParser $args) {
-    $console = PhutilConsole::getConsole();
     $viewer = $this->getViewer();
 
-    $obj_names = $args->getArg('objects');
-    if (!$obj_names) {
+    $object_names = $args->getArg('objects');
+    if (!$object_names) {
       throw new PhutilArgumentUsageException(
         pht('Specify the name of an object to unlock.'));
-    } else if (count($obj_names) > 1) {
+    } else if (count($object_names) > 1) {
       throw new PhutilArgumentUsageException(
         pht('Specify the name of exactly one object to unlock.'));
     }
 
+    $object_name = head($object_names);
+
     $object = id(new PhabricatorObjectQuery())
       ->setViewer($viewer)
-      ->withNames($obj_names)
+      ->withNames(array($object_name))
       ->executeOne();
-
     if (!$object) {
-      $name = head($obj_names);
       throw new PhutilArgumentUsageException(
-        pht("No such object '%s'!", $name));
+        pht(
+          'Unable to find any object with the specified name ("%s").',
+          $object_name));
+    }
+
+    $view_user = $this->loadUser($args->getArg('view'));
+    $edit_user = $this->loadUser($args->getArg('edit'));
+    $owner_user = $this->loadUser($args->getArg('owner'));
+
+    if (!$view_user && !$edit_user && !$owner_user) {
+      throw new PhutilArgumentUsageException(
+        pht(
+          'Choose which capabilities to unlock with "--view", "--edit", '.
+          'or "--owner".'));
     }
 
     $handle = id(new PhabricatorHandleQuery())
@@ -49,84 +81,73 @@ final class PhabricatorPolicyManagementUnlockWorkflow
       ->withPHIDs(array($object->getPHID()))
       ->executeOne();
 
-    if ($object instanceof PhabricatorApplication) {
-      $application = $object;
+    echo tsprintf(
+      "<bg:blue>** %s **</bg> %s\n",
+      pht('UNLOCKING'),
+      pht('Unlocking: %s', $handle->getFullName()));
 
-      $console->writeOut(
-        "%s\n",
-        pht('Unlocking Application: %s', $handle->getFullName()));
+    $engine = PhabricatorUnlockEngine::newUnlockEngineForObject($object);
 
-      // For applications, we can't unlock them in a normal way and don't want
-      // to unlock every capability, just view and edit.
-      $capabilities = array(
-        PhabricatorPolicyCapability::CAN_VIEW,
-        PhabricatorPolicyCapability::CAN_EDIT,
-      );
+    $xactions = array();
+    if ($view_user) {
+      $xactions[] = $engine->newUnlockViewTransactions($object, $view_user);
+    }
+    if ($edit_user) {
+      $xactions[] = $engine->newUnlockEditTransactions($object, $edit_user);
+    }
+    if ($owner_user) {
+      $xactions[] = $engine->newUnlockOwnerTransactions($object, $owner_user);
+    }
+    $xactions = array_mergev($xactions);
 
-      $key = 'phabricator.application-settings';
-      $config_entry = PhabricatorConfigEntry::loadConfigEntry($key);
-      $value = $config_entry->getValue();
+    $policy_application = new PhabricatorPolicyApplication();
+    $content_source = $this->newContentSource();
 
-      foreach ($capabilities as $capability) {
-        if ($application->isCapabilityEditable($capability)) {
-          unset($value[$application->getPHID()]['policy'][$capability]);
-        }
-      }
+    $editor = $object->getApplicationTransactionEditor()
+      ->setActor($viewer)
+      ->setActingAsPHID($policy_application->getPHID())
+      ->setContinueOnMissingFields(true)
+      ->setContinueOnNoEffect(true)
+      ->setContentSource($content_source);
 
-      $config_entry->setValue($value);
-      $config_entry->save();
+    $editor->applyTransactions($object, $xactions);
 
-      $console->writeOut("%s\n", pht('Saved application.'));
+    echo tsprintf(
+      "<bg:green>** %s **</bg> %s\n",
+      pht('UNLOCKED'),
+      pht('Modified object policies.'));
 
-      return 0;
+    $uri = $handle->getURI();
+    if (strlen($uri)) {
+      echo tsprintf(
+        "\n        **%s**: __%s__\n\n",
+        pht('Object URI'),
+        PhabricatorEnv::getURI($uri));
     }
 
-    $console->writeOut("%s\n", pht('Unlocking: %s', $handle->getFullName()));
+    return 0;
+  }
 
-    $updated = false;
-    foreach ($object->getCapabilities() as $capability) {
-      switch ($capability) {
-        case PhabricatorPolicyCapability::CAN_VIEW:
-          try {
-            $object->setViewPolicy(PhabricatorPolicies::POLICY_USER);
-            $console->writeOut("%s\n", pht('Unlocked view policy.'));
-            $updated = true;
-          } catch (Exception $ex) {
-            $console->writeOut("%s\n", pht('View policy is not mutable.'));
-          }
-          break;
-        case PhabricatorPolicyCapability::CAN_EDIT:
-          try {
-            $object->setEditPolicy(PhabricatorPolicies::POLICY_USER);
-            $console->writeOut("%s\n", pht('Unlocked edit policy.'));
-            $updated = true;
-          } catch (Exception $ex) {
-            $console->writeOut("%s\n", pht('Edit policy is not mutable.'));
-          }
-          break;
-        case PhabricatorPolicyCapability::CAN_JOIN:
-          try {
-            $object->setJoinPolicy(PhabricatorPolicies::POLICY_USER);
-            $console->writeOut("%s\n", pht('Unlocked join policy.'));
-            $updated = true;
-          } catch (Exception $ex) {
-            $console->writeOut("%s\n", pht('Join policy is not mutable.'));
-          }
-          break;
-      }
+  private function loadUser($username) {
+    $viewer = $this->getViewer();
+
+    if ($username === null) {
+      return null;
     }
 
-    if ($updated) {
-      $object->save();
-      $console->writeOut("%s\n", pht('Saved object.'));
-    } else {
-      $console->writeOut(
-        "%s\n",
+    $user = id(new PhabricatorPeopleQuery())
+      ->setViewer($viewer)
+      ->withUsernames(array($username))
+      ->executeOne();
+
+    if (!$user) {
+      throw new PhutilArgumentUsageException(
         pht(
-          'Object has no mutable policies. Try unlocking parent/container '.
-          'object instead. For example, to gain access to a commit, unlock '.
-          'the repository it belongs to.'));
+          'No user with username "%s" exists.',
+          $username));
     }
+
+    return $user;
   }
 
 }

src/applications/system/engine/PhabricatorDefaultUnlockEngine.php

@@ -0,0 +1,4 @@
+<?php
+
+final class PhabricatorDefaultUnlockEngine
+  extends PhabricatorUnlockEngine {}

src/applications/system/engine/PhabricatorUnlockEngine.php

@@ -0,0 +1,75 @@
+<?php
+
+abstract class PhabricatorUnlockEngine
+  extends Phobject {
+
+  final public static function newUnlockEngineForObject($object) {
+    if (!($object instanceof PhabricatorApplicationTransactionInterface)) {
+      throw new Exception(
+        pht(
+          'Object ("%s") does not implement interface "%s", so this type '.
+          'of object can not be unlocked.',
+          phutil_describe_type($object),
+          'PhabricatorApplicationTransactionInterface'));
+    }
+
+    return new PhabricatorDefaultUnlockEngine();
+  }
+
+  public function newUnlockViewTransactions($object, $user) {
+    $type_view = PhabricatorTransactions::TYPE_VIEW_POLICY;
+
+    if (!$this->canApplyTransactionType($object, $type_view)) {
+      throw new Exception(
+        pht(
+          'Object view policy can not be unlocked because this object '.
+          'does not have a mutable view policy.'));
+    }
+
+    return array(
+      $this->newTransaction($object)
+        ->setTransactionType($type_view)
+        ->setNewValue($user->getPHID()),
+    );
+  }
+
+  public function newUnlockEditTransactions($object, $user) {
+    $type_edit = PhabricatorTransactions::TYPE_EDIT_POLICY;
+
+    if (!$this->canApplyTransactionType($object, $type_edit)) {
+      throw new Exception(
+        pht(
+          'Object edit policy can not be unlocked because this object '.
+          'does not have a mutable edit policy.'));
+    }
+
+    return array(
+      $this->newTransaction($object)
+        ->setTransactionType($type_edit)
+        ->setNewValue($user->getPHID()),
+    );
+  }
+
+  public function newUnlockOwnerTransactions($object, $user) {
+    throw new Exception(
+      pht(
+        'Object owner can not be unlocked: the unlocking engine ("%s") for '.
+        'this object does not implement an owner unlocking mechanism.',
+        get_class($this)));
+  }
+
+  final protected function canApplyTransactionType($object, $type) {
+    $xaction_types = $object->getApplicationTransactionEditor()
+      ->getTransactionTypesForObject($object);
+
+    $xaction_types = array_fuse($xaction_types);
+
+    return isset($xaction_types[$type]);
+  }
+
+  final protected function newTransaction($object) {
+    return $object->getApplicationTransactionTemplate();
+  }
+
+
+}