-
Notifications
You must be signed in to change notification settings - Fork 420
fix(auth): support TOTP second factors when updating a user #3264
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -17,6 +17,7 @@ | |||||||||||||||||||||||||||||||||||||||
| import * as validator from '../utils/validator'; | ||||||||||||||||||||||||||||||||||||||||
| import { deepCopy } from '../utils/deep-copy'; | ||||||||||||||||||||||||||||||||||||||||
| import { authClientErrorCode, FirebaseAuthError } from './error'; | ||||||||||||||||||||||||||||||||||||||||
| import { TotpInfoResponse } from './user-record'; | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||
| * Interface representing base properties of a user-enrolled second factor for a | ||||||||||||||||||||||||||||||||||||||||
|
|
@@ -93,11 +94,27 @@ export interface UpdatePhoneMultiFactorInfoRequest extends BaseUpdateMultiFactor | |||||||||||||||||||||||||||||||||||||||
| phoneNumber: string; | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||
| * Interface representing a TOTP specific user-enrolled second factor | ||||||||||||||||||||||||||||||||||||||||
| * for an `UpdateRequest`. | ||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||
| export interface UpdateTotpMultiFactorInfoRequest extends BaseUpdateMultiFactorInfoRequest { | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||
| * The TOTP specific metadata of the second factor, as returned by the Auth server when | ||||||||||||||||||||||||||||||||||||||||
| * the factor was enrolled. The Admin SDK cannot enroll a new TOTP factor on behalf of a | ||||||||||||||||||||||||||||||||||||||||
| * user, so this is only ever populated from a previously enrolled factor. | ||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||
| totpInfo: TotpInfoResponse; | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+101
to
+109
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since TOTP factors cannot be newly enrolled via the Admin SDK and must always be carried over from a previously enrolled factor, the enrollment ID (
Suggested change
|
||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||
| * Type representing the properties of a user-enrolled second factor | ||||||||||||||||||||||||||||||||||||||||
| * for an `UpdateRequest`. | ||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||
| export type UpdateMultiFactorInfoRequest = | UpdatePhoneMultiFactorInfoRequest; | ||||||||||||||||||||||||||||||||||||||||
| export type UpdateMultiFactorInfoRequest = | ||||||||||||||||||||||||||||||||||||||||
| | UpdatePhoneMultiFactorInfoRequest | ||||||||||||||||||||||||||||||||||||||||
| | UpdateTotpMultiFactorInfoRequest; | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||
| * The multi-factor related user settings for create operations. | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -20,8 +20,10 @@ import * as utils from '../utils'; | |||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import * as validator from '../utils/validator'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { authClientErrorCode, FirebaseAuthError } from './error'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| UpdateMultiFactorInfoRequest, UpdatePhoneMultiFactorInfoRequest, MultiFactorUpdateSettings | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| UpdateMultiFactorInfoRequest, UpdatePhoneMultiFactorInfoRequest, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| UpdateTotpMultiFactorInfoRequest, MultiFactorUpdateSettings | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } from './auth-config'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { TotpInfoResponse } from './user-record'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export type HashAlgorithmType = 'SCRYPT' | 'STANDARD_SCRYPT' | 'HMAC_SHA512' | | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 'HMAC_SHA256' | 'HMAC_SHA1' | 'HMAC_MD5' | 'MD5' | 'PBKDF_SHA1' | 'BCRYPT' | | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -261,6 +263,7 @@ export interface AuthFactorInfo { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||
| mfaEnrollmentId?: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| displayName?: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| phoneInfo?: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| totpInfo?: TotpInfoResponse; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| enrolledAt?: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| [key: string]: any; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -334,7 +337,6 @@ export function convertMultiFactorInfoToServerFormat(multiFactorInfo: UpdateMult | |||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 'UTC date string.'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Currently only phone second factors are supported. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (isPhoneFactor(multiFactorInfo)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // If any required field is missing or invalid, validation will still fail later. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const authFactorInfo: AuthFactorInfo = { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -344,11 +346,18 @@ export function convertMultiFactorInfoToServerFormat(multiFactorInfo: UpdateMult | |||||||||||||||||||||||||||||||||||||||||||||||||||||||
| phoneInfo: multiFactorInfo.phoneNumber, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| enrolledAt, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for (const objKey in authFactorInfo) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (typeof authFactorInfo[objKey] === 'undefined') { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| delete authFactorInfo[objKey]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| removeUndefinedFields(authFactorInfo); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return authFactorInfo; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else if (isTotpFactor(multiFactorInfo)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // TOTP factors are always carried over from a previously enrolled factor, so the | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // enrollment ID and the TOTP metadata are both preserved as is. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const authFactorInfo: AuthFactorInfo = { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| mfaEnrollmentId: multiFactorInfo.uid, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| displayName: multiFactorInfo.displayName, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| totpInfo: multiFactorInfo.totpInfo, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| enrolledAt, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| removeUndefinedFields(authFactorInfo); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return authFactorInfo; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+351
to
361
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If a developer attempts to update a TOTP factor but omits the
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Unsupported second factor. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -358,11 +367,27 @@ export function convertMultiFactorInfoToServerFormat(multiFactorInfo: UpdateMult | |||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| function removeUndefinedFields(obj: AuthFactorInfo): void { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for (const objKey in obj) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (typeof obj[objKey] === 'undefined') { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| delete obj[objKey]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| function isPhoneFactor(multiFactorInfo: UpdateMultiFactorInfoRequest): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| multiFactorInfo is UpdatePhoneMultiFactorInfoRequest { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return multiFactorInfo.factorId === 'phone'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| function isTotpFactor(multiFactorInfo: UpdateMultiFactorInfoRequest): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| multiFactorInfo is UpdateTotpMultiFactorInfoRequest { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Only factors that carry the TOTP metadata handed out by the server are accepted. A bare | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // secret cannot be enrolled through the Admin SDK, and sending it would leave the user with | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // a factor no authenticator app can generate codes for. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return multiFactorInfo.factorId === 'totp' && 'totpInfo' in multiFactorInfo; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @param {any} obj The object to check for number field within. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @param {string} key The entry key. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since TOTP factors cannot be newly enrolled via the Admin SDK, updating or importing a TOTP factor requires an existing enrollment ID (
uid/mfaEnrollmentId). Adding a check to ensuremfaEnrollmentIdis defined prevents invalid requests from being sent to the server.