Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
See parent issue #3200985: [meta] Fix undesirable access checking on entity query usages for context and test coverage policy. This issue is major because it blocks #2785449: It's too easy to write entity queries with access checks that must not have them.
The menu_ui_get_menu_link_defaults() provides default parent menu items when editing nodes. It uses entity queries that implicitly checks access for the current user, when it should not.
Issue fork drupal-3204140
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 #2
jonathanshawI think this should not be access sensitive, because any access control of menu items would likely be focused on visitors not editors.
MenuLinkContentAccessControlHandler has:
I suggest that under the parent issue's test coverage policy it is acceptable for this not to have test coverage, because it would require unusual customisations to encounter and it's more important to progress the blocked issue.
Comment #4
jonathanshawComment #5
longwaveI think it's unlikely anyone would add access control to individual menu items so I think this is fine.
I did spot another possible but unrelated bug here, the conditions in the two queries are subtly different:
vs
edit: already open as #2665608: Menu UI searches links by incorrect uri
Comment #6
catchHmm I think this should check access because it's showing a list of items in the UI - even though the use case for link access via query alter is very obscure.
Comment #7
jonathanshawFair enough. TRUE preserves the existing behavior and unblocks the parent issue, so it's harmless to do it here and probably no one will ever care. If someone thinks it should be FALSE and actually cares about, let them open an issue for that.
Comment #8
catchThanks I think this is ready to go then.
Comment #9
alexpottCommitted and pushed 1291450a0d to 9.2.x and 8ff460a0d6 to 9.1.x. Thanks!
Backported to 9.1.x because there's no change and keeping things aligned.