Contrib.social

EntityCreateAnyAccessCheck::access() wrongly forbids access when the first bundle forbids access

Open on Drupal.org →
Created on 17 April 2019, over 5 years ago
Updated 12 June 2023, over 1 year ago

Problem/Motivation

I have the following media bundles and they are checked in that order...:
- Custom 1 (forbidden)
- Custom 2 (forbidden)
- Custom 3 (forbidden)
- Image (Allowed)
- Video (Allowed)
- File (Allowed)

The generic button Add media should show up when the a user has access to create at least one of the bundles in the media. Because of how my bundles are ordered, as well as how the code in Drupal\Core\Entity\EntityCreateAnyAccessCheck::access() is implemented, the result in the end is forbidden and the button Add media does not show when it should.

Steps to reproduce

  1. Create a 2 bundles of a specific entity type (first_bundle & last_bundle)
  2. Implement hook_ENTITY_TYPE_create_access() for the specific entity type and return AccessResult::forbidden() for last_bundle
  3. The /entity_type/add page is loaded and only allows to create "first_bundle" items
  4. Modify the hook above to return AccessResult::forbidden() for first_bundle
  5. The add page shows Access Denied, even if it is possible to create last_bundle entities and is possible to create them if you access directly to /entity_type/add/last_bundle

Proposed resolution

Tweaks the implementation to implement what is mentioned in the comments in the following piece of code:

// Check whether an entity of any bundle may be created.
foreach ($bundles as $bundle) {
  $access = $access->orIf($access_control_handler->createAccess($bundle, $account, [], TRUE));
  // In case there is a least one bundle user can create entities for,
  // access is allowed.
  if ($access->isAllowed()) {
    break;
  }
}

The first iteration get's a forbidden passed to the OR in here is causing the overall result to be Forbidden.

What I think is needed here is to aggregate the whole set of access permissions including their cache metadata. Breaking early might cause caheability metadata leaks, as the rest of the allowed access checks are fully ignored.

We can have the break changed, so it will build the whole set of access results and aggregate them (in terms of cache metadata).
Then have an Allow / Forbidden / Neutral based on tuning on the method...

Remaining tasks

Discussion, Patch...

User interface changes

None expected.

API changes

Small internal change to have correct access checking on entity level.

Data model changes

None.

Release notes snippet

TBD.
We will release a bit more actions on the system, as the bug is restricting the operations.
Possibly some more permissions will be alowed.

🐛 Bug report
Status

Fixed

Version

9.5

Component
Entity  →

Last updated about 5 hours ago

Created by

🇧🇬Bulgaria ndobromirov

Live updates comments and jobs are added and updated live.
  • Security

    It is used for security vulnerabilities which do not need a security advisory. For example, security issues in projects which do not have security advisory coverage, or forward-porting a change already disclosed in a security advisory. See Drupal’s security advisory policy for details. Be careful publicly disclosing security vulnerabilities! Use the “Report a security vulnerability” link in the project page’s sidebar. See how to report a security issue for details.

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.

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024