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

fix: OAuth 2.0 Grant Type Authorization: "invalid_client" error / URL Encode of Client ID #2129

Merged
merged 1 commit into from
Jul 26, 2024

Conversation

pietrygamat
Copy link
Contributor

@pietrygamat pietrygamat commented Apr 18, 2024

Description

Fixes issue where OAuth2 parametrs handled by bruno were not url encoded in final request.

fixes #2115
#1003

Contribution Checklist:

  • The pull request only addresses one issue or adds one feature.
  • The pull request does not introduce any breaking changes
  • I have added screenshots or gifs to help explain the change if applicable.
  • I have read the contribution guidelines.
  • Create an issue and link to the pull request.

@pietrygamat pietrygamat changed the title Fix: OAuth 2.0 Grant Type Authorization: "invalid_client" error / URL Encode of Client ID fix: OAuth 2.0 Grant Type Authorization: "invalid_client" error / URL Encode of Client ID Apr 18, 2024
Copy link
Contributor

@andreassiegel andreassiegel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. Using the URL class is a nice improvement compared to string concatenation.

Comment on lines -51 to +54
let oauth2QueryParams =
(authorizationUrl.indexOf('?') > -1 ? '&' : '?') + `client_id=${clientId}&response_type=code`;
const authorizationUrlWithQueryParams = new URL(authorizationUrl);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using the URL class to construct the request URL looks a lot cleaner than the previous string concatenation with some nested ternary operator in the middle. 👍

@ventsislav-georgiev
Copy link

Any progress here?

@werge2121
Copy link

+1 Would like to see this merged in, getting the same error.

@pietrygamat
Copy link
Contributor Author

@sanjai0py bumping this for you attention - please let's have this bug fixed.

@helloanoop
Copy link
Contributor

@lohxt1 please merge this once you have done your testing.
@sanjai0py let's have this targeted for next week release.

@lohxt1 lohxt1 merged commit f2ba351 into usebruno:main Jul 26, 2024
2 of 3 checks passed
@sanjai0py
Copy link
Member

Thank you, @pietrygamat, for taking the time to contribute to this PR. The next release will include your valuable fix!

Also, a big thank you to @andreassiegel for taking the time to review it!

@pietrygamat pietrygamat deleted the bugfix/2115 branch July 26, 2024 15:33
jwetzell pushed a commit to jwetzell/bruno that referenced this pull request Aug 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

OAuth 2.0 Grant Type Authorization: "invalid_client" error / URL Encode of Client ID for vertical bar
7 participants