Discovered in #2869426-71: EntityResource should add _entity_access requirement to REST routes.

CommentFileSizeAuthor
#7 update-delete-message-MediaAccessControlHandler-2950129-7.patch2.77 KBmsankhala
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Wim Leers created an issue. See original summary.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Related issues: +#2950125: Add helpful reason for 'update' and 'delete' access not being allowed to CommentAccessControlHandler, +#2950127: Add helpful reason for 'update' and 'delete' access not being allowed to FileAccessControlHandler
tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler at bio.logis Genetic Information Management GmbH commented
Issue tags: +dcruhr18
owenbush’s picture

owenbush CreditAttribution: owenbush commented

Looking at this at DrupalCon Nashville.

owenbush’s picture

owenbush CreditAttribution: owenbush commented

When looking into this issue it doesn't appear that a definitive 'forbidden' AccessResult is ever returned. If none of the permission checks pass in this case, the access is deferred until later by returning AccessResult::neutral() for both 'update' and 'delete'. Is this actionable as it stands?

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented

Yes, you can add the helpful reason directly in the return AccessResult::neutral()->cachePerPermissions(); calls :)

msankhala’s picture

msankhala CreditAttribution: msankhala as a volunteer and at Srijan | A Material+ Company commented
Status: Active » Needs review
FileSize
update-delete-message-MediaAccessControlHandler-2950129-7.patch2.77 KB

Here is a patch.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Status: Needs review » Reviewed & tested by the community

Thank you, @msankhala!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce commented
Status: Reviewed & tested by the community » Fixed

Crediting @Wim Leers for reviews.

Committed 4022cc1 and pushed to 8.6.x. Thanks!

  • alexpott committed 4022cc1 on 8.6.x 
    Issue #2950129 by msankhala, Wim Leers: Add helpful reason for 'update'...

Status: Fixed » Closed (fixed)

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