@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator

Allow different policy rules for different types of objects

Summary:
Ref T5681. Policy rules can now select objects they can apply to, so a rule like "task author" only shows up where it makes sense (when defining task policies).

This will let us define rules like "members of thread" in Conpherence, "subscribers", etc., to make custom policies more flexible.

Notes:

- Per D13251, we need to do a little work to get the right options for policies like "Maniphest > Default View Policy". This should allow "task" policies.
- This implements a "task author" policy as a simple example.
- The `willApplyRule()` signature now accepts `$objects` to support bulk-loading things like subscribers.

Test Plan:
- Defined a task to be "visible to: task author", verified author could see it and other users could not.
- `var_dump()`'d willApplyRule() inputs, verified they were correct (exactly the objects which use the rule).
- Set `default view policy` to a task-specific policy.
- Verified that other policies like "Can Use Bulk Editor" don't have these options.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5681

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

+220 -41
+11 -11
resources/celerity/map.php
··· 391 391 'rsrc/js/application/phortune/behavior-stripe-payment-form.js' => '3f5d6dbf', 392 392 'rsrc/js/application/phortune/behavior-test-payment-form.js' => 'fc91ab6c', 393 393 'rsrc/js/application/phortune/phortune-credit-card-form.js' => '2290aeef', 394 - 'rsrc/js/application/policy/behavior-policy-control.js' => '9a340b3d', 394 + 'rsrc/js/application/policy/behavior-policy-control.js' => '7d470398', 395 395 'rsrc/js/application/policy/behavior-policy-rule-editor.js' => '5e9f347c', 396 396 'rsrc/js/application/ponder/behavior-votebox.js' => '4e9b766b', 397 397 'rsrc/js/application/projects/behavior-project-boards.js' => 'ba4fa35c', ··· 624 624 'javelin-behavior-pholio-mock-view' => 'fbe497e7', 625 625 'javelin-behavior-phui-dropdown-menu' => '54733475', 626 626 'javelin-behavior-phui-object-box-tabs' => '2bfa2836', 627 - 'javelin-behavior-policy-control' => '9a340b3d', 627 + 'javelin-behavior-policy-control' => '7d470398', 628 628 'javelin-behavior-policy-rule-editor' => '5e9f347c', 629 629 'javelin-behavior-ponder-votebox' => '4e9b766b', 630 630 'javelin-behavior-project-boards' => 'ba4fa35c', ··· 1413 1413 'javelin-request', 1414 1414 'javelin-router', 1415 1415 ), 1416 + '7d470398' => array( 1417 + 'javelin-behavior', 1418 + 'javelin-dom', 1419 + 'javelin-util', 1420 + 'phuix-dropdown-menu', 1421 + 'phuix-action-list-view', 1422 + 'phuix-action-view', 1423 + 'javelin-workflow', 1424 + ), 1416 1425 '7e41274a' => array( 1417 1426 'javelin-install', 1418 1427 ), ··· 1574 1583 'javelin-uri', 1575 1584 'javelin-behavior-device', 1576 1585 'phabricator-title', 1577 - ), 1578 - '9a340b3d' => array( 1579 - 'javelin-behavior', 1580 - 'javelin-dom', 1581 - 'javelin-util', 1582 - 'phuix-dropdown-menu', 1583 - 'phuix-action-list-view', 1584 - 'phuix-action-view', 1585 - 'javelin-workflow', 1586 1586 ), 1587 1587 '9f36c42d' => array( 1588 1588 'javelin-behavior',
+2
src/__phutil_library_map__.php
··· 1051 1051 'ManiphestStatusEmailCommand' => 'applications/maniphest/command/ManiphestStatusEmailCommand.php', 1052 1052 'ManiphestSubpriorityController' => 'applications/maniphest/controller/ManiphestSubpriorityController.php', 1053 1053 'ManiphestTask' => 'applications/maniphest/storage/ManiphestTask.php', 1054 + 'ManiphestTaskAuthorPolicyRule' => 'applications/maniphest/policyrule/ManiphestTaskAuthorPolicyRule.php', 1054 1055 'ManiphestTaskClosedStatusDatasource' => 'applications/maniphest/typeahead/ManiphestTaskClosedStatusDatasource.php', 1055 1056 'ManiphestTaskDependedOnByTaskEdgeType' => 'applications/maniphest/edge/ManiphestTaskDependedOnByTaskEdgeType.php', 1056 1057 'ManiphestTaskDependsOnTaskEdgeType' => 'applications/maniphest/edge/ManiphestTaskDependsOnTaskEdgeType.php', ··· 4412 4413 'PhabricatorProjectInterface', 4413 4414 'PhabricatorSpacesInterface', 4414 4415 ), 4416 + 'ManiphestTaskAuthorPolicyRule' => 'PhabricatorPolicyRule', 4415 4417 'ManiphestTaskClosedStatusDatasource' => 'PhabricatorTypeaheadDatasource', 4416 4418 'ManiphestTaskDependedOnByTaskEdgeType' => 'PhabricatorEdgeType', 4417 4419 'ManiphestTaskDependsOnTaskEdgeType' => 'PhabricatorEdgeType',
+6
src/applications/base/PhabricatorApplication.php
··· 583 583 } 584 584 585 585 public function getCapabilityTemplatePHIDType($capability) { 586 + switch ($capability) { 587 + case PhabricatorPolicyCapability::CAN_VIEW: 588 + case PhabricatorPolicyCapability::CAN_EDIT: 589 + return null; 590 + } 591 + 586 592 $spec = $this->getCustomCapabilitySpecification($capability); 587 593 return idx($spec, 'template'); 588 594 }
+31
src/applications/maniphest/policyrule/ManiphestTaskAuthorPolicyRule.php
··· 1 + <?php 2 + 3 + final class ManiphestTaskAuthorPolicyRule 4 + extends PhabricatorPolicyRule { 5 + 6 + public function getRuleDescription() { 7 + return pht('task author'); 8 + } 9 + 10 + public function canApplyToObject(PhabricatorPolicyInterface $object) { 11 + return ($object instanceof ManiphestTask); 12 + } 13 + 14 + public function applyRule( 15 + PhabricatorUser $viewer, 16 + $value, 17 + PhabricatorPolicyInterface $object) { 18 + 19 + $viewer_phid = $viewer->getPHID(); 20 + if (!$viewer_phid) { 21 + return false; 22 + } 23 + 24 + return ($object->getAuthorPHID() == $viewer_phid); 25 + } 26 + 27 + public function getValueControlType() { 28 + return self::CONTROL_TYPE_NONE; 29 + } 30 + 31 + }
+9 -3
src/applications/meta/controller/PhabricatorApplicationEditController.php
··· 124 124 ->setValue(idx($descriptions, $capability)) 125 125 ->setCaption($caption)); 126 126 } else { 127 - $form->appendChild( 128 - id(new AphrontFormPolicyControl()) 127 + $control = id(new AphrontFormPolicyControl()) 129 128 ->setUser($user) 130 129 ->setDisabled($locked) 131 130 ->setCapability($capability) ··· 133 132 ->setPolicies($policies) 134 133 ->setLabel($label) 135 134 ->setName('policy:'.$capability) 136 - ->setCaption($caption)); 135 + ->setCaption($caption); 136 + 137 + $template = $application->getCapabilityTemplatePHIDType($capability); 138 + if ($template) { 139 + $control->setTemplatePHIDType($template); 140 + } 141 + 142 + $form->appendControl($control); 137 143 } 138 144 139 145 }
+9 -1
src/applications/policy/application/PhabricatorPolicyApplication.php
··· 19 19 '/policy/' => array( 20 20 'explain/(?P<phid>[^/]+)/(?P<capability>[^/]+)/' 21 21 => 'PhabricatorPolicyExplainController', 22 - 'edit/(?:(?P<phid>[^/]+)/)?' => 'PhabricatorPolicyEditController', 22 + 'edit/'. 23 + '(?:'. 24 + 'object/(?P<objectPHID>[^/]+)'. 25 + '|'. 26 + 'type/(?P<objectType>[^/]+)'. 27 + '|'. 28 + 'template/(?P<templateType>[^/]+)'. 29 + ')/'. 30 + '(?:(?P<phid>[^/]+)/)?' => 'PhabricatorPolicyEditController', 23 31 ), 24 32 ); 25 33 }
+38 -2
src/applications/policy/controller/PhabricatorPolicyEditController.php
··· 4 4 extends PhabricatorPolicyController { 5 5 6 6 public function handleRequest(AphrontRequest $request) { 7 - $request = $this->getRequest(); 8 - $viewer = $request->getUser(); 7 + $viewer = $this->getViewer(); 8 + 9 + // TODO: This doesn't do anything yet, but sets up template policies; see 10 + // T6860. 11 + $is_template = false; 12 + 13 + $object_phid = $request->getURIData('objectPHID'); 14 + if ($object_phid) { 15 + $object = id(new PhabricatorObjectQuery()) 16 + ->setViewer($viewer) 17 + ->withPHIDs(array($object_phid)) 18 + ->executeOne(); 19 + if (!$object) { 20 + return new Aphront404Response(); 21 + } 22 + } else { 23 + $object_type = $request->getURIData('objectType'); 24 + if (!$object_type) { 25 + $object_type = $request->getURIData('templateType'); 26 + $is_template = true; 27 + } 28 + 29 + $phid_types = PhabricatorPHIDType::getAllInstalledTypes($viewer); 30 + if (empty($phid_types[$object_type])) { 31 + return new Aphront404Response(); 32 + } 33 + $object = $phid_types[$object_type]->newObject(); 34 + if (!$object) { 35 + return new Aphront404Response(); 36 + } 37 + } 9 38 10 39 $action_options = array( 11 40 PhabricatorPolicy::ACTION_ALLOW => pht('Allow'), ··· 15 44 $rules = id(new PhutilSymbolLoader()) 16 45 ->setAncestorClass('PhabricatorPolicyRule') 17 46 ->loadObjects(); 47 + 48 + foreach ($rules as $key => $rule) { 49 + if (!$rule->canApplyToObject($object)) { 50 + unset($rules[$key]); 51 + } 52 + } 53 + 18 54 $rules = msort($rules, 'getRuleOrder'); 19 55 20 56 $default_rule = array(
+39 -14
src/applications/policy/filter/PhabricatorPolicyFilter.php
··· 149 149 } 150 150 151 151 if ($type == PhabricatorPolicyPHIDTypePolicy::TYPECONST) { 152 - $need_policies[$policy] = $policy; 152 + $need_policies[$policy][] = $object; 153 153 } 154 154 } 155 155 } 156 156 157 157 if ($need_policies) { 158 - $this->loadCustomPolicies(array_keys($need_policies)); 158 + $this->loadCustomPolicies($need_policies); 159 159 } 160 160 161 161 // If we need projects, check if any of the projects we need are also the ··· 500 500 $this->rejectObject($object, $policy, $capability); 501 501 } 502 502 } else if ($type == PhabricatorPolicyPHIDTypePolicy::TYPECONST) { 503 - if ($this->checkCustomPolicy($policy)) { 503 + if ($this->checkCustomPolicy($policy, $object)) { 504 504 return true; 505 505 } else { 506 506 $this->rejectObject($object, $policy, $capability); ··· 573 573 throw $exception; 574 574 } 575 575 576 - private function loadCustomPolicies(array $phids) { 576 + private function loadCustomPolicies(array $map) { 577 577 $viewer = $this->viewer; 578 578 $viewer_phid = $viewer->getPHID(); 579 579 580 580 $custom_policies = id(new PhabricatorPolicyQuery()) 581 581 ->setViewer($viewer) 582 - ->withPHIDs($phids) 582 + ->withPHIDs(array_keys($map)) 583 583 ->execute(); 584 584 $custom_policies = mpull($custom_policies, null, 'getPHID'); 585 - 586 585 587 586 $classes = array(); 588 587 $values = array(); 589 - foreach ($custom_policies as $policy) { 588 + $objects = array(); 589 + foreach ($custom_policies as $policy_phid => $policy) { 590 590 foreach ($policy->getCustomRuleClasses() as $class) { 591 591 $classes[$class] = $class; 592 592 $values[$class][] = $policy->getCustomRuleValues($class); 593 + 594 + foreach (idx($map, $policy_phid, array()) as $object) { 595 + $objects[$class][] = $object; 596 + } 593 597 } 594 598 } 595 599 596 600 foreach ($classes as $class => $ignored) { 597 - $object = newv($class, array()); 598 - $object->willApplyRules($viewer, array_mergev($values[$class])); 599 - $classes[$class] = $object; 601 + $rule_object = newv($class, array()); 602 + 603 + // Filter out any objects which the rule can't apply to. 604 + $target_objects = idx($objects, $class, array()); 605 + foreach ($target_objects as $key => $target_object) { 606 + if (!$rule_object->canApplyToObject($target_object)) { 607 + unset($target_objects[$key]); 608 + } 609 + } 610 + 611 + $rule_object->willApplyRules( 612 + $viewer, 613 + array_mergev($values[$class]), 614 + $target_objects); 615 + 616 + $classes[$class] = $rule_object; 600 617 } 601 618 602 619 foreach ($custom_policies as $policy) { ··· 610 627 $this->customPolicies[$viewer->getPHID()] += $custom_policies; 611 628 } 612 629 613 - private function checkCustomPolicy($policy_phid) { 630 + private function checkCustomPolicy( 631 + $policy_phid, 632 + PhabricatorPolicyInterface $object) { 633 + 614 634 $viewer = $this->viewer; 615 635 $viewer_phid = $viewer->getPHID(); 616 636 ··· 623 643 $objects = $policy->getRuleObjects(); 624 644 $action = null; 625 645 foreach ($policy->getRules() as $rule) { 626 - $object = idx($objects, idx($rule, 'rule')); 627 - if (!$object) { 646 + $rule_object = idx($objects, idx($rule, 'rule')); 647 + if (!$rule_object) { 628 648 // Reject, this policy has a bogus rule. 629 649 return false; 630 650 } 631 651 652 + if (!$rule_object->canApplyToObject($object)) { 653 + // Reject, this policy rule can't be applied to the given object. 654 + return false; 655 + } 656 + 632 657 // If the user matches this rule, use this action. 633 - if ($object->applyRule($viewer, idx($rule, 'value'))) { 658 + if ($rule_object->applyRule($viewer, idx($rule, 'value'), $object)) { 634 659 $action = idx($rule, 'action'); 635 660 break; 636 661 }
+4 -1
src/applications/policy/rule/PhabricatorAdministratorsPolicyRule.php
··· 6 6 return pht('administrators'); 7 7 } 8 8 9 - public function applyRule(PhabricatorUser $viewer, $value) { 9 + public function applyRule( 10 + PhabricatorUser $viewer, 11 + $value, 12 + PhabricatorPolicyInterface $object) { 10 13 return $viewer->getIsAdmin(); 11 14 } 12 15
+11 -2
src/applications/policy/rule/PhabricatorLegalpadSignaturePolicyRule.php
··· 9 9 return pht('signers of legalpad documents'); 10 10 } 11 11 12 - public function willApplyRules(PhabricatorUser $viewer, array $values) { 12 + public function willApplyRules( 13 + PhabricatorUser $viewer, 14 + array $values, 15 + array $objects) { 16 + 13 17 $values = array_unique(array_filter(array_mergev($values))); 14 18 if (!$values) { 15 19 return; ··· 26 30 $this->signatures = mpull($documents, 'getPHID', 'getPHID'); 27 31 } 28 32 29 - public function applyRule(PhabricatorUser $viewer, $value) { 33 + public function applyRule( 34 + PhabricatorUser $viewer, 35 + $value, 36 + PhabricatorPolicyInterface $object) { 37 + 30 38 foreach ($value as $document_phid) { 31 39 if (!isset($this->signatures[$document_phid])) { 32 40 return false; 33 41 } 34 42 } 43 + 35 44 return true; 36 45 } 37 46
+5 -1
src/applications/policy/rule/PhabricatorLunarPhasePolicyRule.php
··· 11 11 return pht('when the moon'); 12 12 } 13 13 14 - public function applyRule(PhabricatorUser $viewer, $value) { 14 + public function applyRule( 15 + PhabricatorUser $viewer, 16 + $value, 17 + PhabricatorPolicyInterface $object) { 18 + 15 19 $moon = new PhutilLunarPhase(PhabricatorTime::getNow()); 16 20 17 21 switch ($value) {
+18 -2
src/applications/policy/rule/PhabricatorPolicyRule.php
··· 8 8 const CONTROL_TYPE_NONE = 'none'; 9 9 10 10 abstract public function getRuleDescription(); 11 - abstract public function applyRule(PhabricatorUser $viewer, $value); 11 + abstract public function applyRule( 12 + PhabricatorUser $viewer, 13 + $value, 14 + PhabricatorPolicyInterface $object); 12 15 13 - public function willApplyRules(PhabricatorUser $viewer, array $values) { 16 + public function willApplyRules( 17 + PhabricatorUser $viewer, 18 + array $values, 19 + array $objects) { 14 20 return; 15 21 } 16 22 ··· 20 26 21 27 public function getValueControlTemplate() { 22 28 return null; 29 + } 30 + 31 + /** 32 + * Return `true` if this rule can be applied to the given object. 33 + * 34 + * Some policy rules may only operation on certain kinds of objects. For 35 + * example, a "task author" rule 36 + */ 37 + public function canApplyToObject(PhabricatorPolicyInterface $object) { 38 + return true; 23 39 } 24 40 25 41 protected function getDatasourceTemplate(
+11 -2
src/applications/policy/rule/PhabricatorProjectsPolicyRule.php
··· 8 8 return pht('members of projects'); 9 9 } 10 10 11 - public function willApplyRules(PhabricatorUser $viewer, array $values) { 11 + public function willApplyRules( 12 + PhabricatorUser $viewer, 13 + array $values, 14 + array $objects) { 15 + 12 16 $values = array_unique(array_filter(array_mergev($values))); 13 17 if (!$values) { 14 18 return; ··· 24 28 } 25 29 } 26 30 27 - public function applyRule(PhabricatorUser $viewer, $value) { 31 + public function applyRule( 32 + PhabricatorUser $viewer, 33 + $value, 34 + PhabricatorPolicyInterface $object) { 35 + 28 36 foreach ($value as $project_phid) { 29 37 if (isset($this->memberships[$viewer->getPHID()][$project_phid])) { 30 38 return true; 31 39 } 32 40 } 41 + 33 42 return false; 34 43 } 35 44
+6 -1
src/applications/policy/rule/PhabricatorUsersPolicyRule.php
··· 6 6 return pht('users'); 7 7 } 8 8 9 - public function applyRule(PhabricatorUser $viewer, $value) { 9 + public function applyRule( 10 + PhabricatorUser $viewer, 11 + $value, 12 + PhabricatorPolicyInterface $object) { 13 + 10 14 foreach ($value as $phid) { 11 15 if ($phid == $viewer->getPHID()) { 12 16 return true; 13 17 } 14 18 } 19 + 15 20 return false; 16 21 } 17 22
+19
src/view/form/control/AphrontFormPolicyControl.php
··· 6 6 private $capability; 7 7 private $policies; 8 8 private $spacePHID; 9 + private $templatePHIDType; 9 10 10 11 public function setPolicyObject(PhabricatorPolicyInterface $object) { 11 12 $this->object = $object; ··· 25 26 26 27 public function getSpacePHID() { 27 28 return $this->spacePHID; 29 + } 30 + 31 + public function setTemplatePHIDType($type) { 32 + $this->templatePHIDType = $type; 33 + return $this; 28 34 } 29 35 30 36 public function setCapability($capability) { ··· 178 184 } 179 185 180 186 187 + if ($this->templatePHIDType) { 188 + $context_path = 'template/'.$this->templatePHIDType.'/'; 189 + } else { 190 + $object_phid = $this->object->getPHID(); 191 + if ($object_phid) { 192 + $context_path = 'object/'.$object_phid.'/'; 193 + } else { 194 + $object_type = phid_get_type($this->object->generatePHID()); 195 + $context_path = 'type/'.$object_type.'/'; 196 + } 197 + } 198 + 181 199 Javelin::initBehavior( 182 200 'policy-control', 183 201 array( ··· 190 208 'labels' => $labels, 191 209 'value' => $this->getValue(), 192 210 'capability' => $this->capability, 211 + 'editURI' => '/policy/edit/'.$context_path, 193 212 'customPlaceholder' => $this->getCustomPolicyPlaceholder(), 194 213 )); 195 214
+1 -1
webroot/rsrc/js/application/policy/behavior-policy-control.js
··· 101 101 * Get the workflow URI to create or edit a policy with a given PHID. 102 102 */ 103 103 var get_custom_uri = function(phid, capability) { 104 - var uri = '/policy/edit/'; 104 + var uri = config.editURI; 105 105 if (phid != config.customPlaceholder) { 106 106 uri += phid + '/'; 107 107 }