bigfork / silverstripe-oauth

SilverStripe OAuth2 authentication, based on the PHP League's OAuth2 client
BSD 3-Clause "New" or "Revised" License
9 stars 11 forks source link

[Bug] Missing [code_verifier] #14

Open Firesphere opened 1 year ago

Firesphere commented 1 year ago

Logging in with Twitter returns an error saying there was "a problem"

Inspecting the actual error, shows that it's missing a code_verifier, as per the PKCE flow.

https://developer.twitter.com/en/docs/authentication/oauth-2-0/user-access-token

This is in Controller:143, where this parameter must be added, I assume.

I've not, however, found what this code exactly should be...

kinglozzer commented 1 year ago

So it looks like it’d need to be injected here https://github.com/bigfork/silverstripe-oauth/blob/760e422d98db1c84bd6d3a3077e79b1cbbfc13f6/src/Control/Controller.php#L143-L145

I’ve not encountered any other providers that have additional parameters like that, but given it’s an array argument there are probably others. So I suppose an extension hook to update that array would the be best solution?

Firesphere commented 1 year ago

Yeah, that's where it needs to be added, if it's Twitter. I am not sure if the TwitterProvider has this data, but if it does, it wouldn't be too hard to build the array up from what the Provider gives, and if it's needed according to the docs? That last bit is a bit iffy...

kinglozzer commented 1 year ago

I am not sure if the TwitterProvider has this data, but if it does, it wouldn't be too hard to build the array up from what the Provider gives

I don’t think the provider does give this info - it comes from the HTTPRequest, we just have to give it to the provider. I suppose in theory we could do something like

$accessToken = $provider->getAccessToken('authorization_code', $request->getVars());

I’m not sure about that though - I don’t know if providers will discard any parameters they don’t need or if there’s potential problems there... an extension hook is probably safest.

Firesphere commented 1 year ago

The provider seems to add it (If I read the URL correctly) though, so I would think the provider has it?

I've not seen it come back in the request... (Then again, it's nearing 11.30PM, I'm breaking my head over this one too much)

kinglozzer commented 1 year ago

Oh perhaps I’m misunderstanding the docs, I thought code_verifier was a random string returned to the Silverstripe site from Twitter, that we then had to send back to Twitter to obtain the access token.

Re-reading the docs, it looks like it might actually just be a hardcoded string code_verifier=challenge?

Firesphere commented 1 year ago

No, challenge is... a placeholder string.

If you try to log in with Twitter on OAuth 2, in the URL, you'll see the code_challenge being a hash, and the s parameter (I think?) being s256 or something, meaning it's a sha256

kinglozzer commented 1 year ago

So it’s a hash in the return URL? i.e. something like https://mysite.com/oauth/callback?code=1234&challenge=5678&s=256

Firesphere commented 1 year ago

Yeah, I think so