Original thoughts

After working on #2409691: EntityAccessControlHandlers miss entity admin access i gave the whole access topic more thought in order to determine which features are required for d8 and whether the rather complex access system of Rules d7 can be simplified.

So here is what I ended up with is the following summary:

  • For determining creation access / listing access (may I create a plugin instance?) for condition and action plugins, mostly the possibility to specify respective permission(s) as part of the annotation should be enough. Example: administer nodes go for all node actions, access content works for all node conditions. After looking at d7 access implementation examples I think this should work fine given the following:
    • For view access, sometimes one of multiple permissions is enough. Thus, we can support optionally specifying multiple permissions, where at least one must be specified. Example: access content & administer nodes.
    • The only case I found which is rather problematic is creation access for rule components without a dedicated permission. D7 checked configuration access of the whole component then, what we cannot do with a permission based approach. However, I think we can simplify this by defaulting to the "bypass rules access checks" permission being defined for re-usable conditions, plus the possibility to specify an alternatively required permission. (In addition to the existing option to provide a custom permission).
  • For determining configuration access (may I edit this plugin configured by someone else?) it should be enough to have some generic access checking logic based upon the information available as part of the ContextConfig and context definitions. For differentiation edit vs view access we could check the 'save' parameter and/or make it a "changed" parameter instead. Use case: The "Data comparison" needs to check view access for the selected context.
  • For being able to check generic 'view' access as needed for entity conditions, we lack a way to generically fetch the 'view permission' for entities. This is in particular required, for the "Data comparison" condition to be able to check its access.
  • We do not have a generic AccessibleInterface() variant for data definitions, only for TypedData instances. In order to allow integration of any kind of data with Rules, support for this needs to be added to data definitions also.
  • For checking ExecutionAccess the core action system already provides an access() method. Use case: Basically VBO. For conditions, a similar need is unlikely to appear.

Implementation plan

* Create a AccessFilter trait with a filterByAccess($user) method. It will filter plugin definitions based on the access rights of the passed user.
* Create a RulesActionManager class that replaces the core ActionManager service - for being able to add in new features.
* Create a new annotation key "configure_permission" for actions and conditions and add it to one action for testing.

Comments

fago’s picture

dasjo’s picture

klausi’s picture

Issue summary: View changes
dasjo’s picture

Issue tags: +Contributor

this should be ready to pick-up

fago’s picture

Issue summary: View changes

Thought more about the differentiation of execution access and checking for configuration access. Core actions have and need execution access, which is different to configuration access. I think for configuration access it should suffice to do a generic implementation on top of the plugins, which take care of checking access based upon the ContextConfig & context definitions.

Torenware’s picture

Assigned: Unassigned » Torenware

At @klausi's suggestion, I'm taking a look at this. I'll create sub-issues for @fago's plan above to start.

Torenware’s picture

Create a new annotation key "configure_permission" for actions and conditions and add it to one action for testing.

This is implemented in #2593139: Create a new annotation key "configure_permission" for actions and conditions. See https://github.com/fago/rules/pull/287 for the implementation.

Sharique’s picture

Is there any action item required on this ticket or we can close it?

TR’s picture

Assigned: Torenware » Unassigned

On the surface, no, it's not complete.

If you want to help out you can search the issue queue for access-related issues and see if some of these issues can be consolidated or restated so they're easier to understand. I know there are many @todo in the Rules plugins that state that access control needs to be implemented. Since this is a [meta] issue, it should have links to all related issues, and this issue should remain open as long as at least one of those related issues remains open.

This appears to be a good, self-contained project for someone who has Drupal skills (it's not a beginner issue) and wants to contribute to Rules. Could that be you @Sharique?

I've unassigned the issue, because @Torenware hasn't touched it in 4 years and my assumption is he's no longer working on this.