Introduce capability for access to nav-menus.php

[westonruter]

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.

[celloexpressions]

+1

Like with customize, it should be a meta capability.

[jesin]

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.

[westonruter]

We'll be working to get this finalized for 4.2, along with related patches to add fine-grained caps for managing Widgets.

[ocean90]

Let's wait for a patch first.

[westonruter]

There is a patch :-)

[celloexpressions]

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.

[westonruter]

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.

[westonruter]

​https://github.com/xwp/wordpress-develop/pull/63

[westonruter]

In 29213.2.diff​:

• Use manage_menus cap instead of edit_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 lacked edit_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.

[westonruter]

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 );

[westonruter]

Change: ​https://github.com/xwp/wordpress-develop/commit/54c9cb865d0e9c66ed3589f4f2fc1fde975dd122

[westonruter]

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

[westonruter]

​https://github.com/xwp/wordpress-develop/commit/d4acea4d793e5ec8ac3f7c5e92939bad4892a4d2#diff-790a19e573938525cbeb6660e3d96a2bR1443

[westonruter]

In 29213.4.diff​: Use manage_menus cap check instead of edit_theme_options in menu WP Ajax requests

[westonruter]

​https://github.com/xwp/wordpress-develop/commit/2546b4e14da9716cf2280a696a7d82377d407ba6

[westonruter]

In 29213.5.diff​: Use manage_menus capability for Customizer section and settings for Navigation

[westonruter]

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?

[DrewAPicture]

Citing ​discussion in Slack with @westonruter and later, @ocean90, punting this to a future release along with #31020.

[westonruter]

Patch needs to be updated to use capability inWP_Customize_Nav_Menus.

[flixos90]

All of these are part of a larger goal.