Page MenuHomePhabricator
Create Task
Maniphest T5681

Allow applications to provide custom policy rules
Closed, ResolvedPublic
Actions

Description

Currently, custom policies can only use global rules like "users", "members of project", etc.

This prevents creating a default edit rule for tasks like "the task author, or administrators". However, there shouldn't be any technical reason that we can't support and evaluate "the task author" as a rule, because we always know (or always can know) what kind of object a policy is being applied to, and policy rules are already pluggable.

This probably doesn't have a huge amount of general purpose utility, but would allow installs to effectively build more custom policy behaviors without needing patches or hacks.

Revisions and Commits

Restricted Differential RevisionRestricted Diffusion Commit
rP Phabricator
D13259rP3de3a72dd8c7 Add a "Subscribers" object policy
D13258rP893c7a26c196 Add a "Thread Members" object policy and some unit tests
D13257rP466755476aed Allow PolicyRules to serve as "Object Policies"
D13252rP7f98a8575d0a Allow different policy rules for different types of objects
D13251rP52f8756c3c7c Add a "template" parameter to application default policies

Related Objects
Search...

Event Timeline

epriestley created this task.Jul 22 2014, 2:00 PM
epriestley raised the priority of this task from to Normal.
epriestley updated the task description. (Show Details)
epriestley added a project: Policy.
epriestley added a subscriber: epriestley.

Some discussion here:

https://secure.phabricator.com/chatlog/channel/6/?at=152617

See also D10012.

epriestley added a comment.Jul 22 2014, 2:07 PM

This requires some changes to the policy framework, but I think nothing too severe:

  • PhabricatorPolicyRule needs to know about the object the rule is being applied to. Currently, it only knows about the viewer. However, it appears trivial to thread the object through to the rule application.
  • PhabricatorPolicyRule needs to have methods like canApplyToObject($object) to allow rules to activate when applied to tasks and deactivate when applied to other types of objects.
  • PhabricatorPolicyEditController needs to be passed the type of object it is editing policies for (probably a PHID?)
  • AphrontFormPolicyControl needs to be passed this information so it can pass it to the EditController.
  • Application policies probably need some additional config.
qgil added a subscriber: qgil.Jul 22 2014, 2:17 PM
mikaye added a subscriber: mikaye.Jul 22 2014, 2:21 PM
sascha-egerer added a subscriber: sascha-egerer.Jul 22 2014, 2:38 PM
sergey.vfx added a subscriber: sergey.vfx.Jul 22 2014, 5:16 PM
epriestley merged a task: T6178: Add "Author" as a parameter for custom policies.Sep 24 2014, 8:33 PM
aklapper added a subscriber: aklapper.Sep 24 2014, 9:57 PM
chad added a subscriber: chad.Nov 4 2014, 3:13 AM
epriestley mentioned this in T6502: Policy to define who can move cards in a workboard.Nov 8 2014, 4:11 PM
joshuaspence added a subscriber: joshuaspence.Nov 9 2014, 10:09 AM
epriestley mentioned this in D10888: Add default policy to Files application.Nov 21 2014, 12:36 AM
Krenair added a subscriber: Krenair.Jan 3 2015, 6:17 PM
epriestley mentioned this in T6860: Allow creation of parameterized application policies.Jan 4 2015, 4:28 PM
epriestley added a parent task: T6306: Add a "members of associated projects" visibility setting to repositories.Jan 5 2015, 6:06 PM
chad mentioned this in T7404: Projects that are joinable by no-one or require legalpad documents should require special privileges to add users.Feb 28 2015, 3:23 AM
devurandom added a subscriber: devurandom.Mar 2 2015, 10:26 AM
eadler added a project: FreeBSD.May 22 2015, 10:15 PM
eadler added a subscriber: eadler.
epriestley mentioned this in T3820: Implement top-level "Spaces" that provide policy isolation to groups of objects.May 28 2015, 6:08 PM
robclancy added a subscriber: robclancy.May 29 2015, 5:04 PM
epriestley mentioned this in T8434: Accommodate the "Security" workflow.Jun 5 2015, 5:05 PM
20after4 added a subscriber: 20after4.Jun 6 2015, 12:39 AM

