Make WordPress Core

Opened 3 years ago

Last modified 3 years ago

#55870 new defect (bug)

WP App Passwords Should be URL Decoded

Reported by: mrahmadawais's profile mrahmadawais Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 5.6
Component: Application Passwords Keywords: reporter-feedback
Focuses: rest-api Cc:

Description (last modified by TimothyBlynJacobs)

Using OAuth 2 based. authentication IETF recommends for client id/secrets URL encoded forms.

Which means, by using some node OpenID clients, we always get Authorization: Basic urlSafeEncodedBase64String('user:pass').

This fails to authenticate as WordPress doesn't decode the user and pass which could also be clientId and clientSecret in OAuth2.

This could be solved by using urldecode( string $str ).

BEFORE

<?PHP
$authenticated = wp_authenticate_application_password( null, $_SERVER['PHP_AUTH_USER'], $_SERVER['PHP_AUTH_PW'] );

AFTER

<?php
$authenticated = wp_authenticate_application_password( null, urldecode($_SERVER['PHP_AUTH_USER']), urldecode($_SERVER['PHP_AUTH_PW']) );

Would you folks be up for a patch for this?

Change History (2)

#1 @TimothyBlynJacobs
3 years ago

  • Description modified (diff)
  • Keywords reporter-feedback added
  • Version changed from trunk to 5.6

Hi @mrahmadawais,

Thanks for the ticket!

I'm not sure why an OpenID client would be used with App Passwords. One of the benefits of App Passwords using Basic Authentication is that it is built in to many HTTP clients by simply passing a username and password field. For instance Axios and Guzzle both have an auth field.

The OAuth specification requires following that encoding process. However, Application Passwords is not OAuth, so I'm not sure why we'd be following the OAuth specification here.

If we did want to match the OAuth specification, changing the encoding format could potentially cause a backward compatibility break. Though it would be unlikely due to the selection of characters that WordPress Core generates.

All that being said, I'm not sure what the practical benefit here is. If the values passed to App Passwords are an OAuth client_id and client_secret they won't be successfully authenticated by wp_authenticate_application_password regardless of the encoding method chosen because they aren't App Passwords.

#2 @mrahmadawais
3 years ago

Hi @TimothyBlynJacobs

Glad to hear back from you. Before creating a ticket I was reading your work on #51939.

I have a similar situation. I don't want App passwords to support OAuth. I want WordPress to do that. The problem I have is that if there's an Authorization: Basic whatever header, WP thinks it's App Passwords. That's not always the case. Especially not in OAuth.

I want to be able to send an Authorization: Basic whatever header — without getting a 401 Unauthorized error — and tell WordPress No this is not an App Pass so don't try to authenticate with that.


My current solution to this seems a bit hacky.

<?php
<?
add_filter('determine_current_user', __NAMESPACE__ . '\\skip_app_pass_auth', 19);

/**
 * Skip App Pass Authentication if a custom header exists by faking 
 *
 * @since 1.0.0
 *
 * @param int|false $input_user User ID if one has been determined, false otherwise.
 * @return int|false The authenticated user ID if successful, false otherwise.
 */
function skip_app_pass_auth($input_user) {
        // Don't authenticate twice.
        if (!empty($input_user)) {
                return $input_user;
        }

        // Check that we're trying to authenticate via Wordless.
        $skip_app_pass_auth = isset($_SERVER['HTTP_SKIP_APP_PASS'])? $_SERVER['HTTP_SKIP_APP_PASS']: false;

        if (!$skip_app_pass_auth) {
                return $input_user;
        }

    // Fake return true to say we have a valid user and allow OAuth token workflow to run from the REST API request which gets stopped by App Passwords auth when WP sees `Authorization: Basic whatever` ← this is required for OAuth token workflow to run and doesn't have user:pass instead has clientID:clientSecret.

    // Could also run OAuth code to get WP user ID and return that, which might be the right idea. 
        return true;

}


See. Not trying to make App passwords support OAuth. Only trying to get in a new auth method but getting stopped too early by WP with a 401 Unauthorized error since WP thinks every Authorization: Basic whatever is an App Pass — whereas it should be more explicit than that. Probably use an extra header to say X-WP-APP-PASS or something.

Also, not sure if this solution is the right way to go.

Thoughts?

Note: See TracTickets for help on using tickets.