Problem/Motivation

Not calling ::accessCheck() has now been deprecated, and all entity queries on content entities should always include an explicit call to ::accessCheck() prior to the query being executed. For Drupal 10 this will be enforced by throwing an exception if ::accessCheck() is not called.

See: https://www.drupal.org/node/3201242

CommentFileSizeAuthor
#6 3240124-6.patch1.67 KBbruno.bicudo
#2 3240124-2.patch831 bytespaulocs

Comments

paulocs created an issue. See original summary.

paulocs’s picture

paulocs
Primary language Portuguese, Brazil
Location Belo Horizonte
commented
Assigned: paulocs » Unassigned
Status: Active » Needs review
StatusFileSize
new 3240124-2.patch831 bytes
gabriel.abdalla’s picture

gabriel.abdalla commented
Status: Needs review » Reviewed & tested by the community

Hi,

Changes look good.

Steps performed:
(1) Downloaded and enabled module.
(2) Applied patch.
(3) Code Review.
(4) Run tests.

Thanks!

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Reviewed & tested by the community » Needs work

There are more entity queries in the module, look for ->getQuery() on storage handlers.

bruno.bicudo’s picture

bruno.bicudo
Primary language Portuguese, Brazil
Location Bragança Paulista - São Paulo
commented
Assigned: Unassigned » bruno.bicudo

I'll try to work on this one.

bruno.bicudo’s picture

bruno.bicudo
Primary language Portuguese, Brazil
Location Bragança Paulista - São Paulo
commented
StatusFileSize
new 3240124-6.patch1.67 KB

I found two more queries which needed the accesCheck() call.

Kindly review it :)

bruno.bicudo’s picture

bruno.bicudo
Primary language Portuguese, Brazil
Location Bragança Paulista - São Paulo
commented
Assigned: bruno.bicudo » Unassigned
Status: Needs work » Needs review

Accidentally marked hide file, sorry.

andregp’s picture

andregp commented
Assigned: Unassigned » andregp

I'll review this

andregp’s picture

andregp commented
Assigned: andregp » Unassigned
Status: Needs review » Reviewed & tested by the community

The remaining getQuerry() calls were updated to include accessCheck(). The patch #7 seems complete.

mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented

Giving my +1 on #6. Ran phpstan-drupal for its accessCheck rule and manually reviewed, seems to nail the 3 occurrences.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Reviewed & tested by the community » Fixed

Thanks, committed.

  • Berdir committed 6a961b8 on 8.x-2.x authored by bruno.bicudo 
    Issue #3240124 by paulocs, bruno.bicudo: Access checking must be...

Status: Fixed » Closed (fixed)

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