Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose the Site Editor Patterns page to all users #61637

Open
t-hamano opened this issue May 14, 2024 · 15 comments
Open

Expose the Site Editor Patterns page to all users #61637

t-hamano opened this issue May 14, 2024 · 15 comments
Labels
[Feature] Patterns A collection of blocks that can be synced (previously reusable blocks) or unsynced [Type] Enhancement A suggestion for improvement.

Comments

@t-hamano
Copy link
Contributor

t-hamano commented May 14, 2024

Further improve #52150

What problem does this address?

#52150 aims to expose the Site Editor Patterns page for every "theme".

However, currently the Site Editor is only accessible to admin users. For non-admin users, links related to the Pattern page link to the classic post list (edit.php?post_type=wp_block).

What is your proposed solution?

We should be able to remove this restriction and allow all users to access the Site Editor Patterns page.

To achieve this, I think we need to implement at least the following points:

  • Even if the current theme is a block theme, non-admin users are not allowed to access anything other than the Patterns page.
  • Check capabilities to restrict actions such as creating, duplicating, deleting, and editing patterns. The "wp_block" post type has complex capabilities.
  • Implement redirects in case the post list page is bookmarked or the post list page is hard-coded by third-party product developers.
@t-hamano t-hamano added [Type] Enhancement A suggestion for improvement. [Feature] Patterns A collection of blocks that can be synced (previously reusable blocks) or unsynced labels May 14, 2024
@t-hamano
Copy link
Contributor Author

There may be a better approach, but the capabilities seem to be available through the API. Works with all roles except Subscriber role.

const currentUserId = wp.data.select('core').getCurrentUser().id
wp.data.select('core').getUser(currentUserId)?.capabilities

image

@youknowriad
Copy link
Contributor

Throughout our code base we generally use canUser "core" selector to check capabilities.

@jsnajdr Just noting that this issue might mean introducing "protected routes" in the site editor routing.

@jsnajdr
Copy link
Member

jsnajdr commented May 14, 2024

That should place any special demands on the router: just register only these routes that correspond to the canUser capabilities, e.g., canUser( 'edit_theme_options' ), and limit what is shown in the top-level sidebar.

We'll have to preload the user REST data, if they aren't being preloaded already.

Security concerns (user can't see certain data) should be fully addressed by the REST API -- the responses shouldn't contain any data that the non-admin user is not supposed to see.

Calypso, the WordPress.com dashboard SPA, has been doing all this (preloading the user, conditional routes and sidebars) since forever, it's not a big deal.

@t-hamano
Copy link
Contributor Author

I hope to address this issue at least a little bit for WP6.7.

Furthermore, if we aim to expose the entire site editor to all users, I think checking user permissions may be necessary not only for the Patterns page but also for other views.

And that issue may also be relevant to moving forward with #62371.

Calypso, the WordPress.com dashboard SPA, has been doing all this (preloading the user, conditional routes and sidebars) since forever, it's not a big deal.

I've tried digging into the Calypso code, but I wonder if adding a selector like the one below to Gutenberg would be a good first step?

https://github.com/Automattic/wp-calypso/blob/f883636501e952a8860b8a67db12037e97ab6a81/client/state/selectors/can-current-user.js

https://github.com/Automattic/wp-calypso/blob/f883636501e952a8860b8a67db12037e97ab6a81/client/my-sites/add-ons/main.tsx#L104

I'd be happy to hear any suggestions on what to do first.

@jsnajdr
Copy link
Member

jsnajdr commented Jun 24, 2024

I wonder if adding a selector like the one below to Gutenberg would be a good first step?

Yes, being able to check any user capability in JS code like this would be great:

wp.data.select( 'core' ).canUser( 'manage_options' );

This should map directly to a server-side PHP call current_user_can( 'manage_options' ). But when I'm looking at it more closely, it looks like non-trivial amount of work 🙂

The current canUser selector looks like it's almost there, but it does something different. It sends an OPTIONS request to a particular REST endpoint (e.g., "create a post") and the return value is the result of that endpoint's permission_callback. These callbacks often call current_user_can, but direct access to generic current_user_can( $capability ) is not available in Core REST API.

Jetpack and WP.com have this as part of the REST endpoint that retrieves info about a site. There is a capabilities field that lists all capabilities of the current user: https://github.com/Automattic/jetpack/blob/trunk/projects/plugins/jetpack/sal/class.json-api-site-base.php#L920-L957 And Calypso uses this field in the canCurrentUser selector.

But Core doesn't have anything like this, as far as I know.

@t-hamano
Copy link
Contributor Author

As I looked into in this comment, it seems that capabilities itself can be obtained via the /users/ endpoint.

wp.apiFetch( {
    path: '/wp/v2/users/1',
    method: 'POST',
} )
    .then( ( response ) => {
        console.log(response)
    } )

capabilities

Do we need an endpoint that returns the result of running current_user_can on the server side, rather than relying directly on capabilities retrieved via this endpoint?

@jsnajdr
Copy link
Member

jsnajdr commented Jun 24, 2024

it seems that capabilities itself can be obtained via the /users/ endpoint.

That's great, I didn't notice it before. The /users/me endpoint returns it only when there is a context=edit parameter. Without it, the field is not there.

Screenshot 2024-06-24 at 12 22 39

The return value of select( 'core' ).getCurrentUser() also doesn't have capabilities.

That means we need to patch the REST request to include context=edit, and then either integrate it into canUser (is that possible in a nice way?) or create a new selector.

@dlh01
Copy link
Contributor

dlh01 commented Jul 11, 2024

Do we need an endpoint that returns the result of running current_user_can on the server side, rather than relying directly on capabilities retrieved via this endpoint?

Maybe this doesn't need to be said or isn't relevant to this particular issue, but, just in case, whether a user has a capability depends not only on the capabilities assigned to their user object but the map_meta_cap and user_has_cap filters in PHP.

For example, using the user_has_cap filter, individual users might be granted edit_theme_options even if their role isn't assigned that by default. Super admins in a multisite can do anything on a site regardless of their role unless they're specifically blocked with the do_not_allow capability, often via the map_meta_cap filter.

The map_meta_cap() function itself also contains various special cases for things like deleting the default category or the privacy policy page.

@jsnajdr
Copy link
Member

jsnajdr commented Jul 11, 2024

whether a user has a capability depends not only on the capabilities assigned to their user object but the map_meta_cap and user_has_cap filters in PHP.

This is a good point. The users REST endpoint handler doesn't call these filters when building the capabilities field in the response. It just returns the plain $user->allcaps field of the user object. You have to call the $user->has_cap() method to get the filters applied.

I think this is a bug in the users endpoint, it should return the real, filtered capabilities. Jetpack has a similar functionality, for retrieving the current user's capabilities on a given site, and it's implemented by enumerating all supported capabilities and calling current_user_can for each of them separately. That gets the filters applied. The Core endpoint should do it the same way.

@dlh01
Copy link
Contributor

dlh01 commented Jul 12, 2024

I think this is a bug in the users endpoint, it should return the real, filtered capabilities. Jetpack has a similar functionality, for retrieving the current user's capabilities on a given site, and it's implemented by enumerating all supported capabilities and calling current_user_can for each of them separately. That gets the filters applied. The Core endpoint should do it the same way.

Yeah, this would certainly improve the accuracy of the endpoint, as long as core didn't assume that a capability has to be in the list for a user to possess it (the super admin example, among other use cases).

@t-hamano
Copy link
Contributor Author

Thank you for all of your research. When I submitted #63623, I learned that the template/template part endpoint permission checks only "edit_theme_options".

In the future, the site editor should be accessible to users without the edit_theme_options capability, so we may need to revise the endpoint to check for a more appropriate capability, such as create_posts.

@jsnajdr
Copy link
Member

jsnajdr commented Jul 22, 2024

In the future, the site editor should be accessible to users without the edit_theme_options capability

Currently all data related to the site editor quite consistently require edit_theme_options even for reading. Look at the register_post_type calls for all the built-in post types. To read wp_template, wp_template_part, wp_global_styles, wp_navigation, wp_font_family, wp_font_face, you need the edit_theme_options capability.

The only exception are nav_menu_item, custom_css and customize_changeset. These require special permissions for writing, but are much more liberal about reading.

@youknowriad Are the strict read permissions really intentional, with good reasons? Or is it mostly an omission?

I learned that the template/template part endpoint permission checks only "edit_theme_options".

By the way, the WP_REST_Templates_Controller::permissions_check method linked to by the link seems a bit redundant to me. The permissions are sufficiently defined by the wp_template post type. Shouldn't the callback be checking these permissions, instead of doing a duplicate explicit edit_theme_options check?

@t-hamano
Copy link
Contributor Author

I'm a bit confused, but at least with regards to patterns, we should be able to create patterns if you have create_posts capability even without the edit_theme_options capability.

However, the current site editor can't be accessed without the edit_theme_options option.

Wouldn't the purpose of this issue be to remove this restriction and allow all users access to the Site Editor Patterns page and control which screens and actions they can perform depending on their capabilities?

@youknowriad
Copy link
Contributor

Currently all data related to the site editor quite consistently require edit_theme_options even for reading.

The site editor shows pages and patterns so this is not entirely true that can be accessed without edit_theme_options.

@youknowriad Are the strict read permissions really intentional, with good reasons? Or is it mostly an omission?

I don't really know for the entities you mentionned.

Wouldn't the purpose of this issue be to remove this restriction and allow all users access to the Site Editor Patterns page and control which screens and actions they can perform depending on their capabilities?

This makes sense to me yes.

@jsnajdr
Copy link
Member

jsnajdr commented Jul 22, 2024

we should be able to create patterns if you have create_posts capability even without the edit_theme_options capability.

That's true because patterns are saved as the wp_block post type, and the permissions for that block type don't use edit_theme_options at all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Patterns A collection of blocks that can be synced (previously reusable blocks) or unsynced [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

No branches or pull requests

4 participants