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.

Sign up for GitHub

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

Jump to bottom

Feature: Implemented MFA APIs #442

Merged
merged 2 commits into from
Jan 7, 2022
Merged

Feature: Implemented MFA APIs #442

merged 2 commits into from
Jan 7, 2022

Conversation

poovamraj
Copy link
Contributor

@poovamraj poovamraj commented Jan 3, 2022 •
edited
Loading

Changes

Implemented APIs to use MFA Login with OTP, OOB, Using recovery code and requesting MFA Challenge

New methods added to the public API - loginWithOTP, loginWithOOB, loginWithRecoveryCode, multifactorChallenge

Information on how to use this has been added to the README file as well

References

Testing

I have added required unit tests and did manual testing to all the features as well. I have tested this with Android platform. Since this runs on JS, the iOS platform behaviour should be same. But never the less manual testing will be done for iOS as well

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

Checklist

@poovamraj poovamraj requested a review from a team as a code owner January 3, 2022 14:24
Widcket
Widcket reviewed Jan 3, 2022
View reviewed changes
src/auth/index.js Outdated Show resolved Hide resolved
Widcket
Widcket reviewed Jan 3, 2022
View reviewed changes
src/auth/index.js Outdated Show resolved Hide resolved
Widcket
Widcket requested changes Jan 3, 2022
View reviewed changes
Copy link
Contributor

@Widcket Widcket left a comment

Choose a reason for hiding this comment

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

Looking good! The multifactorChallenge method has two required parameters that should be optional.

@poovamraj
Copy link
Contributor Author

Thanks @Widcket that was a really nice catch. Will fix these and let you know!

@poovamraj poovamraj force-pushed the feature-otp-api-client-implementation branch from 9f337dd to 0d5694c Compare January 4, 2022 09:25
@poovamraj poovamraj requested a review from Widcket January 4, 2022 09:26
@poovamraj poovamraj force-pushed the feature-otp-api-client-implementation branch from 15d3e08 to b462372 Compare January 4, 2022 09:40
Widcket
Widcket reviewed Jan 4, 2022
View reviewed changes
src/auth/__tests__/index.spec.js Outdated
const parameters = {
mfaToken: '123',
challengeType: '123',
authenticatorId: '123',
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add test cases that verify that challengeType and authenticatorId are optional.

Copy link
Contributor

Choose a reason for hiding this comment

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

Similar to what was done for bindingCode above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added these changes 👍

@poovamraj poovamraj force-pushed the feature-otp-api-client-implementation branch from b462372 to d5a7f47 Compare January 5, 2022 16:36
@poovamraj poovamraj requested a review from Widcket January 5, 2022 16:37
@cocojoe
Copy link
Member

cocojoe commented Jan 6, 2022

I have added required unit tests and did manual testing to all the features as well. I have tested this with Android platform. Since this runs on JS, the iOS platform behaviour should be same. But never the less manual testing will be done for iOS as well

This is vague, what versions exactly have been tested of each platform?

@poovamraj
Copy link
Contributor Author

Currently I have manually tested the APIs in Android API Level 30 (Android 11). I have also tested the working in iOS with version 15.2. These testings were done with React Native version 66.4. I will test in the base version 65 as well

@poovamraj poovamraj merged commit 761ac93 into master Jan 7, 2022
@poovamraj poovamraj mentioned this pull request Jan 7, 2022
Merged
@evansims evansims deleted the feature-otp-api-client-implementation branch July 5, 2022 20:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Widcket Widcket Widcket approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@poovamraj @cocojoe @Widcket