MenuLinkContentAccessControlHandler does not allow "view" access without admin permission, making these entities inaccessible via REST, JSON API and GraphQL

Rerolled patch #80.

The last submitted patch, 82: 2915792-82.patch, failed testing. View results →

Modified #82 patch to allow unrouted links that starts with "/"

Closed 🐛 Custom menu item entity reference is not accessible to anonymous user Closed: duplicate as a duplicate of this. Discussed with @catch in #bugsmash and the possible problems with entity references should probably be handled here too.

Patch #85 not working on Drupal 10.0.9.
It says patch can not be applied.
Any idea?

Patch #85's compatible version for Drupal 10.1.x.

Patch #88 still not working if paragraph is a part of extra field of meu item.
Because it tries to check permission to access entity.menu_link_content.canonical, and it seems there is no way to set custom permission for entity.menu_link_content.canonical, without any additional module installed.

This patch simply ignores all access permission for "view" case, to make sure all menu items are visible.

PLEASE DO NOT USE THIS PATCH, if you need menu item specific permission set up.

@Lendude regarding "Discussed with @catch in #bugsmash and the possible problems with entity references should probably be handled here too." ... Could you give some more info on what needs to be done to get this issue moving again?

@Lendude & @Voleger, we just came to this issue from here: #2925283: New fields visibility and access →

TL;DR: Due to this, entity reference (revision) contents are not visible for guests within the Menu (example: Mega Menu implementation using Paragraphs). Workaround: Use paragraphs_type_permissions submodule, which seems to override this using hook_ENTITY_TYPE_permission()

Still we'd like to help fixing this in core, so we can remove the workaround.

You have both invested a lot of time here and I think you're aware of what has to be done to finish this. What ca we do?
We can reroll the MR against 11.x and 10.3.x but then we should try to finish it and hot have it around for years again... any feedback?

The MR still seems good, a clear improvement and has a lot of positive feedback.

Here's the current state of the MR339 which still works fine against 10.2.x, 10.3.x and eventually 11.x (haven't tried). Great work @voleger!

rest and json_api integration tests are failing

Test are failing because the new link to user/1 is missing in the menu tests. So that should be a very easy fix.

See this link for all tests that are failing.

Not as simple as it seems. The expected cache tags are invalid, since we are swapping the "https://nl.wikipedia.org/wiki/Llama" link with a cacheable internal link to user:1 out. We can implement a dirty workaround and manually prepend the new cache to "getExpectedCacheTags()", but then the test will just throw an error later on.

To be honest, the current test changes generally seem really rushed. Simply replacing a remote link string with an internal link inside already existing tests / test bases without changing anything else just doesn't feel right. It also looks like we removed testing for external links entirely?

I'd say we revert the current test changes and create dedicated tests for internal menu links (json and rest) additionally to the already existing external link tests. Only problem being, that the test structure seems quite complex, so this will probably need quite a bit of work...

Really unsure about this. Does anybody else have an idea? Maybe there is a super simple way to test this, and we only need to add a quick test without changing any existing tests / test bases?

I reverted the test changes and created a new test class inside the jsonapi space. Unsure, whether to create a REST test class as well. But I don't think we should adjust the rest test base, how we did it in "MenuLinkContentResourceTestBase.php" originally, which I reverted here: https://git.drupalcode.org/project/drupal/-/merge_requests/339/diffs?com....

Unfortunately, now the old "MenuLinkContentTest" fails. Here, it expects a 403 status code, but it actually receives an unexpected 200 status code. This might show, that the current MR needs some refactoring? I am not sure. These tests are highly complex with many dependencies.

Attached Drupal 11.1 compatible patch. Would still be great, if someone could finish the great work done here already to get this fixed. Any hero present to move this over the finish line?

The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.