Skip to content

Commit 595dc8f

Browse files
committed
Populate enrolled factors on reload()
1 parent aef653d commit 595dc8f

File tree

6 files changed

+130
-24
lines changed

6 files changed

+130
-24
lines changed

packages-exp/auth-exp/src/core/strategies/phone.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -414,7 +414,7 @@ describe('core/strategies/phone', () => {
414414
});
415415
});
416416

417-
describe("updatePhoneNumber", () => {
417+
describe('updatePhoneNumber', () => {
418418
let user: User;
419419
let reloadMock: fetch.Route;
420420
let signInMock: fetch.Route;

packages-exp/auth-exp/src/core/user/reload.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@ export async function _reloadWithoutSaving(user: User): Promise<void> {
3232
assert(response?.users.length, auth.name);
3333

3434
const coreAccount = response.users[0];
35+
36+
user._notifyReloadListener(coreAccount);
37+
3538
const newProviderData = coreAccount.providerUserInfo?.length
3639
? extractProviderData(coreAccount.providerUserInfo)
3740
: [];

packages-exp/auth-exp/src/core/user/user_impl.ts

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,17 +16,21 @@
1616
*/
1717

1818
import * as externs from '@firebase/auth-types-exp';
19+
import { NextFn } from '@firebase/util';
1920

20-
import { deleteAccount } from '../../api/account_management/account';
21+
import {
22+
APIUserInfo,
23+
deleteAccount
24+
} from '../../api/account_management/account';
25+
import { FinalizeMfaResponse } from '../../api/authentication/mfa';
2126
import { Auth } from '../../model/auth';
2227
import { IdTokenResponse } from '../../model/id_token';
2328
import { User } from '../../model/user';
2429
import { PersistedBlob } from '../persistence';
2530
import { assert } from '../util/assert';
2631
import { getIdTokenResult } from './id_token_result';
27-
import { _reloadWithoutSaving, reload } from './reload';
32+
import { reload, _reloadWithoutSaving } from './reload';
2833
import { StsTokenManager } from './token_manager';
29-
import { FinalizeMfaResponse } from '../../api/authentication/mfa';
3034

3135
export interface UserParameters {
3236
uid: string;
@@ -102,6 +106,28 @@ export class UserImpl implements User {
102106
return reload(this);
103107
}
104108

109+
private reloadUserInfo: APIUserInfo | null = null;
110+
private reloadListener: NextFn<APIUserInfo> | null = null;
111+
112+
_onReload(callback: NextFn<APIUserInfo>): void {
113+
// There should only ever be one listener, and that is a single instance of MultiFactorUser
114+
assert(!this.reloadListener, this.auth.name);
115+
this.reloadListener = callback;
116+
if (this.reloadUserInfo) {
117+
this._notifyReloadListener(this.reloadUserInfo);
118+
this.reloadUserInfo = null;
119+
}
120+
}
121+
122+
_notifyReloadListener(userInfo: APIUserInfo): void {
123+
if (this.reloadListener) {
124+
this.reloadListener(userInfo);
125+
} else {
126+
// If no listener is subscribed yet, save the result so it's available when they do subscribe
127+
this.reloadUserInfo = userInfo;
128+
}
129+
}
130+
105131
async _updateTokensIfNecessary(
106132
response: IdTokenResponse | FinalizeMfaResponse,
107133
reload = false

packages-exp/auth-exp/src/mfa/mfa_user.test.ts

Lines changed: 72 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import { MultiFactorInfo } from './mfa_info';
3131
import { ServerError } from '../api/errors';
3232
import { User } from '../model/user';
3333
import { FinalizeMfaResponse } from '../api/authentication/mfa';
34+
import { ProviderId } from '@firebase/auth-types-exp';
3435

3536
use(chaiAsPromised);
3637

@@ -118,14 +119,14 @@ describe('core/mfa/mfa_user/MultiFactorUser', () => {
118119
expect(await mfaUser.user.getIdToken()).to.eq('final-id-token');
119120
});
120121

121-
// TODO: Uncomment once we integrate with reload()
122-
// it('should update the enrolled Factors', async () => {
123-
// await mfaUser.enroll(assertion);
122+
it('should update the enrolled Factors', async () => {
123+
await mfaUser.enroll(assertion);
124124

125-
// const enrolledFactor = mfaUser.enrolledFactors[1];
126-
// expect(enrolledFactor.factorId).to.eq(ProviderId.PHONE);
127-
// expect(enrolledFactor.uid).to.eq('mfa-id');
128-
// });
125+
expect(mfaUser.enrolledFactors.length).to.eq(1);
126+
const enrolledFactor = mfaUser.enrolledFactors[0];
127+
expect(enrolledFactor.factorId).to.eq(ProviderId.PHONE);
128+
expect(enrolledFactor.uid).to.eq('mfa-id');
129+
});
129130
});
130131

131132
describe('unenroll', () => {
@@ -150,19 +151,11 @@ describe('core/mfa/mfa_user/MultiFactorUser', () => {
150151

151152
const serverUser: APIUserInfo = {
152153
localId: 'local-id',
153-
displayName: 'display-name',
154-
photoUrl: 'photo-url',
155-
email: 'email',
156-
emailVerified: true,
157-
phoneNumber: 'phone-number',
158-
tenantId: 'tenant-id',
159-
createdAt: 123,
160-
lastLoginAt: 456,
161154
mfaInfo: [
162155
{
163156
mfaEnrollmentId: 'other-mfa-id',
164157
enrolledAt: Date.now(),
165-
phoneInfo: 'other-phone-number'
158+
phoneInfo: 'other-phone-info'
166159
}
167160
]
168161
};
@@ -244,4 +237,67 @@ describe('core/mfa/mfa_user/multiFactor', () => {
244237
const mfaUser = multiFactor(user);
245238
expect((mfaUser as MultiFactorUser).user).to.eq(user);
246239
});
240+
241+
it('should only create one instance of an MFA user per User', () => {
242+
const mfaUser = multiFactor(user);
243+
expect(multiFactor(user)).to.eq(mfaUser);
244+
});
245+
246+
context('enrolledFactors', () => {
247+
const serverUser: APIUserInfo = {
248+
localId: 'local-id',
249+
mfaInfo: [
250+
{
251+
mfaEnrollmentId: 'enrollment-id',
252+
enrolledAt: Date.now(),
253+
phoneInfo: 'masked-phone-number'
254+
}
255+
]
256+
};
257+
258+
const updatedServerUser: APIUserInfo = {
259+
localId: 'local-id',
260+
mfaInfo: [
261+
{
262+
mfaEnrollmentId: 'enrollment-id',
263+
enrolledAt: Date.now(),
264+
phoneInfo: 'masked-phone-number'
265+
},
266+
{
267+
mfaEnrollmentId: 'new-enrollment-id',
268+
enrolledAt: Date.now(),
269+
phoneInfo: 'other-masked-phone-number'
270+
}
271+
]
272+
};
273+
274+
beforeEach(() => {
275+
mockFetch.setUp();
276+
mockEndpoint(Endpoint.GET_ACCOUNT_INFO, {
277+
users: [serverUser]
278+
});
279+
});
280+
281+
afterEach(mockFetch.tearDown);
282+
283+
it('should initialize the enrolled factors from the last reload', async () => {
284+
await user.reload();
285+
const mfaUser = multiFactor(user);
286+
expect(mfaUser.enrolledFactors.length).to.eq(1);
287+
const mfaInfo = mfaUser.enrolledFactors[0];
288+
expect(mfaInfo.uid).to.eq('enrollment-id');
289+
expect(mfaInfo.factorId).to.eq(ProviderId.PHONE);
290+
});
291+
292+
it('should update the enrolled factors if the user is reloaded', async () => {
293+
await user.reload();
294+
const mfaUser = multiFactor(user);
295+
expect(mfaUser.enrolledFactors.length).to.eq(1);
296+
mockEndpoint(Endpoint.GET_ACCOUNT_INFO, {
297+
users: [updatedServerUser]
298+
});
299+
await user.reload();
300+
expect(mfaUser.enrolledFactors.length).to.eq(2);
301+
});
302+
});
247303
});

packages-exp/auth-exp/src/mfa/mfa_user.ts

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,20 @@ import { MultiFactorSession } from './mfa_session';
2121
import { MultiFactorAssertion } from './assertions';
2222
import { withdrawMfa } from '../api/account_management/mfa';
2323
import { AuthErrorCode } from '../core/errors';
24+
import { MultiFactorInfo } from './mfa_info';
2425

2526
export class MultiFactorUser implements externs.MultiFactorUser {
26-
// TODO(avolkovi): Set these correctly from getAccountInfo on reload
2727
enrolledFactors: externs.MultiFactorInfo[] = [];
2828

29-
private constructor(readonly user: User) {}
29+
private constructor(readonly user: User) {
30+
user._onReload(userInfo => {
31+
if (userInfo.mfaInfo) {
32+
this.enrolledFactors = userInfo.mfaInfo.map(enrollment =>
33+
MultiFactorInfo._fromServerResponse(user.auth, enrollment)
34+
);
35+
}
36+
});
37+
}
3038

3139
static _fromUser(user: User): MultiFactorUser {
3240
return new MultiFactorUser(user);
@@ -79,6 +87,14 @@ export class MultiFactorUser implements externs.MultiFactorUser {
7987
}
8088
}
8189

90+
const multiFactorUserCache = new WeakMap<
91+
externs.User,
92+
externs.MultiFactorUser
93+
>();
94+
8295
export function multiFactor(user: externs.User): externs.MultiFactorUser {
83-
return MultiFactorUser._fromUser(user as User);
96+
if (!multiFactorUserCache.has(user)) {
97+
multiFactorUserCache.set(user, MultiFactorUser._fromUser(user as User));
98+
}
99+
return multiFactorUserCache.get(user)!;
84100
}

packages-exp/auth-exp/src/model/user.d.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,13 @@
1616
*/
1717

1818
import * as externs from '@firebase/auth-types-exp';
19+
import { NextFn } from '@firebase/util';
1920

21+
import { APIUserInfo } from '../api/account_management/account';
22+
import { FinalizeMfaResponse } from '../api/authentication/mfa';
2023
import { PersistedBlob } from '../core/persistence';
2124
import { Auth } from './auth';
2225
import { IdTokenResponse } from './id_token';
23-
import { FinalizeMfaResponse } from '../api/authentication/mfa';
2426

2527
type MutableUserInfo = {
2628
-readonly [K in keyof externs.UserInfo]: externs.UserInfo[K];
@@ -46,6 +48,9 @@ export interface User extends externs.User {
4648
reload?: boolean
4749
): Promise<void>;
4850

51+
_onReload: (cb: NextFn<APIUserInfo>) => void;
52+
_notifyReloadListener: NextFn<APIUserInfo>;
53+
4954
getIdToken(forceRefresh?: boolean): Promise<string>;
5055
getIdTokenResult(forceRefresh?: boolean): Promise<externs.IdTokenResult>;
5156
reload(): Promise<void>;

0 commit comments

Comments
 (0)