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.

My bad! I linked the wrong issue, the code definitely is already in core. Fixed that part.

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

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

No, that tackles a different issue. I'll have a look at creating an MR if I find the time today.

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 managed to reproduce it on a clean Drupal 10.2.1 install, with the contrib Scheduler module. Unfortunately, in this case, this patch does not seem to work, so there's more going on. Here are the steps to reproduce:

composer create-project drupal/recommended-project drupal.local
cd drupal.local
composer require drush/drush
vendor/bin/drush site:install
vendor/bin/drush user:create Jelle
vendor/bin/drush user:role:add content_editor Jelle
vendor/bin/drush role:perm:add content_editor "access taxonomy overview"

# Login as user 2 and check the toolbar, "Structure" should be there

composer require drupal/scheduler
vendor/bin/drush en scheduler

# Login as user 2 and check the toolbar, "Structure" is gone

It seems to have something to do with this view in scheduler, but I can't figure out what's going wrong:
https://git.drupalcode.org/project/scheduler/-/blob/2.0.1/config/optiona...

After some further digging:

The scheduler module adds the view linked above, which creates a menu link item with entity.taxonomy_vocabulary.collection as its parent. So if you follow the steps to reproduce, you create a user that does have access to entity.taxonomy_vocabulary.collection, but does not have access to its child created by the scheduler module: views_view:views.scheduler_scheduled_taxonomy_term.overview

I'm still trying to wrap my head around the correct implementation details, but basically: once an element in the tree is found that is accessible, and is an actual page, not just an overview of its sublinks, the access check should pass, which it doesn't right now.

@taraskorpach I don't have push access to your MR, but I did find a fix for the problem described above. Not sure how to write a test for this exactly, but in the mean time people experiencing this issue can use the patch attached.

Removed the extra whitespace at the end of the file

Still no access to the MR, but here's the above patch with tests included, and one patch with only the tests to prove they fail.

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

Sorry, I didn't see the big green "Get push access button"... Thanks @taraskorpach for adding the latest patch to the MR :)

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 🐛 Restrict access to empty top level administration pages Fixed - see 🐛 Users with access taxonomy overview permission cannot access admin/structure Needs review , which is on 10.2.x

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

smustgrave → credited Eugen Zerr → .

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 ?

Marked 🐛 Access denied on system.admin_structure route when Admin Toolbar Extra Tools is enabled Active and 🐛 Missing Top Level Menu Item Closed: duplicate as duplicates of this.

This has some huge conflicts with 📌 Restrict access to empty top level administration pages for overview controller Fixed 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 fixes this issue in my 10.2 install.

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

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!

quietone → closed merge request !6100

Automatically closed - issue fixed for 2 weeks with no activity.

khiminrm → changed the visibility of the branch 3413508-admin-page-access to active.

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: 🐛 Admin page access denied even when access is given to child items Needs review