Custom "capabilities" appears broken on custom post types

[jakemgold]

It's impossible to create a functional equivalent to a "contributor" role unique to a new custom content type, no what approach one takes. Consider the code below (the most stripped down code that exemplifies the point):

register_post_type('movies',array(
	'label' => 'Movies',
	'singular_label' => 'Movie',
	'public' => true,
	'capabilities' => array( 'edit_posts' => 'edit_movies' ),
	'supports' => array('title','editor','author')
));

if ( !get_role('producer') )
{
	global $wp_roles;
	if ( !isset( $wp_roles ) ) $wp_roles = new WP_Roles();
	
	$caps = $wp_roles->get_role('subscriber')->capabilities;
	$caps = array_merge( $caps, array( 'edit_movies' => true ) );
	$wp_roles->add_role( 'producer', 'Movie Producer', $caps );
}

Note that the "Movies" admin menu does appear for the "producer" role, along with the "add new" option. However, entering content into the editor produces a "not allowed" type error, and hitting "submit for review" produces a similar error.

Even if I'm missing something (maybe with the meta capabilities?) if the user is not allowed to edit a content type, they should not be presented with the menu options, including "add new".

[nacin]

We didn't implement the mapping of capabilities of custom post types. Right now you have to roll our own handling of custom capabilities entirely, using the map_meta_cap filter. I imagine the errors are due to edit_post being the capability, which then goes through map_meta_cap and maybe it determines you need edit_posts (instead of edit_movies)? I'm not sure.

I'm not sure if it was our intent entirely to offer such fine-grained control and at least not offer the ability for map_meta_cap to do its magic.

[nacin]

We also have this bit of code:

		$post_type = get_post_type_object( $post->post_type );
		if ( $post_type && 'post' != $post_type->capability_type ) {
			$args = array_merge( array( $post_type->cap->edit_post, $user_id ), $args );
			return call_user_func_array( 'map_meta_cap', $args );
		}

But that doesn't take into account that $post_type->capability_type might be 'post', yet $post_type->cap->edit_psot might not be 'edit_post'. Hmm.

Definitely a lot of things to think about here.

[jakemgold]

"I imagine the errors are due to edit_post being the capability, which then goes through map_meta_cap and maybe it determines you need edit_posts (instead of edit_movies)?"

No. Even if you change the code above to use a completely custom "capability_type" (rather than defining granular capabilities) like "movie" (which gets "edit_movie", "edit_movies" etc) this same behavior persists.

Same ability to access Add New screen, same errors in editor and upon saving. Even more strange, assigning "edit_movie" (singular) to "producer" (in example) as a primitive capability will allow the user to successfully save the post, but the editor error still persists. It will also, of course, allow the producer to edit other users posts (despite lacking that capability), but this is to be expected, I suppose, since it's supposed to be a meta capability.

We can debate the definition of a "bug" - it may not be broken / crashing code, but I think it's broken from the perspective of "expected" API behavior for developers. But it seems to me that mapping meta capabilities should be "auto-magic" for custom post types, especially public ones.

[duck_]

Replying to jakemgold:

"I imagine the errors are due to edit_post being the capability, which then goes through map_meta_cap and maybe it determines you need edit_posts (instead of edit_movies)?"

No. Even if you change the code above to use a completely custom "capability_type" (rather than defining granular capabilities) like "movie" (which gets "edit_movie", "edit_movies" etc) this same behavior persists. Same ability to access Add New screen, same errors in editor and upon saving.

nacin's right. If you use 'capabilities' => array( 'edit_posts' => 'edit_movies' ) then map_meta_cap will see that $post_type->capability_type is equal to 'post' and then decide that the edit_post maps to edit_posts. On the other hand, if you use 'capability_type' => 'movie' then map_meta_cap will notice the difference and call itself again checking the edit_movie meta-cap, however there is no matching case for this and so the default is to return edit_movie as the capability required. (In the latter case you could, as suggested, hook onto the map_meta_cap filter and check if $cap == 'edit_movie' etc.)

Even more strange, assigning "edit_movie" (singular) to "producer" (in example) as a primitive capability will allow the user to successfully save the post, but the editor error still persists. It will also, of course, allow the producer to edit other users posts (despite lacking that capability), but this is to be expected, I suppose, since it's supposed to be a meta capability.

When capability_type is movie and my producer has the edit_movie capability I do not see the autosave error in the editor.

[nacin]

Slating this for 3.1 Initial thoughts:

• Opt-in automagic meta capability handling, the way core does it.

• Splitting capability_type based on what appears to be two different functionalities -- re-routing in map_meta_cap, but also controlling the defaults for the capabilities array. That way we're splitting the default naming of the capabilities array from this remapping. Only if we can do this in a back compat way. Ideas welcome.