In case it's helpful: Here's a real-world use case: custom policy rule I wrote to allow "Subscribers of this task" WMFSubscribersPolicyRule

It requires this nasty hack to fill in the task id because I couldn't find any way to get the task id inside of applyRule unless it was hard coded as the rule parameter. So the rule ends up being "Subscribers of task Tnnn" instead of "Subscribers of $this task" with $this being inferred at runtime.

epriestley mentioned this in T8488: Remove "room" vs "message" distinction.Jun 9 2015, 6:43 PM
btrahan added a parent task: T8488: Remove "room" vs "message" distinction.Jun 9 2015, 6:53 PM
epriestley claimed this task.Jun 11 2015, 5:49 PM

We want this for T8488 and it's probably a component in moving T8434 forward.

epriestley added a parent task: T8434: Accommodate the "Security" workflow.Jun 11 2015, 5:50 PM
epriestley mentioned this in T6306: Add a "members of associated projects" visibility setting to repositories.Jun 11 2015, 7:04 PM
epriestley added a parent task: T6860: Allow creation of parameterized application policies.
epriestley added a revision: D13251: Add a "template" parameter to application default policies.Jun 11 2015, 7:08 PM
epriestley added a revision: D13252: Allow different policy rules for different types of objects.Jun 11 2015, 7:24 PM
epriestley added a comment.Jun 11 2015, 7:26 PM

@20after4, D13252 changes the signature of two methods in PhabricatorPolicyRule:

  • willApplyRules() now takes array $objects.
  • applyRule() now takes PhabricatorPolicyInterface $object.

Old rules will need method signatures updated, but will otherwise work the same way they currently do without requiring additional changes.

epriestley added a revision: Restricted Differential Revision.Jun 11 2015, 7:55 PM
epriestley added a commit: rP52f8756c3c7c: Add a "template" parameter to application default policies.Jun 11 2015, 8:25 PM
epriestley added a revision: D13257: Allow PolicyRules to serve as "Object Policies".Jun 11 2015, 11:13 PM
epriestley added a revision: D13258: Add a "Thread Members" object policy and some unit tests.Jun 11 2015, 11:52 PM
epriestley added a revision: D13259: Add a "Subscribers" object policy.Jun 12 2015, 12:44 AM
joshuaspence awarded a token.Jun 12 2015, 11:38 AM
epriestley mentioned this in T8376: Develop Spaces (v1).Jun 12 2015, 7:12 PM
epriestley added a commit: rP7f98a8575d0a: Allow different policy rules for different types of objects.Jun 13 2015, 10:44 PM
epriestley added a commit: rP466755476aed: Allow PolicyRules to serve as "Object Policies".
epriestley added a commit: rP893c7a26c196: Add a "Thread Members" object policy and some unit tests.
epriestley added a commit: rP3de3a72dd8c7: Add a "Subscribers" object policy.
epriestley closed this task as Resolved.Jun 13 2015, 10:50 PM

This now works, and the upstream has implementations for the reasonable-seeming use cases we've seen, particularly the policy that blocks T8488. This is relatively modular and extensible, but I don't think there are any other problems for it to solve right now.

(T6860 is slightly more general; this laid the groundwork for that but can't solve the Passphrase problem on its own.)

epriestley added a commit: Restricted Diffusion Commit.Jun 14 2015, 1:55 PM
epriestley mentioned this in D13091: Modernize Diviner.Jun 16 2015, 2:34 PM
20after4 added a comment.Jun 17 2015, 8:08 AM

@epriestley: thanks for the heads up

btrahan mentioned this in D13351: Conpherence - remove room vs message distinction as far as users are concerned.Jun 18 2015, 11:39 PM
urzds added a subscriber: urzds.Jul 12 2017, 11:13 AM
Phabricator · Built by Phacility