Make WordPress Core

Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#29714 closed defect (bug) (duplicate)

user_can_access_admin_page() returning false for edit.php?post_type=CPT

Reported by: bobbingwide's profile bobbingwide Owned by:
Milestone: Priority: normal
Severity: normal Version: 4.0
Component: Role/Capability Keywords: has-patch needs-unit-tests
Focuses: Cc:

Description

I have a Custom Post Type (CPT) for which I intend to allow registered subscribers the capability to edit posts, but not create posts or manage the custom taxonomies for the posts.

In this example the CPT is "oik_site" - plural label "Sites"

When the registered user is logged and viewing the Dashboard then the admin menu correctly shows the only available option; Sites - which invokes wp-admin/edit.php?post_type=oik_site

When the user clicks on the link WordPress dies with "You do not have sufficient permissions to access this page."

The expected result is that the user should be shown the list of sites, without the Add New button.

I have tracked the problem down to what I believe to be a bug in user_can_access_admin_page().

The "oik_site" CPT is defined with

  $post_type_args['capability_type'] = 'oik_site';
  $post_type_args['capabilities'] = array( 'create_posts' => 'create_oik_sites' );
  $post_type_args['map_meta_cap'] = true;

The 'create_posts' capability is defined as 'create_oik_sites', overriding the default 'edit_oik_sites'.

Subscribers are given the 'edit_oik_sites' capability only.

Where it goes wrong...

The processing in wp-admin/includes/menu.php has correctly checked the user's capability to "edit_oik_sites" and the admin menu has been simplified so that the 'All Sites' sub menu item is no longer displayed.
Since the user doesn't have either create_oik_sites nor manage_categories the Add New and Custom Taxonomy submenu items have been deleted.

Everything seems fine until we call user_can_access_admin_page().
Here it determines $parent to be null and $pagenow to be "edit.php".

$wp_menu_nopriv is correctly set to the menu items that the subscriber cannot use

    [edit.php] => 1
    [upload.php] => 1
    [link-manager.php] => 1
    [edit.php?post_type=page] => 1
    [edit-comments.php] => 1
    ...

Note: edit.php?post_type=oik_site IS NOT SET in the $wp_menu_nopriv array.

