Introduce entity permission providers

Created on 30 September 2016, over 7 years ago
Updated 12 April 2023, about 1 year ago

Problem/Motivation

Right now each content entity type needs to define its set of permissions from scratch, then declare a matching access handler. This is pure boilerplate, an entity type's permissions can very precisely be guessed based on the interfaces it implements and the permission granularity it specifies. Furthermore, requiring each developer to create a new access handler each time leaves room for frequent bugs, such as wrong cacheability metadata.

Proposed resolution

The permissions currently vary based on two factors:

  • EntityOwnerInterface
  • Permission granularity (bundle / entity_type)

Future iterations of the patch / issue followups would also take into account EntityPublishedInterface.

Generated permissions:

  • "administer $entity_type_id" (god mode permission)
  • "access $entity_type_id overview" (the basic permission for listings)
  • "view $entity_type_id" OR "view own $entity_type_id" / "view any $entity_type_id" depending on EntityOwnerInterface
  • create/update/delete permissions per bundle or per entity type, also taking into account EntityOwnerInterface

Note that view permissions are never per-bundle cause we have no way to enforce it, we'd need query access for that (ala node access).

Just like we did for route providers, we introduce the concept of permission providers. That makes this generation opt-in.
Each participating entity type would define the core's permission provider, and the matching access handler. Core calls the permission provider of each entity type when building permissions.

The proposed solution was implemented in the Entity API contrib module ( #2801031: Provide a generic entity access handler and permissions → ) and is used by Commerce and other contrib modules.

Remaining tasks

Roll the patch

📌 Task
Status

Needs work

Version

10.1 ✨

Component
Entity  →

Last updated about 21 hours ago

  • Maintained by
  • 🇬🇧United Kingdom @catch
  • 🇨🇭Switzerland @Berdir
  • 🇩🇪Germany @hchonov
Created by

🇫🇮Finland holist

Live updates comments and jobs are added and updated live.
  • Needs issue summary update

    Issue summaries save everyone time if they are kept up-to-date. See Update issue summary task instructions.

  • Needs subsystem maintainer review

    It is used to alert the maintainer(s) of a particular core subsystem that an issue significantly impacts their subsystem, and their signoff is needed (see the governance policy draft for more information). Also, if you use this tag, make sure the issue component is set to the correct subsystem. If an issue significantly impacts more than one subsystem, use needs framework manager review instead.

  • Needs change record

    A change record needs to be drafted before an issue is committed. Note: Change records used to be called change notifications.

Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • 🇩🇪Germany Anybody Porta Westfalica
  • 🇩🇪Germany Anybody Porta Westfalica

    @bojanz in #53 you wrote:

    New feature (resolving #9 from this issue):
    #2977231: EntityPermissionProvider should provide per-bundle view permissions

    but the issue summary still says:

    Note that view permissions are never per-bundle cause we have no way to enforce it, we'd need query access for that (ala node access).

    I wasn't confident enough to remove that from the issue summary, so could you perhaps have a look to be sure it should be removed?

    Looks like the MR already contains the relevant code:

      /**
       * Whether per-bundle permissions should be provided for the 'view' operation.
       *
       * Note that self::$bundleGranularity must also be TRUE in order for this
       * setting to be taken into consideration.
       *
       * @var bool
       */
      protected $viewBundleGranularity = FALSE;
    

    (https://git.drupalcode.org/project/drupal/-/merge_requests/2912/diffs#10...)

    I just created ✨ Add "view $bundle media" permission Active where exactly that is needed, but I can't tell if it's still an issue. I think it isn't?
    Thanks!

    Next steps: Solve #85 and #98

  • 🇩🇪Germany Grevil

    We're currently writing a new permission module: https://www.drupal.org/project/entity_access_by_reference_field →

    For that reason, we had a look into the different hook_entity_access implementations in core:

    • node
    • media
    • taxonomy term
    • user

    all of them, but node, use "update", only node uses "edit".

    Also the documentation at https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Entity%21... says

    The operation that is to be performed on $entity. Usually one of:
    "view"
    "update"
    "delete"

    but the code in the MR uses "edit" instead of "update". That's quite confusing and we think there should be a consensus on one correct term!

    Here's the current implementation from the MR:

        /**
       * @var array
       */
      protected $permissionMap = [
        'admin' => NULL,
        'collection' => NULL,
        'view any published' => NULL,
        'view own published' => NULL,
        'view own unpublished' => NULL,
        'edit any' => NULL,
        'edit own' => NULL,
        'delete any' => NULL,
        'delete own' => NULL,
        'create' => NULL,
      ];
    

    So we'd stronly suggest to change "edit" to "update". Also the method names use "update", like getUpdatePermission()

  • 🇩🇪Germany Grevil

    My bad, the operation is always "update", but the permission is "edit" (which is still quite confusing).

  • Open on Drupal.org →
    Environment: PHP 8.2 & MySQL 8
    last update 12 months ago
    Not currently mergeable.
  • last update 12 months ago
    29,323 pass, 37 fail
Production build 0.69.0 2024