feat(mfa): Implement Multi-Factor Authentication (MFA) support#1517
feat(mfa): Implement Multi-Factor Authentication (MFA) support#1517subhankarmaiti wants to merge 10 commits intomasterfrom
Conversation
Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
| } | ||
|
|
||
| @ReactMethod | ||
| override fun mfaGetAuthenticators(mfaToken: String, factorsAllowed: ReadableArray?, promise: Promise) { |
There was a problem hiding this comment.
Rename to getMfaAuthenticators ?
There was a problem hiding this comment.
mfaGetAuthenticators Shouldn't this also be renamed to getMfaAuthenticators
Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
| @@ -177,6 +181,7 @@ class A0Auth0Module(private val reactContext: ReactApplicationContext) : A0Auth0 | |||
|
|
|||
| this.useDPoP = useDPoP ?: true | |||
There was a problem hiding this comment.
completely unrelated but shouldn't the default behaviour for Dpop be false ?
| override fun mfaVerify(mfaToken: String, type: String, code: String, bindingCode: String?, promise: Promise) { | ||
| mfaBridge?.verify(mfaToken, type, code, bindingCode, promise) | ||
| ?: promise.reject("NOT_INITIALIZED", "Auth0 not initialized") | ||
| } |
There was a problem hiding this comment.
Noted you are following a pattern of mfa prefix . Ensure this is inline with what the auth0/react SDK also follows to maintain consistency ?
| } | ||
|
|
||
| fun getAuthenticators(mfaToken: String, factorsAllowed: ReadableArray?, promise: Promise) { | ||
| val mfaClient = createAuthClient().mfaClient(mfaToken) |
There was a problem hiding this comment.
Can't we just create one instance on AuthenticationAPIClient for this entire class ?
private val client by lazy { AuthenticationAPIClient(auth0).apply{
if (useDPoP) {
useDPoP(reactContext)
}
}
}
| ) | ||
| } | ||
|
|
||
| fun enroll(mfaToken: String, type: String, value: String?, promise: Promise) { |
There was a problem hiding this comment.
Define type as enum so that its always limited to the supported factor
| applicationId "com.auth0example" | ||
| minSdkVersion rootProject.ext.minSdkVersion | ||
| targetSdkVersion rootProject.ext.targetSdkVersion | ||
| manifestPlaceholders = [auth0Domain: "dev-481o0jxuhhma72l4.us.auth0.com", auth0Scheme: "${applicationId}.auth0"] |
There was a problem hiding this comment.
remove the domain value from here
| self.useDPoP = useDPoP | ||
| } | ||
|
|
||
| private func createAuthentication() -> Authentication { |
There was a problem hiding this comment.
Same comment as in Android. Can't we have 1 instance on Authentication fot this bridge class
| } | ||
| } | ||
|
|
||
| func enroll(mfaToken: String, type: String, value: String?, resolve: @escaping RCTPromiseResolveBlock, reject: @escaping RCTPromiseRejectBlock) { |
There was a problem hiding this comment.
Make type as an enum instead of String
71772d0 to
714794d
Compare
No description provided.