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

Remove parameterless tokenExchange() method #573

Merged
merged 4 commits into from
Dec 7, 2021
Merged

Conversation

Widcket
Copy link
Contributor

@Widcket Widcket commented Dec 6, 2021

Changes

⚠️ THIS PR CONTAINS BREAKING CHANGES

Auth0.swift 1.x has a tokenExchange(withParameters:) method, besides tokenExchange(withCode:codeVerifier:redirectURI:). Since we added Request.parameters(_:) and removed all the existing parameters: parameters, tokenExchange(withParameters:) ended up as tokenExchange().

Auth0.Android does not have a similar parameterless method, only fun token(authorizationCode: String, codeVerifier: String, redirectUri: String ), that is, the equivalent to tokenExchange(withCode:codeVerifier:redirectURI:).

Since we already have tokenExchange(withCode:codeVerifier:redirectURI:), and given the fact developers can add custom parameters with Request.parameters(_:), it does not make sense to keep the parameterless tokenExchange() method. This PR removes it.

Testing

  • This change adds unit test coverage
  • This change has been tested on the latest version of the platform/language or why not

Checklist

@Widcket Widcket requested a review from a team as a code owner December 6, 2021 23:37
@Widcket Widcket added the review:small Small review label Dec 6, 2021
@@ -360,6 +347,20 @@ private extension Auth0Authentication {
telemetry: self.telemetry)
}

func tokenExchange() -> Request<Credentials, AuthenticationError> {
Copy link
Contributor

@adamjmcgrath adamjmcgrath Dec 7, 2021

Choose a reason for hiding this comment

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

Not sure if you want to change these, but strictly speaking there's no "Token Exchange" going on with these defaults, you're just calling the "Get Token" endpoint https://auth0.com/docs/api/authentication#get-token

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Renamed the private method to just token() after the endpoint in 8f5a656.

@@ -97,6 +97,20 @@ Use `createUser(email:username:password:connection:userMetadata:rootAttributes:`

Use `userInfo(withAccessToken:)` instead.

#### `tokenExchange(withParameters:)`

Use `tokenExchange(withCode:codeVerifier:redirectURI:)` instead. To pass custom parameters, use the `parameters(_:)` method from `Request`:
Copy link
Contributor

Choose a reason for hiding this comment

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

Similarly, this should really be called codeExchange - since you're doing Proof Key for Code Exchange (not sure what Android calls it)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, renamed the method in 8f5a656.
Android calls it token().

@Widcket Widcket merged commit aee55ac into beta Dec 7, 2021
@Widcket Widcket deleted the v2/token-exchange branch December 7, 2021 11:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
review:small Small review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants