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

Expose recovery code [SDK-2661] #487

Merged
merged 1 commit into from
Jul 16, 2021
Merged

Conversation

Widcket
Copy link
Contributor

@Widcket Widcket commented Jul 15, 2021

Changes

The documentation for the Verify with Recovery Code endpoint states that the response might include a new recovery_code that the app should display to the user. The Credentials class currently does not include that field, so it is not exposed to the developer.

Screen Shot 2021-07-15 at 17 50 41

Testing

Unit tests were added, and the change was also tested manually on an iOS simulator:

Screen Shot 2021-07-15 at 17 46 10

  • 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 July 15, 2021 20:57
@Widcket Widcket merged commit 7758a3b into master Jul 16, 2021

@objc public init(accessToken: String? = nil, tokenType: String? = nil, idToken: String? = nil, refreshToken: String? = nil, expiresIn: Date? = nil, scope: String? = nil) {
@objc public init(accessToken: String? = nil, tokenType: String? = nil, idToken: String? = nil, refreshToken: String? = nil, expiresIn: Date? = nil, scope: String? = nil, recoveryCode: String? = nil) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@Widcket Given that this class and constructor is public. If someone is instantiating a Credentials object today without using named parameters, wouldn't this change be a breaking change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, check the tests. When the value is not provided, it ends up as nil.

Copy link
Contributor

Choose a reason for hiding this comment

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

@Widcket The tests in CredentialsSpec.swift all use the constructor that takes a json input.

https://github.com/auth0/Auth0.swift/pull/487/files/8bc15053675b00025de5f162f40b9241f8ed82c6#diff-77a2df3b8a702b22e6181fc1f662e39a2012290011ad69caf52cb08bbb8c04c3R42

If not that file, what is the file I should be checking that makes use of the alternate constructor that takes all the arguments?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The JSON constructor is calling the one with the parameters: https://github.com/auth0/Auth0.swift/blob/master/Auth0/Credentials.swift#L68

If someone is instantiating a Credentials object today without using named parameters

The parameter is expressed as recoveryCode: String? = nil, so there's no instantiating it without named parameters. The parameters are there, even if you do Credentials():

Screen Shot 2021-07-16 at 11 12 47

@Widcket Widcket deleted the feature/expose-recovery-code branch July 16, 2021 11:43
@Widcket Widcket added this to the 1.35.0 milestone Jul 19, 2021
@Widcket Widcket mentioned this pull request Jul 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants