With Administration Menu installed, administrative users (other than uid 1) without the 'administer site configuration' privilege can see several menu items below admin/settings, which they should not see. If a user clicks on one of those items, an 'Access forbidden' page is displayed.
Attached patch fixes this bug by adding the missing access restriction to menu items below admin/settings. It seems that only menu items recently added to Drupal 5 are affected.
| Comment | File | Size | Author |
|---|---|---|---|
| #3 | system-DRUPAL-5.patch | 4.14 KB | sun |
| drupal-DRUPAL-5_0.patch | 4.14 KB | sun |
Comments
Comment #1
sunI've tested this patch on several Drupal sites in the meanwhile. Everything works just as usual and admin_menu does not display menu entries a user is not allowed to access.
Comment #2
drummIf a menu item does not set 'access', the parent menu item's access should be used.
Comment #3
sunI believe you are making the assumption that anything below 'Site configuration' is only accessible if a user has the corresponding 'administer site configuration' permission. That might be true for Drupal core, but it's definitely not true for contrib modules. Why should I need to give a user the access permissions to alter system settings if the user should be able to alter contrib module settings only?
Even if admin_menu checks access permissions of the parent menu item (which is 'Site configuration' here), the following items are displayed, because the user has access to the 'Site configuration' menu item although she does not have the 'administer site configuration' permission:
This is because 'Site configuration' is a grouping menu item. If one of its children is accessible, a user gains access to 'Site configuration'. Thus, above menu items are displayed, but a user gets an 'Access forbidden' page by clicking on them.
I hope this clarifies that menu rendering modules like admin_menu have no chance to omit those items and this simple and unobtrusive patch needs to hit core. It not only fixes this bug but also makes menu items in system.module more consistent, which I'd say is always good.
Updated patch against 5.3 dev.
Comment #4
drummThe assumption for developers is that inherited access restrictions will work. If we explicitly set these, there will be other similar problems in core or contributions.
In Drupal core, access is not checked using the current parents, but by using the original paths. See http://api.drupal.org/api/function/_menu_item_is_accessible/5 for an example of the proper way to check access.
Comment #5
seutje commentedI hate working with this .patch shit
the unxutils thingy doesn't do anything, just sits there blinking like a kid with an extra chromosome :(
so I tried to edit the shit manually (yeh, go ahead and bitch) and now nothing works \o/
so after finally managing to figure out which parts of the original to keep and which to replace, I'm back to getting no menu at all on any other user besides superadmin
weird thing is, it only goes trough the loop like 8 times or so, while I used to be getting 10+ items in the menu I wouldn't have access to
and when I added a line to show me the result of _menu_item_is_accessible($mid) it only spits out 2 zeros... why not 8? since the loop runs 8 times and nothing is shown, they should all return 0 and not just 2 of them
the other 6 don't seem to return anything at all, shit's fucked up
u know what, fuck this crappy ass administrator menu shit all together, can't believe I just spent over 3 hours trying to get some half-ass module to actually work