Since these tests are true the function returns false.

 if ( empty( $parent) ) {
    if ( isset( $_wp_menu_nopriv[$pagenow] ) )
      return false;

Had the second test taken into account the post_type being edited then this would not have failed.

Proposed fix

Adding the following code after the test on empty( $parent ) gives the expected results.

    if ( $pagenow == "edit.php" && isset( $_REQUEST['post_type'] ) ) {
      $pagenow .= '?post_type=' . $_REQUEST['post_type' ];
    }

Attachments (2)

29714.diff (517 bytes) - added by bobbingwide 10 years ago.
If $pagenow = edit.php update it to reflect the post_type being edited
29714-2.diff (1.5 KB) - added by bobbingwide 10 years ago.
don't update the global $pagenow. Use a copy in $pagetest.

Download all attachments as: .zip

Change History (12)

#1 @bobbingwide
10 years ago

Workaround
The above fix can be implemented as a workaround.

Create a filter function for "user_has_cap". In that filter function, when the capability being checked for is that which you have defined ( e.g. "edit_oik_sites"), add the following logic.

global $pagenow;
if ( $pagenow == "edit.php" && isset( $_REQUEST['post_type'] ) ) {
        $pagenow .= '?post_type=' . $_REQUEST['post_type' ];
}

This will then allow the failing function ( user_can_access_admin_page() ) to return true.

@bobbingwide
10 years ago

If $pagenow = edit.php update it to reflect the post_type being edited

#2 @bobbingwide
10 years ago

  • Keywords has-patch added

I've added my proposed fix.

#3 @bobbingwide
10 years ago

  • Keywords reporter-feedback added

I've just discovered that the change to the $pagenow global variable has unwanted side effects during later processing. Certain action and filter names get corrupted.

e.g. for a post_type of oik_site we get the following incorrect names

admin_footer-edit.php?post_type=oik_site
admin_head-edit.php?post_type=oik_site
admin_print_scripts-edit.php?post_type=oik_site
admin_print_styles-edit.php?post_type=oik_site
bulk_actions-edit.php?post_type=oik_site
get_user_option_manageedit.php?post_type=oik_sitecolumnshidden
load-edit.php?post_type=oik_site
manage_edit.php?post_type=oik_site_columns
manage_edit.php?post_type=oik_site_sortable_columns
views_edit.php?post_type=oik_site

In all cases the 'edit.php?post_type=' part of the name should just be 'edit-' or 'edit.php'

Obviously, some more work is needed.

#4 @bobbingwide
10 years ago

To overcome the unwanted side effects of changing $pagenow the second half of the workaround resets the value as soon as possible after user_can_access_admin_page(). The next filter that is invoked is 'add_menu_classes' invoked by add_menu_classes().

add_filter( "add_menu_classes", "oik_sites_add_menu_classes" );
/**
 * Implement "add_menu_classes" filter for oik-sites
 *
 * Part 2 of the Workaround for TRAC #29714  
 */
function oik_sites_add_menu_classes( $menu ) {
  global $pagenow;
  if ( false !== strpos( $pagenow, "edit.php" ) ) {
    $pagenow = "edit.php";
  }
  return( $menu );
}

@bobbingwide
10 years ago

don't update the global $pagenow. Use a copy in $pagetest.

#5 @bobbingwide
10 years ago

  • Keywords needs-unit-tests added; reporter-feedback removed

The new patch (29714-2.diff) does not change $pagenow. It copies the value to $pagetest before modifying it. This won't create the side effects. The rest of the function uses $pagetest instead of $pagenow.

#6 @johnrom
10 years ago

In the example workaround above, $pagenow will be changed again the next time user_has_cap is called. I would switch to the "custom_menu_order" hook as it is only called once.

Alternately, I came up with a similar workaround before I found this ticket (I found this when writing out a TRAC ticket, "similar threads" is very useful). Instead of manipulating something so mission critical as $pagenow, I added a menu item to the submenu exactly where the check would expect it to be, and then removed it right after the checkpoint.

/**
 * Workaround for TRAC #29714
 * @hooked custom_menu_order
 */
function hack_menu_to_allow_add_child_cpt( $custom_menu_order ) {
	global $submenu;

	$parent_post_type = 'cpt';
	$my_post_type = 'child_cpt';

	$my_post_type_object = get_post_type_object( $my_post_type );
	$create_cpt_capability = $my_post_type_object->cap->create_posts;

	$parent_post_type_edit = "edit.php?post_type={$parent_post_type}";
	$child_post_type_new = "post-new.php?post_type={$my_post_type}";

	if ( ! empty( $submenu[ $parent_post_type_edit ] ) ) {
		$submenu[ $parent_post_type_edit ][] = array(
			'Add Child CPT',
			$create_cpt_capability,
			"post-new.php?post_type={$my_post_type}",
			'New Child CPT',
		);
	}

	// we didn't do anything with this
	return $custom_menu_order;
}
add_filter('custom_menu_order', 'hack_menu_to_allow_add_child_cpt');


/**
 * Part 2: Workaround for TRAC #29714
 * @hooked add_menu_classes
 */
function hack_unset_menu_item_after_child_cpt_perms( $menu ) {
	global $submenu;

	$parent_post_type = 'cpt';
	$my_post_type = 'child_cpt';

	$parent_post_type_edit = "edit.php?post_type={$parent_post_type}";
	$child_post_type_new = "post-new.php?post_type={$my_post_type}";

	if ( ! empty( $submenu[ $parent_post_type_edit ] ) ) {
		foreach( $submenu[ $parent_post_type_edit ] as $submenu_item_key => $submenu_item ) {

			if ( $child_post_type_new == $submenu_item[2] )
				unset( $submenu[ $parent_post_type_edit ][ $submenu_item_key ] );
		}
	}

	// we didn't do anything with this either
	return $menu;
}
add_filter('add_menu_classes', 'hack_unset_menu_item_after_child_cpt_perms');

#7 @bobbingwide
10 years ago

Hi johnrom, glad I'm not the only one with the problem.
Did you try using the patch 229714-2.diff?
Obviously I'd prefer to see a fix rather than having to continue to use a workaround.

I was hoping that this defect might actually get reviewed one day

#8 @johnrom
10 years ago

I did not and will not have the availability for a few days. However, I can see that it is possible that $_wp_submenu_nopriv would have never been populated with the correct values that you are trying to test for, since $pagenow was not set to the same value during the creation of $_wp_submenu_nopriv

That's all I've got for now, and it's only a guess. Have you tested to ensure that a user role without the privileges to edit your custom post type (but with and without edit_posts and create_posts) is indeed unable to reach the post-new page?

#9 @bobbingwide
10 years ago

  • Resolution set to duplicate
  • Status changed from new to closed

After discussion with johnbillion at #wpldn contributor day we've agreed that this defect is a duplicate of #22895.

We also note that my 29714-2.diff is a better fix than nacin's original since it doesn't have any downstream side effects.

Also note that the workaround DOES work in 4.1-beta2.

Last edited 10 years ago by bobbingwide (previous) (diff)

#10 @DrewAPicture
10 years ago

  • Milestone Awaiting Review deleted

Duplicate of #22895.

Note: See TracTickets for help on using tickets.