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 @@ +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[^/]+)/(?P[^/]+)/' => 'PhabricatorPolicyExplainController', - 'edit/(?:(?P[^/]+)/)?' => 'PhabricatorPolicyEditController', + 'edit/'. + '(?:'. + 'object/(?P[^/]+)'. + '|'. + 'type/(?P[^/]+)'. + '|'. + 'template/(?P[^/]+)'. + ')/'. + '(?:(?P[^/]+)/)?' => '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 + '/'; }