Update token expiration logic to account for missing expiresIn in cer…

…tain requests. (#4085)
firebase · Nov 17, 2020 · 0c4f794 · 0c4f794
1 parent 34973cd
commit 0c4f794
Show file tree

Hide file tree

Showing 7 changed files with 97 additions and 31 deletions.
diff --git a/packages-exp/auth-exp/src/api/authentication/token.ts b/packages-exp/auth-exp/src/api/authentication/token.ts
@@ -33,15 +33,15 @@ export const enum Endpoint {
 
 /** The server responses with snake_case; we convert to camelCase */
 interface RequestStsTokenServerResponse {
-  access_token?: string;
-  expires_in?: string;
-  refresh_token?: string;
+  access_token: string;
+  expires_in: string;
+  refresh_token: string;
 }
 
 export interface RequestStsTokenResponse {
-  accessToken?: string;
-  expiresIn?: string;
-  refreshToken?: string;
+  accessToken: string;
+  expiresIn: string;
+  refreshToken: string;
 }
 
 export async function requestStsToken(

diff --git a/packages-exp/auth-exp/src/core/strategies/action_code_settings.ts b/packages-exp/auth-exp/src/core/strategies/action_code_settings.ts
@@ -27,7 +27,7 @@ export function _setActionCodeSettingsOnRequest(
   actionCodeSettings: ActionCodeSettings
 ): void {
   _assert(
-    actionCodeSettings.url.length > 0,
+    actionCodeSettings.url?.length > 0,
     auth,
     AuthErrorCode.INVALID_CONTINUE_URI
   );

diff --git a/packages-exp/auth-exp/src/core/user/id_token_result.ts b/packages-exp/auth-exp/src/core/user/id_token_result.ts
@@ -118,3 +118,16 @@ export function _parseToken(token: string): externs.ParsedToken | null {
     return null;
   }
 }
+
+/**
+ * Extract expiresIn TTL from a token by subtracting the expiration from the issuance.
+ *
+ * @internal
+ */
+export function _tokenExpiresIn(token: string): number {
+  const parsedToken = _parseToken(token);
+  _assert(parsedToken, AuthErrorCode.INTERNAL_ERROR);
+  _assert(typeof parsedToken.exp !== 'undefined', AuthErrorCode.INTERNAL_ERROR);
+  _assert(typeof parsedToken.iat !== 'undefined', AuthErrorCode.INTERNAL_ERROR);
+  return Number(parsedToken.exp) - Number(parsedToken.iat);
+}
diff --git a/packages-exp/auth-exp/src/core/user/token_manager.test.ts b/packages-exp/auth-exp/src/core/user/token_manager.test.ts
@@ -26,6 +26,8 @@ import * as fetch from '../../../test/helpers/mock_fetch';
 import { Endpoint } from '../../api/authentication/token';
 import { IdTokenResponse } from '../../model/id_token';
 import { StsTokenManager, Buffer } from './token_manager';
+import { FinalizeMfaResponse } from '../../api/authentication/mfa';
+import { makeJWT } from '../../../test/helpers/jwt';
 
 use(chaiAsPromised);
 
@@ -74,6 +76,18 @@ describe('core/user/token_manager', () => {
       expect(stsTokenManager.accessToken).to.eq('id-token');
       expect(stsTokenManager.refreshToken).to.eq('refresh-token');
     });
+
+    it('falls back to exp and iat when expiresIn is ommited (ie: MFA)', () => {
+      const idToken = makeJWT({ 'exp': '180', 'iat': '120' });
+      stsTokenManager.updateFromServerResponse({
+        idToken,
+        refreshToken: 'refresh-token'
+      } as FinalizeMfaResponse);
+
+      expect(stsTokenManager.expirationTime).to.eq(now + 60_000);
+      expect(stsTokenManager.accessToken).to.eq(idToken);
+      expect(stsTokenManager.refreshToken).to.eq('refresh-token');
+    });
   });
 
   describe('#clearRefreshToken', () => {

diff --git a/packages-exp/auth-exp/src/core/user/token_manager.ts b/packages-exp/auth-exp/src/core/user/token_manager.ts
@@ -22,6 +22,7 @@ import { IdTokenResponse } from '../../model/id_token';
 import { AuthErrorCode } from '../errors';
 import { PersistedBlob } from '../persistence';
 import { _assert, debugFail } from '../util/assert';
+import { _tokenExpiresIn } from './id_token_result';
 
 /**
  * The number of milliseconds before the official expiration time of a token
@@ -48,10 +49,23 @@ export class StsTokenManager {
   updateFromServerResponse(
     response: IdTokenResponse | FinalizeMfaResponse
   ): void {
+    _assert(response.idToken, AuthErrorCode.INTERNAL_ERROR);
+    _assert(
+      typeof response.idToken !== 'undefined',
+      AuthErrorCode.INTERNAL_ERROR
+    );
+    _assert(
+      typeof response.refreshToken !== 'undefined',
+      AuthErrorCode.INTERNAL_ERROR
+    );
+    const expiresIn =
+      'expiresIn' in response && typeof response.expiresIn !== 'undefined'
+        ? Number(response.expiresIn)
+        : _tokenExpiresIn(response.idToken);
     this.updateTokensAndExpiration(
       response.idToken,
       response.refreshToken,
-      'expiresIn' in response ? response.expiresIn : undefined
+      expiresIn
     );
   }
 
@@ -83,19 +97,21 @@ export class StsTokenManager {
       auth,
       oldToken
     );
-    this.updateTokensAndExpiration(accessToken, refreshToken, expiresIn);
+    this.updateTokensAndExpiration(
+      accessToken,
+      refreshToken,
+      Number(expiresIn)
+    );
   }
 
   private updateTokensAndExpiration(
-    accessToken?: string,
-    refreshToken?: string,
-    expiresInSec?: string
+    accessToken: string,
+    refreshToken: string,
+    expiresInSec: number
   ): void {
     this.refreshToken = refreshToken || null;
     this.accessToken = accessToken || null;
-    if (expiresInSec) {
-      this.expirationTime = Date.now() + Number(expiresInSec) * 1000;
-    }
+    this.expirationTime = Date.now() + expiresInSec * 1000;
   }
 
   static fromJSON(appName: string, object: PersistedBlob): StsTokenManager {

diff --git a/packages-exp/auth-exp/src/mfa/mfa_resolver.test.ts b/packages-exp/auth-exp/src/mfa/mfa_resolver.test.ts
@@ -17,6 +17,7 @@
 
 import { expect, use } from 'chai';
 import * as chaiAsPromised from 'chai-as-promised';
+import * as sinon from 'sinon';
 
 import { OperationType, ProviderId } from '@firebase/auth-types-exp';
 import { FirebaseError } from '@firebase/util';
@@ -36,16 +37,20 @@ import { PhoneMultiFactorAssertion } from '../platform_browser/mfa/assertions/ph
 import { MultiFactorError } from './mfa_error';
 import { getMultiFactorResolver, MultiFactorResolver } from './mfa_resolver';
 import { _createError } from '../core/util/assert';
+import { makeJWT } from '../../test/helpers/jwt';
 
 use(chaiAsPromised);
 
 describe('core/mfa/mfa_resolver/MultiFactorResolver', () => {
+  const finalIdToken = makeJWT({ 'exp': '3600', 'iat': '1200' });
   let auth: TestAuth;
   let underlyingError: FirebaseError;
   let error: MultiFactorError;
   let primaryFactorCredential: AuthCredential;
+  let clock: sinon.SinonFakeTimers;
 
   beforeEach(async () => {
+    clock = sinon.useFakeTimers();
     auth = await testAuth();
     auth.tenantId = 'tenant-id';
     primaryFactorCredential = EmailAuthProvider.credential(
@@ -55,19 +60,23 @@ describe('core/mfa/mfa_resolver/MultiFactorResolver', () => {
     underlyingError = _createError(auth, AuthErrorCode.MFA_REQUIRED, {
       serverResponse: {
         localId: 'local-id',
-        expiresIn: '3600',
         mfaPendingCredential: 'mfa-pending-credential',
         mfaInfo: [
           {
             mfaEnrollmentId: 'mfa-enrollment-id',
             enrolledAt: Date.now(),
-            phoneInfo: 'phone-info'
+            phoneInfo: '+*******1234',
+            displayName: ''
           }
         ]
       }
     });
   });
 
+  afterEach(() => {
+    sinon.restore();
+  });
+
   describe('MultiFactorResolver', () => {
     let assertion: MultiFactorAssertion;
     let secondFactorCredential: PhoneAuthCredential;
@@ -110,7 +119,7 @@ describe('core/mfa/mfa_resolver/MultiFactorResolver', () => {
 
       beforeEach(() => {
         mock = mockEndpoint(Endpoint.FINALIZE_PHONE_MFA_SIGN_IN, {
-          idToken: 'final-id-token',
+          idToken: finalIdToken,
           refreshToken: 'final-refresh-token'
         });
 
@@ -135,13 +144,13 @@ describe('core/mfa/mfa_resolver/MultiFactorResolver', () => {
             assertion
           )) as UserCredential;
           expect(userCredential.user.uid).to.eq('local-id');
-          expect(await userCredential.user.getIdToken()).to.eq(
-            'final-id-token'
+          expect(await userCredential.user.getIdToken()).to.eq(finalIdToken);
+          expect(userCredential.user.stsTokenManager.expirationTime).to.eq(
+            clock.now + 2400 * 1000
           );
           expect(userCredential._tokenResponse).to.eql({
             localId: 'local-id',
-            expiresIn: '3600',
-            idToken: 'final-id-token',
+            idToken: finalIdToken,
             refreshToken: 'final-refresh-token'
           });
           expect(mock.calls[0].request).to.eql({
@@ -175,13 +184,13 @@ describe('core/mfa/mfa_resolver/MultiFactorResolver', () => {
             assertion
           )) as UserCredential;
           expect(userCredential.user).to.eq(user);
-          expect(await userCredential.user.getIdToken()).to.eq(
-            'final-id-token'
+          expect(await userCredential.user.getIdToken()).to.eq(finalIdToken);
+          expect(userCredential.user.stsTokenManager.expirationTime).to.eq(
+            clock.now + 2400 * 1000
           );
           expect(userCredential._tokenResponse).to.eql({
             localId: 'local-id',
-            expiresIn: '3600',
-            idToken: 'final-id-token',
+            idToken: finalIdToken,
             refreshToken: 'final-refresh-token'
           });
           expect(mock.calls[0].request).to.eql({

diff --git a/packages-exp/auth-exp/src/mfa/mfa_user.test.ts b/packages-exp/auth-exp/src/mfa/mfa_user.test.ts
@@ -17,6 +17,7 @@
 
 import { expect, use } from 'chai';
 import * as chaiAsPromised from 'chai-as-promised';
+import * as sinon from 'sinon';
 
 import { ProviderId } from '@firebase/auth-types-exp';
 
@@ -33,6 +34,7 @@ import { MultiFactorSession, MultiFactorSessionType } from './mfa_session';
 import { multiFactor, MultiFactorUser } from './mfa_user';
 import { MultiFactorAssertion } from './mfa_assertion';
 import { Auth } from '../model/auth';
+import { makeJWT } from '../../test/helpers/jwt';
 
 use(chaiAsPromised);
 
@@ -57,16 +59,22 @@ class MockMultiFactorAssertion extends MultiFactorAssertion {
 }
 
 describe('core/mfa/mfa_user/MultiFactorUser', () => {
+  const idToken = makeJWT({ 'exp': '3600', 'iat': '1200' });
   let auth: TestAuth;
   let mfaUser: MultiFactorUser;
+  let clock: sinon.SinonFakeTimers;
 
   beforeEach(async () => {
     auth = await testAuth();
     mockFetch.setUp();
+    clock = sinon.useFakeTimers();
     mfaUser = MultiFactorUser._fromUser(testUser(auth, 'uid', undefined, true));
   });
 
-  afterEach(mockFetch.tearDown);
+  afterEach(() => {
+    mockFetch.tearDown();
+    sinon.restore();
+  });
 
   describe('getSession', () => {
     it('should return the id token', async () => {
@@ -99,7 +107,7 @@ describe('core/mfa/mfa_user/MultiFactorUser', () => {
     };
 
     const serverResponse: FinalizeMfaResponse = {
-      idToken: 'final-id-token',
+      idToken,
       refreshToken: 'refresh-token'
     };
 
@@ -114,7 +122,10 @@ describe('core/mfa/mfa_user/MultiFactorUser', () => {
     it('should update the tokens', async () => {
       await mfaUser.enroll(assertion);
 
-      expect(await mfaUser.user.getIdToken()).to.eq('final-id-token');
+      expect(await mfaUser.user.getIdToken()).to.eq(idToken);
+      expect(mfaUser.user.stsTokenManager.expirationTime).to.eq(
+        clock.now + 2400 * 1000
+      );
     });
 
     it('should update the enrolled Factors', async () => {
@@ -131,7 +142,7 @@ describe('core/mfa/mfa_user/MultiFactorUser', () => {
     let withdrawMfaEnrollmentMock: mockFetch.Route;
 
     const serverResponse: FinalizeMfaResponse = {
-      idToken: 'final-id-token',
+      idToken,
       refreshToken: 'refresh-token'
     };
 
@@ -199,7 +210,10 @@ describe('core/mfa/mfa_user/MultiFactorUser', () => {
     it('should update the tokens', async () => {
       await mfaUser.unenroll(mfaInfo);
 
-      expect(await mfaUser.user.getIdToken()).to.eq('final-id-token');
+      expect(await mfaUser.user.getIdToken()).to.eq(idToken);
+      expect(mfaUser.user.stsTokenManager.expirationTime).to.eq(
+        clock.now + 2400 * 1000
+      );
     });
 
     context('token revoked by backend', () => {