MediaLibraryFieldWidgetOpener::checkAccess denies access when modifying field values on a translated entity revision [#3197416]

Problem/Motivation

When attempting to attach a media entity via the Media Library to a translated revisioned entity, it gives a 403 access denied – no reason.

The problem is in \Drupal\media_library\MediaLibraryFieldWidgetOpener::checkAccess

If the parameters from the media library state have a revision ID, it is loaded without fetching the translation from context.

    if (!empty($parameters['revision_id'])) {
      $entity = $storage->loadRevision($parameters['revision_id']);

The following access check is always forbidden:

$entity_access = $access_handler->access($entity, 'update', $account, TRUE);

When loading the translation from context, access is now allowed.

    if (!empty($parameters['revision_id'])) {
      $entity = $storage->loadRevision($parameters['revision_id']);
      $entity_repository = \Drupal::getContainer()->get('entity.repository');
      $entity = $entity_repository->getTranslationFromContext($entity);

Steps to reproduce

User with permissions:

  - 'create content translations'
  - 'delete content translations'
  - 'translate any entity'
  - 'translate editable entities'
  - 'update content translations'
  - 'create file media'
  - 'create image media'
  - 'create remote_video media'
  - 'update media'

Proposed resolution

Pass the entity to the entity repository getTranslationFromContext method.
Pending finding a solution for the case of entity translation creation (the translation can not be fetched).

Remaining tasks

Write a test.
Cover the entity translation case.
Create a Change Record.
Add trigger_error for backwards compatibility.

User interface changes

API changes

Data model changes

Release notes snippet

Comment	File	Size	Author
#23	3197416-nr-bot.txt	170 bytes	needs-review-queue-bot
#20	3197416-medialibrary-fieldwidgetopenercheckaccess-denies-access-on-a-translated-entity-revision.patch	5.39 KB	miksha
#13	core-3197416-medialibraryfieldwidgetopenercheckaccess-denies-access-13.patch	3.26 KB	eduardo morales alberti

Issue fork drupal-3197416

Show commands

Start within a Git clone of the project using the version control instructions.

Add & fetch this issue fork’s repository

Or, if you do not have SSH keys set up on git.drupalcode.org:

Add & fetch this issue fork’s repository

3197416-medialibraryfieldwidgetopenercheckaccess-denies-access-11-x changes, plain diff MR !4516
Check out this branch for the first time

Check out existing branch, if you already have it locally
3197416-medialibrary-fieldwidgetopenercheckaccess-denies-access-on-a-translated-entity-revision changes, plain diff MR !9450
Check out this branch for the first time

Check out existing branch, if you already have it locally
3197416-medialibraryfieldwidgetopenercheckaccess-denies-access-9-5-x changes, plain diff MR !4515
Check out this branch for the first time

Check out existing branch, if you already have it locally
11.x compare
Check out this branch for the first time

Check out existing branch, if you already have it locally
9.2.x compare
Check out this branch for the first time

Check out existing branch, if you already have it locally

1 hidden branch

3197416-medialibraryfieldwidgetopenercheckaccess-denies-access

changes, plain diff MR !303

Check out this branch for the first time

Check out existing branch, if you already have it locally

About issue forks

Comments

Comment #1

9 February 2021 at 02:50

mglaman created an issue. See original summary.

Comment #2

9 February 2021 at 02:56

mglaman opened merge request !303

Comment #4

9 February 2021 at 02:56

Version:

9.2.x-dev

» 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Comment #5

9 February 2021 at 02:56

Version:

9.3.x-dev

» 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Comment #6

9 February 2021 at 02:56

Version:

9.4.x-dev

» 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Comment #7

smustgrave commented 10 August 2022 at 17:20

Status:	Needs review	» Needs work
Issue tags:		+Bug Smash Initiative

Can you update the MR to the 9.5 or 10.1.x branch and check for errors?

Also for the tests.

Great issue summary!

Comment #8

10 August 2022 at 17:20

Version:

9.5.x-dev

» 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Comment #9

10 August 2022 at 17:20

Version:

10.1.x-dev

» 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Comment #10

eduardo morales alberti

Spanish

Spain, 🇪🇺

commented 31 July 2023 at 15:18

Same problem here!
I will try the patch

Comment #11

31 July 2023 at 15:25

Eduardo Morales Alberti opened merge request !4515

Comment #12

31 July 2023 at 15:27

Eduardo Morales Alberti opened merge request !4516

Comment #13

eduardo morales alberti

Spanish

Spain, 🇪🇺

commented 31 July 2023 at 15:30

Status	File	Size
new	core-3197416-medialibraryfieldwidgetopenercheckaccess-denies-access-13.patch	3.26 KB

Added patch from the MR.

Comment #14

31 July 2023 at 16:02

mglaman closed merge request !303

Comment #15

eduardo morales alberti

Spanish

Spain, 🇪🇺

commented 1 August 2023 at 09:40

Status:

Needs work

» Reviewed & tested by the community

@smustgrave is there anything else that should be reviewed? In our case, the patch seems to work well.

Comment #16

smustgrave commented 1 August 2023 at 14:07

Status:

Reviewed & tested by the community

» Needs work

Left some comments on the MR and this will still need a test

Comment #17

eduardo morales alberti

Spanish

Spain, 🇪🇺

commented 2 August 2023 at 10:07

This seems that is not working on the create translation operation because the method getTranslationFromContext checks translations that already exist.

  public function getTranslationFromContext(EntityInterface $entity, $langcode = NULL, $context = []) {
    $translation = $entity;

    if ($entity instanceof TranslatableDataInterface && count($entity->getTranslationLanguages()) > 1) {

Comment #18

eduardo morales alberti

Spanish

Spain, 🇪🇺

commented 3 August 2023 at 08:14

Issue summary:

View changes

Comment #19

berdir

German

Switzerland

commented 17 August 2023 at 07:45

I'm running into an edge case that's even trickier, in combination with #3047022: Layout builder fails to assign inline block access dependencies for the overrides section storage on entities with pending revisions as well as contrib layout builder modules, somehow the revision doesn't match what layout builder has.

But I suspect it's neither this nor the other issue, because what matters for layout builder is the revision id, which is the same. And i'm also seeing access denied results within the layout builder edit form for the delete button access check, so it's also confused about that. The site also uses modules like layout_builder_st and the content was cloned, so there so many possible things that could have and did go wrong.

Just documenting my findings for now in case someone else runs into a similar issue. Considering a custom patch that just grants full unconditional access to anything as we have no access restrictions for editors.

Comment #20

miksha commented 18 November 2023 at 13:18

Status	File	Size
new	3197416-medialibrary-fieldwidgetopenercheckaccess-denies-access-on-a-translated-entity-revision.patch	5.39 KB

I have a translator role that doesn't have edit permissions. I also encountered the same when adding/updating translations with this role that AJAX error happens with message that user needs edit permission for entity type where media field is calling media library in order to change field value. I made a patch for this but not sure if it's the right approach or if it covers all situations but what I tested and works is that user with just `translate any entity` and without `edit` permissions is able to create new translation and also update media field. Editing existing translation and updating media field also works. Solution also covers translating entities other than nodes e.g. taxonomies with media field etc.

This issue is related to https://www.drupal.org/project/drupal/issues/3038254 where access check was also delegated further.

Comment #21

matt b

English

commented 25 May 2024 at 06:54

I've just hit the same issue. I'm busy enabling translation on my site, having had issues years back with Media when it was a contrib module, I'd resisted implementing. Enabled it in D10 and hit this. Media does give a nice interface, but this issue means it is still unusable.

Comment #22

matt b

English

commented 25 May 2024 at 07:38

Status:

Needs work

» Reviewed & tested by the community

I've applied both patchs and the issue is fixed for me.

Comment #23

needs-review-queue-bot commented 26 May 2024 at 04:18

Status:

Reviewed & tested by the community

» Needs work

Status	File	Size
new	3197416-nr-bot.txt	170 bytes

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Comment #24

matt b

English

commented 3 June 2024 at 04:22

Just re-ran composer update and second patch (#20) won't apply, but I'm working fine with the first patch (#13)

Comment #25

9 September 2024 at 15:01

miksha opened merge request !9450

Comment #26

miksha commented 9 September 2024 at 15:06

Status:

Needs work

» Needs review

I tested 11.x locally and it still has the same issue that my patch in #20 fixes. I created a new MR against 11.x and combined changes from both #13 & #20 patches. For me this fixes the AJAX error when translator role user has permissions like defined in issue description under Steps to reproduce but at the same time has no edit permissions.

Comment #27

smustgrave commented 9 September 2024 at 18:43

Status:

Needs review

» Needs work

Was previously tagged for tests so moving to NW for those.

Comment #28

9 September 2024 at 18:43

Version:

11.x-dev

» main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.