From bec46531a5ded99953a64c4b882deae8d638efa5 Mon Sep 17 00:00:00 2001 From: Kevin Cheung Date: Tue, 27 Jun 2023 11:58:44 -0700 Subject: [PATCH 1/3] Fixes to password policy validation --- src/auth/auth-config.ts | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/src/auth/auth-config.ts b/src/auth/auth-config.ts index f5022b3b14..0b54171689 100644 --- a/src/auth/auth-config.ts +++ b/src/auth/auth-config.ts @@ -2146,21 +2146,21 @@ export class PasswordPolicyAuthConfig implements PasswordPolicyConfig { ); } } - if (typeof options.constraints.requireUppercase !== undefined && + if (typeof options.constraints.requireUppercase !== 'undefined' && !validator.isBoolean(options.constraints.requireUppercase)) { throw new FirebaseAuthError( AuthClientErrorCode.INVALID_CONFIG, '"PasswordPolicyConfig.constraints.requireUppercase" must be a boolean.', ); } - if (typeof options.constraints.requireLowercase !== undefined && + if (typeof options.constraints.requireLowercase !== 'undefined' && !validator.isBoolean(options.constraints.requireLowercase)) { throw new FirebaseAuthError( AuthClientErrorCode.INVALID_CONFIG, '"PasswordPolicyConfig.constraints.requireLowercase" must be a boolean.', ); } - if (typeof options.constraints.requireNonAlphanumeric !== undefined && + if (typeof options.constraints.requireNonAlphanumeric !== 'undefined' && !validator.isBoolean(options.constraints.requireNonAlphanumeric)) { throw new FirebaseAuthError( AuthClientErrorCode.INVALID_CONFIG, @@ -2168,27 +2168,20 @@ export class PasswordPolicyAuthConfig implements PasswordPolicyConfig { ' must be a boolean.', ); } - if (typeof options.constraints.requireNumeric !== undefined && + if (typeof options.constraints.requireNumeric !== 'undefined' && !validator.isBoolean(options.constraints.requireNumeric)) { throw new FirebaseAuthError( AuthClientErrorCode.INVALID_CONFIG, '"PasswordPolicyConfig.constraints.requireNumeric" must be a boolean.', ); } - if (!validator.isNumber(options.constraints.minLength)) { + if (typeof options.constraints.minLength === 'undefined') { + options.constraints.minLength = 6; + } else if (!validator.isNumber(options.constraints.minLength)) { throw new FirebaseAuthError( AuthClientErrorCode.INVALID_CONFIG, '"PasswordPolicyConfig.constraints.minLength" must be a number.', ); - } - if (!validator.isNumber(options.constraints.maxLength)) { - throw new FirebaseAuthError( - AuthClientErrorCode.INVALID_CONFIG, - '"PasswordPolicyConfig.constraints.maxLength" must be a number.', - ); - } - if (options.constraints.minLength === undefined) { - options.constraints.minLength = 6; } else { if (!(options.constraints.minLength >= 6 && options.constraints.minLength <= 30)) { @@ -2199,8 +2192,13 @@ export class PasswordPolicyAuthConfig implements PasswordPolicyConfig { ); } } - if (options.constraints.maxLength === undefined) { + if (typeof options.constraints.maxLength === 'undefined') { options.constraints.maxLength = 4096; + } else if (!validator.isNumber(options.constraints.maxLength)) { + throw new FirebaseAuthError( + AuthClientErrorCode.INVALID_CONFIG, + '"PasswordPolicyConfig.constraints.maxLength" must be a number.', + ); } else { if (!(options.constraints.maxLength >= options.constraints.minLength && options.constraints.maxLength <= 4096)) { From 6a0fbdd072c73f4dc5f5cca38753cb53eaf215d5 Mon Sep 17 00:00:00 2001 From: Kevin Cheung Date: Tue, 27 Jun 2023 13:35:36 -0700 Subject: [PATCH 2/3] add test --- test/unit/auth/auth-config.spec.ts | 118 ++++++++++++++++++----------- 1 file changed, 72 insertions(+), 46 deletions(-) diff --git a/test/unit/auth/auth-config.spec.ts b/test/unit/auth/auth-config.spec.ts index b94db6d38c..8894bdc5ff 100644 --- a/test/unit/auth/auth-config.spec.ts +++ b/test/unit/auth/auth-config.spec.ts @@ -1239,60 +1239,86 @@ describe('OIDCConfig', () => { }); }); }); - describe('PasswordPolicyAuthConfig',() => { - describe('constructor',() => { - const validConfig = new PasswordPolicyAuthConfig({ - passwordPolicyEnforcementState: 'ENFORCE', +}); + +describe('PasswordPolicyAuthConfig',() => { + describe('constructor',() => { + const validConfig = new PasswordPolicyAuthConfig({ + passwordPolicyEnforcementState: 'ENFORCE', + passwordPolicyVersions: [ + { + customStrengthOptions: { + containsNumericCharacter: true, + containsLowercaseCharacter: true, + containsNonAlphanumericCharacter: true, + containsUppercaseCharacter: true, + minPasswordLength: 8, + maxPasswordLength: 30, + }, + }, + ], + forceUpgradeOnSignin: true, + }); + + it('should throw an error on missing state',() => { + expect(() => new PasswordPolicyAuthConfig({ passwordPolicyVersions: [ { - customStrengthOptions: { - containsNumericCharacter: true, - containsLowercaseCharacter: true, - containsNonAlphanumericCharacter: true, - containsUppercaseCharacter: true, - minPasswordLength: 8, - maxPasswordLength: 30, - }, - }, + customStrengthOptions: {}, + } ], - forceUpgradeOnSignin: true, - }); + } as any)).to.throw('INTERNAL ASSERT FAILED: Invalid password policy configuration response'); + }); - it('should throw an error on missing state',() => { - expect(() => new PasswordPolicyAuthConfig({ - passwordPolicyVersions: [ - { - customStrengthOptions: {}, - } - ], - } as any)).to.throw('INTERNAL ASSERT FAILED: Invalid password policy configuration response'); - }); + it('should set readonly property "enforcementState" to ENFORCE on state enforced',() => { + expect(validConfig.enforcementState).to.equal('ENFORCE'); + }); - it('should set readonly property "enforcementState" to ENFORCE on state enforced',() => { - expect(validConfig.enforcementState).to.equal('ENFORCE'); + it('should set readonly property "enforcementState" to OFF on state disabling',() => { + const offStateConfig=new PasswordPolicyAuthConfig({ + passwordPolicyEnforcementState: 'OFF', }); + expect(offStateConfig.enforcementState).to.equal('OFF'); + }); - it('should set readonly property "enforcementState" to OFF on state disabling',() => { - const offStateConfig=new PasswordPolicyAuthConfig({ - passwordPolicyEnforcementState: 'OFF', - }); - expect(offStateConfig.enforcementState).to.equal('OFF'); - }); + it('should set readonly property "constraints"',() => { + const expectedConstraints: CustomStrengthOptionsConfig = { + requireUppercase: true, + requireLowercase: true, + requireNonAlphanumeric: true, + requireNumeric: true, + minLength: 8, + maxLength: 30, + } + expect(validConfig.constraints).to.deep.equal(expectedConstraints); + }); - it('should set readonly property "constraints"',() => { - const expectedConstraints: CustomStrengthOptionsConfig = { - requireUppercase: true, - requireLowercase: true, - requireNonAlphanumeric: true, - requireNumeric: true, - minLength: 8, - maxLength: 30, - } - expect(validConfig.constraints).to.deep.equal(expectedConstraints); - }); + it('should set readonly property "forceUpgradeOnSignin"',() => { + expect(validConfig.forceUpgradeOnSignin).to.deep.equal(true); + }); + }); - it('should set readonly property "forceUpgradeOnSignin"',() => { - expect(validConfig.forceUpgradeOnSignin).to.deep.equal(true); + describe('buildServerRequest()', () => { + it('should return server request with default constraints', () => { + expect(PasswordPolicyAuthConfig.buildServerRequest({ + enforcementState: 'ENFORCE', + constraints: {}, + })).to.deep.equal({ + passwordPolicyEnforcementState: 'ENFORCE', + forceUpgradeOnSignin: false, + passwordPolicyVersions: [ + { + customStrengthOptions: { + containsLowercaseCharacter: false, + containsUppercaseCharacter: false, + containsNumericCharacter: false, + containsNonAlphanumericCharacter: false, + minPasswordLength: 6, + maxPasswordLength: 4096, + } + } + ] }); }); - });}); + }); +}); From 6a9a0028e8e8826fd54821bcdbc9a7bad90347e9 Mon Sep 17 00:00:00 2001 From: pragatimodi <110490169+pragatimodi@users.noreply.github.com> Date: Tue, 27 Jun 2023 11:47:03 -0700 Subject: [PATCH 3/3] feat(auth): Add `TotpInfo` field to `UserRecord` (#2197) * Adding TotpInfo to userRecord * Changing type from `any` to `unknown` for type safety. * Addressing feedback --- src/auth/user-record.ts | 78 +++++++++++++- test/unit/auth/user-record.spec.ts | 163 ++++++++++++++++++++++++++++- 2 files changed, 231 insertions(+), 10 deletions(-) diff --git a/src/auth/user-record.ts b/src/auth/user-record.ts index 2bd2fdacbd..18725c8279 100644 --- a/src/auth/user-record.ts +++ b/src/auth/user-record.ts @@ -47,8 +47,13 @@ export interface MultiFactorInfoResponse { mfaEnrollmentId: string; displayName?: string; phoneInfo?: string; + totpInfo?: TotpInfoResponse; enrolledAt?: string; - [key: string]: any; + [key: string]: unknown; +} + +export interface TotpInfoResponse { + [key: string]: unknown; } export interface ProviderUserInfoResponse { @@ -84,6 +89,7 @@ export interface GetAccountInfoUserResponse { enum MultiFactorId { Phone = 'phone', + Totp = 'totp', } /** @@ -102,7 +108,9 @@ export abstract class MultiFactorInfo { public readonly displayName?: string; /** - * The type identifier of the second factor. For SMS second factors, this is `phone`. + * The type identifier of the second factor. + * For SMS second factors, this is `phone`. + * For TOTP second factors, this is `totp`. */ public readonly factorId: string; @@ -120,9 +128,15 @@ export abstract class MultiFactorInfo { */ public static initMultiFactorInfo(response: MultiFactorInfoResponse): MultiFactorInfo | null { let multiFactorInfo: MultiFactorInfo | null = null; - // Only PhoneMultiFactorInfo currently available. + // PhoneMultiFactorInfo, TotpMultiFactorInfo currently available. try { - multiFactorInfo = new PhoneMultiFactorInfo(response); + if (response.phoneInfo !== undefined) { + multiFactorInfo = new PhoneMultiFactorInfo(response); + } else if (response.totpInfo !== undefined) { + multiFactorInfo = new TotpMultiFactorInfo(response); + } else { + // Ignore the other SDK unsupported MFA factors to prevent blocking developers using the current SDK. + } } catch (e) { // Ignore error. } @@ -240,6 +254,60 @@ export class PhoneMultiFactorInfo extends MultiFactorInfo { } } +/** + * TotpInfo struct associated with a second factor + */ +export class TotpInfo { + +} + +/** + * Interface representing a TOTP specific user-enrolled second factor. + */ +export class TotpMultiFactorInfo extends MultiFactorInfo { + + /** + * TotpInfo struct associated with a second factor + */ + public readonly totpInfo: TotpInfo; + + /** + * Initializes the TotpMultiFactorInfo object using the server side response. + * + * @param response - The server side response. + * @constructor + * @internal + */ + constructor(response: MultiFactorInfoResponse) { + super(response); + utils.addReadonlyGetter(this, 'totpInfo', response.totpInfo); + } + + /** + * {@inheritdoc MultiFactorInfo.toJSON} + */ + public toJSON(): object { + return Object.assign( + super.toJSON(), + { + totpInfo: this.totpInfo, + }); + } + + /** + * Returns the factor ID based on the response provided. + * + * @param response - The server side response. + * @returns The multi-factor ID associated with the provided response. If the response is + * not associated with any known multi-factor ID, null is returned. + * + * @internal + */ + protected getFactorId(response: MultiFactorInfoResponse): string | null { + return (response && response.totpInfo) ? MultiFactorId.Totp : null; + } +} + /** * The multi-factor related user settings. */ @@ -247,7 +315,7 @@ export class MultiFactorSettings { /** * List of second factors enrolled with the current user. - * Currently only phone second factors are supported. + * Currently only phone and totp second factors are supported. */ public enrolledFactors: MultiFactorInfo[]; diff --git a/test/unit/auth/user-record.spec.ts b/test/unit/auth/user-record.spec.ts index 4ec43af305..dc332c13b9 100644 --- a/test/unit/auth/user-record.spec.ts +++ b/test/unit/auth/user-record.spec.ts @@ -21,7 +21,7 @@ import * as chaiAsPromised from 'chai-as-promised'; import { deepCopy } from '../../../src/utils/deep-copy'; import { - GetAccountInfoUserResponse, ProviderUserInfoResponse, MultiFactorInfoResponse, + GetAccountInfoUserResponse, ProviderUserInfoResponse, MultiFactorInfoResponse, TotpMultiFactorInfo, } from '../../../src/auth/user-record'; import { UserInfo, UserMetadata, UserRecord, MultiFactorSettings, MultiFactorInfo, PhoneMultiFactorInfo, @@ -379,18 +379,157 @@ describe('PhoneMultiFactorInfo', () => { }); }); -describe('MultiFactorInfo', () => { +describe('TotpMultiFactorInfo', () => { const serverResponse: MultiFactorInfoResponse = { + mfaEnrollmentId: 'enrollmentId1', + displayName: 'displayName1', + enrolledAt: now.toISOString(), + totpInfo: {}, + }; + const totpMultiFactorInfo = new TotpMultiFactorInfo(serverResponse); + const totpMultiFactorInfoMissingFields = new TotpMultiFactorInfo({ + mfaEnrollmentId: serverResponse.mfaEnrollmentId, + totpInfo: serverResponse.totpInfo, + }); + + describe('constructor', () => { + it('should throw when an empty object is provided', () => { + expect(() => { + return new TotpMultiFactorInfo({} as any); + }).to.throw('INTERNAL ASSERT FAILED: Invalid multi-factor info response'); + }); + + it('should throw when an undefined response is provided', () => { + expect(() => { + return new TotpMultiFactorInfo(undefined as any); + }).to.throw('INTERNAL ASSERT FAILED: Invalid multi-factor info response'); + }); + + it('should succeed when mfaEnrollmentId and totpInfo are both provided', () => { + expect(() => { + return new TotpMultiFactorInfo({ + mfaEnrollmentId: 'enrollmentId1', + totpInfo: {}, + }); + }).not.to.throw(Error); + }); + + it('should throw when only mfaEnrollmentId is provided', () => { + expect(() => { + return new TotpMultiFactorInfo({ + mfaEnrollmentId: 'enrollmentId1', + } as any); + }).to.throw('INTERNAL ASSERT FAILED: Invalid multi-factor info response'); + }); + + it('should throw when only totpInfo is provided', () => { + expect(() => { + return new TotpMultiFactorInfo({ + totpInfo: {}, + } as any); + }).to.throw('INTERNAL ASSERT FAILED: Invalid multi-factor info response'); + }); + }); + + describe('getters', () => { + it('should set missing optional fields to null', () => { + expect(totpMultiFactorInfoMissingFields.uid).to.equal(serverResponse.mfaEnrollmentId); + expect(totpMultiFactorInfoMissingFields.displayName).to.be.undefined; + expect(totpMultiFactorInfoMissingFields.totpInfo).to.equal(serverResponse.totpInfo); + expect(totpMultiFactorInfoMissingFields.enrollmentTime).to.be.null; + expect(totpMultiFactorInfoMissingFields.factorId).to.equal('totp'); + }); + + it('should return expected factorId', () => { + expect(totpMultiFactorInfo.factorId).to.equal('totp'); + }); + + it('should throw when modifying readonly factorId property', () => { + expect(() => { + (totpMultiFactorInfo as any).factorId = 'other'; + }).to.throw(Error); + }); + + it('should return expected displayName', () => { + expect(totpMultiFactorInfo.displayName).to.equal(serverResponse.displayName); + }); + + it('should throw when modifying readonly displayName property', () => { + expect(() => { + (totpMultiFactorInfo as any).displayName = 'Modified'; + }).to.throw(Error); + }); + + it('should return expected totpInfo object', () => { + expect(totpMultiFactorInfo.totpInfo).to.equal(serverResponse.totpInfo); + }); + + it('should return expected uid', () => { + expect(totpMultiFactorInfo.uid).to.equal(serverResponse.mfaEnrollmentId); + }); + + it('should throw when modifying readonly uid property', () => { + expect(() => { + (totpMultiFactorInfo as any).uid = 'modifiedEnrollmentId'; + }).to.throw(Error); + }); + + it('should return expected enrollmentTime', () => { + expect(totpMultiFactorInfo.enrollmentTime).to.equal(now.toUTCString()); + }); + + it('should throw when modifying readonly uid property', () => { + expect(() => { + (totpMultiFactorInfo as any).enrollmentTime = new Date().toISOString(); + }).to.throw(Error); + }); + }); + + describe('toJSON', () => { + it('should return expected JSON object', () => { + expect(totpMultiFactorInfo.toJSON()).to.deep.equal({ + uid: 'enrollmentId1', + displayName: 'displayName1', + enrollmentTime: now.toUTCString(), + totpInfo: {}, + factorId: 'totp', + }); + }); + + it('should return expected JSON object with missing fields set to null', () => { + expect(totpMultiFactorInfoMissingFields.toJSON()).to.deep.equal({ + uid: 'enrollmentId1', + displayName: undefined, + enrollmentTime: null, + totpInfo: {}, + factorId: 'totp', + }); + }); + }); +}); + +describe('MultiFactorInfo', () => { + const phoneServerResponse: MultiFactorInfoResponse = { mfaEnrollmentId: 'enrollmentId1', displayName: 'displayName1', enrolledAt: now.toISOString(), phoneInfo: '+16505551234', }; - const phoneMultiFactorInfo = new PhoneMultiFactorInfo(serverResponse); + const phoneMultiFactorInfo = new PhoneMultiFactorInfo(phoneServerResponse); + const totpServerResponse: MultiFactorInfoResponse = { + mfaEnrollmentId: 'enrollmentId1', + displayName: 'displayName1', + enrolledAt: now.toISOString(), + totpInfo: {}, + }; + const totpMultiFactorInfo = new TotpMultiFactorInfo(totpServerResponse); describe('initMultiFactorInfo', () => { it('should return expected PhoneMultiFactorInfo', () => { - expect(MultiFactorInfo.initMultiFactorInfo(serverResponse)).to.deep.equal(phoneMultiFactorInfo); + expect(MultiFactorInfo.initMultiFactorInfo(phoneServerResponse)).to.deep.equal(phoneMultiFactorInfo); + }); + it('should return expected TotpMultiFactorInfo', () => { + expect(MultiFactorInfo.initMultiFactorInfo(totpServerResponse)).to.deep.equal(totpMultiFactorInfo); }); it('should return null for invalid MultiFactorInfo', () => { @@ -425,6 +564,12 @@ describe('MultiFactorSettings', () => { enrolledAt: now.toISOString(), secretKey: 'SECRET_KEY', }, + { + mfaEnrollmentId: 'enrollmentId5', + displayName: 'displayName1', + enrolledAt: now.toISOString(), + totpInfo: {}, + }, ], }; const expectedMultiFactorInfo = [ @@ -439,6 +584,12 @@ describe('MultiFactorSettings', () => { enrolledAt: now.toISOString(), phoneInfo: '+16505556789', }), + new TotpMultiFactorInfo({ + mfaEnrollmentId: 'enrollmentId5', + displayName: 'displayName1', + enrolledAt: now.toISOString(), + totpInfo: {}, + }) ]; describe('constructor', () => { @@ -457,9 +608,10 @@ describe('MultiFactorSettings', () => { it('should populate expected enrolledFactors', () => { const multiFactor = new MultiFactorSettings(serverResponse); - expect(multiFactor.enrolledFactors.length).to.equal(2); + expect(multiFactor.enrolledFactors.length).to.equal(3); expect(multiFactor.enrolledFactors[0]).to.deep.equal(expectedMultiFactorInfo[0]); expect(multiFactor.enrolledFactors[1]).to.deep.equal(expectedMultiFactorInfo[1]); + expect(multiFactor.enrolledFactors[2]).to.deep.equal(expectedMultiFactorInfo[2]); }); }); @@ -504,6 +656,7 @@ describe('MultiFactorSettings', () => { enrolledFactors: [ expectedMultiFactorInfo[0].toJSON(), expectedMultiFactorInfo[1].toJSON(), + expectedMultiFactorInfo[2].toJSON(), ], }); });