Simplify locking of Almanac cluster services

Summary:
Fixes T6741. Ref T10246. Broadly, we want to protect Almanac cluster services:

  - Today, against users in the Phacility cluster accidentally breaking their own instances.
  - In the future, against attackers compromising administrative accounts and adding a new "cluster database" which points at hardware they control.

The way this works right now is really complicated: there's a global "can create cluster services" setting, and then separate per-service and per-device locks.

Instead, change "Can Create Cluster Services" into "Can Manage Cluster Services". Require this permission (in addition to normal permissions) to edit or create any cluster service.

This permission can be locked to "No One" via config (as we do in the Phacility cluster) so we only need this one simple setting.

There's also zero reason to individually lock //some// of the cluster services.

Also improve extended policy errors.

The UI here is still a little heavy-handed, but should be good enough for the moment.

Test Plan:
  - Ran migrations.
  - Verified that cluster services and bindings reported that they belonged to the cluster.
  - Edited a cluster binding.
  - Verified that the bound device was marked as a cluster device
  - Moved a cluster binding, verified the old device was unmarked as a cluster device.
  - Tried to edit a cluster device as an unprivileged user, got a sensible error.

{F1126552}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T6741, T10246

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

resources/sql/autopatches/20160223.almanac.1.bound.sql

@@ -0,0 +1,2 @@
+ALTER TABLE {$NAMESPACE}_almanac.almanac_device
+  ADD isBoundToClusterService BOOL NOT NULL;

resources/sql/autopatches/20160223.almanac.2.lockbind.sql

@@ -0,0 +1,2 @@
+UPDATE {$NAMESPACE}_almanac.almanac_device
+  SET isBoundToClusterService = isLocked;

resources/sql/autopatches/20160223.almanac.3.devicelock.sql

@@ -0,0 +1,2 @@
+ALTER TABLE {$NAMESPACE}_almanac.almanac_device
+  DROP isLocked;

resources/sql/autopatches/20160223.almanac.4.servicelock.sql

@@ -0,0 +1,2 @@
+ALTER TABLE {$NAMESPACE}_almanac.almanac_service
+  DROP isLocked;

src/__phutil_library_map__.php

