Problem/Motivation

While working on upgrades to Drupal 10 Upgrade status provided these errors as needing to be resolved manually:

On Line 123 of web/modules/contrib/admin_toolbar/admin_toolbar_search/src/SearchLinks.php

Relying on entity queries to check access by default is deprecated in drupal:9.2.0 and an error will be thrown from drupal:10.0.0. Call \Drupal\Core\Entity\Query\QueryInterface::accessCheck() with TRUE or FALSE to specify whether access should be checked.

On Line 114 of web/modules/contrib/admin_toolbar/admin_toolbar_tools/src/Plugin/Derivative/ExtraLinks.php

Relying on entity queries to check access by default is deprecated in drupal:9.2.0 and an error will be thrown from drupal:10.0.0. Call \Drupal\Core\Entity\Query\QueryInterface::accessCheck() with TRUE or FALSE to specify whether access should be checked.

Proposed resolution

Call \Drupal\Core\Entity\Query\QueryInterface::accessCheck() with TRUE

Remaining tasks

Call \Drupal\Core\Entity\Query\QueryInterface::accessCheck() with TRUE

CommentFileSizeAuthor
#26 3338408-26.patch1.99 KByuvania
#25 3338408-access_check_errors_for_D10_compatibility.patch2.38 KBsashainparis
#24 access_check_errors_for_D10_compatibility.patch2.26 KBradheymkumar
#18 3338408_12.patch1.99 KBprabuela
#14 3338408-fix-access-check-error.patch1.99 KBedmund.dunn
#11 3338408-11.patch1.93 KBrajab natshah
#2 admin_toolbar-resolve_access_check-3338408-2.patch1.92 KBelkaro

Issue fork admin_toolbar-3338408

Command icon Show commands

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

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

About issue forks

Comments

lelkneralfaro created an issue. See original summary.

elkaro’s picture

elkaro commented
StatusFileSize
new admin_toolbar-resolve_access_check-3338408-2.patch1.92 KB
elkaro’s picture

elkaro commented
Status: Active » Needs review
jjcarrion’s picture

jjcarrion
Primary language Spanish
Location Spain
commented

Patch #2 worked for me for the Drupal 10 upgrade,

Thanks!

jheinon_finland’s picture

jheinon_finland commented
Status: Needs review » Reviewed & tested by the community

Greetings!

I tested this on my local installation for a Drupal 9 site. The patch applied successfully and I have thus updated the status from Needs review to Reviewed & tested by the community.

rajab natshah’s picture

rajab natshah
he/him
commented

The patch #2 is working.
Thank you, lelkneralfaro!

kunal_sahu made their first commit to this issue’s fork.

kunal_sahu opened merge request !48

kunal_sahu’s picture

kunal_sahu commented
Assigned: elkaro » Unassigned

Hi @all Tested the patch. The patch LGTM.

kunal@LAPTOP-G4FTKS5H MINGW64 /c/xampp/htdocs/contributions/web/modules/contrib/admin_toolbar-3338408 (3338408-resolve-access-check)
$ git apply -v admin_toolbar-resolve_access_check-3338408-2.patch
Checking patch admin_toolbar_search/src/SearchLinks.php...
Checking patch admin_toolbar_tools/src/Plugin/Derivative/ExtraLinks.php...
Applied patch admin_toolbar_search/src/SearchLinks.php cleanly.
Applied patch admin_toolbar_tools/src/Plugin/Derivative/ExtraLinks.php cleanly.

RTBC++ #2.

Creating a MR for helping the maintainer.

please review and merge this MR.

Thanks

berdir’s picture

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

it's pretty much a false positive, those are *very likely* config entities (never seen a content bundle entity, doesn't make sense), it won't do anything but if we do something it should be an accessCheck(FALSE) just in case core ever actually adds access checks/query altering to config entity queries.

Either way, you can safely ignore this and do not need to apply it as a patch to your site.

rajab natshah’s picture

rajab natshah
he/him
commented
StatusFileSize
new 3338408-11.patch1.93 KB

Rajab Natshah opened merge request !51

rajab natshah’s picture

rajab natshah
he/him
commented
Status: Needs work » Needs review
edmund.dunn’s picture

edmund.dunn commented
StatusFileSize
new 3338408-fix-access-check-error.patch1.99 KB

#11 would not apply. I rerolled it for the latest release(3.4.1).

Status: Needs review » Needs work

The last submitted patch, 14: 3338408-fix-access-check-error.patch, failed testing. View results

prabuela’s picture

prabuela commented
Assigned: Unassigned » prabuela
prabuela’s picture

prabuela commented
prabuela’s picture

prabuela commented
StatusFileSize
new 3338408_12.patch1.99 KB
prabuela’s picture

prabuela commented
Assigned: prabuela » Unassigned
Status: Needs work » Needs review
szato’s picture

szato commented
Status: Needs review » Reviewed & tested by the community

Checked patch from #14 (and re-run automated tests for D10.1, D9.5 with php 8.1/Mysql 8 - passed).
Works for me.

@PrabuEla
I hid your patch because you changed accessCheck(FALSE) to accessCheck(TRUE) - which was already discussed in #10 and implemented in #11 and in #14.

szato’s picture

szato commented

I hid patch from #2 too.

dillix’s picture

dillix commented

Please make new release with this patch, so we can update sites to D10.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

You don't need this patch, the module works perfectly well on D10 without it. See #10.

radheymkumar’s picture

radheymkumar commented
Version: 3.x-dev » 3.4.2
Assigned: Unassigned » radheymkumar
Status: Reviewed & tested by the community » Needs review
Issue tags: +Resolve access check errors for D10 compatibility
StatusFileSize
new access_check_errors_for_D10_compatibility.patch2.26 KB

I fixed access check errors for D10 compatibility. attached patch.
Thanks

sashainparis’s picture

sashainparis
Primary language French
Location Provins
commented
StatusFileSize
new 3338408-access_check_errors_for_D10_compatibility.patch2.38 KB

#24: was founded on the project base path.
Update patch to consider the sort() added in between on these lines.
Using the module base path.

yuvania’s picture

yuvania
she/her
Primary language Spanish
commented
StatusFileSize
new 3338408-26.patch1.99 KB

I fixed access check errors for D10 compatibility

ericvl’s picture

ericvl
Primary language Dutch
Location Vichte, Belgium
commented

@yuvania
This is the same patch as in #14.
See also remarks in #10 and #23.

saganakat’s picture

saganakat commented

The patch in comment #14 has worked for drupal 10.1

rajab natshah’s picture

rajab natshah
he/him
commented
Status: Needs review » Reviewed & tested by the community
skaught’s picture

skaught
Primary language English
commented

Agree: tested patch #14 [3338408-fix-access-check-error.patch] applies as expected.

Prashant.c made their first commit to this issue’s fork.

prashant.c’s picture

prashant.c
he/him
Primary language English
Location Dharamshala
commented
Status: Reviewed & tested by the community » Needs review

Re-based with 3.x, needs to be reviewed.
Thanks

keshavv’s picture

keshavv commented

Added a comment in the MR. If someone review and update?
https://git.drupalcode.org/project/admin_toolbar/-/merge_requests/51

japerry’s picture

japerry
Location KOMK
commented
Status: Needs review » Closed (outdated)