Skip to content

Populates UserRecord with multi-factor related info #681

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Oct 24, 2019
Merged

Populates UserRecord with multi-factor related info #681

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
83b8484
Updates UserRecord to parse multi-factor related information from the…
bojeil-google Oct 21, 2019
2ade29d
Cleans up comments.
bojeil-google Oct 21, 2019
4a6a908
Addresses review comments.
bojeil-google Oct 23, 2019
ad542df
Adds response validation before getting factor ID.
bojeil-google Oct 24, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 29 additions & 36 deletions src/auth/user-record.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,7 @@ export interface CreateRequest extends UpdateRequest {
uid?: string;
}

export interface Metadata {
createdAt?: string;
lastLoginAt?: string;
}

export interface MfaInfo {
export interface AuthFactorInfo {
mfaEnrollmentId: string;
displayName?: string;
phoneInfo?: string;
Expand All @@ -81,11 +76,7 @@ export interface ProviderUserInfo {
federatedId?: string;
}

export interface Mfa {
mfaInfo?: MfaInfo[];
}

export interface GetAccountInfoUserResponse extends Metadata, Mfa {
export interface GetAccountInfoUserResponse {
localId: string;
email?: string;
emailVerified?: boolean;
Expand All @@ -99,6 +90,9 @@ export interface GetAccountInfoUserResponse extends Metadata, Mfa {
validSince?: string;
tenantId?: string;
providerUserInfo?: ProviderUserInfo[];
mfaInfo?: AuthFactorInfo[];
createdAt?: string;
lastLoginAt?: string;
[key: string]: any;
}

Expand All @@ -123,7 +117,7 @@ export abstract class MultiFactorInfo {
* @param response The server side response.
* @constructor
*/
public static initMultiFactorInfo(response: MfaInfo): MultiFactorInfo | null {
public static initMultiFactorInfo(response: AuthFactorInfo): MultiFactorInfo | null {
let multiFactorInfo: MultiFactorInfo | null = null;
// Only PhoneMultiFactorInfo currently available.
try {
Expand All @@ -140,9 +134,8 @@ export abstract class MultiFactorInfo {
* @param response The server side response.
* @constructor
*/
constructor(response: MfaInfo) {
const factorId = this.getFactorId(response);
this.initFromServerResponse(response, factorId);
constructor(response: AuthFactorInfo) {
this.initFromServerResponse(response);
}

/** @return The plain object representation. */
Expand All @@ -162,33 +155,33 @@ export abstract class MultiFactorInfo {
* @return The multi-factor ID associated with the provided response. If the response is
* not associated with any known multi-factor ID, null is returned.
*/
protected abstract getFactorId(response: MfaInfo): MultiFactorId | null;
protected abstract getFactorId(response: AuthFactorInfo): MultiFactorId | null;

/**
* Initializes the MultiFactorInfo object using the provided server response.
*
* @param response The server side response.
*/
private initFromServerResponse(response: MfaInfo, factorId: MultiFactorId | null) {
if (factorId && response && response.mfaEnrollmentId) {
utils.addReadonlyGetter(this, 'uid', response.mfaEnrollmentId);
utils.addReadonlyGetter(this, 'factorId', factorId);
utils.addReadonlyGetter(this, 'displayName', response.displayName || null);
// Encoded using [RFC 3339](https://www.ietf.org/rfc/rfc3339.txt) format.
// For example, "2017-01-15T01:30:15.01Z".
// This can be parsed directly via Date constructor.
// This can be computed using Data.prototype.toISOString.
if (response.enrolledAt) {
utils.addReadonlyGetter(
this, 'enrollmentTime', new Date(response.enrolledAt).toUTCString());
} else {
utils.addReadonlyGetter(this, 'enrollmentTime', null);
}
} else {
private initFromServerResponse(response: AuthFactorInfo) {
const factorId = this.getFactorId(response);
if (!factorId || !response || !response.mfaEnrollmentId) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we check response before passing it to getFactorId()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

throw new FirebaseAuthError(
AuthClientErrorCode.INTERNAL_ERROR,
'INTERNAL ASSERT FAILED: Invalid multi-factor info response');
}
utils.addReadonlyGetter(this, 'uid', response.mfaEnrollmentId);
utils.addReadonlyGetter(this, 'factorId', factorId);
utils.addReadonlyGetter(this, 'displayName', response.displayName || null);
// Encoded using [RFC 3339](https://www.ietf.org/rfc/rfc3339.txt) format.
// For example, "2017-01-15T01:30:15.01Z".
// This can be parsed directly via Date constructor.
// This can be computed using Data.prototype.toISOString.
if (response.enrolledAt) {
utils.addReadonlyGetter(
this, 'enrollmentTime', new Date(response.enrolledAt).toUTCString());
} else {
utils.addReadonlyGetter(this, 'enrollmentTime', null);
}
}
}

Expand All @@ -202,7 +195,7 @@ export class PhoneMultiFactorInfo extends MultiFactorInfo {
* @param response The server side response.
* @constructor
*/
constructor(response: MfaInfo) {
constructor(response: AuthFactorInfo) {
super(response);
utils.addReadonlyGetter(this, 'phoneNumber', response.phoneInfo);
}
Expand All @@ -223,7 +216,7 @@ export class PhoneMultiFactorInfo extends MultiFactorInfo {
* @return The multi-factor ID associated with the provided response. If the response is
* not associated with any known multi-factor ID, null is returned.
*/
protected getFactorId(response: MfaInfo): MultiFactorId | null {
protected getFactorId(response: AuthFactorInfo): MultiFactorId | null {
return !!response.phoneInfo ? MultiFactorId.Phone : null;
}
}
Expand All @@ -238,7 +231,7 @@ export class MultiFactor {
* @param response The server side response.
* @constructor
*/
constructor(response: Mfa) {
constructor(response: GetAccountInfoUserResponse) {
const parsedEnrolledFactors: MultiFactorInfo[] = [];
if (!isNonNullObject(response)) {
throw new FirebaseAuthError(
Expand Down Expand Up @@ -277,7 +270,7 @@ export class UserMetadata {
public readonly creationTime: string;
public readonly lastSignInTime: string;

constructor(response: Metadata) {
constructor(response: GetAccountInfoUserResponse) {
// Creation date should always be available but due to some backend bugs there
// were cases in the past where users did not have creation date properly set.
// This included legacy Firebase migrating project users and some anonymous users.
Expand Down
34 changes: 19 additions & 15 deletions test/unit/auth/user-record.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import * as chaiAsPromised from 'chai-as-promised';
import {deepCopy} from '../../../src/utils/deep-copy';
import {
UserInfo, UserMetadata, UserRecord, GetAccountInfoUserResponse, ProviderUserInfo,
MultiFactor, PhoneMultiFactorInfo, MultiFactorInfo,
MultiFactor, PhoneMultiFactorInfo, MultiFactorInfo, AuthFactorInfo,
} from '../../../src/auth/user-record';


Expand Down Expand Up @@ -240,7 +240,7 @@ function getUserInfoWithPhoneNumberJSON(): object {
}

describe('PhoneMultiFactorInfo', () => {
const serverResponse: any = {
const serverResponse: AuthFactorInfo = {
mfaEnrollmentId: 'enrollmentId1',
displayName: 'displayName1',
enrolledAt: now.toISOString(),
Expand All @@ -256,7 +256,7 @@ describe('PhoneMultiFactorInfo', () => {
it('should throw when an empty object is provided', () => {
expect(() => {
return new PhoneMultiFactorInfo({} as any);
}).to.throw(Error);
}).to.throw('INTERNAL ASSERT FAILED: Invalid multi-factor info response');
});

it('should succeed when mfaEnrollmentId and phoneInfo are both provided', () => {
Expand All @@ -273,15 +273,15 @@ describe('PhoneMultiFactorInfo', () => {
return new PhoneMultiFactorInfo({
mfaEnrollmentId: 'enrollmentId1',
} as any);
}).to.throw(Error);
}).to.throw('INTERNAL ASSERT FAILED: Invalid multi-factor info response');
});

it('should throw when only phoneInfo is provided', () => {
expect(() => {
return new PhoneMultiFactorInfo({
phoneInfo: '+16505551234',
} as any);
}).to.throw(Error);
}).to.throw('INTERNAL ASSERT FAILED: Invalid multi-factor info response');
});
});

Expand Down Expand Up @@ -369,7 +369,7 @@ describe('PhoneMultiFactorInfo', () => {
});

describe('MultiFactorInfo', () => {
const serverResponse: any = {
const serverResponse: AuthFactorInfo = {
mfaEnrollmentId: 'enrollmentId1',
displayName: 'displayName1',
enrolledAt: now.toISOString(),
Expand All @@ -390,6 +390,7 @@ describe('MultiFactorInfo', () => {

describe('MultiFactor', () => {
const serverResponse = {
localId: 'uid123',
mfaInfo: [
{
mfaEnrollmentId: 'enrollmentId1',
Expand Down Expand Up @@ -433,11 +434,11 @@ describe('MultiFactor', () => {
it('should throw when a non object is provided', () => {
expect(() => {
return new MultiFactor(undefined as any);
}).to.throw(Error);
}).to.throw('INTERNAL ASSERT FAILED: Invalid multi-factor response');
});

it('should populate an empty enrolledFactors array when given an empty object', () => {
const multiFactor = new MultiFactor({});
const multiFactor = new MultiFactor({} as any);

expect(multiFactor.enrolledFactors.length).to.equal(0);
});
Expand Down Expand Up @@ -478,7 +479,7 @@ describe('MultiFactor', () => {

describe('toJSON', () => {
it('should return expected JSON object when given an empty response', () => {
const multiFactor = new MultiFactor({});
const multiFactor = new MultiFactor({} as any);

expect(multiFactor.toJSON()).to.deep.equal({
enrolledFactors: [],
Expand Down Expand Up @@ -610,6 +611,7 @@ describe('UserMetadata', () => {
const expectedLastLoginAt = 1476235905000;
const expectedCreatedAt = 1476136676000;
const actualMetadata: UserMetadata = new UserMetadata({
localId: 'uid123',
lastLoginAt: expectedLastLoginAt.toString(),
createdAt: expectedCreatedAt.toString(),
});
Expand All @@ -621,20 +623,20 @@ describe('UserMetadata', () => {
describe('constructor', () => {
it('should initialize as expected when a valid creationTime is provided', () => {
expect(() => {
return new UserMetadata({createdAt: '1476136676000'});
return new UserMetadata({createdAt: '1476136676000'} as any);
}).not.to.throw(Error);
});

it('should set creationTime and lastSignInTime to null when not provided', () => {
const metadata = new UserMetadata({});
const metadata = new UserMetadata({} as any);
expect(metadata.creationTime).to.be.null;
expect(metadata.lastSignInTime).to.be.null;
});

it('should set creationTime to null when creationTime value is invalid', () => {
const metadata = new UserMetadata({
createdAt: 'invalid',
});
} as any);
expect(metadata.creationTime).to.be.null;
expect(metadata.lastSignInTime).to.be.null;
});
Expand All @@ -643,7 +645,7 @@ describe('UserMetadata', () => {
const metadata = new UserMetadata({
createdAt: '1476235905000',
lastLoginAt: 'invalid',
});
} as any);
expect(metadata.lastSignInTime).to.be.null;
});
});
Expand Down Expand Up @@ -864,7 +866,7 @@ describe('UserRecord', () => {
const metadata = new UserMetadata({
createdAt: '1476136676000',
lastLoginAt: '1476235905000',
});
} as any);
expect(userRecord.metadata).to.deep.equal(metadata);
});

Expand All @@ -873,7 +875,7 @@ describe('UserRecord', () => {
(userRecord as any).metadata = new UserMetadata({
createdAt: new Date().toUTCString(),
lastLoginAt: new Date().toUTCString(),
});
} as any);
}).to.throw(Error);
});

Expand Down Expand Up @@ -957,6 +959,7 @@ describe('UserRecord', () => {

it('should return expected multiFactor', () => {
const multiFactor = new MultiFactor({
localId: 'uid123',
mfaInfo: [
{
mfaEnrollmentId: 'enrollmentId1',
Expand Down Expand Up @@ -986,6 +989,7 @@ describe('UserRecord', () => {
it('should throw when modifying readonly multiFactor property', () => {
expect(() => {
(userRecord as any).multiFactor = new MultiFactor({
localId: 'uid123',
mfaInfo: [{
mfaEnrollmentId: 'enrollmentId3',
displayName: 'displayName3',
Expand Down