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

Add client credentials grant with basic auth #422

Conversation

patrickcarlohickman
Copy link
Contributor

The currently supported OAuth2 client credentials grant includes the client_id and the client_secret in the body of the request. Some APIs require that the client_id and client_secret are sent using the HTTP Basic Authentication scheme. This PR adds a new client credentials request that supports the basic authentication method instead of the request body method.

The request only adds new files and should not cause any backwards compatibility issues.

RFC reference: https://datatracker.ietf.org/doc/html/rfc6749#section-2.3.1

Implementation Notes:

  • The new GetClientCredentialsTokenBasicAuthRequest was copied from the existing GetClientCredentialsTokenRequest. The defaultBody() method was updated to remove the client_id/client_secret from the body, and the defaultAuth() method was added to implement the basic authentication.
  • The new ClientCredentialsBasicAuthGrant trait uses the existing ClientCredentialsGrant trait, and just redefines the resolveAccessTokenRequest() method to use the new request file.
  • A new test was added to ensure the request body and Authentication header are as expected.

Copy link
Member

@Sammyjo20 Sammyjo20 left a comment

Choose a reason for hiding this comment

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

Awesome PR @patrickcarlohickman thank you! I actually ran into this issue recently with an API, so it's great this will be native now.

@Sammyjo20 Sammyjo20 merged commit 74b7f67 into saloonphp:v3 Aug 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants