Make WordPress Core

Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#20111 closed feature request (wontfix)

Ability to set permission level for admin settings pages using filters

Reported by: bananastalktome's profile bananastalktome Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: Administration Keywords:
Focuses: Cc:

Description

Removing admin pages from the menu tree is made possible through using either the $submenu global or the remove_submenu_page function, however preventing users from manually accessing the page is not an easy endeavor (afaik). Rather than hard-coding in the minimum role, would it make better sense to utilize filters to allow developers to change the privilege?

For example, a change on line 13 of wp-admin/options-discussion.php:

- if ( ! current_user_can( 'manage_options' ) )
+ if ( ! current_user_can( apply_filters('admin_options_discussion_page_privilege', 'manage_options') ) )

Removing access to this page would then be as easy as add_filter('admin_options_discussion_page_privilege', create_function('', 'return "manage_network";')); , for example.

Change History (10)

#1 follow-up: @nacin
12 years ago

You can do something like this:

add_filter( 'map_meta_cap', function( $caps, $cap ) {
   if ( $cap == 'manage_options' )
      $caps[] = 'manage_network';
   return $caps;
} );

#2 in reply to: ↑ 1 @bananastalktome
12 years ago

Replying to nacin:

You can do something like this:

add_filter( 'map_meta_cap', function( $caps, $cap ) {
   if ( $cap == 'manage_options' )
      $caps[] = 'manage_network';
   return $caps;
} );

The problem with this approach (correct me if I am wrong) is that I do not want 'manage_options' to map to 'manage_network' in all places since there are many places WordPress (as well as plugins) uses 'manage_options'. The desired effect is to be able to remove access to a particular options page without impacting anything else (more desirable would be to remove individual options on some pages rather than the entire page, but that's a story for another day ;) ).

#3 @scribu
12 years ago

  • Cc scribu added

#4 follow-up: @ocean90
12 years ago

I didn't test it, but it should be possible to work with $current_screen or $pagenow in your filter function.

#5 in reply to: ↑ 4 @bananastalktome
12 years ago

Replying to ocean90:

I didn't test it, but it should be possible to work with $current_screen or $pagenow in your filter function.

I had dabbled with using $current_screen, but it was difficult to always know which attributes to check when I wanted to restrict a particular admin page for users. For example checking $current_screen->base was generally the only attribute I needed to use when blocking access to admin pages; however checking for the value 'post' did not, as I had thought, block only Post type posts but also Pages. It was ambiguities such as this which caused me to have some concerns over whether the checks I had used were really blocking what I had intended. I figured using filters, with each distinct admin page in the nav menu having a separate filter which can be used to redefine permission levels, might make the process a little easier.

I realize that this would, in some ways, complement what $current_screen can already do, but I think it might make it a little more straightforward and clear.

A pie-in-the-sky wish (which actually negates the need for my proposed admin page permission filters) would be that the remove_(sub)menu_page functions would not only remove the page form the navigation, but also remove access to them (this way I could have the appropriate pages removed within if (current_user_can('blah')) blocks). But there may be a reason I am overlooking as to why this is not already done.

#6 @scribu
12 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to wontfix
  • Status changed from new to closed

I figured using filters, with each distinct admin page in the nav menu having a separate filter which can be used to redefine permission levels, might make the process a little easier.

Except what is a "distinct admin page"? For example, it's not obvious if edit.php?post_type=post should have a different filter than edit.php?post_type=page.

That's why we have $current_screen (which also has a post_type property, btw), so that you can choose your own level of granularity.

Having remove_(sub)menu_page() also block access is an interesting idea, but it suffers from the same problem: do you block the whole of edit.php or just parts of it? There's no 1-to-1 mapping between menu items and admin screens.

#7 @nacin
12 years ago

One more thing to think about: options.php is not protected in these situations. You'll want to guard it there, too.

#8 @bananastalktome
12 years ago

Replying to scribu:

Except what is a "distinct admin page"? For example, it's not obvious if edit.php?post_type=post should have a different filter than edit.php?post_type=page.

IMHO, a distinct admin page is any page which corresponds to a nav menu item (as well as any interaction which occurs within those pages, such as clicking 'Save', 'Publish', etc/, and any sub-pages which may or may not be in the nav menu themselves). So in that case, edit.php(query parameter post_type=post is default) and edit.php?post_type=page would be two distinct pages since each has a direct point of entry from the nav menu.

That's why we have $current_screen (which also has a post_type property, btw), so that you can choose your own level of granularity.

Having remove_(sub)menu_page() also block access is an interesting idea, but it suffers from the same problem: do you block the whole of edit.php or just parts of it? There's no 1-to-1 mapping between menu items and admin screens.

By using remove_menu_page(remove_menu_page('edit.php'); it already only removes the 'Posts' page and not the 'Pages' page.

I think what it comes down to is that when I decide to remove a page, why is it still accessible? I can't think of a compelling reason why someone would remove a page but still want users to be able to access it. Part of what causes confusion for me (as well as from anecdotal accounts of others on websites I have come across when trying to find a 'best way' to remove admin pages, some of which terrifyingly proposes simply using css to hide the menu items with display: none), is that when I remove a menu page, it does not really 'remove' the page so much as hide it, which as far as security goes is little better than the css approach. Semantics, maybe, but IMHO given the current behavior, the function should be remove_menu_item('blah'); as it really doesn't remove the page itself.

Also, can you please clarify what you mean by "the 1-to-1 mapping between menu items and admin screens"? I thought there already was a 1-to-1 mapping between each menu item and screens.

I realize this may have gone beyond the scope of this original ticket, and I would be happy to move this conversation elsewhere if deemed more appropriate.

Replying to nacin:

One more thing to think about: options.php is not protected in these situations. You'll want to guard it there, too.

Thanks for the tip!

#9 follow-up: @scribu
12 years ago

Examples of non 1-to-1 relationships between menu items and screens:

  • post.php?post=1331&action=edit has no menu item
  • Appearance -> Themes houses two screens: themes.php and theme-install.php
  • on multisite: site-info.php, site-users.php etc. also have no dedicated menu items
Last edited 12 years ago by scribu (previous) (diff)

#10 in reply to: ↑ 9 @bananastalktome
12 years ago

Replying to scribu:

Examples of non 1-to-1 relationships between menu items and screens:

  • post.php?post=1331&action=edit has no menu item
  • Appearance -> Themes houses two screens: themes.php and theme-install.php
  • on multisite: site-info.php, site-users.php etc. also have no dedicated menu items

Ah, ok, I see what you mean now. This does break down my filters suggestion, I'll admit :). I still do not agree with the fact that removing an admin menu page doesn't really remove it, but I guess with proper access checks it can be secured to an extent.

Note: See TracTickets for help on using tickets.