WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 14 months ago

Last modified 14 months ago

#16847 closed defect (bug) (invalid)

Capability check fails for custom post type revision edit (& map_meta_cap no good)

Reported by: anmari Owned by:
Milestone: Priority: normal
Severity: minor Version: 3.0
Component: Revisions Keywords: reporter-feedback has-patch dev-feedback
Focuses: Cc:

Description (last modified by SergeyBiryukov)

I am using nightly build (1 day old).

Steps to reproduce:

  1. Register post type with capability 'event'.
  1. Edit custom post type till you have some revisions.
  1. Attempt to view a revision. One gets sent to the normal posts edit.php screen.

I looked at revision.php and managed to work out that it was failing at

	if ( !current_user_can( 'read_post', $revision->ID ) || !current_user_can( 'read_post', $post->ID ) )


I tried

	if ( !current_user_can( 'read_'.$post->post_type, $revision->ID ) || !current_user_can( 'read_'.$post->post_type, $post->ID ) )

but that still failed.

I commented out the check and was then able to view the revision.

I think this affects the autosave too as that was how I started looking at it.

So it looks like the problem is in the current_user_can check somehow not working out that the author or admin user is allowed to view or edit the revision ?

Ticket #14122 may be relevant (detailed discussion about meta-caps?)

I found also Ticket #14749 says it fixed something similar, but that was not a capability problem, so not relevant.

I back tested and behaviour occurs in 3.0, 3.1 and the nightly build

Attachments (1)

16847.diff (2.1 KB) - added by adamsilverstein 15 months ago.
match cap checks in revisions to cap check used in edit

Download all attachments as: .zip

Change History (20)

comment:1 scribu3 years ago

  • Component changed from Revisions to Role/Capability
  • Keywords reporter-feedback added; needs-patch removed

Please paste the code you're using to define the post type.

Also, add the following code somewhere and paste the output:

function debug_role() {
	global $wp_roles;

	echo '<pre>';
	print_r( $wp_roles->get_role('your_role')->capabilities );
}
add_action('init', 'debug_role');

Obviously, replace 'your_role' with whatever role you're seeing the problem with.

comment:2 anmari3 years ago

Hello,

I worked out that the problem was that (in my plugin) while the admin user had plural capabilities "view_events", "edit_events", they did not have singular capabilities "view_event", "edit_event".

I had also recreated problem with another post type using brad's custom post type UI plugin (to rule out my code) and then justin's members plugin to add the custom capabilities to admin.

Using the CPT UI with default capability 'post' all is fine, can access revisions.
Change to custom capability, then one MUST add plural and singular of the capabilities. Plural not adequate access.

I then looked at edit.php to see what it was checking.

it does this:

if ( !current_user_can($post_type_object->cap->edit_posts) )

while revision.php does not use the post type object and is also a singular check:

if ( !current_user_can( 'read_post', $revision->ID ) || !current_user_can( 'read_post', $post->ID ) )

SO anyway immediate fix is:

make sure that the user has the singular capability at the very least, not just plural.

Philosophically should a user who has the plural capability be allowed to access the post even if they do not have the singular capability. This would make it functions same as edit screens etc?

Your debug code would have revealed this I think - do you need me to do anymore?

regards, anmari

comment:3 scribu3 years ago

  • Cc nacin added

plural capabilities = primitive capabilities (are actually assigned to the role)

singular capabilities = meta capabilities (map to one or more primitive capabilities)

Long story short, if you add 'map_meta_cap' => true to your post type definition, it should work.

Maybe nacin can shed some light on wether we should change from the raw 'read_post' to ->cap->read_post in revision.php

comment:4 anmari3 years ago

  • Summary changed from Capability check fails for custom post type revision edit to Capability check fails for custom post type revision edit (& map_meta_cap no good)
  • Version changed from 3.1 to 3.3

Hi,
added 'map_meta_cap' => true, to the post_type registration

Number of things then happen (latest nightly build and 3.2.1):

1) edit, and quick edit and trash links on custom post type manage screen dissappears (only have view) - take out the map_meta_cap and at least I can edit.

2) Bunch of notices on every edit screen which seem to relate to the fact that a 'null' post is picked up initially

Notice: Trying to get property of non-object in C:\web\wpbeta\wp\wp-includes\capabilities.php on line 992

