Problem/Motivation
Since Drupal 9.2.0 the access checking must be explicitly specified on entity query prior to the query being executed. See the following change record for more details: https://www.drupal.org/node/3201242.
I found 2 spots in Token module, that should be handled:
- https://git.drupalcode.org/project/token/-/blob/8.x-1.x/token.tokens.inc...
- https://git.drupalcode.org/project/token/-/blob/8.x-1.x/tests/src/Functi...
Proposed resolution
Explicitly set `accessCheck` for the list entity queries.
Remaining tasks
1. Review listed usages and decide whether the access should be checked or not;
2. Submit a merge request with proposed changes;
3. Review, commit.
Issue fork token-3231436
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:
Comments
Comment #3
Matroskeen1) I think it should be TRUE in https://git.drupalcode.org/project/token/-/blob/8.x-1.x/token.tokens.inc..., so user can see the amount of nodes he has access to (default value was correct);
2) I think it should be FALSE in https://git.drupalcode.org/project/token/-/blob/8.x-1.x/tests/src/Functi..., because the test should care whether the link exists disregarding the access.
Comment #4
MatroskeenComment #5
andypostLooks reasonable to keep both as false
Comment #6
TR CreditAttribution: TR commented@andypost: Why the RTBC if you want to make both FALSE? Right now the patch sets one to TRUE and one to FALSE.
As explained in the change record https://www.drupal.org/node/3201242, the query without the call to
accessCheck()
defaults toaccessCheck(TRUE)
.So the MR and the suggestion in #5 both amount to a functional change in the token module. IMO as a "task" to address a deprecation in D9.2, the only change in scope of this issue would be to add
accessCheck(TRUE)
to both queries to preserve the existing behavior.But yes, the question of what is the proper access control to use for these queries should be discussed, and if access is changed then a test should be provided to ensure that this module respects the access control settings.
Comment #7
andypostProbably I was tired, thank you for review!
Comment #8
andregp CreditAttribution: andregp at CI&T commentedI'll change it.
Comment #9
andregp CreditAttribution: andregp at CI&T commentedComment #10
andregp CreditAttribution: andregp at CI&T commentedSorry, I forgot to unassign myself
Comment #13
BerdirMerged, thanks.