@@ -27,7 +27,6 @@ phutil_register_library_map(array(
     'AlmanacConduitAPIMethod' => 'applications/almanac/conduit/AlmanacConduitAPIMethod.php',
     'AlmanacConsoleController' => 'applications/almanac/controller/AlmanacConsoleController.php',
     'AlmanacController' => 'applications/almanac/controller/AlmanacController.php',
-    'AlmanacCreateClusterServicesCapability' => 'applications/almanac/capability/AlmanacCreateClusterServicesCapability.php',
     'AlmanacCreateDevicesCapability' => 'applications/almanac/capability/AlmanacCreateDevicesCapability.php',
     'AlmanacCreateNamespacesCapability' => 'applications/almanac/capability/AlmanacCreateNamespacesCapability.php',
     'AlmanacCreateNetworksCapability' => 'applications/almanac/capability/AlmanacCreateNetworksCapability.php',
@@ -57,10 +56,9 @@ phutil_register_library_map(array(
     'AlmanacInterfaceQuery' => 'applications/almanac/query/AlmanacInterfaceQuery.php',
     'AlmanacInterfaceTableView' => 'applications/almanac/view/AlmanacInterfaceTableView.php',
     'AlmanacKeys' => 'applications/almanac/util/AlmanacKeys.php',
-    'AlmanacManagementLockWorkflow' => 'applications/almanac/management/AlmanacManagementLockWorkflow.php',
+    'AlmanacManageClusterServicesCapability' => 'applications/almanac/capability/AlmanacManageClusterServicesCapability.php',
     'AlmanacManagementRegisterWorkflow' => 'applications/almanac/management/AlmanacManagementRegisterWorkflow.php',
     'AlmanacManagementTrustKeyWorkflow' => 'applications/almanac/management/AlmanacManagementTrustKeyWorkflow.php',
-    'AlmanacManagementUnlockWorkflow' => 'applications/almanac/management/AlmanacManagementUnlockWorkflow.php',
     'AlmanacManagementUntrustKeyWorkflow' => 'applications/almanac/management/AlmanacManagementUntrustKeyWorkflow.php',
     'AlmanacManagementWorkflow' => 'applications/almanac/management/AlmanacManagementWorkflow.php',
     'AlmanacNames' => 'applications/almanac/util/AlmanacNames.php',
@@ -3996,6 +3994,7 @@ phutil_register_library_map(array(
       'PhabricatorApplicationTransactionInterface',
       'AlmanacPropertyInterface',
       'PhabricatorDestructibleInterface',
+      'PhabricatorExtendedPolicyInterface',
     ),
     'AlmanacBindingEditController' => 'AlmanacServiceController',
     'AlmanacBindingEditor' => 'AlmanacEditor',
@@ -4013,7 +4012,6 @@ phutil_register_library_map(array(
     'AlmanacConduitAPIMethod' => 'ConduitAPIMethod',
     'AlmanacConsoleController' => 'AlmanacController',
     'AlmanacController' => 'PhabricatorController',
-    'AlmanacCreateClusterServicesCapability' => 'PhabricatorPolicyCapability',
     'AlmanacCreateDevicesCapability' => 'PhabricatorPolicyCapability',
     'AlmanacCreateNamespacesCapability' => 'PhabricatorPolicyCapability',
     'AlmanacCreateNetworksCapability' => 'PhabricatorPolicyCapability',
@@ -4030,6 +4028,7 @@ phutil_register_library_map(array(
       'PhabricatorDestructibleInterface',
       'PhabricatorNgramsInterface',
       'PhabricatorConduitResultInterface',
+      'PhabricatorExtendedPolicyInterface',
     ),
     'AlmanacDeviceController' => 'AlmanacController',
     'AlmanacDeviceEditController' => 'AlmanacDeviceController',
@@ -4057,10 +4056,9 @@ phutil_register_library_map(array(
     'AlmanacInterfaceQuery' => 'AlmanacQuery',
     'AlmanacInterfaceTableView' => 'AphrontView',
     'AlmanacKeys' => 'Phobject',
-    'AlmanacManagementLockWorkflow' => 'AlmanacManagementWorkflow',
+    'AlmanacManageClusterServicesCapability' => 'PhabricatorPolicyCapability',
     'AlmanacManagementRegisterWorkflow' => 'AlmanacManagementWorkflow',
     'AlmanacManagementTrustKeyWorkflow' => 'AlmanacManagementWorkflow',
-    'AlmanacManagementUnlockWorkflow' => 'AlmanacManagementWorkflow',
     'AlmanacManagementUntrustKeyWorkflow' => 'AlmanacManagementWorkflow',
     'AlmanacManagementWorkflow' => 'PhabricatorManagementWorkflow',
     'AlmanacNames' => 'Phobject',
@@ -4130,6 +4128,7 @@ phutil_register_library_map(array(
       'PhabricatorDestructibleInterface',
       'PhabricatorNgramsInterface',
       'PhabricatorConduitResultInterface',
+      'PhabricatorExtendedPolicyInterface',
     ),
     'AlmanacServiceController' => 'AlmanacController',
     'AlmanacServiceDatasource' => 'PhabricatorTypeaheadDatasource',

src/applications/almanac/application/PhabricatorAlmanacApplication.php

@@ -93,7 +93,7 @@ final class PhabricatorAlmanacApplication extends PhabricatorApplication {
       AlmanacCreateNamespacesCapability::CAPABILITY => array(
         'default' => PhabricatorPolicies::POLICY_ADMIN,
       ),
-      AlmanacCreateClusterServicesCapability::CAPABILITY => array(
+      AlmanacManageClusterServicesCapability::CAPABILITY => array(
         'default' => PhabricatorPolicies::POLICY_ADMIN,
       ),
     );

src/applications/almanac/capability/AlmanacManageClusterServicesCapability.php

@@ -1,17 +1,17 @@
 <?php
 
-final class AlmanacCreateClusterServicesCapability
+final class AlmanacManageClusterServicesCapability
   extends PhabricatorPolicyCapability {
 
   const CAPABILITY = 'almanac.cluster';
 
   public function getCapabilityName() {
-    return pht('Can Create Cluster Services');
+    return pht('Can Manage Cluster Services');
   }
 
   public function describeCapabilityRejection() {
     return pht(
-      'You do not have permission to create Almanac cluster services.');
+      'You do not have permission to manage Almanac cluster services.');
   }
 
 }

src/applications/almanac/controller/AlmanacBindingViewController.php

@@ -39,11 +39,13 @@ final class AlmanacBindingViewController
       ->setHeader($header)
       ->addPropertyList($property_list);
 
-    if ($binding->getService()->getIsLocked()) {
+    if ($binding->getService()->isClusterService()) {
-      $this->addLockMessage(
+      $this->addClusterMessage(
         $box,
+        pht('The service for this binding is a cluster service.'),
         pht(
-          'This service for this binding is locked, so the binding can '.
+          'The service for this binding is a cluster service. You do not '.
+          'have permission to manage cluster services, so this binding can '.
           'not be edited.'));
     }
 

src/applications/almanac/controller/AlmanacController.php

@@ -166,7 +166,14 @@ abstract class AlmanacController
       ->setTable($table);
   }
 
-  protected function addLockMessage(PHUIObjectBoxView $box, $message) {
+  protected function addClusterMessage(
+    PHUIObjectBoxView $box,
+    $positive,
+    $negative) {
+
+    $can_manage = $this->hasApplicationCapability(
+      AlmanacManageClusterServicesCapability::CAPABILITY);
+
     $doc_link = phutil_tag(
       'a',
       array(
@@ -175,11 +182,22 @@ abstract class AlmanacController
       ),
       pht('Learn More'));
 
+    if ($can_manage) {
+      $severity = PHUIInfoView::SEVERITY_NOTICE;
+      $message = $positive;
+    } else {
+      $severity = PHUIInfoView::SEVERITY_WARNING;
+      $message = $negative;
+    }
+
+    $icon = id(new PHUIIconView())
+      ->setIcon('fa-sitemap');
+
     $error_view = id(new PHUIInfoView())
-      ->setSeverity(PHUIInfoView::SEVERITY_WARNING)
+      ->setSeverity($severity)
       ->setErrors(
         array(
-          array($message, ' ', $doc_link),
+          array($icon, ' ', $message, ' ', $doc_link),
         ));
 
     $box->setInfoView($error_view);

src/applications/almanac/controller/AlmanacDeviceViewController.php

@@ -21,10 +21,6 @@ final class AlmanacDeviceViewController
       return new Aphront404Response();
     }
 
-    // We rebuild locks on a device when viewing the detail page, so they
-    // automatically get corrected if they fall out of sync.
-    $device->rebuildDeviceLocks();
-
     $title = pht('Device %s', $device->getName());
 
     $property_list = $this->buildPropertyList($device);
@@ -40,12 +36,14 @@ final class AlmanacDeviceViewController
       ->setHeader($header)
       ->addPropertyList($property_list);
 
-    if ($device->getIsLocked()) {
+    if ($device->isClusterDevice()) {
-      $this->addLockMessage(
+      $this->addClusterMessage(
         $box,
+        pht('This device is bound to a cluster service.'),
         pht(
-          'This device is bound to a locked service, so it can not be '.
+          'This device is bound to a cluster service. You do not have '.
-          'edited.'));
+          'permission to manage cluster services, so the device can not '.
+          'be edited.'));
     }
 
     $interfaces = $this->buildInterfaceList($device);
@@ -219,14 +217,14 @@ final class AlmanacDeviceViewController
 
     $handles = $viewer->loadHandles(mpull($services, 'getPHID'));
 
-    $icon_lock = id(new PHUIIconView())
+    $icon_cluster = id(new PHUIIconView())
-      ->setIcon('fa-lock');
+      ->setIcon('fa-sitemap');
 
     $rows = array();
     foreach ($services as $service) {
       $rows[] = array(
-        ($service->getIsLocked()
+        ($service->isClusterService()
-          ? $icon_lock
+          ? $icon_cluster
           : null),
         $handles->renderHandle($service->getPHID()),
       );

src/applications/almanac/controller/AlmanacServiceEditController.php

@@ -43,7 +43,7 @@ final class AlmanacServiceEditController
       $service_type = $service_types[$service_class];
       if ($service_type->isClusterServiceType()) {
         $this->requireApplicationCapability(
-          AlmanacCreateClusterServicesCapability::CAPABILITY);
+          AlmanacManageClusterServicesCapability::CAPABILITY);
       }
 
       $service = AlmanacService::initializeNewService();
@@ -190,7 +190,7 @@ final class AlmanacServiceEditController
     }
 
     list($can_cluster, $cluster_link) = $this->explainApplicationCapability(
-      AlmanacCreateClusterServicesCapability::CAPABILITY,
+      AlmanacManageClusterServicesCapability::CAPABILITY,
       pht('You have permission to create cluster services.'),
       pht('You do not have permission to create new cluster services.'));
 

src/applications/almanac/controller/AlmanacServiceViewController.php

@@ -36,15 +36,13 @@ final class AlmanacServiceViewController
       ->setHeader($header)
       ->addPropertyList($property_list);
 
-    $messages = $service->getServiceType()->getStatusMessages($service);
+    if ($service->isClusterService()) {
-    if ($messages) {
+      $this->addClusterMessage(
-      $box->setFormErrors($messages);
-    }
-
-    if ($service->getIsLocked()) {
-      $this->addLockMessage(
         $box,
-        pht('This service is locked, and can not be edited.'));
+        pht('This is a cluster service.'),
+        pht(
+          'This service is a cluster service. You do not have permission to '.
+          'edit cluster services, so you can not edit this service.'));
     }
 
     $bindings = $this->buildBindingList($service);

src/applications/almanac/editor/AlmanacBindingEditor.php

@@ -3,6 +3,8 @@
 final class AlmanacBindingEditor
   extends AlmanacEditor {
 
+  private $devicePHID;
+
   public function getEditorObjectsDescription() {
     return pht('Almanac Binding');
   }
@@ -62,6 +64,34 @@ final class AlmanacBindingEditor
 
     switch ($xaction->getTransactionType()) {
       case AlmanacBindingTransaction::TYPE_INTERFACE:
+        $interface_phids = array();
+
+        $interface_phids[] = $xaction->getOldValue();
+        $interface_phids[] = $xaction->getNewValue();
+
+        $interface_phids = array_filter($interface_phids);
+        $interface_phids = array_unique($interface_phids);
+
+        $interfaces = id(new AlmanacInterfaceQuery())
+          ->setViewer(PhabricatorUser::getOmnipotentUser())
+          ->withPHIDs($interface_phids)
+          ->execute();
+
+        $device_phids = array();
+        foreach ($interfaces as $interface) {
+          $device_phids[] = $interface->getDevicePHID();
+        }
+
+        $device_phids = array_unique($device_phids);
+
+        $devices = id(new AlmanacDeviceQuery())
+          ->setViewer(PhabricatorUser::getOmnipotentUser())
+          ->withPHIDs($device_phids)
+          ->execute();
+
+        foreach ($devices as $device) {
+          $device->rebuildClusterBindingStatus();
+        }
         return;
     }
 

src/applications/almanac/editor/AlmanacServiceEditor.php

@@ -11,7 +11,6 @@ final class AlmanacServiceEditor
     $types = parent::getTransactionTypes();
 
     $types[] = AlmanacServiceTransaction::TYPE_NAME;
-    $types[] = AlmanacServiceTransaction::TYPE_LOCK;
 
     $types[] = PhabricatorTransactions::TYPE_VIEW_POLICY;
     $types[] = PhabricatorTransactions::TYPE_EDIT_POLICY;
@@ -25,8 +24,6 @@ final class AlmanacServiceEditor
     switch ($xaction->getTransactionType()) {
       case AlmanacServiceTransaction::TYPE_NAME:
         return $object->getName();
-      case AlmanacServiceTransaction::TYPE_LOCK:
-        return (bool)$object->getIsLocked();
     }
 
     return parent::getCustomTransactionOldValue($object, $xaction);
@@ -39,8 +36,6 @@ final class AlmanacServiceEditor
     switch ($xaction->getTransactionType()) {
       case AlmanacServiceTransaction::TYPE_NAME:
         return $xaction->getNewValue();
-      case AlmanacServiceTransaction::TYPE_LOCK:
-        return (bool)$xaction->getNewValue();
     }
 
     return parent::getCustomTransactionNewValue($object, $xaction);
@@ -54,9 +49,6 @@ final class AlmanacServiceEditor
       case AlmanacServiceTransaction::TYPE_NAME:
         $object->setName($xaction->getNewValue());
         return;
-      case AlmanacServiceTransaction::TYPE_LOCK:
-        $object->setIsLocked((int)$xaction->getNewValue());
-        return;
     }
 
     return parent::applyCustomInternalTransaction($object, $xaction);
@@ -69,23 +61,6 @@ final class AlmanacServiceEditor
     switch ($xaction->getTransactionType()) {
       case AlmanacServiceTransaction::TYPE_NAME:
         return;
-      case AlmanacServiceTransaction::TYPE_LOCK:
-        $service = id(new AlmanacServiceQuery())
-          ->setViewer(PhabricatorUser::getOmnipotentUser())
-          ->withPHIDs(array($object->getPHID()))
-          ->needBindings(true)
-          ->executeOne();
-
-        $devices = array();
-        foreach ($service->getBindings() as $binding) {
-          $device = $binding->getInterface()->getDevice();
-          $devices[$device->getPHID()] = $device;
-        }
-
-        foreach ($devices as $device) {
-          $device->rebuildDeviceLocks();
-        }
-        return;
     }
 
     return parent::applyCustomExternalTransaction($object, $xaction);

src/applications/almanac/management/AlmanacManagementLockWorkflow.php

@@ -1,49 +0,0 @@
-<?php
-
-final class AlmanacManagementLockWorkflow
-  extends AlmanacManagementWorkflow {
-
-  protected function didConstruct() {
-    $this
-      ->setName('lock')
-      ->setSynopsis(pht('Lock a service to prevent it from being edited.'))
-      ->setArguments(
-        array(
-          array(
-            'name' => 'services',
-            'wildcard' => true,
-          ),
-        ));
-  }
-
-  public function execute(PhutilArgumentParser $args) {
-    $console = PhutilConsole::getConsole();
-
-    $services = $this->loadServices($args->getArg('services'));
-    if (!$services) {
-      throw new PhutilArgumentUsageException(
-        pht('Specify at least one service to lock.'));
-    }
-
-    foreach ($services as $service) {
-      if ($service->getIsLocked()) {
-        throw new PhutilArgumentUsageException(
-          pht(
-            'Service "%s" is already locked!',
-            $service->getName()));
-      }
-    }
-
-    foreach ($services as $service) {
-      $this->updateServiceLock($service, true);
-
-      $console->writeOut(
-        "**<bg:green> %s </bg>** %s\n",
-        pht('LOCKED'),
-        pht('Service "%s" was locked.', $service->getName()));
-    }
-
-    return 0;
-  }
-
-}

src/applications/almanac/management/AlmanacManagementUnlockWorkflow.php

@@ -1,49 +0,0 @@
-<?php
-
-final class AlmanacManagementUnlockWorkflow
-  extends AlmanacManagementWorkflow {
-
-  protected function didConstruct() {
-    $this
-      ->setName('unlock')
-      ->setSynopsis(pht('Unlock a service to allow it to be edited.'))
-      ->setArguments(
-        array(
-          array(
-            'name' => 'services',
-            'wildcard' => true,
-          ),
-        ));
-  }
-
-  public function execute(PhutilArgumentParser $args) {
-    $console = PhutilConsole::getConsole();
-
-    $services = $this->loadServices($args->getArg('services'));
-    if (!$services) {
-      throw new PhutilArgumentUsageException(
-        pht('Specify at least one service to unlock.'));
-    }
-
-    foreach ($services as $service) {
-      if (!$service->getIsLocked()) {
-        throw new PhutilArgumentUsageException(
-          pht(
-            'Service "%s" is not locked!',
-            $service->getName()));
-      }
-    }
-
-    foreach ($services as $service) {
-      $this->updateServiceLock($service, false);
-
-      $console->writeOut(
-        "**<bg:green> %s </bg>** %s\n",
-        pht('UNLOCKED'),
-        pht('Service "%s" was unlocked.', $service->getName()));
-    }
-
-    return 0;
-  }
-
-}

src/applications/almanac/query/AlmanacDeviceSearchEngine.php

@@ -84,6 +84,10 @@ final class AlmanacDeviceSearchEngine
         ->setHref($device->getURI())
         ->setObject($device);
 
+      if ($device->isClusterDevice()) {
+        $item->addIcon('fa-sitemap', pht('Cluster Device'));
+      }
+
       $list->addItem($item);
     }
 

src/applications/almanac/query/AlmanacServiceQuery.php

@@ -8,7 +8,6 @@ final class AlmanacServiceQuery
   private $names;
   private $serviceClasses;
   private $devicePHIDs;
-  private $locked;
   private $namePrefix;
   private $nameSuffix;
 
@@ -39,11 +38,6 @@ final class AlmanacServiceQuery
     return $this;
   }
 
-  public function withLocked($locked) {
-    $this->locked = $locked;
-    return $this;
-  }
-
   public function withNamePrefix($prefix) {
     $this->namePrefix = $prefix;
     return $this;
@@ -129,13 +123,6 @@ final class AlmanacServiceQuery
         $this->devicePHIDs);
     }
 
-    if ($this->locked !== null) {
-      $where[] = qsprintf(
-        $conn,
-        'service.isLocked = %d',
-        (int)$this->locked);
-    }
-
     if ($this->namePrefix !== null) {
       $where[] = qsprintf(
         $conn,

src/applications/almanac/query/AlmanacServiceSearchEngine.php

@@ -101,15 +101,6 @@ final class AlmanacServiceSearchEngine
           $service->getServiceType()->getServiceTypeIcon(),
           $service->getServiceType()->getServiceTypeShortName());
 
-      if ($service->getIsLocked() ||
-          $service->getServiceType()->isClusterServiceType()) {
-        if ($service->getIsLocked()) {
-          $item->addIcon('fa-lock', pht('Locked'));
-        } else {
-          $item->addIcon('fa-unlock-alt red', pht('Unlocked'));
-        }
-      }
-
       $list->addItem($item);
     }
 

src/applications/almanac/servicetype/AlmanacClusterServiceType.php

@@ -11,28 +11,4 @@ abstract class AlmanacClusterServiceType
     return 'fa-sitemap';
   }
 
-  public function getStatusMessages(AlmanacService $service) {
-    $messages = parent::getStatusMessages($service);
-
-    if (!$service->getIsLocked()) {
-      $doc_href = PhabricatorEnv::getDoclink(
-        'User Guide: Phabricator Clusters');
-
-      $doc_link = phutil_tag(
-        'a',
-        array(
-          'href' => $doc_href,
-          'target' => '_blank',
-        ),
-        pht('Learn More'));
-
-      $messages[] = pht(
-        'This is an unlocked cluster service. After you finish editing '.
-        'it, you should lock it. %s.',
-        $doc_link);
-    }
-
-    return $messages;
-  }
-
 }

src/applications/almanac/servicetype/AlmanacServiceType.php

@@ -55,10 +55,6 @@ abstract class AlmanacServiceType extends Phobject {
     return array();
   }
 
-  public function getStatusMessages(AlmanacService $service) {
-    return array();
-  }
-
   /**
    * List all available service type implementations.
    *

src/applications/almanac/storage/AlmanacBinding.php

@@ -6,7 +6,8 @@ final class AlmanacBinding
     PhabricatorPolicyInterface,
     PhabricatorApplicationTransactionInterface,
     AlmanacPropertyInterface,
-    PhabricatorDestructibleInterface {
+    PhabricatorDestructibleInterface,
+    PhabricatorExtendedPolicyInterface {
 
   protected $servicePHID;
   protected $devicePHID;
@@ -157,17 +158,30 @@ final class AlmanacBinding
         'interface.'),
     );
 
-    if ($capability === PhabricatorPolicyCapability::CAN_EDIT) {
-      if ($this->getService()->getIsLocked()) {
-        $notes[] = pht(
-          'The service for this binding is locked, so it can not be edited.');
-      }
-    }
-
     return $notes;
   }
 
 
+/* -(  PhabricatorExtendedPolicyInterface  )--------------------------------- */
+
+
+  public function getExtendedPolicy($capability, PhabricatorUser $viewer) {
+    switch ($capability) {
+      case PhabricatorPolicyCapability::CAN_EDIT:
+        if ($this->getService()->isClusterService()) {
+          return array(
+            array(
+              new PhabricatorAlmanacApplication(),
+              AlmanacManageClusterServicesCapability::CAPABILITY,
+            ),
+          );
+        }
+        break;
+    }
+
+    return array();
+  }
+
 /* -(  PhabricatorApplicationTransactionInterface  )------------------------- */
 
 

src/applications/almanac/storage/AlmanacDevice.php

@@ -10,14 +10,15 @@ final class AlmanacDevice
     AlmanacPropertyInterface,
     PhabricatorDestructibleInterface,
     PhabricatorNgramsInterface,
-    PhabricatorConduitResultInterface {
+    PhabricatorConduitResultInterface,
+    PhabricatorExtendedPolicyInterface {
 
   protected $name;
   protected $nameIndex;
   protected $mailKey;
   protected $viewPolicy;
   protected $editPolicy;
-  protected $isLocked;
+  protected $isBoundToClusterService;
 
   private $almanacProperties = self::ATTACHABLE;
 
@@ -26,7 +27,7 @@ final class AlmanacDevice
       ->setViewPolicy(PhabricatorPolicies::POLICY_USER)
       ->setEditPolicy(PhabricatorPolicies::POLICY_ADMIN)
       ->attachAlmanacProperties(array())
-      ->setIsLocked(0);
+      ->setIsBoundToClusterService(0);
   }
 
   protected function getConfiguration() {
@@ -36,7 +37,7 @@ final class AlmanacDevice
         'name' => 'text128',
         'nameIndex' => 'bytes12',
         'mailKey' => 'bytes20',
-        'isLocked' => 'bool',
+        'isBoundToClusterService' => 'bool',
       ),
       self::CONFIG_KEY_SCHEMA => array(
         'key_name' => array(
@@ -70,30 +71,28 @@ final class AlmanacDevice
     return '/almanac/device/view/'.$this->getName().'/';
   }
 
-
+  public function rebuildClusterBindingStatus() {
-  /**
-   * Find locked services which are bound to this device, updating the device
-   * lock flag if necessary.
-   *
-   * @return list<phid> List of locking service PHIDs.
-   */
-  public function rebuildDeviceLocks() {
     $services = id(new AlmanacServiceQuery())
       ->setViewer(PhabricatorUser::getOmnipotentUser())
       ->withDevicePHIDs(array($this->getPHID()))
-      ->withLocked(true)
       ->execute();
 
-    $locked = (bool)count($services);
+    $is_cluster = false;
+    foreach ($services as $service) {
+      if ($service->isClusterService()) {
+        $is_cluster = true;
+        break;
+      }
+    }
 
-    if ($locked != $this->getIsLocked()) {
+    if ($is_cluster != $this->getIsBoundToClusterService()) {
-      $this->setIsLocked((int)$locked);
+      $this->setIsBoundToClusterService((int)$is_cluster);
       $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
         queryfx(
           $this->establishConnection('w'),
-          'UPDATE %T SET isLocked = %d WHERE id = %d',
+          'UPDATE %T SET isBoundToClusterService = %d WHERE id = %d',
           $this->getTableName(),
-          $this->getIsLocked(),
+          $this->getIsBoundToClusterService(),
           $this->getID());
       unset($unguarded);
     }
@@ -101,6 +100,10 @@ final class AlmanacDevice
     return $this;
   }
 
+  public function isClusterDevice() {
+    return $this->getIsBoundToClusterService();
+  }
+
 
 /* -(  AlmanacPropertyInterface  )------------------------------------------- */
 
@@ -156,28 +159,37 @@ final class AlmanacDevice
       case PhabricatorPolicyCapability::CAN_VIEW:
         return $this->getViewPolicy();
       case PhabricatorPolicyCapability::CAN_EDIT:
-        if ($this->getIsLocked()) {
-          return PhabricatorPolicies::POLICY_NOONE;
-        } else {
         return $this->getEditPolicy();
     }
   }
-  }
 
   public function hasAutomaticCapability($capability, PhabricatorUser $viewer) {
     return false;
   }
 
   public function describeAutomaticCapability($capability) {
-    if ($capability === PhabricatorPolicyCapability::CAN_EDIT) {
+    return null;
-      if ($this->getIsLocked()) {
-        return pht(
-          'This device is bound to a locked service, so it can not '.
-          'be edited.');
-      }
   }
 
-    return null;
+
+/* -(  PhabricatorExtendedPolicyInterface  )--------------------------------- */
+
+
+  public function getExtendedPolicy($capability, PhabricatorUser $viewer) {
+    switch ($capability) {
+      case PhabricatorPolicyCapability::CAN_EDIT:
+        if ($this->isClusterDevice()) {
+          return array(
+            array(
+              new PhabricatorAlmanacApplication(),
+              AlmanacManageClusterServicesCapability::CAPABILITY,
+            ),
+          );
+        }
+        break;
+    }
+
+    return array();
   }
 
 

src/applications/almanac/storage/AlmanacInterface.php

@@ -101,13 +101,6 @@ final class AlmanacInterface
         'view the interface.'),
     );
 
-    if ($capability === PhabricatorPolicyCapability::CAN_EDIT) {
-      if ($this->getDevice()->getIsLocked()) {
-        $notes[] = pht(
-          'The device for this interface is locked, so it can not be edited.');
-      }
-    }
-
     return $notes;
   }
 

src/applications/almanac/storage/AlmanacService.php

@@ -9,7 +9,8 @@ final class AlmanacService
     AlmanacPropertyInterface,
     PhabricatorDestructibleInterface,
     PhabricatorNgramsInterface,
-    PhabricatorConduitResultInterface {
+    PhabricatorConduitResultInterface,
+    PhabricatorExtendedPolicyInterface {
 
   protected $name;
   protected $nameIndex;
@@ -17,7 +18,6 @@ final class AlmanacService
   protected $viewPolicy;
   protected $editPolicy;
   protected $serviceClass;
-  protected $isLocked;
 
   private $almanacProperties = self::ATTACHABLE;
   private $bindings = self::ATTACHABLE;
@@ -27,8 +27,7 @@ final class AlmanacService
     return id(new AlmanacService())
       ->setViewPolicy(PhabricatorPolicies::POLICY_USER)
       ->setEditPolicy(PhabricatorPolicies::POLICY_ADMIN)
-      ->attachAlmanacProperties(array())
+      ->attachAlmanacProperties(array());
-      ->setIsLocked(0);
   }
 
   protected function getConfiguration() {
@@ -39,7 +38,6 @@ final class AlmanacService
         'nameIndex' => 'bytes12',
         'mailKey' => 'bytes20',
         'serviceClass' => 'text64',
-        'isLocked' => 'bool',
       ),
       self::CONFIG_KEY_SCHEMA => array(
         'key_name' => array(
@@ -94,6 +92,10 @@ final class AlmanacService
     return $this;
   }
 
+  public function isClusterService() {
+    return $this->getServiceType()->isClusterServiceType();
+  }
+
 
 /* -(  AlmanacPropertyInterface  )------------------------------------------- */
 
@@ -149,28 +151,37 @@ final class AlmanacService
       case PhabricatorPolicyCapability::CAN_VIEW:
         return $this->getViewPolicy();
       case PhabricatorPolicyCapability::CAN_EDIT:
-        if ($this->getIsLocked()) {
-          return PhabricatorPolicies::POLICY_NOONE;
-        } else {
         return $this->getEditPolicy();
     }
   }
-  }
 
   public function hasAutomaticCapability($capability, PhabricatorUser $viewer) {
     return false;
   }
 
   public function describeAutomaticCapability($capability) {
+    return null;
+  }
+
+
+/* -(  PhabricatorExtendedPolicyInterface  )--------------------------------- */
+
+
+  public function getExtendedPolicy($capability, PhabricatorUser $viewer) {
     switch ($capability) {
       case PhabricatorPolicyCapability::CAN_EDIT:
-        if ($this->getIsLocked()) {
+        if ($this->isClusterService()) {
-          return pht('This service is locked and can not be edited.');
+          return array(
+            array(
+              new PhabricatorAlmanacApplication(),
+              AlmanacManageClusterServicesCapability::CAPABILITY,
+            ),
+          );
         }
         break;
     }
 
-    return null;
+    return array();
   }
 
 

src/applications/policy/filter/PhabricatorPolicyFilter.php

@@ -381,10 +381,13 @@ final class PhabricatorPolicyFilter extends Phobject {
           $reject = $extended_objects[$extended_key];
           unset($extended_objects[$extended_key]);
 
-          // TODO: This isn't as user-friendly as it could be. It's possible
+          // It's possible that we're rejecting this object for multiple
-          // that we're rejecting this object for multiple capability/policy
+          // capability/policy failures, but just pick the first one to show
-          // failures, though.
+          // to the user.
-          $this->rejectObject($reject, false, '<extended>');
+          $first_capability = head($capabilities);
+          $first_policy = $object_in->getPolicy($first_capability);
+
+          $this->rejectObject($reject, $first_policy, $first_capability);
         }
       }
     }

src/docs/user/configuration/cluster.diviner

@@ -12,20 +12,7 @@ yet. This document is mostly a placeholder.
 Locking Services
 ================
 
-Because cluster configuration is defined in Phabricator itself, an attacker
+Very briefly, you can set "Can Manage Cluster Services" to "No One" to lock
-who compromises an account that can edit the cluster definition has significant
+the cluster definition.
-power. For example, the attacker might be able to configure Phabricator to
-replicate the database to a server they control.
 
-To mitigate this attack, services in Almanac can be locked to prevent them
+See also @{article:Almanac User Guide}.
-from being edited from the web UI. An attacker would then need significantly
-greater access (to the CLI, or directly to the database) in order to change
-the cluster configuration.
-
-You should normally keep cluster services in a locked state, and unlock them
-only to edit them. Once you're finished making changes, lock the service again.
-The web UI will warn you when you're viewing an unlocked cluster service, as
-a reminder that you should lock it again once you're finished editing.
-
-For details on how to lock and unlock a service, see
-@{article:Almanac User Guide}.

src/docs/user/userguide/almanac.diviner

@@ -177,35 +177,3 @@ Users can edit services and devices within a namespace if they have edit
 permission on the service or device itself, as long as they don't try to rename
 the service or device to move it into a namespace they don't have permission
 to access.
-
-
-Locking and Unlocking Services
-==============================
-
-Services can be locked to prevent edits from the web UI. This primarily hardens
-Almanac against attacks involving account compromise. Notably, locking cluster
-services prevents an attacker from modifying the Phabricator cluster definition.
-For more details on this scenario, see
-@{article:User Guide: Phabricator Clusters}.
-
-Beyond hardening cluster definitions, you might also want to lock a critical
-service to prevent accidental edits.
-
-To lock a service, run:
-
-  phabricator/ $ ./bin/almanac lock <service>
-
-To unlock a service later, run:
-
-  phabricator/ $ ./bin/almanac unlock <service>
-
-Locking a service also locks all of the service's bindings and properties, as
-well as the devices connected to the service. Generally, no part of the
-service definition can be modified while it is locked.
-
-Devices (and their properties) will remain locked as long as they are bound to
-at least one locked service. To edit a device, you'll need to unlock all the
-services it is bound to.
-
-Locked services and devices will show that they are locked in the web UI, and
-editing options will be unavailable.