Page MenuHomePhabricator

D13252.diff
No OneTemporary

D13252.diff

diff --git a/resources/celerity/map.php b/resources/celerity/map.php
--- a/resources/celerity/map.php
+++ b/resources/celerity/map.php
@@ -391,7 +391,7 @@
'rsrc/js/application/phortune/behavior-stripe-payment-form.js' => '3f5d6dbf',
'rsrc/js/application/phortune/behavior-test-payment-form.js' => 'fc91ab6c',
'rsrc/js/application/phortune/phortune-credit-card-form.js' => '2290aeef',
- 'rsrc/js/application/policy/behavior-policy-control.js' => '9a340b3d',
+ 'rsrc/js/application/policy/behavior-policy-control.js' => '7d470398',
'rsrc/js/application/policy/behavior-policy-rule-editor.js' => '5e9f347c',
'rsrc/js/application/ponder/behavior-votebox.js' => '4e9b766b',
'rsrc/js/application/projects/behavior-project-boards.js' => 'ba4fa35c',
@@ -624,7 +624,7 @@
'javelin-behavior-pholio-mock-view' => 'fbe497e7',
'javelin-behavior-phui-dropdown-menu' => '54733475',
'javelin-behavior-phui-object-box-tabs' => '2bfa2836',
- 'javelin-behavior-policy-control' => '9a340b3d',
+ 'javelin-behavior-policy-control' => '7d470398',
'javelin-behavior-policy-rule-editor' => '5e9f347c',
'javelin-behavior-ponder-votebox' => '4e9b766b',
'javelin-behavior-project-boards' => 'ba4fa35c',
@@ -1413,6 +1413,15 @@
'javelin-request',
'javelin-router',
),
+ '7d470398' => array(
+ 'javelin-behavior',
+ 'javelin-dom',
+ 'javelin-util',
+ 'phuix-dropdown-menu',
+ 'phuix-action-list-view',
+ 'phuix-action-view',
+ 'javelin-workflow',
+ ),
'7e41274a' => array(
'javelin-install',
),
@@ -1575,15 +1584,6 @@
'javelin-behavior-device',
'phabricator-title',
),
- '9a340b3d' => array(
- 'javelin-behavior',
- 'javelin-dom',
- 'javelin-util',
- 'phuix-dropdown-menu',
- 'phuix-action-list-view',
- 'phuix-action-view',
- 'javelin-workflow',
- ),
'9f36c42d' => array(
'javelin-behavior',
'javelin-stratcom',
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
@@ -1051,6 +1051,7 @@
'ManiphestStatusEmailCommand' => 'applications/maniphest/command/ManiphestStatusEmailCommand.php',
'ManiphestSubpriorityController' => 'applications/maniphest/controller/ManiphestSubpriorityController.php',
'ManiphestTask' => 'applications/maniphest/storage/ManiphestTask.php',
+ 'ManiphestTaskAuthorPolicyRule' => 'applications/maniphest/policyrule/ManiphestTaskAuthorPolicyRule.php',
'ManiphestTaskClosedStatusDatasource' => 'applications/maniphest/typeahead/ManiphestTaskClosedStatusDatasource.php',
'ManiphestTaskDependedOnByTaskEdgeType' => 'applications/maniphest/edge/ManiphestTaskDependedOnByTaskEdgeType.php',
'ManiphestTaskDependsOnTaskEdgeType' => 'applications/maniphest/edge/ManiphestTaskDependsOnTaskEdgeType.php',
@@ -4412,6 +4413,7 @@
'PhabricatorProjectInterface',
'PhabricatorSpacesInterface',
),
+ 'ManiphestTaskAuthorPolicyRule' => 'PhabricatorPolicyRule',
'ManiphestTaskClosedStatusDatasource' => 'PhabricatorTypeaheadDatasource',
'ManiphestTaskDependedOnByTaskEdgeType' => 'PhabricatorEdgeType',
'ManiphestTaskDependsOnTaskEdgeType' => 'PhabricatorEdgeType',
diff --git a/src/applications/base/PhabricatorApplication.php b/src/applications/base/PhabricatorApplication.php
--- a/src/applications/base/PhabricatorApplication.php
+++ b/src/applications/base/PhabricatorApplication.php
@@ -583,6 +583,12 @@
}
public function getCapabilityTemplatePHIDType($capability) {
+ switch ($capability) {
+ case PhabricatorPolicyCapability::CAN_VIEW:
+ case PhabricatorPolicyCapability::CAN_EDIT:
+ return null;
+ }
+
$spec = $this->getCustomCapabilitySpecification($capability);
return idx($spec, 'template');
}
diff --git a/src/applications/maniphest/policyrule/ManiphestTaskAuthorPolicyRule.php b/src/applications/maniphest/policyrule/ManiphestTaskAuthorPolicyRule.php
new file mode 100644
--- /dev/null
+++ b/src/applications/maniphest/policyrule/ManiphestTaskAuthorPolicyRule.php
@@ -0,0 +1,31 @@
+<?php
+
+final class ManiphestTaskAuthorPolicyRule
+ extends PhabricatorPolicyRule {
+
+ public function getRuleDescription() {
+ return pht('task author');
+ }
+
+ public function canApplyToObject(PhabricatorPolicyInterface $object) {
+ return ($object instanceof ManiphestTask);
+ }
+
+ public function applyRule(
+ PhabricatorUser $viewer,
+ $value,
+ PhabricatorPolicyInterface $object) {
+
+ $viewer_phid = $viewer->getPHID();
+ if (!$viewer_phid) {
+ return false;
+ }
+
+ return ($object->getAuthorPHID() == $viewer_phid);
+ }
+
+ public function getValueControlType() {
+ return self::CONTROL_TYPE_NONE;
+ }
+
+}
diff --git a/src/applications/meta/controller/PhabricatorApplicationEditController.php b/src/applications/meta/controller/PhabricatorApplicationEditController.php
--- a/src/applications/meta/controller/PhabricatorApplicationEditController.php
+++ b/src/applications/meta/controller/PhabricatorApplicationEditController.php
@@ -124,8 +124,7 @@
->setValue(idx($descriptions, $capability))
->setCaption($caption));
} else {
- $form->appendChild(
- id(new AphrontFormPolicyControl())
+ $control = id(new AphrontFormPolicyControl())
->setUser($user)
->setDisabled($locked)
->setCapability($capability)
@@ -133,7 +132,14 @@
->setPolicies($policies)
->setLabel($label)
->setName('policy:'.$capability)
- ->setCaption($caption));
+ ->setCaption($caption);
+
+ $template = $application->getCapabilityTemplatePHIDType($capability);
+ if ($template) {
+ $control->setTemplatePHIDType($template);
+ }
+
+ $form->appendControl($control);
}
}
diff --git a/src/applications/policy/application/PhabricatorPolicyApplication.php b/src/applications/policy/application/PhabricatorPolicyApplication.php
--- a/src/applications/policy/application/PhabricatorPolicyApplication.php
+++ b/src/applications/policy/application/PhabricatorPolicyApplication.php
@@ -19,7 +19,15 @@
'/policy/' => array(
'explain/(?P<phid>[^/]+)/(?P<capability>[^/]+)/'
=> 'PhabricatorPolicyExplainController',
- 'edit/(?:(?P<phid>[^/]+)/)?' => 'PhabricatorPolicyEditController',
+ 'edit/'.
+ '(?:'.
+ 'object/(?P<objectPHID>[^/]+)'.
+ '|'.
+ 'type/(?P<objectType>[^/]+)'.
+ '|'.
+ 'template/(?P<templateType>[^/]+)'.
+ ')/'.
+ '(?:(?P<phid>[^/]+)/)?' => 'PhabricatorPolicyEditController',
),
);
}
diff --git a/src/applications/policy/controller/PhabricatorPolicyEditController.php b/src/applications/policy/controller/PhabricatorPolicyEditController.php
--- a/src/applications/policy/controller/PhabricatorPolicyEditController.php
+++ b/src/applications/policy/controller/PhabricatorPolicyEditController.php
@@ -4,8 +4,37 @@
extends PhabricatorPolicyController {
public function handleRequest(AphrontRequest $request) {
- $request = $this->getRequest();
- $viewer = $request->getUser();
+ $viewer = $this->getViewer();
+
+ // TODO: This doesn't do anything yet, but sets up template policies; see
+ // T6860.
+ $is_template = false;
+
+ $object_phid = $request->getURIData('objectPHID');
+ if ($object_phid) {
+ $object = id(new PhabricatorObjectQuery())
+ ->setViewer($viewer)
+ ->withPHIDs(array($object_phid))
+ ->executeOne();
+ if (!$object) {
+ return new Aphront404Response();
+ }
+ } else {
+ $object_type = $request->getURIData('objectType');
+ if (!$object_type) {
+ $object_type = $request->getURIData('templateType');
+ $is_template = true;
+ }
+
+ $phid_types = PhabricatorPHIDType::getAllInstalledTypes($viewer);
+ if (empty($phid_types[$object_type])) {
+ return new Aphront404Response();
+ }
+ $object = $phid_types[$object_type]->newObject();
+ if (!$object) {
+ return new Aphront404Response();
+ }
+ }
$action_options = array(
PhabricatorPolicy::ACTION_ALLOW => pht('Allow'),
@@ -15,6 +44,13 @@
$rules = id(new PhutilSymbolLoader())
->setAncestorClass('PhabricatorPolicyRule')
->loadObjects();
+
+ foreach ($rules as $key => $rule) {
+ if (!$rule->canApplyToObject($object)) {
+ unset($rules[$key]);
+ }
+ }
+
$rules = msort($rules, 'getRuleOrder');
$default_rule = 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
@@ -149,13 +149,13 @@
}
if ($type == PhabricatorPolicyPHIDTypePolicy::TYPECONST) {
- $need_policies[$policy] = $policy;
+ $need_policies[$policy][] = $object;
}
}
}
if ($need_policies) {
- $this->loadCustomPolicies(array_keys($need_policies));
+ $this->loadCustomPolicies($need_policies);
}
// If we need projects, check if any of the projects we need are also the
@@ -500,7 +500,7 @@
$this->rejectObject($object, $policy, $capability);
}
} else if ($type == PhabricatorPolicyPHIDTypePolicy::TYPECONST) {
- if ($this->checkCustomPolicy($policy)) {
+ if ($this->checkCustomPolicy($policy, $object)) {
return true;
} else {
$this->rejectObject($object, $policy, $capability);
@@ -573,30 +573,47 @@
throw $exception;
}
- private function loadCustomPolicies(array $phids) {
+ private function loadCustomPolicies(array $map) {
$viewer = $this->viewer;
$viewer_phid = $viewer->getPHID();
$custom_policies = id(new PhabricatorPolicyQuery())
->setViewer($viewer)
- ->withPHIDs($phids)
+ ->withPHIDs(array_keys($map))
->execute();
$custom_policies = mpull($custom_policies, null, 'getPHID');
-
$classes = array();
$values = array();
- foreach ($custom_policies as $policy) {
+ $objects = array();
+ foreach ($custom_policies as $policy_phid => $policy) {
foreach ($policy->getCustomRuleClasses() as $class) {
$classes[$class] = $class;
$values[$class][] = $policy->getCustomRuleValues($class);
+
+ foreach (idx($map, $policy_phid, array()) as $object) {
+ $objects[$class][] = $object;
+ }
}
}
foreach ($classes as $class => $ignored) {
- $object = newv($class, array());
- $object->willApplyRules($viewer, array_mergev($values[$class]));
- $classes[$class] = $object;
+ $rule_object = newv($class, array());
+
+ // Filter out any objects which the rule can't apply to.
+ $target_objects = idx($objects, $class, array());
+ foreach ($target_objects as $key => $target_object) {
+ if (!$rule_object->canApplyToObject($target_object)) {
+ unset($target_objects[$key]);
+ }
+ }
+
+ $rule_object->willApplyRules(
+ $viewer,
+ array_mergev($values[$class]),
+ $target_objects);
+
+ $classes[$class] = $rule_object;
}
foreach ($custom_policies as $policy) {
@@ -610,7 +627,10 @@
$this->customPolicies[$viewer->getPHID()] += $custom_policies;
}
- private function checkCustomPolicy($policy_phid) {
+ private function checkCustomPolicy(
+ $policy_phid,
+ PhabricatorPolicyInterface $object) {
+
$viewer = $this->viewer;
$viewer_phid = $viewer->getPHID();
@@ -623,14 +643,19 @@
$objects = $policy->getRuleObjects();
$action = null;
foreach ($policy->getRules() as $rule) {
- $object = idx($objects, idx($rule, 'rule'));
- if (!$object) {
+ $rule_object = idx($objects, idx($rule, 'rule'));
+ if (!$rule_object) {
// Reject, this policy has a bogus rule.
return false;
}
+ if (!$rule_object->canApplyToObject($object)) {
+ // Reject, this policy rule can't be applied to the given object.
+ return false;
+ }
+
// If the user matches this rule, use this action.
- if ($object->applyRule($viewer, idx($rule, 'value'))) {
+ if ($rule_object->applyRule($viewer, idx($rule, 'value'), $object)) {
$action = idx($rule, 'action');
break;
}
diff --git a/src/applications/policy/rule/PhabricatorAdministratorsPolicyRule.php b/src/applications/policy/rule/PhabricatorAdministratorsPolicyRule.php
--- a/src/applications/policy/rule/PhabricatorAdministratorsPolicyRule.php
+++ b/src/applications/policy/rule/PhabricatorAdministratorsPolicyRule.php
@@ -6,7 +6,10 @@
return pht('administrators');
}
- public function applyRule(PhabricatorUser $viewer, $value) {
+ public function applyRule(
+ PhabricatorUser $viewer,
+ $value,
+ PhabricatorPolicyInterface $object) {
return $viewer->getIsAdmin();
}
diff --git a/src/applications/policy/rule/PhabricatorLegalpadSignaturePolicyRule.php b/src/applications/policy/rule/PhabricatorLegalpadSignaturePolicyRule.php
--- a/src/applications/policy/rule/PhabricatorLegalpadSignaturePolicyRule.php
+++ b/src/applications/policy/rule/PhabricatorLegalpadSignaturePolicyRule.php
@@ -9,7 +9,11 @@
return pht('signers of legalpad documents');
}
- public function willApplyRules(PhabricatorUser $viewer, array $values) {
+ public function willApplyRules(
+ PhabricatorUser $viewer,
+ array $values,
+ array $objects) {
+
$values = array_unique(array_filter(array_mergev($values)));
if (!$values) {
return;
@@ -26,12 +30,17 @@
$this->signatures = mpull($documents, 'getPHID', 'getPHID');
}
- public function applyRule(PhabricatorUser $viewer, $value) {
+ public function applyRule(
+ PhabricatorUser $viewer,
+ $value,
+ PhabricatorPolicyInterface $object) {
+
foreach ($value as $document_phid) {
if (!isset($this->signatures[$document_phid])) {
return false;
}
}
+
return true;
}
diff --git a/src/applications/policy/rule/PhabricatorLunarPhasePolicyRule.php b/src/applications/policy/rule/PhabricatorLunarPhasePolicyRule.php
--- a/src/applications/policy/rule/PhabricatorLunarPhasePolicyRule.php
+++ b/src/applications/policy/rule/PhabricatorLunarPhasePolicyRule.php
@@ -11,7 +11,11 @@
return pht('when the moon');
}
- public function applyRule(PhabricatorUser $viewer, $value) {
+ public function applyRule(
+ PhabricatorUser $viewer,
+ $value,
+ PhabricatorPolicyInterface $object) {
+
$moon = new PhutilLunarPhase(PhabricatorTime::getNow());
switch ($value) {
diff --git a/src/applications/policy/rule/PhabricatorPolicyRule.php b/src/applications/policy/rule/PhabricatorPolicyRule.php
--- a/src/applications/policy/rule/PhabricatorPolicyRule.php
+++ b/src/applications/policy/rule/PhabricatorPolicyRule.php
@@ -8,9 +8,15 @@
const CONTROL_TYPE_NONE = 'none';
abstract public function getRuleDescription();
- abstract public function applyRule(PhabricatorUser $viewer, $value);
+ abstract public function applyRule(
+ PhabricatorUser $viewer,
+ $value,
+ PhabricatorPolicyInterface $object);
- public function willApplyRules(PhabricatorUser $viewer, array $values) {
+ public function willApplyRules(
+ PhabricatorUser $viewer,
+ array $values,
+ array $objects) {
return;
}
@@ -22,6 +28,16 @@
return null;
}
+ /**
+ * Return `true` if this rule can be applied to the given object.
+ *
+ * Some policy rules may only operation on certain kinds of objects. For
+ * example, a "task author" rule
+ */
+ public function canApplyToObject(PhabricatorPolicyInterface $object) {
+ return true;
+ }
+
protected function getDatasourceTemplate(
PhabricatorTypeaheadDatasource $datasource) {
return array(
diff --git a/src/applications/policy/rule/PhabricatorProjectsPolicyRule.php b/src/applications/policy/rule/PhabricatorProjectsPolicyRule.php
--- a/src/applications/policy/rule/PhabricatorProjectsPolicyRule.php
+++ b/src/applications/policy/rule/PhabricatorProjectsPolicyRule.php
@@ -8,7 +8,11 @@
return pht('members of projects');
}
- public function willApplyRules(PhabricatorUser $viewer, array $values) {
+ public function willApplyRules(
+ PhabricatorUser $viewer,
+ array $values,
+ array $objects) {
+
$values = array_unique(array_filter(array_mergev($values)));
if (!$values) {
return;
@@ -24,12 +28,17 @@
}
}
- public function applyRule(PhabricatorUser $viewer, $value) {
+ public function applyRule(
+ PhabricatorUser $viewer,
+ $value,
+ PhabricatorPolicyInterface $object) {
+
foreach ($value as $project_phid) {
if (isset($this->memberships[$viewer->getPHID()][$project_phid])) {
return true;
}
}
+
return false;
}
diff --git a/src/applications/policy/rule/PhabricatorUsersPolicyRule.php b/src/applications/policy/rule/PhabricatorUsersPolicyRule.php
--- a/src/applications/policy/rule/PhabricatorUsersPolicyRule.php
+++ b/src/applications/policy/rule/PhabricatorUsersPolicyRule.php
@@ -6,12 +6,17 @@
return pht('users');
}
- public function applyRule(PhabricatorUser $viewer, $value) {
+ public function applyRule(
+ PhabricatorUser $viewer,
+ $value,
+ PhabricatorPolicyInterface $object) {
+
foreach ($value as $phid) {
if ($phid == $viewer->getPHID()) {
return true;
}
}
+
return false;
}
diff --git a/src/view/form/control/AphrontFormPolicyControl.php b/src/view/form/control/AphrontFormPolicyControl.php
--- a/src/view/form/control/AphrontFormPolicyControl.php
+++ b/src/view/form/control/AphrontFormPolicyControl.php
@@ -6,6 +6,7 @@
private $capability;
private $policies;
private $spacePHID;
+ private $templatePHIDType;
public function setPolicyObject(PhabricatorPolicyInterface $object) {
$this->object = $object;
@@ -27,6 +28,11 @@
return $this->spacePHID;
}
+ public function setTemplatePHIDType($type) {
+ $this->templatePHIDType = $type;
+ return $this;
+ }
+
public function setCapability($capability) {
$this->capability = $capability;
@@ -178,6 +184,18 @@
}
+ if ($this->templatePHIDType) {
+ $context_path = 'template/'.$this->templatePHIDType.'/';
+ } else {
+ $object_phid = $this->object->getPHID();
+ if ($object_phid) {
+ $context_path = 'object/'.$object_phid.'/';
+ } else {
+ $object_type = phid_get_type($this->object->generatePHID());
+ $context_path = 'type/'.$object_type.'/';
+ }
+ }
+
Javelin::initBehavior(
'policy-control',
array(
@@ -190,6 +208,7 @@
'labels' => $labels,
'value' => $this->getValue(),
'capability' => $this->capability,
+ 'editURI' => '/policy/edit/'.$context_path,
'customPlaceholder' => $this->getCustomPolicyPlaceholder(),
));
diff --git a/webroot/rsrc/js/application/policy/behavior-policy-control.js b/webroot/rsrc/js/application/policy/behavior-policy-control.js
--- a/webroot/rsrc/js/application/policy/behavior-policy-control.js
+++ b/webroot/rsrc/js/application/policy/behavior-policy-control.js
@@ -101,7 +101,7 @@
* Get the workflow URI to create or edit a policy with a given PHID.
*/
var get_custom_uri = function(phid, capability) {
- var uri = '/policy/edit/';
+ var uri = config.editURI;
if (phid != config.customPlaceholder) {
uri += phid + '/';
}

File Metadata

Mime Type
text/plain
Expires
Tue, May 21, 2:46 PM (1 w, 5 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6269854
Default Alt Text
D13252.diff (19 KB)

Event Timeline