[Meta, Plan] Pitch-Burgh: Policy based access in core

Created on 29 June 2023, over 1 year ago
Updated 12 April 2024, 8 months ago

Problem/Motivation

This is the meta-issue for fulfillment of deliverables for the winning "Pitch-Burgh" proposal shown here. It will be updated as subtasks are identified/completed.

Completed tasks

Remaining tasks

๐ŸŒฑ Plan
Status

Fixed

Version

10.3 โœจ

Component
Baseย  โ†’

Last updated 2 days ago

Created by

๐Ÿ‡ง๐Ÿ‡ชBelgium kristiaanvandeneynde Antwerp, Belgium

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @kristiaanvandeneynde
  • ๐Ÿ‡ซ๐Ÿ‡ฎFinland lauriii Finland

    I think it would be great to have framework managers review the plan on a high level to make sure that it's something that fits in core.

  • ๐Ÿ‡ง๐Ÿ‡ชBelgium kristiaanvandeneynde Antwerp, Belgium

    For those interested, the current pitch is about getting this into core. I was asked about what would happen if we did not get this into core and ended up with the IS as shown here. The current pitch looks more like this:

    The scope of the work is to add an API to Drupal core that allows people to assign permissions via other means than user roles. These permissions will be assigned based on custom policies such as time of day, safety level of the account, etc.

    The initial goal is to convert Drupal's current 2 policies (user roles, UID 1) into the new API and clearly document how to utilize the API. A stretch goal would be to add an example module that already implements the "office hours" policy.

    The milestones are threefold, but two have been reached already:

    • DONE
    • DONE
    • Convert Flexible Permissions into core code and also convert UID 1 / user roles into policies

    At the end of the project you should have a merge request in the Drupal core issue queue, some documentation on how to use this (will need guidance on where to write this) and hopefully as a stretch goal an example module.

    More specifically, we are looking at the following (lots of copy-paste, code already exists):

    1. Move all of Flexible Permissions code into the Drupal\Core\Session namespace and add AccessPolicyInterface::SCOPE_DRUPAL
    2. Change the PermissionHashGenerator to mimic GroupPermissionHashGenerator
    3. Change AccountPermissionsCacheContext to mimic GroupPermissionsCacheContext, namely using cacheable metadata from the hash generator
    4. Write a RoleBasedAccessPolicy (name pending) class that calculates permissions based on which user roles you have
    5. Write a SuperUserAccessPolicy (name pending) class that grants all permissions to user ID 1
    6. Check if all tests still go green and adjust where needed

    Should the DA ask to focus on getting this to work in contrib rather than core, then we could skip step 1, try to swap out coreโ€™s services for step 2 and 3 and still carry out step 4 and 5. Step 6 would be difficult because coreโ€™s tests would not know about us swapping out the access layer.

  • ๐Ÿ‡ง๐Ÿ‡ชBelgium kristiaanvandeneynde Antwerp, Belgium

    Whoops

  • ๐Ÿ‡ฆ๐Ÿ‡บAustralia larowlan ๐Ÿ‡ฆ๐Ÿ‡บ๐Ÿ.au GMT+10

    This sounds like a great plan, and a good way to go about matching velocity with deliverables.

    Removing the tag

  • ๐Ÿ‡ฆ๐Ÿ‡บAustralia larowlan ๐Ÿ‡ฆ๐Ÿ‡บ๐Ÿ.au GMT+10

    Adding a really old related issue that this might provide a path forward on

  • ๐Ÿ‡ง๐Ÿ‡ชBelgium kristiaanvandeneynde Antwerp, Belgium

    Updated IS to reflect #3 getting approval

  • ๐Ÿ‡ง๐Ÿ‡ชBelgium kristiaanvandeneynde Antwerp, Belgium
  • ๐Ÿ‡ง๐Ÿ‡ชBelgium kristiaanvandeneynde Antwerp, Belgium

    Updated the IS to point to child tasks for an explanation of the remaining work.

  • ๐Ÿ‡ง๐Ÿ‡ชBelgium kristiaanvandeneynde Antwerp, Belgium
  • ๐Ÿ‡ง๐Ÿ‡ชBelgium kristiaanvandeneynde Antwerp, Belgium

    Added the documentation that could go into the official drupal.org documentation here: ๐Ÿ“Œ Document the new access policy API Needs review

  • ๐Ÿ‡ง๐Ÿ‡ชBelgium kristiaanvandeneynde Antwerp, Belgium
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States frob US

    There is a 13 year old core issue about expanding the Entity Access system to match that of the Node Access system that should be considered here as well. โœจ Add an entity query access API and deprecate hook_query_ENTITY_TYPE_access_alter() Needs work Work has been done on that issue and a contrib module has been created to provide the functionality described there. https://www.drupal.org/project/raft_entity_access โ†’

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States frob US
  • ๐Ÿ‡ง๐Ÿ‡ชBelgium kristiaanvandeneynde Antwerp, Belgium

    That is about query access, which is completely different to what is being worked on here. The work here precedes that by building your permissions following policies. You can still take your built permissions to a query and try to get entity query access working.

    Core currently has no generic entity query access, only node query access, and I even helped work on the system that went into Entity API to fix that problem space in contrib. It still needs fixing in core in my opinion, but that's not what this issue is about.

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States frob US

    I think I've got it. This isn't about access at all but rather rules surrounding permissions and roles. Instead of a permission being an absolute value of yes or no it becomes an "it depends" type of check.

    The reason I linked the other issue is that I could see policy that is checking a value within the entity (or a linking entity) to see if the policy is applied.

    Some example scenarios:

    • Delete nodes created after a certain date
    • Manage nodes that link to a taxonomy or term

    Something in core that could depend on this API are the CRUD based own node permissions.

    • View own nodes
    • Edit own nodes
    • Delete own nodes

    View published content is another place I could see switching to a policy over a permission.

  • ๐Ÿ‡ง๐Ÿ‡ชBelgium kristiaanvandeneynde Antwerp, Belgium

    No not quite.

    Policies determine which permissions you get in the system based on things you can deduce from the global context. As long as that context doesn't change, you always get the same permissions from these policies. What you're describing isn't Policy Based Access Control (PBAC), but Attribute Based Access Control (ABAC), which we already have in core in the form of hook_entity_access. You could perfectly take care of both your example scenarios with simple entity access hooks.

    Where policy based access shines is that any permission check, doesn't even have to be entity related, can now have its outcome affected by which policies are currently active for the user. It could be that you receive entity-related permissions from a policy, but it's definitely not limited to that.

    So it's more reasonable to consider ABAC a subset of PBAC. ABAC typically checks for both attributes and a permission, e.g.: "view unpublished content" is the permission part (PBAC) but the fact that this permission will always be checked in tandem with the "status" field is the ABAC part.

    You can have PBAC without ABAC, but the other way around would often be really weird (although not inconceivable).

  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom rivimey

    In #3 you mention the stretch goal -- I would suggest that this and a number of other policies (that make sense) are implemented to ensure that the API is in fact sufficient to implement them, even if those additional policies are later left as guide examples or even abandoned. With this sort of API it is really hard to ensure you have all the right interfaces and the right data available up front...

  • ๐Ÿ‡ง๐Ÿ‡ชBelgium kristiaanvandeneynde Antwerp, Belgium

    Agreed that it's rather hard to predict what people will be doing with the API, but keep in mind that Group has been running on this for quite a while already. So there is an example of it being useful to contrib. Having said that, the contract stipulates that the primary goal is to get this into core. If there's any budget left after that, I can definitely cook up a mini module or two that cover a single access policy.

  • ๐Ÿ‡ญ๐Ÿ‡บHungary nevergone Nyรญregyhรกza, Hungary, Europe

    Is this expected in 11.0?

  • ๐Ÿ‡ง๐Ÿ‡ชBelgium kristiaanvandeneynde Antwerp, Belgium

    10.3 if we can get the last issue committed in time. Part of it already is in 10.3

  • ๐Ÿ‡ญ๐Ÿ‡บHungary Gรกbor Hojtsy Hungary

    ๐Ÿ“Œ Implement the new access policy API Needs work landed, woot! What else is left here!

  • Issue was unassigned.
  • Status changed to Fixed 9 months ago
  • ๐Ÿ‡ง๐Ÿ‡ชBelgium kristiaanvandeneynde Antwerp, Belgium

    Nothing :)

    Crediting those I spoke to about getting the ball rolling + those who helped with the many reviews. Feel free to ping me if I forgot anyone.

  • ๐Ÿ‡ซ๐Ÿ‡ทFrance andypost

    Thank you! Awaiting good write-ups to spread the knowledge

  • ๐Ÿ‡ง๐Ÿ‡ชBelgium kristiaanvandeneynde Antwerp, Belgium

    I submitted a session on this subject, would be cool if it got accepted.

  • Thank you everyone for the great work.
    One less dependency for the sites using group :)

  • Automatically closed - issue fixed for 2 weeks with no activity.

Production build 0.71.5 2024