• Allowing capability_type to take an array() such as array( 'box', 'boxes' ), to handle odd plurals.

[hakre]

Any follow-up to the initial thoughts we have so far?

[thenbrent]

For Jake and others looking to achieve this, here is a plugin that maps the meta caps for a custom post:
​http://wordpress.org/extend/plugins/map-cap

Took a while to figure out how to do it for my own needs so thought I'd release to help others.

It's a bit different to the solution required for the core. I also imagine Andrew has a better idea for how to map the capabilties in the core, but if this feature goes ahead, the plugin should provide a good start for someone else thinking about providing the patch.

[nacin]

This needs some love. Anyone want to pick it up?

[kevinB]

I've been pondering this topic extensively over the last few months and have some code to throw into the fray. Will plan on submitting a patch by Oct 13th.

[nacin]

Replying to kevinB:

I've been pondering this topic extensively over the last few months and have some code to throw into the fray. Will plan on submitting a patch by Oct 13th.

Our feature freeze is next week, and I don't want to cut it close, so I must warn you that I or someone else might end up getting to this first.

[kevinB]

Okay, I didn't realize it was that close. I'll try to put something together Saturday night.

[nacin]

Entirely untested. Probably missing some components.

[nacin]

(In [15890]) Rough first pass on map_meta_cap for custom post types. see #14122.

[nacin]

(In [15891]) Only check the post author if the post type supports authors. see #14122.

[nacin]

[15892]

[kevinB]

Are you interested in making map_meta_cap deal with custom stati?

I have a revision that does, but have been waiting to see if the status cap approach I proposed for #12706 will be accepted.

Along with the custom stati patch there, I attached a plugin which calls register_post_status and provides meta cap mapping which supports custom type and custom status.

If you and ptah take a look at that patch and plugin, I think you will find my code more advanced and WP-standards-compliant than my trac/svn protocol.

[Utkarsh]

Notice: Undefined index: delete_posts in ...\wp-includes\post.php on line 1022

[nacin]

(In [15897]) Only dumb down delete_others_posts if it deserves to exist. fixes a notice. see #14122.

[Utkarsh]

One more, possibly related.

Notice: Undefined property: stdClass::$read in ...\wp-includes\capabilities.php on line 916

[scribu]

Yeah, seeing that in the post revisions box.

[nacin]

Replying to scribu:

Yeah, seeing that in the post revisions box.

That's what I get for temporarily turning off WP_DEBUG and forgetting about it.

Looking through everything now.

[nacin]

(In [15909]) Revisions and attachments should also generate meta capabilities as part of their cap object. see #14122.

[automattor]

(In [15934]) Allow capability_type to be an array, for odd plural situations such as story/storys/stories. After registration it reverts to a singular string. Lots of documentation for meta capabilities and post types, also some cleanups for register_post_type documentation. see #14122.

[nacin]

(In [15935]) Remove some capability_type code in WP_Query. Only build faux caps if we're querying for multiple post types or if the post type object can't be found. see #14122.

[jane]

There are a couple more days before we call freeze if you guys think you can wrap this up to get it into 3.1.

[nacin]

There's one specific bug reported over wp-testers I want to investigate, otherwise I'm prepared to close this and suggest new tickets should be opened.

[scribu]

1. Register a public post type:

register_post_type( 'foo', array('public' => true) );

2. Go to /wp-admin/post-new.php?post_type=foo

The following notice shows in the Publish box:

Notice: Undefined property: stdClass::$delete_posts in /home/cristi/svn/wp/wp-includes/capabilities.php on line 852

[WraithKenny]

In addition to scribu's test, on /wp-admin/edit.php?post_type=foo the same notice is present.

Interestingly, if you don't publish, and just save draft, the entry is still editable.

If you publish, then the /wp-admin/edit.php?post_type=foo screen shows,
Notice: Undefined property: stdClass::$edit_published_posts in /mnt/stor1-wc1-dfw1/384190/384265/ken.unfocus.com/web/content/wp-includes/capabilities.php on line 890
twice above the table, and once below the view link, and also a
Notice: Undefined property: stdClass::$delete_published_posts in /mnt/stor1-wc1-dfw1/384190/384265/ken.unfocus.com/web/content/wp-includes/capabilities.php on line 846 above the link.

The entry is also editable.

[WraithKenny]

UN-editable is what I meant above.

[WraithKenny]

$edit_published_posts, $delete_posts, and $delete_published_posts (etc.) are checked for, but aren't set by register_post_type unless 'map_meta_cap' is true, but the default is set to false. (Not setting 'map_meta_cap' didn't break permissions in 3.0 so this is a regression?)