$post = get_post( $args[0] );
if ( 'revision' == $post->post_type ) {

Notice: Trying to get property of non-object in C:\web\wpbeta\wp\wp-includes\capabilities.php on line 996

Notice: Trying to get property of non-object in C:\web\wpbeta\wp\wp-includes\capabilities.php on line 998

Notice: Trying to get property of non-object in C:\web\wpbeta\wp\wp-includes\capabilities.php on line 999

Notice: Trying to get property of non-object in C:\web\wpbeta\wp\wp-includes\capabilities.php on line 999

Notice: Trying to get property of non-object in C:\web\wpbeta\wp\wp-includes\capabilities.php on line 1002

Notice: Trying to get property of non-object in C:\web\wpbeta\wp\wp-includes\capabilities.php on line 1002

comment:5 anmari3 years ago

  • Version changed from 3.3 to 3.2.1

comment:6 westi15 months ago

  • Keywords revisions-3.6 added

Adding to the list of potential items for the Revisions Feature push in 3.6.

comment:7 follow-up: westi15 months ago

  • Component changed from Role/Capability to Revisions
  • Keywords revisions-3.6 removed
  • Milestone changed from Awaiting Review to 3.6

Moving to the Revisions Component for 3.6

Next steps:

  • Concrete reproductive steps on the ticket for the bug.

comment:8 in reply to: ↑ 7 adamsilverstein15 months ago

Replying to westi:

Moving to the Revisions Component for 3.6

Next steps:

  • Concrete reproductive steps on the ticket for the bug.

i am also unclear how to reproduce the bug.

comment:9 nacin15 months ago

I doubt there is a bug here at all.

Revisions simply use its parent post for cap checking. This is baked into map_meta_cap directly.

comment:10 SergeyBiryukov15 months ago

  • Description modified (diff)
  • Version changed from 3.2.1 to 3.0

comment:11 adamsilverstein15 months ago

  • Keywords has-patch added

i have been re-reading the ticket again trying to understand it. the root of the issue is that the edit screen makes a different check for capabilities before allowing access than the revisions review screen.

its pointed out above, but worth repeating with current links --

edit.php checks current_user_can( $post_type_object->cap->edit_posts ) - At line 21

revisions.php checks if ( !current_user_can( 'read_post', $revision->ID ) || !current_user_can( 'read_post', $post->ID ) ) At Line 111

the difference in the check apparently lead to the (difficult to reproduce) bug in this ticket.

if i understand it correctly the bug is still there and probably best would be solved by replacing the check in revisions to match the edit.php.

i have attached a patch for review that changes each of several calls in revisions.php to the same check used in edit.php, this should lead to better expected behavior, that is same capabilities required for editing a post than for editing a revision, which wasn't _quite_ the case before.

along the way i noticed the same calls repeating several times in different parts of the switch loop (you will see that in my patch). shouldn't we just move these common checks to the top of the switch loop.

Last edited 15 months ago by SergeyBiryukov (previous) (diff)

adamsilverstein15 months ago

match cap checks in revisions to cap check used in edit

comment:12 adamsilverstein15 months ago

  • Keywords dev-feedback added

comment:13 ethitter15 months ago

  • Cc erick@… added

comment:14 adamsilverstein15 months ago

  • Cc adamsilverstein@… added

comment:15 westi14 months ago

In 1212/tests:

Revisions: Test the capability checks used in wp-admin/revision.php against how we expect them to behave both with a built-in and a custom post type

See #16847 - Trying to explore CPTs and Custom Capabilities with revisions edit/view/restore

comment:16 westi14 months ago

I'm pretty sure there isn't a bug here that needs fixing which is what I tried to show in the above unit-tests.

Would like some feedback from someone who has done more CPT with Custom Capabilities work in core on my tests before closing this down - maybe @nacin can review the tests ;)

comment:17 nacin14 months ago

In 1213/tests:

Add a revisions capability test for when the post cannot be edited once published because the edit_published_posts cap is denied. See [1212/tests]. See #16847. Trims westi's whitespace.

comment:18 nacin14 months ago

  • Milestone 3.6 deleted
  • Resolution set to invalid
  • Status changed from new to closed

Not a bug from what I can find.

Note: See TracTickets for help on using tickets.