Skip to content

Conversation

Xiaoshouzi-gh
Copy link
Contributor

@Xiaoshouzi-gh Xiaoshouzi-gh commented Jun 1, 2023

Added Assertion, generator and resolver logic for TOTP MFA.
Updated the backend rpc call finalizeMFASignInRequest to take TOTP parameters.

This PR is an updated version of #11217.

@google-oss-bot
Copy link

1 Warning
⚠️ Did you forget to add a changelog entry? (Add #no-changelog to the PR description to silence this warning.)

Generated by 🚫 Danger

@@ -53,6 +53,15 @@ NS_SWIFT_NAME(TOTPMultiFactorGenerator) API_UNAVAILABLE(macos, tvos, watchos)
+ (FIRTOTPMultiFactorAssertion *)assertionForEnrollmentWithSecret:(FIRTOTPSecret *)secret
oneTimePassword:(NSString *)oneTimePassword;

/** @fn assertionForSignInWithenrollmentID:
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: can we move @fn to a new line?

Copy link
Contributor

@renkelvin renkelvin left a comment

Choose a reason for hiding this comment

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

LGTM with 1 comment.

finalizedMFARequestInfo = [[FIRAuthProtoFinalizeMFAPhoneRequestInfo alloc]
initWithSessionInfo:phoneAssertion.authCredential.verificationID
verificationCode:phoneAssertion.authCredential.verificationCode];
} else {
Copy link
Contributor

Choose a reason for hiding this comment

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

I would be safer withelse if (is totp assertion){} and then check if finalizedMFARequestInfo is nil before assigning at L76.

@Xiaoshouzi-gh Xiaoshouzi-gh merged commit b8c33e1 into totp-release-0413 Jun 2, 2023
@Xiaoshouzi-gh Xiaoshouzi-gh deleted the totp-release-gen branch June 2, 2023 00:39
@firebase firebase locked and limited conversation to collaborators Jul 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants