'users' appearing twice in menu [#39639]

I wasn't really sure where to file this, but it's a usability problem. If a given user has "administer users" permission but does not have "administer site configuration" permission, then their navigation menu ends up with two entries for "users". One is admin/users, the other is admin/settings/users, but both show up as "users". Normally the second would be under admin/settings in the menu, but without "administer site configuration" the "admin" and "settings" menus are not shown. That means both "users" entries fall up the menu structure and end up at the top level, where they're identical.

I've not tried it, but I suspect the multiple uses of "menu" and "forum" and such would have a similar problem.

Comment	File	Size	Author
#8	admin_settings_permissions_0.patch	2.67 KB	Richard Archer
#8
#2	admin_permissions.patch	1.63 KB	Richard Archer
#2
#1	admin_settings_permissions.patch	2.67 KB	Richard Archer
#1

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

Richard Archer CreditAttribution: Richard Archer commented 16 December 2005 at 11:34

File	Size
admin_settings_permissions.patch	2.67 KB

There are two problems here.

First is that access to admin/settings/* is supposed to be governed by the "administer site configuration" option. To do this a module should not define an 'access' attribute on the admin/settings/module callback, thus letting the admin/settings 'access' attribute prevail. But some modules do define 'access' so their settings menu appears if the user has the "administer module" permission. This is probably oversight on behalf of the module authors... I'm responsible for the one in menu.module and I realise now it is an error.

The attached patch fixes this.

The second problem is that the administer menu does not appear unless the user has the "access administration pages" permission. The only thing this gives a user access to is the watchdog logs so I really don't see the harm in granting this permission.

That said, I do have a workaround for this... which is to fiddle the settings of the admin menu. I'll attach the patch separately because I really don't think it's core-worthy.

Comment #2

Richard Archer CreditAttribution: Richard Archer commented 16 December 2005 at 11:41

File	Size
admin_permissions.patch	1.63 KB

And here's the patch that allows the admin menu to be shown if the user has access to admin/* menus but you don't want them to see the watchdog log.

It's a pretty fragile patch... much better IMHO to apply whichever patch it is that lets the default admin page be specified and set it to menus or users. Oh, that's right... that patch segfaulted on apache... but it will eventually make it into core.

So NOTE the main patch for this problem is attached to comment #1.

Comment #3

Richard Archer CreditAttribution: Richard Archer commented 23 December 2005 at 11:06

Status:

Active

» Needs review

Crell, would you mind reviewing the patch in comment #1? I've seen this bug reported a couple of times and it would be nice to get it fixed.

Comment #4

Crell CreditAttribution: Crell commented 25 December 2005 at 08:01

Status:

Needs review

» Reviewed & tested by the community

Sorry, Richard, I didn't notice your replies earlier. Tried patch in #1, works as advertised for user, menu, and forum modules. No side effects that I've noticed.

I hadn't realized that "administer site configuration" meant "module configuration" until you explained it here. That's probably something that should be clarified at some point.

Comment #5

Dries CreditAttribution: Dries commented 26 December 2005 at 11:24

Why the node_page in + 'callback' => user_access('access administration pages') ? 'watchdog_overview' : 'node_page',?

Comment #6

Richard Archer CreditAttribution: Richard Archer commented 26 December 2005 at 19:18

Note that the real patch to address this issue is attached to comment #1.

The patch on comment #2 allows the admin menu to be made visible if it contains children without giving the user access to the watchdog page. This patch instead shows node_page if user does not have permissions to view watchdog.

I don't like this second patch... I just posted it to fully address Crell's original issue. If it was to be included in core there would need to be more work done on the MENU_VISIBLE_IF_HAS_CHILDREN menu item flag, and I'm not ready to go there yet.

But patch #1 should be considered for core, please.

Comment #7

Crell CreditAttribution: Crell commented 26 December 2005 at 22:10

Even if #1 doesn't address every possible issue, it addresses the one I raised in this issue. It also doesn't break anything as far as I can tell, but is a usability improvement. As I said there's some documenting that could also be done, but we'll deal with that later. #1 is a good patch as is. #2 should probably be a separate issue thread.