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

Handle API response for mobile OTP code incorrect. #371

Merged
merged 3 commits into from
Nov 5, 2020

Conversation

nicbell
Copy link
Contributor

@nicbell nicbell commented Nov 4, 2020

Changes

When logging in with a mobile phone and SMS code:

AuthenticationAPIClient(account)
            .loginWithPhoneNumber(mobileNumber, code, smsConnection)
            .setScope(scope)
            .setAudience(audience)
            .start

With an incorrectly entered code I expect to for AuthenticationException.isMultifactorCodeInvalid to be true.

This PR just adds another string match.

References

image

Testing

Test logging in with mobile and SMS code, entering the code incorrectly. AuthenticationException.isMultifactorCodeInvalid should be true.

Checklist

@nicbell nicbell requested a review from a team as a code owner November 4, 2020 19:23
@lbalmaceda lbalmaceda added the needs investigation An issue that has more questions to answer or otherwise needs work to fully understand the issue label Nov 4, 2020
@lbalmaceda
Copy link
Contributor

@nicbell thanks for this. I assume this changed at some point to prevent user enumeration. I've checked on my side using email and I receive a different error message. Can you please update the PR to add support to this other description as well? Thanks

image

@lbalmaceda lbalmaceda added waiting for customer This issue is waiting for a response from the issue or PR author and removed needs investigation An issue that has more questions to answer or otherwise needs work to fully understand the issue labels Nov 4, 2020
@nicbell
Copy link
Contributor Author

nicbell commented Nov 5, 2020

@lbalmaceda thanks, I've added email OTP and tests for both.

@lbalmaceda lbalmaceda closed this Nov 5, 2020
@lbalmaceda lbalmaceda reopened this Nov 5, 2020
@lbalmaceda lbalmaceda merged commit fa96a5d into auth0:master Nov 5, 2020
@lbalmaceda
Copy link
Contributor

Thanks for making the changes. I imagine this can be put into the next release train, in one week from now.

@lbalmaceda lbalmaceda added CH: Fixed and removed waiting for customer This issue is waiting for a response from the issue or PR author labels Nov 5, 2020
@lbalmaceda lbalmaceda modified the milestones: v1-Next, 1.29.1 Nov 5, 2020
@lbalmaceda
Copy link
Contributor

@nicbell Patch released!

@lbalmaceda
Copy link
Contributor

Having a second look at this after released. The isMultifactorCodeInvalid method is meant to be used for MFA flows, and while I understand that Passwordless could be confused with that, these are indeed different flows.

We already had a method that related to this error case. It is currently missing the Passwordless error descriptions so it's not currently matching that. It's more accurate if we move this PR's changes to the isInvalidCredentials method instead. I'll make the changes and ship that as another patch release. Just wanted you give you a heads up, since you're the only one that reached out for this use case, so you can update your usage.

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.

None yet

2 participants