Admin page access denied even when access is given to child items

The linked issue hasn't been committed so instead of a separate issue think this should be closed and that issue moved back to Needs work.

Will need test coverage either way.

This should be converted to a Merge Request. Change Status to Needs Review if you think it's ready.

#3381929: Restrict access to empty top level administration pages for overview controller touches the same function as this patch. Is that issue fixing the same problem?

I've created a PR to simplify Jelle_S's life. I'm not sure if the code will work properly, but based on #7, I'm marking the issue as 'Needs Review'.

Thanks but MR should target 11.x

And will need test coverage

Should I create a new branch based on the 11.x?

You can change the target branch on the current MR, will have to rebase most likely though just FYI.

11.x is the current development branch. So issues land there first and then backported to other supported branches when committed.

As I mentioned in #10, I'm unsure if the code is correct, so I followed the steps from the summary. Unfortunately, I'm unable to reproduce the issue.

My user has the 'Administer menus and menu links' permission but lacks the 'Access the taxonomy vocabulary overview page' permission. In this case, I have access to /admin/structure, and the 'Structure' link also appears in the menu.

I'm marking this as postponed since we need more information to reproduce it.

I've updated MR according to your patch. Can I grant access to the MR, or does it solely belong to me?

Test only job failed with expected message: https://git.drupalcode.org/issue/drupal-3413508/-/jobs/614074#L54

Hiding patches for clarity.

Confirmed test-only job (same as #25)

 There was 1 error:
1) Drupal\Tests\system\Functional\Menu\MenuAccessTest::testSystemAdminMenuBlockAccessCheck
Behat\Mink\Exception\ExpectationException: Current response status code is 403, but 200 expected.
/builds/issue/drupal-3413508/vendor/behat/mink/src/WebAssert.php:794
/builds/issue/drupal-3413508/vendor/behat/mink/src/WebAssert.php:130
/builds/issue/drupal-3413508/core/modules/system/tests/src/Functional/Menu/MenuAccessTest.php:318
/builds/issue/drupal-3413508/core/modules/system/tests/src/Functional/Menu/MenuAccessTest.php:336
/builds/issue/drupal-3413508/core/modules/system/tests/src/Functional/Menu/MenuAccessTest.php:246
/builds/issue/drupal-3413508/vendor/phpunit/phpunit/src/Framework/TestResult.php:728
ERRORS!
Tests: 2, Assertions: 232, Errors: 1.

Issue summary appears to be complete and solution matches change. Believe this is good.

I don't think that this is specific to 11.x after the changes for #296693: Restrict access to empty top level administration pages - see #3415709: Users with access taxonomy overview permission cannot access admin/structure, which is on 10.2.x

As noted there, the equivalent change for 10.2 seems to fix the problem.

