D15339.diff
No OneTemporary
Actions

Size

36 KB

Referenced Files

None

Subscribers

None

D15339.diff
View Options

	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 @@
	<?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.');
	}

	}
	diff --git a/src/applications/almanac/controller/AlmanacBindingViewController.php b/src/applications/almanac/controller/AlmanacBindingViewController.php
	--- a/src/applications/almanac/controller/AlmanacBindingViewController.php
	+++ b/src/applications/almanac/controller/AlmanacBindingViewController.php
	@@ -39,11 +39,13 @@
	->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 @@
	-<?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;
	- }
	-
	-}
	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 @@
	-<?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;
	- }
	-
	-}
	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<phid> 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, '<extended>');
	+ // 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 <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.

File Metadata

Mime Type: text/plain
Expires: Thu, Oct 24, 8:13 AM (3 w, 3 d ago)
Storage Engine: amazon-s3
Storage Format: Encrypted (AES-256-CBC)
Storage Handle: phabricator/secure/7l/xs/axqffxi775p6emoz
Default Alt Text: D15339.diff (36 KB)

D15339.diffNo OneTemporaryActions

D15339.diffView Options

File Metadata

Event Timeline

D15339.diff
No OneTemporary
Actions

D15339.diff
View Options