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

[SDK-4142] Add support for /oauth/par #470

Merged
merged 6 commits into from
Apr 24, 2023
Merged

[SDK-4142] Add support for /oauth/par #470

merged 6 commits into from
Apr 24, 2023

Conversation

stevehobbsdev
Copy link
Contributor

@stevehobbsdev stevehobbsdev commented Apr 21, 2023

Changes

This PR implements two new auth API methods:

  • pushed_authorization_request: calls the /oauth/par endpoint with all the parameters that would normally be sent to /authorize, and returns the payload including request_uri
  • par_authorization_url: accepts request_uri and builds a URL that includes it along with client_id

Note

Something I would like some feedback on: the SDK already exposes authorization_url (source) that builds a URL for /authorize with all the params. However, I decided not to support PAR here for two reasons:

  • It currently takes a required parameter redirect_url that is not required for the PAR request
  • It currently takes a set of additional parameters to include in the request, all of which will be ignored by the server when sending a request_uri

It made sense to build a new method. However, also looking for feedback on the name par_authorization_url.

Testing

Please describe how this can be tested by reviewers. Be specific about anything not tested and reasons why. If this library has unit and/or integration testing, tests should be added for new functionality and existing tests should complete without errors.

  • This change adds unit test coverage
  • This change adds integration test coverage
  • This change has been tested on the latest version of Ruby

Checklist

@stevehobbsdev stevehobbsdev added the review:medium Medium review label Apr 21, 2023
@stevehobbsdev stevehobbsdev requested a review from a team as a code owner April 21, 2023 10:54
@@ -16,7 +16,7 @@ module HTTPProxy
BASE_DELAY = 100

# proxying requests from instance methods to HTTP class methods
%i(get post post_file put patch delete delete_with_body).each do |method|
%i(get post post_file post_form put patch delete delete_with_body).each do |method|
Copy link
Contributor Author

@stevehobbsdev stevehobbsdev Apr 21, 2023

Choose a reason for hiding this comment

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

Until now the SDK didn't have a way to send a POST request with application/x-www-form-urlencoded, it always assumed JSON. To support this, I added a new method type here (which ultimately ends up as a POST, similar to how :post_file is used).

The alternative was to add an option to the method that signals the developer wants to use form data, but there was no way to do this without introducing a potential breaking change.

adamjmcgrath
adamjmcgrath previously approved these changes Apr 21, 2023
@stevehobbsdev stevehobbsdev merged commit 2075bb7 into master Apr 24, 2023
@stevehobbsdev stevehobbsdev deleted the sdk-4141/par branch April 24, 2023 13:43
@stevehobbsdev stevehobbsdev mentioned this pull request Apr 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
review:medium Medium review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants