Opened 9 years ago
Last modified 17 months ago
#29213 assigned enhancement
Introduce capability for access to nav-menus.php
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | Future Release | Priority: | normal |
Severity: | normal | Version: | 3.0 |
Component: | Menus | Keywords: | dev-feedback needs-unit-tests needs-patch granular-capabilities |
Focuses: | administration | Cc: |
Description (last modified by )
Management of the nav menus currently requires edit_theme_options
capability, a capability associated with administrators which grants the power to make many wide sweeping changes. There should be a discrete capability edit_nav_menus
just for managing menus, one that is inherited for anyone who has edit_theme_options
by default. This was done for Customizer access in #28605 with the introduction of a customize
capability.
Originally brought up in #14386.
For introducing a manage_widgets
capability, see #31020.
Attachments (5)
Change History (32)
#2
@
9 years ago
I created patch 29213.diff as a starting point based on changesheet [29170] and added edit_nav_menus
capability through the map_meta_cap
filter. However accessing wp-admin/nav-menus.php displayed the following error:
You do not have sufficient permissions to access this page.
This was because user_can_access_admin_page()
was returning false
. Digging deeper I found that $_wp_menu_nopriv
contained the following value:
$_wp_menu_nopriv["nav-menus.php"] = true;
To test how customize
worked I used the same filter and printed $_wp_menu_nopriv
. This is what I saw:
$_wp_menu_nopriv["customize.php?return=%2Fwp-admin%2Fcustomize.php"] = true;
But wp-admin/customize.php was accessible, so I narrowed down to the user_can_access_admin_page()
function's following lines:
if ( isset( $_wp_menu_nopriv[$pagenow] ) ) return false;
The $pagenow
variable only contains the filename without query strings so when customize
was used this line checked for isset( $_wp_menu_nopriv["customize.php"] )
which obviously caused this if
loop to be skipped with user_can_access_admin_page()
returning true
.
Now the question is why did customize.php and nav-menus.php end up in the $_wp_menu_nopriv
list when appropriate capabilities were being used.
The answer is in lines 93 - 117 of file wp-admin/includes/menu.php. Specifically the following lines:
if ( $new_parent != $old_parent ) { $_wp_real_parent_file[$old_parent] = $new_parent; $menu[$id][2] = $new_parent;
This piece of code replaces the parent menu's filename with the filename of its child menu. This results in the following values:
Array ( [0] => Appearance [1] => edit_theme_options [2] => nav-menus.php [3] => [4] => menu-top menu-icon-appearance [5] => menu-appearance [6] => dashicons-admin-appearance )
Notice how this menu got its parent's capability requirement. This causes nav-menus.php (or customize.php?return=%2Fwp-admin%2Fcustomize.php) to be added to the $_wp_menu_nopriv
list.
So when filenames are being replaced we should also replace their capability requirement.
#4
@
8 years ago
- Milestone changed from Future Release to 4.2
We'll be working to get this finalized for 4.2, along with related patches to add fine-grained caps for managing Widgets.
#7
@
8 years ago
- Keywords has-patch added; needs-patch removed
- Milestone changed from Future Release to 4.2
- Owner set to westonruter
- Status changed from new to assigned
#8
@
8 years ago
I'm not sure about the name edit_nav_menus
. Menu items are post objects (menus are taxonomies), and as such there are several related capabilities that could exist at a more fine-grained level. I think for the tasks of creating, editing, and deleting menus, manage_nav_menus
would be appropriate as all-encompassing of being able to do stuff with menus.
#9
@
8 years ago
Good call. For accessing the admin UI, the manage_nav_menus
makes sense I think. And then for users who can edit_theme_options
could then also by default have the edit_nav_menus
, delete_nav_menu
, etc capabilities.
#10
@
8 years ago
- Keywords dev-feedback added
- Owner changed from westonruter to ocean90
- Status changed from assigned to reviewing
In 29213.2.diff:
- Use
manage_menus
cap instead ofedit_nav_menus
. The use of “menus” makes sense instead of “nav_menus” because the theme support is called “menus”. Though at the same time, the post type is called “nav_menu_item” and the taxonomy is called “nav_menu”. I am not partial to either capability. I think celloexpressions should have the last word :-) - Bubble-up submenu admin page capability in addition to location; replace label if no children left. This is based on the great research by jesin. If the user lacks the capability for the original menu parent, then the capability and location of the first sub-menu item is assigned to the parent. If there is only one submenu item anyway, then the label is also copied up to the parent item.
- Show the “Menus” link in the admin bar if the user can
manage_menus
. Also, the change here also allows other menu items (specifically “Customize”) to appear in the admin bar if they have the required capability. Previously, the entire menu would short-circuit if the user lackededit_theme_options
.
Note that the changes to allow the Menus link to appear in the admin menu and the admin bar were also initially done in the Customizer for #28605. However, it was decided on IRC that Core support for displaying the Customize links would be removed and it would have to be manually added in by a plugin that also granted the user the customize
capability. We'll need to decide if that decision remains.
#11
@
8 years ago
By the way, here is a sample plugin which grants access to manage_menus
for a user who can edit_posts
:
<?php function allow_users_who_can_edit_posts_to_customize( $caps, $cap, $user_id ) { $required_cap = 'edit_posts'; if ( 'manage_menus' === $cap && user_can( $user_id, $required_cap ) ) { $caps = array( $required_cap ); } return $caps; } add_filter( 'map_meta_cap', 'allow_users_who_can_edit_posts_to_customize', 10, 3 );
#13
@
8 years ago
In 29213.3.diff: If there are no menus, check manage_menus
cap in WP_Nav_Menu
widget, and if the user lacks the cap, don't show a link to nav-menus.php
#14
@
8 years ago
In 29213.4.diff: Use manage_menus
cap check instead of edit_theme_options
in menu WP Ajax requests
#15
@
8 years ago
In 29213.5.diff: Use manage_menus
capability for Customizer section and settings for Navigation
#16
@
8 years ago
- Keywords 2nd-opinion added
This change is causing a problem:
Bubble-up submenu admin page capability in addition to location; replace label if no children left. This is based on the great research by jesin. If the user lacks the capability for the original menu parent, then the capability and location of the first sub-menu item is assigned to the parent. If there is only one submenu item anyway, then the label is also copied up to the parent item.
The legacy "Link Categories" menu to appear as a top-level admin menu item when the current user can manage_categories
but cannot manage_links
. In the beautiful wp-admin/menus.php
we have:
<?php // ... $menu[15] = array( __('Links'), 'manage_links', 'link-manager.php', '', 'menu-top menu-icon-links', 'menu-links', 'dashicons-admin-links' ); $submenu['link-manager.php'][5] = array( _x('All Links', 'admin menu'), 'manage_links', 'link-manager.php' ); /* translators: add new links */ $submenu['link-manager.php'][10] = array( _x('Add New', 'link'), 'manage_links', 'link-add.php' ); $submenu['link-manager.php'][15] = array( __('Link Categories'), 'manage_categories', 'edit-tags.php?taxonomy=link_category' );
As described above, the behavior is to move a submenu item to the parent if there is only one submenu item. In this case, the parent item was also inaccessible. The quick fix here could be to wrap this entire block in a if ( get_option( 'link_manager_enabled' ) )
check. However, I'm afraid that there could be other instances where submenu items inadvertently bubble up to the top admin menu, due to the brittleness of wp-admin/menus.php
.
For the admin menu, at least, we may need to go the route of the customize
cap and just require that plugins manually add admin menu items for the managing menus.
The links in the admin bar, however, could be a different story since it is not brittle.
Thoughts?
This ticket was mentioned in Slack in #core by drew. View the logs.
8 years ago
This ticket was mentioned in Slack in #core by drew. View the logs.
8 years ago
This ticket was mentioned in Slack in #core by drew. View the logs.
8 years ago
#20
@
8 years ago
- Milestone changed from 4.2 to Future Release
Citing discussion in Slack with @westonruter and later, @ocean90, punting this to a future release along with #31020.
#21
@
7 years ago
- Owner changed from ocean90 to westonruter
- Status changed from reviewing to accepted
This ticket was mentioned in Slack in #core by sergey. View the logs.
7 years ago
#23
@
7 years ago
- Keywords early added
- Owner changed from westonruter to johnbillion
- Status changed from accepted to assigned
+1
Like with
customize
, it should be a meta capability.