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

[C++] OAuth 2.0 request's content type is not x-www-form-urlencoded #12334

Closed
BewareMyPower opened this issue Oct 12, 2021 · 1 comment · Fixed by #12341
Closed

[C++] OAuth 2.0 request's content type is not x-www-form-urlencoded #12334

BewareMyPower opened this issue Oct 12, 2021 · 1 comment · Fixed by #12341
Assignees
Labels
type/bug The PR fixed a bug or issue reported a bug

Comments

@BewareMyPower
Copy link
Contributor

Describe the bug
When a HTTP request is sent to an OAuth 2.0 server for an access token, the content type should be x-www-form-urlencoded. However, current C++ implementation uses application/json as the content type. Sometimes it works but for some strict validated OAuth 2.0 server, it could fail.

To Reproduce
Here are the error logs when sending a request whose content type is application/json.

ERROR [0x70000db06000] AuthOauth2:341 | Response failed for issuerurl https://login.microsoftonline.com/(xxx). response Code 400 passedin: 

The error code is 400, which means bad request.

Expected behavior
The request's content type should be x-www-form-urlencoded.

@EronWright
Copy link
Contributor

Good job in diagnosing this.

BewareMyPower added a commit that referenced this issue Oct 13, 2021

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Fixes #12334

### Motivation

When C++ client sends a HTTP request for the access token from a OAuth 2.0 server, the content type is JSON, which is incorrect and might not work in some cases.

### Modifications

- Replace `generateJsonBody` with `generateParamMap` to create a map that contains the necessary keys from `CredentialsFlow`.
- Add a `buildClientCredentialsBody` method to convert the map to URL encoded string.
- Change the content type from `json` to `x-www-form-urlencoded`.

### Verifying this change

- [x] Make sure that the change passes the CI checks.

This change is already covered by existing tests, such as `AuthPluginTest.testOauth2RequestBody`. This PR changes the test to verify `generateParamMap` because `generateJsonBody` is removed.
tuteng pushed a commit that referenced this issue Oct 13, 2021

Verified

This commit was signed with the committer’s verified signature.
tuteng Guangning E
Fixes #12334

### Motivation

When C++ client sends a HTTP request for the access token from a OAuth 2.0 server, the content type is JSON, which is incorrect and might not work in some cases.

### Modifications

- Replace `generateJsonBody` with `generateParamMap` to create a map that contains the necessary keys from `CredentialsFlow`.
- Add a `buildClientCredentialsBody` method to convert the map to URL encoded string.
- Change the content type from `json` to `x-www-form-urlencoded`.

### Verifying this change

- [x] Make sure that the change passes the CI checks.

This change is already covered by existing tests, such as `AuthPluginTest.testOauth2RequestBody`. This PR changes the test to verify `generateParamMap` because `generateJsonBody` is removed.

(cherry picked from commit 4ae7f6a)
bharanic-dev pushed a commit to bharanic-dev/pulsar that referenced this issue Mar 18, 2022
…he#12341)

Fixes apache#12334

### Motivation

When C++ client sends a HTTP request for the access token from a OAuth 2.0 server, the content type is JSON, which is incorrect and might not work in some cases.

### Modifications

- Replace `generateJsonBody` with `generateParamMap` to create a map that contains the necessary keys from `CredentialsFlow`.
- Add a `buildClientCredentialsBody` method to convert the map to URL encoded string.
- Change the content type from `json` to `x-www-form-urlencoded`.

### Verifying this change

- [x] Make sure that the change passes the CI checks.

This change is already covered by existing tests, such as `AuthPluginTest.testOauth2RequestBody`. This PR changes the test to verify `generateParamMap` because `generateJsonBody` is removed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug The PR fixed a bug or issue reported a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants