Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F13955631
D15339.id36992.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
36 KB
Referenced Files
None
Subscribers
None
D15339.id36992.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',
@@ -4002,6 +4000,7 @@
'PhabricatorApplicationTransactionInterface',
'AlmanacPropertyInterface',
'PhabricatorDestructibleInterface',
+ 'PhabricatorExtendedPolicyInterface',
),
'AlmanacBindingEditController' => 'AlmanacServiceController',
'AlmanacBindingEditor' => 'AlmanacEditor',
@@ -4019,7 +4018,6 @@
'AlmanacConduitAPIMethod' => 'ConduitAPIMethod',
'AlmanacConsoleController' => 'AlmanacController',
'AlmanacController' => 'PhabricatorController',
- 'AlmanacCreateClusterServicesCapability' => 'PhabricatorPolicyCapability',
'AlmanacCreateDevicesCapability' => 'PhabricatorPolicyCapability',
'AlmanacCreateNamespacesCapability' => 'PhabricatorPolicyCapability',
'AlmanacCreateNetworksCapability' => 'PhabricatorPolicyCapability',
@@ -4036,6 +4034,7 @@
'PhabricatorDestructibleInterface',
'PhabricatorNgramsInterface',
'PhabricatorConduitResultInterface',
+ 'PhabricatorExtendedPolicyInterface',
),
'AlmanacDeviceController' => 'AlmanacController',
'AlmanacDeviceEditController' => 'AlmanacDeviceController',
@@ -4063,10 +4062,9 @@
'AlmanacInterfaceQuery' => 'AlmanacQuery',
'AlmanacInterfaceTableView' => 'AphrontView',
'AlmanacKeys' => 'Phobject',
- 'AlmanacManagementLockWorkflow' => 'AlmanacManagementWorkflow',
+ 'AlmanacManageClusterServicesCapability' => 'PhabricatorPolicyCapability',
'AlmanacManagementRegisterWorkflow' => 'AlmanacManagementWorkflow',
'AlmanacManagementTrustKeyWorkflow' => 'AlmanacManagementWorkflow',
- 'AlmanacManagementUnlockWorkflow' => 'AlmanacManagementWorkflow',
'AlmanacManagementUntrustKeyWorkflow' => 'AlmanacManagementWorkflow',
'AlmanacManagementWorkflow' => 'PhabricatorManagementWorkflow',
'AlmanacNames' => 'Phobject',
@@ -4136,6 +4134,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
Details
Attached
Mime Type
text/plain
Expires
Oct 15 2024, 2:17 AM (4 w, 6 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6710937
Default Alt Text
D15339.id36992.diff (36 KB)
Attached To
Mode
D15339: Simplify locking of Almanac cluster services
Attached
Detach File
Event Timeline
Log In to Comment