​http://core.trac.wordpress.org/browser/trunk/wp-includes/post.php#L1066

[scribu]

So I guess 'map_meta_cap' should default to true, no?

[WraithKenny]

 if ( empty($args->capability_type) )
	                $args->capability_type = 'post';

This check doesn't seem to do anything because the default value is 'post' so it should never actually be empty right? (Though I suppose you could pass it )

If you add below that:

 if ( 'post' == $args->capability_type && empty($args->capabilities) ) )
	                $args->map_meta_cap = true;

and then the default can stay the same if its important that it not change... I'm not sure of the decisions that where made there.

[WraithKenny]

Adds a check to see if register_post_type is using default capabilities and turns on map_meta_cap

[WraithKenny]

Some of the other errors I thought were from this (but weren't) are because I used Upper-case/underscore in the $post_type name.

See #14910 (unrelated but the errors would look similar)

[nacin]

So here's what I've been able to track down. My code was based on a false assumption.

Let's say you have a thing post type: capability_type = post (default), capabilities = array( edit_things, edit_thing, etc. )

The problem is that capability_type acts not only as a base, or as a switch for map_meta_cap, but also as a faulty switch in map_meta_cap. The end result is that if you want to edit_thing 4, you end up with also needing edit_published_posts etc. This only occurs when capability_type = post, of course. Even 'page' will not trigger this kind of handling.

To me, it's faulty meta capability handling, so I'm calling this a bug, and I intend to change functionality so edit_published_posts will not be required for editing thing 4, regardless of capability_type.

The solution (which I'll work on) is to re-work the code in map_meta_cap to not map when map_meta_cap is false, end of discussion.

Does anyone see an issue with this? Alternative would be WraithKenny's patch, but keep in mind it would then notice out when capability_type = post and a custom capabilities array is set, so it's not enough.

[WraithKenny]

The only issue I see is that ​http://core.trac.wordpress.org/browser/trunk/wp-includes/capabilities.php#L890 checks the edit_published_post capability which means any post_type registered with map_meta_cap => false can not be deleted after publishing, even by admins, right?

My patch was merely a work-around for default settings, but unless I'm missing something, it's a bigger issue with map_meta_cap => false. I would figure, admins should be granted permissions for all post_types.

[WraithKenny]

Perhaps the use case is a special user with exclusive rights (that even admins don't have I guess) to a CPT, in which case, a custom role with custom caps would be created, so they may as well set map_meta_cap explicitly to false. This would be a rare and limited case and would suggest that the default be true on map_meta_cap.

[nacin]

Replying to WraithKenny:

The only issue I see is that ​http://core.trac.wordpress.org/browser/trunk/wp-includes/capabilities.php#L890 checks the edit_published_post capability which means any post_type registered with map_meta_cap => false can not be deleted after publishing, even by admins, right?

No, I'm saying, that code would be completely skipped. No mapping if map_meta_cap = false, regardless of capability_type.

My patch was merely a work-around for default settings, but unless I'm missing something, it's a bigger issue with map_meta_cap => false. I would figure, admins should be granted permissions for all post_types.

This would be a rare and limited case and would suggest that the default be true on map_meta_cap.

It's by default false already. The only exception is when capability_type = post, but I consider that to be a bug in most cases. On the other hand a straight capability_type = post with no capabilities array should probably be map_meta_cap = true. That I can agree with. I think I will need to diagram it out.

[WraithKenny]

#15334 closed as duplicate of this one.

[nacin]

Here's a big issue with 3.0 functionality.

{{{
register_post_type( 'book', array(
		'capability_type' => 'post',
		'capabilities' => array( 'edit_post' => 'edit_book' ),
	)
);
$user_id = 1;
$post_id = wp_insert_post( array( 'post_title' => 'book', 'post_type' => 'book', 'post_status' => 'private', 'post_author' => 1 ) );
var_dump( map_meta_cap( 'edit_post', $user_id, $post_id ) );
$post_type_obj = get_post_type_object( 'book' );
var_dump( map_meta_cap( $post_type_obj->cap->edit_post, 1, $post_id ) );
}}}
You should get two arrays that match. They won't, though. Checking through the post type cap will give you edit_book (no mapping), but checking through edit_post will give you edit_private_posts based on the faulty meta cap.

The bug everyone has been seeing is caused by a direct edit_post call in the list table. While core shouldn't have any of these, we need to remain compat for plugins. They should work the same.

[nacin]

Re-post:

---

Here's a big issue with 3.0 functionality.

register_post_type( 'book', array(
		'capability_type' => 'post',
		'capabilities' => array( 'edit_post' => 'edit_book' ),
	)
);
$user_id = 1;
$post_id = wp_insert_post( array( 'post_title' => 'book', 'post_type' => 'book', 'post_status' => 'private', 'post_author' => 1 ) );
var_dump( map_meta_cap( 'edit_post', $user_id, $post_id ) );
$post_type_obj = get_post_type_object( 'book' );
var_dump( map_meta_cap( $post_type_obj->cap->edit_post, 1, $post_id ) );

You should get two arrays that match. They won't, though. Checking through the post type cap will give you edit_book (no mapping), but checking through edit_post will give you edit_private_posts based on the faulty meta cap.

The bug everyone has been seeing is caused by a direct edit_post call in the list table. While core shouldn't have any of these, we need to remain compat for plugins. They should work the same.

[nacin]

(In [16273]) Cripple capability_type. Produced inconsistent, janky meta cap mapping; now only acts as a capability base. see #14122.

[nacin]

(In [16337]) Use cap->edit_post in WP_Posts_List_Table. see #14122.

[nacin]

Removes the checks on post_type_supports('author'). Leaving this in would be a change from 3.0, but if most people are using 'supports' for meta boxes but still storing values in post_author, then I'm unsure of the side effects here.

[nacin]

map_meta_cap becomes true if capability_type = post and no capabilities were specified. This allows capability_type = post (and nothing else) to work exactly as it did in 3.0, so I'm leaning towards commit on this one.

[nacin]

Two more patches attached based on further testing for inconsistencies and regressions.

14122.capability_type.post.diff​ allows capability_type = 'post' to kick on map_meta_cap, provided that no custom caps were defined. This makes sense to commit.

14122.remove.post_type_supports.author.diff​ would remove the post_type_supports('author') checks on edit_others_posts and delete_others_posts. The side effects could be interesting if people are storing and retrieving post_author (and even expecting caps to be mapped on it), and simply using 'supports' for the meta boxes and quick edit field. This seems to be what people have been doing with the 'supports' args, unfortunately, so it is probably best to pull this.

[nacin]

Justifying 14122.capability_type.post.diff further:

In 3.0, per duck_ earlier in this ticket:

If you use 'capabilities' => array( 'edit_posts' => 'edit_movies' ) then map_meta_cap will see that $post_type->capability_type is equal to 'post' and then decide that the edit_post maps to edit_posts.

Because we used strings in map_meta_cap until now, then if capabilities is not empty, you should assume mapping did not work (without a filter).

Continuing:

On the other hand, if you use 'capability_type' => 'movie' then map_meta_cap will notice the difference and call itself again checking the edit_movie meta-cap, however there is no matching case for this and so the default is to return edit_movie as the capability required. (In the latter case you could, as suggested, hook onto the map_meta_cap filter and check if $cap == 'edit_movie' etc.)

Thus, capability_type = post && empty( capabilities ) means that mapping should be triggered.

[nacin]

(In [16338]) Set map_meta_cap to true when capability_type = post and no custom caps are specified. see #14122.

[nacin]

This should get a 31compat post on wpdevel, but who knows where to start.

[nacin]

Another issue I see is that a plugin may have been using meta capability handling via capability_type = post but only partially. i.e. they mapped edit_posts to edit_movies and may have handed that on their own, but then let delete_post take its course. (Very similar to the OP.)

At that point, we'd need fine-grained control for map_meta_cap, not just on/off, but rather on/off for read, edit, and delete independently. That seems silly.

Other than a decision on 14122.remove.post_type_supports.author.diff​, I am inclined to let this sit, and wait for bug reports once beta is out.

[nacin]

(In [16387]) capability_type = page and no custom caps should also kick on map_meta_cap. see #14122.

[nacin]

Okay, for 14122.remove.post_type_supports.author.diff​, I realized that the changes to each file can be optionally considered separately.

We can make the default capabilities be edit_posts/delete_posts if the post type doesn't support an author -- that doesn't prevent further customization from relying on the author, however, so no real restriction. So I would not revert the change from wp-includes/post.php.

On the other hand, we can continue to check post_author in map_meta_cap() even if the post type doesn't support author. If it ends up using cap->edit_others_posts, then it'll be either the default 'edit_posts', or whatever is specified.

Going to remove the bits from map_meta_cap().

---

Unit tests can be handled in ​#UT11.

[nacin]

(In [16422]) Don't check post_type_supports in map_meta_cap. see #14122.

[nacin]

(In [16825]) Remove post_type_supports check from register_post_type caps all together. fixes #14122.

[nacin]

Aftermath: #15779

[nacin]

​http://unit-tests.trac.wordpress.org/changeset/424