Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OAuth2 client_credentials authentication does not seem to be RFC6749 compliant #5909

Open
lolleko opened this issue Dec 22, 2023 · 3 comments
Labels

Comments

@lolleko
Copy link

lolleko commented Dec 22, 2023

Branch/Environment/Version

  • Branch/Version: 1.1.
  • Environment: On-prem

Describe the bug

Failing to authenticate via client_credentials Because client_id & client_secret do not seem to be urlendecoded after base64 decoding.

Reproduction steps

  1. We will use the following client credentials in the example:
client_id: $s6BhdRkqt3@
client_secret: $7Fjfp0ZBr1KtDRbnfVdmIw@
  1. Urlencode the 2 as sepcified by RFC6749
client_id(urlencoded): %24s6BhdRkqt3%40
client_secret(urlencoded): %247Fjfp0ZBr1KtDRbnfVdmIw%40
  1. Encoding the urlencoded variants to baset64 separated by a colon will lead to the following header:
Authorization: Basic JTI0czZCaGRSa3F0MyU0MDolMjQ3RmpmcDBaQnIxS3REUmJuZlZkbUl3JTQw
  1. Providing this header to the oauth/token endpoint via curl:
curl --location 'abc.com/oauth/token' --header 'Content-Type: application/x-www-form-urlencoded' --header 'Authorization: Basic JTI0czZCaGRSa3F0MyU0MDolMjQ3RmpmcDBaQnIxS3REUmJuZlZkbUl3JTQw'  --data-urlencode 'grant_type=client_credentials' 

Actual behavior

No token is returned :(

{"error":"invalid_request","error_description":"The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed."}

Expected behavior

A token should be returned.

Additional context

In fact, a token is returned if we omit step 2 from the reproduction steps.

client_id: $s6BhdRkqt3@
client_secret: $7Fjfp0ZBr1KtDRbnfVdmIw@

Header encoded directly in base64 (skipping urlencode)

Authorization: Basic JHM2QmhkUmtxdDNAOiQ3RmpmcDBaQnIxS3REUmJuZlZkbUl3QA

Passing this token using the curl command from step 5 will return a token.

If I understand RFC6749 correctly urlencoding id and secret is required when using the authorization header.
This is also how it is implemented in the x/oauth2 pkg: https://github.com/golang/oauth2/blob/master/internal/token.go#L199
In fact, if I remove the url.QueryEscape from that file, I am also able to authenticate with tyk using x/oauth2

So I think tyk or whatever pkg you are using for oauth2 (osin?) should also follow this behaviour to be RFC6749 compliant. Therefore, first decode the base64 header, second split at the colon, third urlendecode.

@lolleko lolleko added the bug label Dec 22, 2023
@mhuaco
Copy link

mhuaco commented Feb 13, 2024

@lolleko Thank you for bringing this to our attention. How are you generating the client_id and secret. Could you please share the doc you're referencing? I ask because when I create an OAuth client via our Tyk dashboard, I'm not seeing the special character '$@' included in the client ID and secret.

Regards
Marvin

@lolleko
Copy link
Author

lolleko commented Mar 6, 2024

Hey Marvin,

Thanks for getting back to me.
We are generating our ID and secret ourselves, we are not using the dashboard.
I manually created ID and secret, including special characters '$@' to showcase this issue.
Like I said, tyk OAuth works as expected as long as we do not include these special characters in our generated IDs/Secrets.

This issue occurs not only for '$@', but for all characters that need to be urlenencoded (https://docs.microfocus.com/OMi/10.62/Content/OMi/ExtGuide/ExtApps/URL_encoding.htm)
Also, it doesn't matter where you put them (I just put them in the beginning and end as a simple example)

So something like s6#Bhd?Rkqt3 or sdaskdop@mlkj$li will also cause issues.

@lolleko
Copy link
Author

lolleko commented Mar 8, 2024

Ahhh and regarding the referenced docs, I think I put all the relevant references in the Issue description, which one are you missing?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants