Make WordPress Core

Opened 4 years ago

Closed 4 years ago

#52082 closed defect (bug) (maybelater)

Application Passwords issue with wordpress_logged_in cookie

Reported by: sebsz's profile SeBsZ Owned by:
Milestone: Priority: normal
Severity: normal Version: 5.6
Component: Application Passwords Keywords:
Focuses: Cc:

Description

I've been trying to debug this all day. I found that when making a REST API request with just the Authorization header, everything seems to work perfectly. However, if I happen to be using a browser that was logged into wp-admin and then make a REST API request (using Javascript for example), cookies are transmitted along with the request.

In my case, the presence of the wordpress_logged_in_xxxxxxxxxxxxxxxxx=xxxxxxxxx cookie causes authentication to fail and it results in a HTTP 401 'rest_forbidden' -> 'Sorry, you are not allowed to do that.' response.

{
    "code": "rest_forbidden",
    "message": "Sorry, you are not allowed to do that.",
    "data": {
        "status": 401
    }
}

I've traced this to line 437 in class-wp-rest-server.php:

<?php
$result = $this->check_authentication();

which causes the $current_user global to be set to 0. Before this code runs, it is important to note that $current_user has the correct ID of the cookie logged in user.

Interestingly, I've found that if I set the $current_user global to null in my own register_rest_route 'permission_callback', then it works fine. It seems to re-authenticate and correctly use the basic auth credentials.

I am not familiar enough with this code to be able to write a patch for this, I hope someone can tell me this is indeed a bug or desired behavior?

I feel that a valid wordpress_logged_in cookie should allow the REST authentication to succeed instead of fail. In addition, the presence of this cookie should not cause the API request to fail if there is a valid basic auth header.

Change History (7)

#1 @TimothyBlynJacobs
4 years ago

Hi @SeBsZ,

This is expected behavior, you can see this comment for why it happens: https://make.wordpress.org/core/2020/11/05/application-passwords-integration-guide/#comment-40414 That doesn't mean we can't look to change that behavior, but I think it would be tricky because of how during a REST API request we aren't doing the full user validation in wp_validate_auth_cookie, the nonce check happens in rest_cookie_check_errors.

In what scenario are you using App Passwords that you also end up with auth cookies?

#2 @archon810
4 years ago

In our case, we're logged into WP and there's a Chrome extension installed that uses the API, which breaks as of WP 5.6.

#3 follow-up: @TimothyBlynJacobs
4 years ago

I see the same behavior using the Application Passwords feature plugin. Were you using a different mechanism previously? I'd recommend omitting credentials when making the CORS requests.

#4 in reply to: ↑ 3 @SeBsZ
4 years ago

Replying to TimothyBlynJacobs:

I see the same behavior using the Application Passwords feature plugin. Were you using a different mechanism previously? I'd recommend omitting credentials when making the CORS requests.

You may be right about this, we seem to have had a custom fix for this which is broken now because there is no 'rest_api_auth_handler' function anymore in core:

<?php
$app_pw = Application_Passwords::rest_api_auth_handler(false);
if ($app_pw !== false) {
    wp_set_current_user($app_pw);
}

This code would force set the current user in my 'permission_callback' and everything would work fine.

As I mentioned in my original post, setting $current_user = null; in the permission_callback actually works with WP 5.6 - this seems to force re-authentication and then the REST request works. I was just wondering if this is the right workaround or if this needs fixing in WP core?

Last edited 4 years ago by SeBsZ (previous) (diff)

#5 @TimothyBlynJacobs
4 years ago

As I mentioned in my original post, setting $current_user = null; in the permission_callback actually works with WP 5.6 - this seems to force re-authentication and then the REST request works. I was just wondering if this is the right workaround or if this needs fixing in WP core?

This is definitely not a good solution, since it will allow for cookie auth without passing a nonce. The correct solution is to not send cookies. As a last resort, you could use the return value from wp_validate_application_password in your callback, but I'd highly recommend avoiding that.

This ticket was mentioned in Slack in #core by lukecarbis. View the logs.


4 years ago

#7 @kirasong
4 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to maybelater
  • Status changed from new to closed

After this comment and conversation about the ticket in triage, going to resolve as maybelater per @TimothyBlynJacobs' suggestion.

Please feel free to continue to chat or reopen the ticket if you don't think this is appropriate.

Thanks much!

Note: See TracTickets for help on using tickets.