Fix Signature and Callback (Issues WP-API/OAuth1#59, WP-API/OAuth1#64, WP-API/OAuth1#91)

[coderkevin]

This Pull Request integrates commits from 2 other Pull Requests.

This Pull Request accomplishes:

1. Replaces urlencode/urldecode with rawurlencode/rawurldecode calls for more URI correctness.
2. Stops the decode/re-encode of parameters which may already have portions of them encoded.
3. Correctly generates an oauth_signature regardless of '+' or encoded parameters contained within the signature string.
4. Stores the oauth_callback parameter for use with oauth_callback_confirmed in the 1.0a spec.

This Pull Request fixes the issues:

• #59
• #64
• #91

AlexC's commit: Ensure OAuth1 signature is created as per the spec

This commit fixes signature generation for certain cases, however it adds an extra encoding in addition to normalize_parameters which can cause an issue (addressed below.)

coderkevin's commit: Remove normalize_parameters function

This commit fixes WP-API/OAuth1#91 and relies on AlexC's commit above to encode inside of join_with_equals_sign() to encode parameters as the param string is generated.

sblaz's commit: Fix issue WP-API/OAuth#59, OAuth callback isn't called

This commit stores and carries over the oauth_callback parameter for post-authentication. It relies on the above two commits to ensure the callback parameter remains correctly encoded.

[rmccue]

Hi, thanks for the PR! Usually I'd close out combined PRs, but it seems like this relies on the others?

Can you give an example of requests that don't work beforehand, and do work after?

[coderkevin]

Hi,

I originally had mirrored the other two commits myself, but I brought in the commits from @sblaz and @AlexC to give credit where it was due. I just made them work together.

What this fixes is any time you have an oauth_callback parameter that is already URL encoded twice. For me, I'm integrating hello.js with this, and it sends an encoded URL with an encoded parameter in the query string.

Here's the progression (hand-writing URLs here, but I hope you get the idea)

1. Inside my server, I package up the following state parameter into the oauth_callback: state={ id: '123', status: 'authorizing' }
2. Then I encode the parameters and add to the oauth_callback: http://myserver/oauth-proxy?state=%7B%20id%3A%20%27123%27%2C%20status%3A%20%27authorizing%27%20%7D
3. From my server, I package up and send an auth request (state encoded twice, oauth_callback encoded once): http://wpserver/oauth1/authorize?oauth_callback=http%3A%2F%2Fmyserver%2Foauth-proxy%3Fstate%3D%257B%2520id%253A%2520%2527123%2527%252C%2520status%253A%2520%2527authorizing%2527%2520%257D
4. In OAuth1, it uses the PHP GET data, which is decoded once (matches step 2): http://myserver/oauth-proxy?state=%7B%20id%3A%20%27123%27%2C%20status%3A%20%27authorizing%27%20%7D
5. In OAuth1 normalize_parameters, all parameters are decoded... http://myserver/oauth-proxy?state={ id: '123', status: 'authorizing' }
6. then re-encoded again. (now state is encoded once, oauth_callback encoded once) http%3A%2F%2Fmyserver%2Foauth-proxy%3Fstate%3D%7B%20id%3A%20%27123%27%2C%20status%3A%20%27authorizing%27%20%7D

This is the problem. Because when the signature is generated off of this string, it doesn't match the one that was calculated by my server, because the oauth_callback parameter was modified via normalize_parameters and is now incorrect. Also, even if you get past the signature, you can't call a URL like this: http://myserver/oauth-proxy?state={ id: '123', status: 'authorizing' }

I think this only comes up if the oauth_callback URL has an encoded parameter, so if others sent a callback URL without any encoded parameters, they would not run afoul of this particular issue.

[rmccue]

These should all be fixed in #98 now, I think. Can you give it a try and confirm please? :)

[coderkevin]

I just tried this out, and I'm afraid it hasn't helped fix the problem. Here's the code in question, and it appears to be still intact:

https://github.com/WP-API/OAuth1/blob/141f4f80d260ed9c2acb10687b2dc0e136fea4c7/lib/class-wp-rest-oauth1.php#L733

    protected function normalize_parameters( &$key, &$value ) {
        $key = rawurlencode( rawurldecode( $key ) );
        $value = rawurlencode( rawurldecode( $value ) );
    }

This code is what is decoding and then re-encoding, which makes the signatures not match and the URL unusable (see example comment above). I'll post a comment below this one with cut-and-pastes of the strings that are being signed on both ends.

[coderkevin]

From my Client:

String to sign: "GET&http%3A%2F%2Fvagrant.local%2Foauth1%2Frequest&oauth_callback%3Dhttp%253A%252F%252Flocalhost%253A8080%252Fredirect.html%253Fproxy_url%253Dhttp%25253A%25252F%25252Flocalhost%25253A8080%25252F%2526state%253D%25257B%252522client_id%252522%25253A%252522ke3cgrDMqOXq%252522%25252C%252522network%252522%25253A%252522wpApi%252522%25252C%252522display%252522%25253A%252522popup%252522%25252C%252522callback%252522%25253A%252522_hellojs_3u0c4lde%252522%25252C%252522state%252522%25253A%252522%252522%25252C%252522redirect_uri%252522%25253A%252522http%25253A%25252F%25252Flocalhost%25253A8080%25252Fredirect.html%252522%25252C%252522scope%252522%25253A%252522basic%252522%25252C%252522oauth%252522%25253A%25257B%252522version%252522%25253A%2525221.0a%252522%25252C%252522auth%252522%25253A%252522http%25253A%25252F%25252Fvagrant.local%25252Foauth1%25252Fauthorize%252522%25252C%252522request%252522%25253A%252522http%25253A%25252F%25252Fvagrant.local%25252Foauth1%25252Frequest%252522%25252C%252522token%252522%25253A%252522http%25253A%25252F%25252Fvagrant.local%25252Foauth1%25252Faccess%252522%25257D%25252C%252522oauth_proxy%252522%25253A%252522http%25253A%25252F%25252Flocalhost%25253A8080%25252Foauthproxy%252522%25257D%2526client_id%253Dke3cgrDMqOXq%26oauth_consumer_key%3Dke3cgrDMqOXq%26oauth_nonce%3D3765a3ae273396000%26oauth_signature_method%3DHMAC-SHA1%26oauth_timestamp%3D1449476002%26oauth_version%3D1.0"

From the Server (OAuth1 plugin):

[07-Dec-2015 08:13:23 UTC] $string_to_sign="GET&http%3A%2F%2Fvagrant.local%2Foauth1%2Frequest&oauth_callback%3Dhttp%253A%252F%252Flocalhost%253A8080%252Fredirect.html%253Fproxy_url%253Dhttp%253A%252F%252Flocalhost%253A8080%252F%2526state%253D%257B%2522client_id%2522%253A%2522ke3cgrDMqOXq%2522%252C%2522network%2522%253A%2522wpApi%2522%252C%2522display%2522%253A%2522popup%2522%252C%2522callback%2522%253A%2522_hellojs_3u0c4lde%2522%252C%2522state%2522%253A%2522%2522%252C%2522redirect_uri%2522%253A%2522http%253A%252F%252Flocalhost%253A8080%252Fredirect.html%2522%252C%2522scope%2522%253A%2522basic%2522%252C%2522oauth%2522%253A%257B%2522version%2522%253A%25221.0a%2522%252C%2522auth%2522%253A%2522http%253A%252F%252Fvagrant.local%252Foauth1%252Fauthorize%2522%252C%2522request%2522%253A%2522http%253A%252F%252Fvagrant.local%252Foauth1%252Frequest%2522%252C%2522token%2522%253A%2522http%253A%252F%252Fvagrant.local%252Foauth1%252Faccess%2522%257D%252C%2522oauth_proxy%2522%253A%2522http%253A%252F%252Flocalhost%253A8080%252Foauthproxy%2522%257D%2526client_id%253Dke3cgrDMqOXq%26oauth_consumer_key%3Dke3cgrDMqOXq%26oauth_nonce%3D3765a3ae273396000%26oauth_signature_method%3DHMAC-SHA1%26oauth_timestamp%3D1449476002%26oauth_version%3D1.0"

[coderkevin]

@rmccue It's been a while since we last had activity here. Is this still something you'd like to address?

[jtsternberg]

I'm interested in this discussion as well.