I was also seeing this problem on my site. The latest patch does fix the problem for me (I'm on 10.2.x).

Also noticed this issue on a 10.2.x site.
Copied the diff locally, used it as patch with composer -> it works!

I also noticed this on 10.2.x site and the patch fixed it.

Confirm that I also noticed this on the 10.2.2 site and the patch fixed it.

This seriously affects the ability to administer sites without a documented work around so bumping the priority.

The code is pretty confusing and its not clear to me how it fixes the problem but i don't seen anything broken.

Same issue here. Users with no permissions for the first link get an access denied. MR 6100 fix the issue

Closed as duplicate.

MR6100 works for my use case as well - see https://github.com/drush-ops/drush/issues/5864

-mike

Our project is on Drupal 10.2.1.
I was breaking my head about why a content editor cannot see the Structure menu tab, even though he has permissions to edit certain menus and taxonomies and view webform subscriptions.
I looked through the issie queues of admin_toolbar, admin_toolbar_links_access_filter, gin_toolbar, gin, ... but could not find it.

The MR6100 applies and the admin -> structure links are again available in the toolbar for the content editor

BUT

The user now sees all menus, not only the ones he has access to via permissions.
Also, its not just listing, he can also edit all the menus he's not supposed to be able to, like the administration menu.
That should not be possible, security wise. Can anyone confirm ?

UPDATE

Apparently, now I suddenly must no longer grant the permission 'Administer menus and menu links'. If that permission is granted, user can see and edit ALL possible menus. Without that permission, all is working as expected and user only has access to those menus that are granted via 'Menu Admin per Menu' permissions.

Marked #3421825: Access denied on system.admin_structure route when Admin Toolbar Extra Tools is enabled and #3423502: Missing Top Level Menu Item as duplicates of this.

This has some huge conflicts with #3381929: Restrict access to empty top level administration pages for overview controller going to try and resolve them today. Unfortunately both used a 4th child with similar but slightly different access rules.

I've done my best to merge in the conflicts, but the tests were quite mind bending to get passing.

I've also replaced "super" with "grand" in most cases as that seemed to be a large part of the conflicts.

The test only changes passed so something isn't right...

MR 6100 fixes this issue in my 10.2 install.

Not sure if something else is amiss, but im not seeing the issues from #37 with a user who has 'administer menu' permissions.

Yeah it sounds like from the UPDATE in #37 that it was user error.

DOH! Took me far too long to figure out what I'd done wrong... I hadn't put the test menu items back in the merge conflict madness.

Now that I've done that and fixed up the tests, they are correctly failing without the fix:

Behat\Mink\Exception\ExpectationException : Routes do not match. 
Expected routes:
Accessible routes: menu_test.parent_test, menu_test.child4_test, menu_test.child4_test_overview
Inaccessible routes: menu_test.child1_test, menu_test.child2_test, menu_test.child3_test_block, menu_test.grand_child1_test, menu_test.grand_child2_test, menu_test.grand_child3_test, menu_test.great_grand_child1_test, menu_test.grand_child4_test
Actual routes: 
Accessible routes: menu_test.child4_test, menu_test.child4_test_overview
Inaccessible routes: menu_test.parent_test, menu_test.child1_test, menu_test.child2_test, menu_test.child3_test_block, menu_test.grand_child1_test, menu_test.grand_child2_test, menu_test.grand_child3_test, menu_test.great_grand_child1_test, menu_test.grand_child4_test

EDIT: Worth mentioning that I also tried to get around adding more menu items, but we need some that aren't using the menu overview controller hence the new ones are required.

With the changes pushed with the last commit, we could not apply the patch to Drupal Core 10.2.3.
Can you please add a patch file for 10.2.3 version?

Ran test-only feature

1) Drupal\Tests\system\Functional\Menu\MenuAccessTest::testSystemAdminMenuBlockAccessCheck
Behat\Mink\Exception\ExpectationException: Routes do not match. 
Expected routes:
Accessible routes: menu_test.parent_test, menu_test.child4_test, menu_test.child4_test_overview
Inaccessible routes: menu_test.child1_test, menu_test.child2_test, menu_test.child3_test_block, menu_test.grand_child1_test, menu_test.grand_child2_test, menu_test.grand_child3_test, menu_test.great_grand_child1_test, menu_test.grand_child4_test
Actual routes: 
Accessible routes: menu_test.child4_test, menu_test.child4_test_overview
Inaccessible routes: menu_test.parent_test, menu_test.child1_test, menu_test.child2_test, menu_test.child3_test_block, menu_test.grand_child1_test, menu_test.grand_child2_test, menu_test.grand_child3_test, menu_test.great_grand_child1_test, menu_test.grand_child4_test
/builds/issue/drupal-3413508/core/tests/Drupal/Tests/WebAssert.php:580
/builds/issue/drupal-3413508/core/modules/system/tests/src/Functional/Menu/MenuAccessTest.php:411
/builds/issue/drupal-3413508/core/modules/system/tests/src/Functional/Menu/MenuAccessTest.php:283
/builds/issue/drupal-3413508/vendor/phpunit/phpunit/src/Framework/TestResult.php:728
ERRORS!
Tests: 2, Assertions: 258, Errors: 1.

Issue summary appears correct.

Verified the solution does fix the problem described.

@Alina Basarabeanu does https://www.drupal.org/files/issues/2024-01-11/3413508-22.patch still work for you on 10.2?

Yes,
Previously I was using the merge request as was pointing to 10.x branch.
Thanks

Same error here on 10.2.3, the patch https://www.drupal.org/files/issues/2024-01-11/3413508-22.patch solves our issue.

Added some comments to the MR.

Also I think we need a change record and a release note. This change might result in users seeing menus items they couldn't before. I think that this means we should tell users about this.

Added the CR https://www.drupal.org/node/3431191

Brief release note added, feel free to make amendments.

Random fail on unit tests in the pipeline, have rerun.

Rebased the MR and that Unit test failure was random.

Feedback appears to be addressed, release note seems good. Mentions the problem that is solved.

Committed/pushed to 11.x and cherry-picked to 10.3.x, thanks!

Patch #22 is still needed on the latest version of Drupal Core 10.2.6. I don't know why this is only cherry picked to 10.3.x? This is a necessary fix for all Drupal sites since it's now only checking the first menu item instead of any menu item.

Please re-open and add code change to 10.2.x as well, patch can be used in the meantime. (Until a stable version of 10.3.x is released for production)

Opened bug report against 10.2 with MR of same cherry-pick, please review: #3460872: Admin page access denied even when access is given to child items (requesting 10.2 backport)