Cleanup TOTP code to include Firebase User reference in MultiFactorSession. (#7346)

* cleanup TOTP code to include Firebase user reference in MultiFactorSession

* removed helper comments

* Added fakeUid const to organize code

* edited test to match changes in MultiFactorSession

* resolved TestAuth incompatible type error

* resolved more TestAuth incompatible type error

* resubmitting

* fixed formatting & resolved issue with undefined

* fixed format issue

* fixed formatting

* tried to resolve issue with user being undefined

* fixed if statement formatting

* addressed feedback: changed expect to compare user

* fixed formatting

* fixed the issue with undefined

* instantiate user

* organized import statements

* changed import formatting

* resubmitting for import formatting issue

* ran yarn format

Author: AngelAngelXie

packages/auth/src/mfa/assertions/totp.test.ts

@@ -38,6 +38,8 @@ import { MultiFactorAssertionImpl } from '../mfa_assertion';
 
 use(chaiAsPromised);
 
+const fakeUid: string = 'uid';
+
 describe('core/mfa/assertions/totp/TotpMultiFactorGenerator', () => {
   let auth: TestAuth;
   let session: MultiFactorSessionImpl;
@@ -82,7 +84,7 @@ describe('core/mfa/assertions/totp/TotpMultiFactorGenerator', () => {
     });
     afterEach(mockFetch.tearDown);
 
-    it('should throw error if auth instance is not found in mfaSession', async () => {
+    it('should throw error if user instance is not found in mfaSession', async () => {
       try {
         session = MultiFactorSessionImpl._fromIdtoken(
           'enrollment-id-token',
@@ -100,9 +102,10 @@ describe('core/mfa/assertions/totp/TotpMultiFactorGenerator', () => {
       );
 
       auth = await testAuth();
+      const user = await testUser(auth, fakeUid);
       session = MultiFactorSessionImpl._fromIdtoken(
         'enrollment-id-token',
-        auth
+        user
       );
       const secret = await TotpMultiFactorGenerator.generateSecret(session);
       expect(mock.calls[0].request).to.eql({
@@ -159,10 +162,11 @@ describe('core/mfa/totp/assertions/TotpMultiFactorAssertionImpl', () => {
   afterEach(mockFetch.tearDown);
 
   describe('enroll', () => {
+    const user = testUser(auth, fakeUid);
     beforeEach(() => {
       session = MultiFactorSessionImpl._fromIdtoken(
         'enrollment-id-token',
-        auth
+        user
       );
     });
 
@@ -180,7 +184,8 @@ describe('core/mfa/totp/assertions/TotpMultiFactorAssertionImpl', () => {
           sessionInfo: 'verification-id'
         }
       });
-      expect(session.auth).to.eql(auth);
+      expect(session.user).to.not.be.undefined;
+      expect(session.user).to.eql(user);
     });
 
     context('with display name', () => {
@@ -203,7 +208,8 @@ describe('core/mfa/totp/assertions/TotpMultiFactorAssertionImpl', () => {
             sessionInfo: 'verification-id'
           }
         });
-        expect(session.auth).to.eql(auth);
+        expect(session.user).to.not.be.undefined;
+        expect(session.user).to.eql(user);
       });
     });
   });
@@ -299,7 +305,7 @@ describe('core/mfa/assertions/totp/TotpSecret', async () => {
   describe('generateQrCodeUrl', () => {
     beforeEach(async () => {
       await auth.updateCurrentUser(
-        testUser(_castAuth(auth), 'uid', fakeEmail, true)
+        testUser(_castAuth(auth), fakeUid, fakeEmail, true)
       );
     });
 

packages/auth/src/mfa/assertions/totp.ts

@@ -91,16 +91,16 @@ export class TotpMultiFactorGenerator {
   ): Promise<TotpSecret> {
     const mfaSession = session as MultiFactorSessionImpl;
     _assert(
-      typeof mfaSession.auth !== 'undefined',
+      typeof mfaSession.user?.auth !== 'undefined',
       AuthErrorCode.INTERNAL_ERROR
     );
-    const response = await startEnrollTotpMfa(mfaSession.auth, {
+    const response = await startEnrollTotpMfa(mfaSession.user.auth, {
       idToken: mfaSession.credential,
       totpEnrollmentInfo: {}
     });
     return TotpSecret._fromStartTotpMfaEnrollmentResponse(
       response,
-      mfaSession.auth
+      mfaSession.user.auth
     );
   }
 

packages/auth/src/mfa/mfa_session.ts

@@ -14,7 +14,8 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-import { AuthInternal } from '../model/auth';
+
+import { UserInternal } from '../model/user';
 import { MultiFactorSession } from '../model/public_types';
 
 export const enum MultiFactorSessionType {
@@ -33,17 +34,17 @@ export class MultiFactorSessionImpl implements MultiFactorSession {
   private constructor(
     readonly type: MultiFactorSessionType,
     readonly credential: string,
-    readonly auth?: AuthInternal
+    readonly user?: UserInternal
   ) {}
 
   static _fromIdtoken(
     idToken: string,
-    auth?: AuthInternal
+    user?: UserInternal
   ): MultiFactorSessionImpl {
     return new MultiFactorSessionImpl(
       MultiFactorSessionType.ENROLL,
       idToken,
-      auth
+      user
     );
   }
 

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

@@ -88,7 +88,7 @@ describe('core/mfa/mfa_user/MultiFactorUser', () => {
       const mfaSession = (await mfaUser.getSession()) as MultiFactorSessionImpl;
       expect(mfaSession.type).to.eq(MultiFactorSessionType.ENROLL);
       expect(mfaSession.credential).to.eq('access-token');
-      expect(mfaSession.auth).to.eq(auth);
+      expect(mfaSession.user?.auth).to.eq(auth);
     });
   });
 

packages/auth/src/mfa/mfa_user.ts

@@ -50,7 +50,7 @@ export class MultiFactorUserImpl implements MultiFactorUser {
   async getSession(): Promise<MultiFactorSession> {
     return MultiFactorSessionImpl._fromIdtoken(
       await this.user.getIdToken(),
-      this.user.auth
+      this.user
     );
   }
 

packages/auth/src/platform_browser/mfa/assertions/phone.test.ts

@@ -20,7 +20,11 @@ import { expect, use } from 'chai';
 import chaiAsPromised from 'chai-as-promised';
 
 import { mockEndpoint } from '../../../../test/helpers/api/helper';
-import { testAuth, TestAuth } from '../../../../test/helpers/mock_auth';
+import {
+  testAuth,
+  TestAuth,
+  testUser
+} from '../../../../test/helpers/mock_auth';
 import * as mockFetch from '../../../../test/helpers/mock_fetch';
 import { Endpoint } from '../../../api';
 import { FinalizeMfaResponse } from '../../../api/authentication/mfa';
@@ -57,10 +61,11 @@ describe('platform_browser/mfa/phone', () => {
   afterEach(mockFetch.tearDown);
 
   describe('enroll', () => {
+    const user = testUser(auth, 'uid');
     beforeEach(() => {
       session = MultiFactorSessionImpl._fromIdtoken(
         'enrollment-id-token',
-        auth
+        user
       );
     });
 
@@ -78,7 +83,8 @@ describe('platform_browser/mfa/phone', () => {
           sessionInfo: 'verification-id'
         }
       });
-      expect(session.auth).to.eql(auth);
+      expect(session.user).to.not.be.undefined;
+      expect(session.user).to.eql(user);
     });
 
     context('with display name', () => {
@@ -101,7 +107,8 @@ describe('platform_browser/mfa/phone', () => {
             sessionInfo: 'verification-id'
           }
         });
-        expect(session.auth).to.eql(auth);
+        expect(session.user).to.not.be.undefined;
+        expect(session.user).to.eql(user);
       });
     });
   });
@@ -124,7 +131,7 @@ describe('platform_browser/mfa/phone', () => {
           sessionInfo: 'verification-id'
         }
       });
-      expect(session.auth).to.eql(undefined);
+      expect(session.user).to.be.undefined;
     });
   });
 });