Populates UserRecord with multi-factor related info #681
Conversation
… GetAccountInfo server response. First step in supporting multi-factor authentication with SMS as a second factor.
There was a problem hiding this comment.
Looks pretty good. My main concerns are around some of the names. There are just many similar sounding names, and it makes it a little difficult to follow. My primary suggestions are:
- Embed
MfaandMetadatatypes directly intoGetAccountInfoUserResponseand remove them. - Rename
MfaInfotoAuthFactorInfo. - Rename
MultiFactorInfotoAuthFactorandPhoneMultiFactorInfoto justPhoneFactor.
There was a problem hiding this comment.
Thanks for the review and feedback. I made most of the requested changes except some of the public API renames as they have been approved by the API council and already implemented in the client SDKs (currently in alpha phase). It is hard to change it at this point across all platforms.
| } else { | ||
| private initFromServerResponse(response: AuthFactorInfo) { | ||
| const factorId = this.getFactorId(response); | ||
| if (!factorId || !response || !response.mfaEnrollmentId) { |
There was a problem hiding this comment.
Should we check response before passing it to getFactorId()?
There was a problem hiding this comment.
I have added a check for this. I also added a test when undefined is passed to the constructor that the expected error is caught and thrown.
Updates UserRecord to parse multi-factor related information from the GetAccountInfo server response.
First step in supporting multi-factor authentication with SMS as a second factor.