WordPress / wordpress-playground

Run WordPress in the browser via WebAssembly PHP
https://w.org/playground/
GNU General Public License v2.0
1.65k stars 262 forks source link

Incorrect user capabilities result in unexpected 401 responses from the REST API #1611

Open dcalhoun opened 4 months ago

dcalhoun commented 4 months ago

When querying the REST API with successful authentication, endpoints return unexpected 401 response statuses. It appears the user's capabilities are not accurately portrayed in the context of WordPress Playground.

I discovered this issue while using curl to query a site running with the Studio app, but I also reproduced the issue using wp-now. Contrastingly, performing the following steps with a site running with wp-env result in the expected outcome.

Steps to Reproduce

  1. Start a Playground-powered site: npx @wp-now/wp-now start
  2. Modify the site's ~/.wp-now/wordpress-versions/latest/wp-config.php to set the environment to local: define('WP_ENVIRONMENT_TYPE', 'local');
  3. Visit the site's WP Admin and navigate to UsersEdit (the admin user).
  4. Create and copy an application password.
  5. Create and copy a Base64 version of the combined username and application password: echo -n 'admin:<application_password>' | base64 | pbcopy
  6. Generate a curl request for the block types endpoint: curl --header "Authorization: Basic <base64_string>" -L http://localhost:<port>/?rest_route=/wp/v2/block-types

Expected Outcome

A 200 response containing the site's block types is returned.

Actual Outcome

A 401 response is returned:

{
  "code": "rest_block_type_cannot_view",
  "message": "Sorry, you are not allowed to manage block types.",
  "data": { "status": 401 }
}
brandonpayton commented 4 months ago

I'm curious whether this has anything to do with #1539 and will grab this to test after the other PR is merged.

dcalhoun commented 4 months ago

@brandonpayton thank you for the support!

When I investigated briefly, the permissions check unexpectedly returned false. If I recall correctly, when I logged $this->capabilities on the user class, it was empty.

brandonpayton commented 2 months ago

@dcalhoun, it looks like the auth is not working for some reason. I'm getting the same message, and if I instrument the permissions check to error_log get_current_user_id() the value is 0.

Hoping to look into this a bit more later.

brandonpayton commented 4 weeks ago

@dcalhoun After some debugging, it looks like Playground is auto-sending cookies and interfering with application password auth. Here's how:

Playground is auto-sending login cookies here: https://github.com/WordPress/wordpress-playground/blob/2b1f0b635809d0b8723460a88460b04bcfadd1ce/packages/php-wasm/universal/src/lib/php-request-handler.ts#L485-L496

When Playground auto-adds cookies to these requests that contain Authorization headers, it causes WordPress to skip evaluation of the application password. Instead, cookie-based auth initially succeeds in an earlier filter here:

add_filter( 'determine_current_user', 'wp_validate_auth_cookie' );
add_filter( 'determine_current_user', 'wp_validate_logged_in_cookie', 20 );
add_filter( 'determine_current_user', 'wp_validate_application_password', 20 );

Then when rest_cookie_check_errors() looks for the nonce required by the REST API for cookie-based auth, it finds there is no nonce and clears the current user here:

if ( null === $nonce ) {
    // No nonce at all, so act as if it's an unauthenticated request.
    wp_set_current_user( 0 );
    return true;
}

This is an interesting one. Maybe it would make sense for the PHPRequestHandler to omit cookies if a request appears to contain another auth-related header. But cookies aren't just for auth. So it seems presumptuous to remove cookies when we don't know the user's intent. We could filter out specific WP cookies, but it still might be solving a problem caused by a bit of magic using a bit more magic.

Read and understand our cookie store code better would probably help generate other ideas.

@adamziel @bgrgicak do you have any thoughts on this?

adamziel commented 4 weeks ago

I wonder if it's time to get rid of the internal cookie store and scope all cookies to the specific scoped WordPress site path