diff --git a/resources/sql/autopatches/20160223.almanac.1.bound.sql b/resources/sql/autopatches/20160223.almanac.1.bound.sql new file mode 100644 --- /dev/null +++ b/resources/sql/autopatches/20160223.almanac.1.bound.sql @@ -0,0 +1,2 @@ +ALTER TABLE {$NAMESPACE}_almanac.almanac_device + ADD isBoundToClusterService BOOL NOT NULL; diff --git a/resources/sql/autopatches/20160223.almanac.2.lockbind.sql b/resources/sql/autopatches/20160223.almanac.2.lockbind.sql new file mode 100644 --- /dev/null +++ b/resources/sql/autopatches/20160223.almanac.2.lockbind.sql @@ -0,0 +1,2 @@ +UPDATE {$NAMESPACE}_almanac.almanac_device + SET isBoundToClusterService = isLocked; diff --git a/resources/sql/autopatches/20160223.almanac.3.devicelock.sql b/resources/sql/autopatches/20160223.almanac.3.devicelock.sql new file mode 100644 --- /dev/null +++ b/resources/sql/autopatches/20160223.almanac.3.devicelock.sql @@ -0,0 +1,2 @@ +ALTER TABLE {$NAMESPACE}_almanac.almanac_device + DROP isLocked; diff --git a/resources/sql/autopatches/20160223.almanac.4.servicelock.sql b/resources/sql/autopatches/20160223.almanac.4.servicelock.sql new file mode 100644 --- /dev/null +++ b/resources/sql/autopatches/20160223.almanac.4.servicelock.sql @@ -0,0 +1,2 @@ +ALTER TABLE {$NAMESPACE}_almanac.almanac_service + DROP isLocked; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -27,7 +27,6 @@ '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 @@ '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 @@ 'PhabricatorApplicationTransactionInterface', 'AlmanacPropertyInterface', 'PhabricatorDestructibleInterface', + 'PhabricatorExtendedPolicyInterface', ), 'AlmanacBindingEditController' => 'AlmanacServiceController', 'AlmanacBindingEditor' => 'AlmanacEditor', @@ -4013,7 +4012,6 @@ 'AlmanacConduitAPIMethod' => 'ConduitAPIMethod', 'AlmanacConsoleController' => 'AlmanacController', 'AlmanacController' => 'PhabricatorController', - 'AlmanacCreateClusterServicesCapability' => 'PhabricatorPolicyCapability', 'AlmanacCreateDevicesCapability' => 'PhabricatorPolicyCapability', 'AlmanacCreateNamespacesCapability' => 'PhabricatorPolicyCapability', 'AlmanacCreateNetworksCapability' => 'PhabricatorPolicyCapability', @@ -4030,6 +4028,7 @@ 'PhabricatorDestructibleInterface', 'PhabricatorNgramsInterface', 'PhabricatorConduitResultInterface', + 'PhabricatorExtendedPolicyInterface', ), 'AlmanacDeviceController' => 'AlmanacController', 'AlmanacDeviceEditController' => 'AlmanacDeviceController', @@ -4057,10 +4056,9 @@ '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 @@ 'PhabricatorDestructibleInterface', 'PhabricatorNgramsInterface', 'PhabricatorConduitResultInterface', + 'PhabricatorExtendedPolicyInterface', ), 'AlmanacServiceController' => 'AlmanacController', 'AlmanacServiceDatasource' => 'PhabricatorTypeaheadDatasource', diff --git a/src/applications/almanac/application/PhabricatorAlmanacApplication.php b/src/applications/almanac/application/PhabricatorAlmanacApplication.php --- a/src/applications/almanac/application/PhabricatorAlmanacApplication.php +++ b/src/applications/almanac/application/PhabricatorAlmanacApplication.php @@ -93,7 +93,7 @@ AlmanacCreateNamespacesCapability::CAPABILITY => array( 'default' => PhabricatorPolicies::POLICY_ADMIN, ), - AlmanacCreateClusterServicesCapability::CAPABILITY => array( + AlmanacManageClusterServicesCapability::CAPABILITY => array( 'default' => PhabricatorPolicies::POLICY_ADMIN, ), ); diff --git a/src/applications/almanac/capability/AlmanacCreateClusterServicesCapability.php b/src/applications/almanac/capability/AlmanacManageClusterServicesCapability.php rename from src/applications/almanac/capability/AlmanacCreateClusterServicesCapability.php rename to src/applications/almanac/capability/AlmanacManageClusterServicesCapability.php --- a/src/applications/almanac/capability/AlmanacCreateClusterServicesCapability.php +++ b/src/applications/almanac/capability/AlmanacManageClusterServicesCapability.php @@ -1,17 +1,17 @@ setHeader($header) ->addPropertyList($property_list); - if ($binding->getService()->getIsLocked()) { - $this->addLockMessage( + if ($binding->getService()->isClusterService()) { + $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.')); } diff --git a/src/applications/almanac/controller/AlmanacController.php b/src/applications/almanac/controller/AlmanacController.php --- a/src/applications/almanac/controller/AlmanacController.php +++ b/src/applications/almanac/controller/AlmanacController.php @@ -166,7 +166,14 @@ ->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 @@ ), 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); diff --git a/src/applications/almanac/controller/AlmanacDeviceViewController.php b/src/applications/almanac/controller/AlmanacDeviceViewController.php --- a/src/applications/almanac/controller/AlmanacDeviceViewController.php +++ b/src/applications/almanac/controller/AlmanacDeviceViewController.php @@ -21,10 +21,6 @@ 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 @@ ->setHeader($header) ->addPropertyList($property_list); - if ($device->getIsLocked()) { - $this->addLockMessage( + if ($device->isClusterDevice()) { + $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 '. - 'edited.')); + 'This device is bound to a cluster service. You do not have '. + 'permission to manage cluster services, so the device can not '. + 'be edited.')); } $interfaces = $this->buildInterfaceList($device); @@ -219,14 +217,14 @@ $handles = $viewer->loadHandles(mpull($services, 'getPHID')); - $icon_lock = id(new PHUIIconView()) - ->setIcon('fa-lock'); + $icon_cluster = id(new PHUIIconView()) + ->setIcon('fa-sitemap'); $rows = array(); foreach ($services as $service) { $rows[] = array( - ($service->getIsLocked() - ? $icon_lock + ($service->isClusterService() + ? $icon_cluster : null), $handles->renderHandle($service->getPHID()), ); diff --git a/src/applications/almanac/controller/AlmanacServiceEditController.php b/src/applications/almanac/controller/AlmanacServiceEditController.php --- a/src/applications/almanac/controller/AlmanacServiceEditController.php +++ b/src/applications/almanac/controller/AlmanacServiceEditController.php @@ -43,7 +43,7 @@ $service_type = $service_types[$service_class]; if ($service_type->isClusterServiceType()) { $this->requireApplicationCapability( - AlmanacCreateClusterServicesCapability::CAPABILITY); + AlmanacManageClusterServicesCapability::CAPABILITY); } $service = AlmanacService::initializeNewService(); @@ -190,7 +190,7 @@ } 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.')); diff --git a/src/applications/almanac/controller/AlmanacServiceViewController.php b/src/applications/almanac/controller/AlmanacServiceViewController.php --- a/src/applications/almanac/controller/AlmanacServiceViewController.php +++ b/src/applications/almanac/controller/AlmanacServiceViewController.php @@ -36,15 +36,13 @@ ->setHeader($header) ->addPropertyList($property_list); - $messages = $service->getServiceType()->getStatusMessages($service); - if ($messages) { - $box->setFormErrors($messages); - } - - if ($service->getIsLocked()) { - $this->addLockMessage( + if ($service->isClusterService()) { + $this->addClusterMessage( $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); diff --git a/src/applications/almanac/editor/AlmanacBindingEditor.php b/src/applications/almanac/editor/AlmanacBindingEditor.php --- a/src/applications/almanac/editor/AlmanacBindingEditor.php +++ b/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 @@ 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; } diff --git a/src/applications/almanac/editor/AlmanacServiceEditor.php b/src/applications/almanac/editor/AlmanacServiceEditor.php --- a/src/applications/almanac/editor/AlmanacServiceEditor.php +++ b/src/applications/almanac/editor/AlmanacServiceEditor.php @@ -11,7 +11,6 @@ $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 @@ 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 @@ 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 @@ 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 @@ 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); diff --git a/src/applications/almanac/management/AlmanacManagementLockWorkflow.php b/src/applications/almanac/management/AlmanacManagementLockWorkflow.php deleted file mode 100644 --- a/src/applications/almanac/management/AlmanacManagementLockWorkflow.php +++ /dev/null @@ -1,49 +0,0 @@ -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( - "** %s ** %s\n", - pht('LOCKED'), - pht('Service "%s" was locked.', $service->getName())); - } - - return 0; - } - -} diff --git a/src/applications/almanac/management/AlmanacManagementUnlockWorkflow.php b/src/applications/almanac/management/AlmanacManagementUnlockWorkflow.php deleted file mode 100644 --- a/src/applications/almanac/management/AlmanacManagementUnlockWorkflow.php +++ /dev/null @@ -1,49 +0,0 @@ -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( - "** %s ** %s\n", - pht('UNLOCKED'), - pht('Service "%s" was unlocked.', $service->getName())); - } - - return 0; - } - -} diff --git a/src/applications/almanac/query/AlmanacDeviceSearchEngine.php b/src/applications/almanac/query/AlmanacDeviceSearchEngine.php --- a/src/applications/almanac/query/AlmanacDeviceSearchEngine.php +++ b/src/applications/almanac/query/AlmanacDeviceSearchEngine.php @@ -84,6 +84,10 @@ ->setHref($device->getURI()) ->setObject($device); + if ($device->isClusterDevice()) { + $item->addIcon('fa-sitemap', pht('Cluster Device')); + } + $list->addItem($item); } diff --git a/src/applications/almanac/query/AlmanacServiceQuery.php b/src/applications/almanac/query/AlmanacServiceQuery.php --- a/src/applications/almanac/query/AlmanacServiceQuery.php +++ b/src/applications/almanac/query/AlmanacServiceQuery.php @@ -8,7 +8,6 @@ private $names; private $serviceClasses; private $devicePHIDs; - private $locked; private $namePrefix; private $nameSuffix; @@ -39,11 +38,6 @@ return $this; } - public function withLocked($locked) { - $this->locked = $locked; - return $this; - } - public function withNamePrefix($prefix) { $this->namePrefix = $prefix; return $this; @@ -129,13 +123,6 @@ $this->devicePHIDs); } - if ($this->locked !== null) { - $where[] = qsprintf( - $conn, - 'service.isLocked = %d', - (int)$this->locked); - } - if ($this->namePrefix !== null) { $where[] = qsprintf( $conn, diff --git a/src/applications/almanac/query/AlmanacServiceSearchEngine.php b/src/applications/almanac/query/AlmanacServiceSearchEngine.php --- a/src/applications/almanac/query/AlmanacServiceSearchEngine.php +++ b/src/applications/almanac/query/AlmanacServiceSearchEngine.php @@ -101,15 +101,6 @@ $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); } diff --git a/src/applications/almanac/servicetype/AlmanacClusterServiceType.php b/src/applications/almanac/servicetype/AlmanacClusterServiceType.php --- a/src/applications/almanac/servicetype/AlmanacClusterServiceType.php +++ b/src/applications/almanac/servicetype/AlmanacClusterServiceType.php @@ -11,28 +11,4 @@ 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; - } - } diff --git a/src/applications/almanac/servicetype/AlmanacServiceType.php b/src/applications/almanac/servicetype/AlmanacServiceType.php --- a/src/applications/almanac/servicetype/AlmanacServiceType.php +++ b/src/applications/almanac/servicetype/AlmanacServiceType.php @@ -55,10 +55,6 @@ return array(); } - public function getStatusMessages(AlmanacService $service) { - return array(); - } - /** * List all available service type implementations. * diff --git a/src/applications/almanac/storage/AlmanacBinding.php b/src/applications/almanac/storage/AlmanacBinding.php --- a/src/applications/almanac/storage/AlmanacBinding.php +++ b/src/applications/almanac/storage/AlmanacBinding.php @@ -6,7 +6,8 @@ PhabricatorPolicyInterface, PhabricatorApplicationTransactionInterface, AlmanacPropertyInterface, - PhabricatorDestructibleInterface { + PhabricatorDestructibleInterface, + PhabricatorExtendedPolicyInterface { protected $servicePHID; protected $devicePHID; @@ -157,17 +158,30 @@ '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 )------------------------- */ diff --git a/src/applications/almanac/storage/AlmanacDevice.php b/src/applications/almanac/storage/AlmanacDevice.php --- a/src/applications/almanac/storage/AlmanacDevice.php +++ b/src/applications/almanac/storage/AlmanacDevice.php @@ -10,14 +10,15 @@ 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 @@ ->setViewPolicy(PhabricatorPolicies::POLICY_USER) ->setEditPolicy(PhabricatorPolicies::POLICY_ADMIN) ->attachAlmanacProperties(array()) - ->setIsLocked(0); + ->setIsBoundToClusterService(0); } protected function getConfiguration() { @@ -36,7 +37,7 @@ 'name' => 'text128', 'nameIndex' => 'bytes12', 'mailKey' => 'bytes20', - 'isLocked' => 'bool', + 'isBoundToClusterService' => 'bool', ), self::CONFIG_KEY_SCHEMA => array( 'key_name' => array( @@ -70,30 +71,28 @@ return '/almanac/device/view/'.$this->getName().'/'; } - - /** - * Find locked services which are bound to this device, updating the device - * lock flag if necessary. - * - * @return list List of locking service PHIDs. - */ - public function rebuildDeviceLocks() { + public function rebuildClusterBindingStatus() { $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()) { - $this->setIsLocked((int)$locked); + if ($is_cluster != $this->getIsBoundToClusterService()) { + $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 @@ return $this; } + public function isClusterDevice() { + return $this->getIsBoundToClusterService(); + } + /* -( AlmanacPropertyInterface )------------------------------------------- */ @@ -156,11 +159,7 @@ case PhabricatorPolicyCapability::CAN_VIEW: return $this->getViewPolicy(); case PhabricatorPolicyCapability::CAN_EDIT: - if ($this->getIsLocked()) { - return PhabricatorPolicies::POLICY_NOONE; - } else { - return $this->getEditPolicy(); - } + return $this->getEditPolicy(); } } @@ -169,15 +168,28 @@ } public function describeAutomaticCapability($capability) { - if ($capability === PhabricatorPolicyCapability::CAN_EDIT) { - 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 null; + return array(); } diff --git a/src/applications/almanac/storage/AlmanacInterface.php b/src/applications/almanac/storage/AlmanacInterface.php --- a/src/applications/almanac/storage/AlmanacInterface.php +++ b/src/applications/almanac/storage/AlmanacInterface.php @@ -101,13 +101,6 @@ '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; } diff --git a/src/applications/almanac/storage/AlmanacService.php b/src/applications/almanac/storage/AlmanacService.php --- a/src/applications/almanac/storage/AlmanacService.php +++ b/src/applications/almanac/storage/AlmanacService.php @@ -9,7 +9,8 @@ AlmanacPropertyInterface, PhabricatorDestructibleInterface, PhabricatorNgramsInterface, - PhabricatorConduitResultInterface { + PhabricatorConduitResultInterface, + PhabricatorExtendedPolicyInterface { protected $name; protected $nameIndex; @@ -17,7 +18,6 @@ protected $viewPolicy; protected $editPolicy; protected $serviceClass; - protected $isLocked; private $almanacProperties = self::ATTACHABLE; private $bindings = self::ATTACHABLE; @@ -27,8 +27,7 @@ return id(new AlmanacService()) ->setViewPolicy(PhabricatorPolicies::POLICY_USER) ->setEditPolicy(PhabricatorPolicies::POLICY_ADMIN) - ->attachAlmanacProperties(array()) - ->setIsLocked(0); + ->attachAlmanacProperties(array()); } protected function getConfiguration() { @@ -39,7 +38,6 @@ 'nameIndex' => 'bytes12', 'mailKey' => 'bytes20', 'serviceClass' => 'text64', - 'isLocked' => 'bool', ), self::CONFIG_KEY_SCHEMA => array( 'key_name' => array( @@ -94,6 +92,10 @@ return $this; } + public function isClusterService() { + return $this->getServiceType()->isClusterServiceType(); + } + /* -( AlmanacPropertyInterface )------------------------------------------- */ @@ -149,11 +151,7 @@ case PhabricatorPolicyCapability::CAN_VIEW: return $this->getViewPolicy(); case PhabricatorPolicyCapability::CAN_EDIT: - if ($this->getIsLocked()) { - return PhabricatorPolicies::POLICY_NOONE; - } else { - return $this->getEditPolicy(); - } + return $this->getEditPolicy(); } } @@ -162,15 +160,28 @@ } public function describeAutomaticCapability($capability) { + return null; + } + + +/* -( PhabricatorExtendedPolicyInterface )--------------------------------- */ + + + public function getExtendedPolicy($capability, PhabricatorUser $viewer) { switch ($capability) { case PhabricatorPolicyCapability::CAN_EDIT: - if ($this->getIsLocked()) { - return pht('This service is locked and can not be edited.'); + if ($this->isClusterService()) { + return array( + array( + new PhabricatorAlmanacApplication(), + AlmanacManageClusterServicesCapability::CAPABILITY, + ), + ); } break; } - return null; + return array(); } diff --git a/src/applications/policy/filter/PhabricatorPolicyFilter.php b/src/applications/policy/filter/PhabricatorPolicyFilter.php --- a/src/applications/policy/filter/PhabricatorPolicyFilter.php +++ b/src/applications/policy/filter/PhabricatorPolicyFilter.php @@ -381,10 +381,13 @@ $reject = $extended_objects[$extended_key]; unset($extended_objects[$extended_key]); - // TODO: This isn't as user-friendly as it could be. It's possible - // that we're rejecting this object for multiple capability/policy - // failures, though. - $this->rejectObject($reject, false, ''); + // It's possible that we're rejecting this object for multiple + // capability/policy failures, but just pick the first one to show + // to the user. + $first_capability = head($capabilities); + $first_policy = $object_in->getPolicy($first_capability); + + $this->rejectObject($reject, $first_policy, $first_capability); } } } diff --git a/src/docs/user/configuration/cluster.diviner b/src/docs/user/configuration/cluster.diviner --- a/src/docs/user/configuration/cluster.diviner +++ b/src/docs/user/configuration/cluster.diviner @@ -12,20 +12,7 @@ Locking Services ================ -Because cluster configuration is defined in Phabricator itself, an attacker -who compromises an account that can edit the cluster definition has significant -power. For example, the attacker might be able to configure Phabricator to -replicate the database to a server they control. +Very briefly, you can set "Can Manage Cluster Services" to "No One" to lock +the cluster definition. -To mitigate this attack, services in Almanac can be locked to prevent them -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}. +See also @{article:Almanac User Guide}. diff --git a/src/docs/user/userguide/almanac.diviner b/src/docs/user/userguide/almanac.diviner --- a/src/docs/user/userguide/almanac.diviner +++ b/src/docs/user/userguide/almanac.diviner @@ -177,35 +177,3 @@ 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 - -To unlock a service later, run: - - phabricator/ $ ./bin/almanac unlock - -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.