Avoid direct use of user capabilities in client-side code

[danielbachhuber]

In a variety of places throughout the codebase, user capabilities are directly referenced to determine whether UI should be displayed. For example, here's the check for displaying post sticky UI:

const userCan = get( user.data, 'post_type_capabilities', false );
if ( postType !== 'post' || ! userCan.publish_posts || ! userCan.edit_others_posts ) {
	return null;
}

This type of check, one where user capabilities are directly inspected, are incorrect because WordPress expects capabilities to be evaluated, and potentially modified, at runtime.

To illustrate, in the original edit_post() sticky post capability check, result of current_user_can() can be modified by the map_meta_cap and user_can filters:

if ( current_user_can( $ptype->cap->edit_others_posts ) && current_user_can( $ptype->cap->publish_posts ) ) {
	if ( ! empty( $post_data['sticky'] ) ) {
		stick_post( $post_ID );
	} else {
		unstick_post( $post_ID );
	}
}

The REST API provides this information for us on a high-level through the Allow header:

$ http --auth daniel:daniel OPTIONS wordpress-develop.test/wp-json/wp/v2/posts --headers
HTTP/1.1 200 OK
Access-Control-Allow-Headers: Authorization, Content-Type
Access-Control-Expose-Headers: X-WP-Total, X-WP-TotalPages
Allow: GET, POST

However, we don't yet have this information on an attribute by attribute basis (e.g. setting sticky requires publish_posts and edit_others_posts, while assigning an author requires edit_others_posts).

[Lewiscowles1986]

Is it just that the filters are not being used before localising information on caps for Gutenberg here, or are there more issues? I've looked at the other linked issue, but aside from both relating to capabilities, I'm not seeing a link, and this looks like more than one issue.

[danielbachhuber]

Is it just that the filters are not being used before localising information on caps for Gutenberg here, or are there more issues?

Yes. Fundamentally, the problem is that the map_meta_cap and user_can filters can (and do) change WordPress capabilities system, so we can't rely upon the static capability values. We need to execute the capabilities (as close to their runtime context) in order to know their true value.

Impacted components include:

• post-author
• post-pending-status
• post-publish-button
• post-publish-panel
• post-schedule
• post-sticky
• post-visibility
• [ ] Image / media upload (solved by Allows header instead).
• unfiltered_html checks

[TimothyBJacobs]

We chatted abit about this in the REST channel today. One of the ways we could do this is to utilize the targetSchema property of JSON Hyper Schema. An example of what this would look like for sticky post functionality.

{
  "_links": {
    "self": [],
    "wp:action-sticky_post": [
      {
        "title": "Sticky Post",
        "href": "https://w.org/wp-json/wp/v2/posts/5",
        "targetSchema": {
          "type": "object",
          "properties": {
            "sticky": {
              "type": "boolean",
              "description": "Whether or not the object should be treated as sticky."
            }
          }
        }
      }
    ]
  }
}

We would only include the wp:action-sticky_post link in the response if the current user has permission to perform that action.

The targetSchema part of things is showing what a PUT to the resource would conform to, mostly just a snapshot of the existing schema definition for the property. So for changing the author it'd be:

{
  "_links": {
    "self": [],
    "wp:action-author": [
      {
        "title": "Author",
        "href": "https://w.org/wp-json/wp/v2/posts/5",
        "targetSchema": {
          "type": "object",
          "properties": {
            "author": {
              "type": "integer",
              "description": "The ID for the author of the object."
            }
          }
        }
      }
    ]
  }
}

In JS land we'd check if the link with whichever relation we wanted exists in the item's response.

[danielbachhuber]

Thanks @TimothyBJacobs. This seems to be exactly what we want.

[felixarntz]

I agree this approach appears to solve the problem. Would we always return these actions as _links on every response, or should a request flag be required to get them?

We should probably provide some base logic and utility methods in WP_REST_Controller. Then we'd need a list of all the actions that should be handled for the posts controller. Other controllers could be updated as necessary. We might also consider introducing a filter for these links so that devs can integrate further actions in the same manner, when working with custom UI, or even as plugins inside of Gutenberg.

[danielbachhuber]

Would we always return these actions as _links on every response, or should a request flag be required to get them?

The former: the actions would be included on every response (but only when the user has permission to perform the action).

We should probably provide some base logic and utility methods in WP_REST_Controller. Then we'd need a list of all the actions that should be handled for the posts controller. Other controllers could be updated as necessary.

I think we should take on that abstraction when it proves necessary. At this point, we only need a few actions on the Posts controller.

We might also consider introducing a filter for these links so that devs can integrate further actions in the same manner, when working with custom UI, or even as plugins inside of Gutenberg.

The existing rest_prepare_{$post_type} seems sufficient for our needs (see implementation on the pull request.

[felixarntz]

The existing rest_prepare_{$post_type} seems sufficient for our needs (see implementation on the pull request.

Given that this filter works (via the PR), I'm okay with leaving away an abstraction for now. I'd only advise against actually changing the controller internally without considering an abstraction.

[danielbachhuber]

I'd only advise against actually changing the controller internally without considering an abstraction.

Can you explain what your abstraction might be, and how it would be a benefit?

[felixarntz]

Can you explain what your abstraction might be, and how it would be a benefit?

Not yet, I haven't really put thoughts into it, since we were not gonna do it. I guess I would consider introducing utility methods in WP_REST_Controller to reduce the amount of code we need to write for just a single action - like for the capability check, action identifier, and particularly the target schema, for which we could usually just grab a field from the typical item schema.

My main point is that if we start implementing that here (which I support), we should at some point publish a post on make.wordpress.org/core/ about this procedure, and open tickets to add similar actions to other endpoints. This is out of scope for Gutenberg obviously, but I'd like to ensure we follow a certain pattern for this functionality across all areas.

[danielbachhuber]

I've created a Trac ticket to get this into core: https://core.trac.wordpress.org/ticket/44287

[oandregal]

@danielbachhuber #7667 has landed a new canUserUseUnfilteredHTML selector and in #7865 the remaining uses of the unfiltered capability have been migrated to use this selector as well.

[danielbachhuber]

@nosolosw Awesome!