Fix tests & write some new ones

Author: sam-gc

packages-exp/auth-exp/src/api/account_management/profile.test.ts

@@ -33,7 +33,8 @@ describe('api/account_management/updateProfile', () => {
   const request = {
     idToken: 'my-token',
     email: '[email protected]',
-    password: 'my-password'
+    password: 'my-password',
+    returnSecureToken: true,
   };
 
   let auth: TestAuth;
@@ -48,7 +49,7 @@ describe('api/account_management/updateProfile', () => {
   it('should POST to the correct endpoint', async () => {
     const mock = mockEndpoint(Endpoint.SET_ACCOUNT_INFO, {
       displayName: 'my-name',
-      email: '[email protected]'
+      email: '[email protected]',
     });
 
     const response = await updateProfile(auth, request);

packages-exp/auth-exp/src/api/account_management/profile.ts

@@ -23,6 +23,7 @@ export interface UpdateProfileRequest {
   idToken: string;
   displayName?: string | null;
   photoUrl?: string | null;
+  returnSecureToken: boolean;
 }
 
 export interface UpdateProfileResponse extends IdTokenResponse {

packages-exp/auth-exp/src/core/auth/auth_impl.ts

@@ -168,6 +168,7 @@ export class AuthImpl implements Auth, _FirebaseService {
       // If the stored user (i.e. the old "currentUser") has a redirectId that
       // matches the redirect user, then we want to initially sign in with the
       // new user object from result.
+      // TODO(samgho): More thoroughly test all of this
       if (
         (!redirectUserEventId || redirectUserEventId === storedUserEventId) &&
         result?.user

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

@@ -42,7 +42,8 @@ import { AUTH_ERROR_FACTORY, AuthErrorCode } from '../errors';
 import {
   linkWithCredential,
   reauthenticateWithCredential,
-  signInWithCredential
+  signInWithCredential,
+  _signInWithCredential
 } from './credential';
 
 use(chaiAsPromised);
@@ -111,6 +112,15 @@ describe('core/strategies/credential', () => {
       expect(auth.currentUser).to.eq(user);
     });
 
+    it('does not update the current user if bypass is true', async () => {
+      stub(authCredential, '_getIdTokenResponse').returns(
+        Promise.resolve(idTokenResponse)
+      );
+      const { user } = await _signInWithCredential(auth, authCredential, true);
+      expect(auth.currentUser).to.be.null;
+      expect(user).not.to.be.null;
+    });
+
     it('should handle MFA', async () => {
       const serverResponse: IdTokenMfaResponse = {
         localId: 'uid',

packages-exp/auth-exp/src/core/user/account_info.test.ts

@@ -73,7 +73,8 @@ describe('core/user/profile', () => {
       expect(ep.calls[0].request).to.eql({
         idToken: 'access-token',
         displayName: 'displayname',
-        photoUrl: 'photo'
+        photoUrl: 'photo',
+        returnSecureToken: true,
       });
     });
 
@@ -118,7 +119,8 @@ describe('core/user/profile', () => {
       await updateEmail(user, '[email protected]');
       expect(set.calls[0].request).to.eql({
         idToken: 'access-token',
-        email: '[email protected]'
+        email: '[email protected]',
+        returnSecureToken: true,
       });
 
       expect(user.uid).to.eq('new-uid-to-prove-refresh-got-called');
@@ -135,7 +137,8 @@ describe('core/user/profile', () => {
       await updatePassword(user, 'pass');
       expect(set.calls[0].request).to.eql({
         idToken: 'access-token',
-        password: 'pass'
+        password: 'pass',
+        returnSecureToken: true,
       });
 
       expect(user.uid).to.eq('new-uid-to-prove-refresh-got-called');

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

@@ -49,7 +49,7 @@ export async function updateProfile(
 
   const userInternal = user as User;
   const idToken = await user.getIdToken();
-  const profileRequest = { idToken, displayName, photoUrl };
+  const profileRequest = { idToken, displayName, photoUrl, returnSecureToken: true };
   const response = await _logoutIfInvalidated(
     userInternal,
     apiUpdateProfile(userInternal.auth, profileRequest)
@@ -121,7 +121,7 @@ async function updateEmailOrPassword(
 ): Promise<void> {
   const { auth } = user;
   const idToken = await user.getIdToken();
-  const request: UpdateEmailPasswordRequest = { idToken };
+  const request: UpdateEmailPasswordRequest = { idToken, returnSecureToken: true };
 
   if (email) {
     request.email = email;

packages-exp/auth-exp/src/core/user/invalidation.test.ts

@@ -63,6 +63,12 @@ describe('src/core/user/invalidation', () => {
     expect(auth.currentUser).to.be.null;
   });
 
+  it('does not log out if bypass auth state is true', async () => {
+    const error = makeError(AuthErrorCode.USER_DISABLED);
+    try { await _logoutIfInvalidated(user, Promise.reject(error), true); } catch {}
+    expect(auth.currentUser).to.eq(user);
+  });
+
   it('logs out the user if the error is token_expired', async () => {
     const error = makeError(AuthErrorCode.TOKEN_EXPIRED);
     await expect(

packages-exp/auth-exp/src/platform_browser/auth.test.ts

@@ -43,6 +43,9 @@ import { _getInstance } from '../core/util/instantiator';
 import { _getClientVersion, ClientPlatform } from '../core/util/version';
 import { Auth } from '../model/auth';
 import { browserPopupRedirectResolver } from './popup_redirect';
+import { PopupRedirectResolver } from '../model/popup_redirect';
+import { UserCredentialImpl } from '../core/user/user_credential_impl';
+import { User } from '../model/user';
 
 use(sinonChai);
 use(chaiAsPromised);
@@ -102,13 +105,15 @@ describe('core/auth/initializeAuth', () => {
     let createManagerStub: sinon.SinonSpy;
     let reloadStub: sinon.SinonStub;
     let oldAuth: Auth;
+    let completeRedirectFnStub: sinon.SinonStub;
 
     beforeEach(async () => {
       oldAuth = await testAuth();
       createManagerStub = sinon.spy(PersistenceUserManager, 'create');
       reloadStub = sinon
         .stub(reload, '_reloadWithoutSaving')
         .returns(Promise.resolve());
+      completeRedirectFnStub = sinon.stub(_getInstance<PopupRedirectResolver>(browserPopupRedirectResolver), '_completeRedirectFn').returns(Promise.resolve(null));
     });
 
     async function initAndWait(
@@ -173,6 +178,23 @@ describe('core/auth/initializeAuth', () => {
       expect(reload._reloadWithoutSaving).not.to.have.been.called;
     });
 
+    it('signs in the redirect user if found', async () => {
+      let user: User|null = null;
+      completeRedirectFnStub.callsFake((auth: Auth) => {
+        user = testUser(auth, 'uid', '[email protected]');
+        return Promise.resolve(new UserCredentialImpl({
+        operationType: externs.OperationType.SIGN_IN,
+        user,
+        providerId: null
+        }));
+
+      });
+
+      const auth = await initAndWait([inMemoryPersistence], browserPopupRedirectResolver);
+      expect(user).not.to.be.null;
+      expect(auth.currentUser).to.eq(user);
+    });
+
     it('reloads non-redirect users', async () => {
       sinon
         .stub(_getInstance<Persistence>(inMemoryPersistence), '_get')

packages-exp/auth-exp/src/platform_browser/popup_redirect.ts

@@ -40,7 +40,7 @@ import { _setWindowLocation } from './auth_window';
 import { _openIframe } from './iframe/iframe';
 import { browserSessionPersistence } from './persistence/session_storage';
 import { _open, AuthPopup } from './util/popup';
-import { getRedirectResult } from './strategies/redirect';
+import {_getRedirectResult } from './strategies/redirect';
 
 /**
  * URL for Authentication widget which will initiate the OAuth handshake
@@ -200,7 +200,7 @@ class BrowserPopupRedirectResolver implements PopupRedirectResolver {
     return this.originValidationPromises[key];
   }
 
-  _completeRedirectFn = getRedirectResult;
+  _completeRedirectFn = _getRedirectResult;
 }
 
 /**

packages-exp/auth-exp/src/platform_browser/strategies/abstract_popup_redirect_operation.test.ts

@@ -185,7 +185,8 @@ describe('src/core/strategies/abstract_popup_redirect_operation', () => {
           sessionId: BASE_AUTH_EVENT.sessionId!,
           tenantId: BASE_AUTH_EVENT.tenantId || undefined,
           postBody: BASE_AUTH_EVENT.postBody || undefined,
-          user: undefined
+          user: undefined,
+          bypassAuthState: false,
         };
       }
 
@@ -236,6 +237,25 @@ describe('src/core/strategies/abstract_popup_redirect_operation', () => {
         await operation.execute();
         expect(idp._reauth).to.have.been.calledWith(expectedIdpTaskParams());
       });
+
+      it('includes the bypassAuthState parameter', async () => {
+        operation = new WrapperOperation(
+          auth,
+          AuthEventType.REAUTH_VIA_REDIRECT,
+          resolver,
+          undefined,
+          /** bypassAuthState */ true
+        );
+
+        const type = AuthEventType.REAUTH_VIA_REDIRECT;
+        updateFilter(type);
+        finishPromise(authEvent({ type }));
+        await operation.execute();
+        expect(idp._reauth).to.have.been.calledWith({
+          ...expectedIdpTaskParams(),
+          bypassAuthState: true,
+        });
+      });
     });
   });
 });

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

@@ -424,7 +424,8 @@ describe('core/strategies/phone', () => {
         users: [{ uid: 'uid' }]
       });
       signInMock = mockEndpoint(Endpoint.SIGN_IN_WITH_PHONE_NUMBER, {
-        idToken: 'new-access-token'
+        idToken: 'new-access-token',
+        refreshToken: 'refresh-token',
       });
       credential = PhoneAuthCredential._fromVerification(
         'session-info',

packages-exp/auth-exp/src/platform_browser/strategies/redirect.test.ts

@@ -51,7 +51,7 @@ import {
   getRedirectResult,
   linkWithRedirect,
   reauthenticateWithRedirect,
-  signInWithRedirect
+  signInWithRedirect, _getRedirectResult
 } from './redirect';
 import { FirebaseError } from '@firebase/util';
 
@@ -426,5 +426,29 @@ describe('src/core/strategies/redirect', () => {
       expect(auth.persistenceLayer.lastObjectSet?._redirectEventId).to.be
         .undefined;
     });
+
+    it('does not mutate authstate if bypassAuthState is true', async () => {
+      await reInitAuthWithRedirectUser(MATCHING_EVENT_ID);
+      const redirectPersistence: Persistence = _getInstance(
+        RedirectPersistence
+      );
+      sinon.spy(redirectPersistence, '_remove');
+
+      const cred = new UserCredentialImpl({
+        user: auth._currentUser!,
+        providerId: externs.ProviderId.GOOGLE,
+        operationType: externs.OperationType.LINK
+      });
+      idpStubs._link.returns(Promise.resolve(cred));
+      const promise = _getRedirectResult(auth, resolver, true);
+      iframeEvent({
+        type: AuthEventType.LINK_VIA_REDIRECT
+      });
+      expect(await promise).to.eq(cred);
+      expect(redirectPersistence._remove).not.to.have.been.called;
+      expect(auth._currentUser?._redirectEventId).not.to.be.undefined;
+      expect(auth.persistenceLayer.lastObjectSet?._redirectEventId).not.to.be
+        .undefined;
+    });
   });
 });

packages-exp/auth-exp/src/platform_browser/strategies/redirect.ts

@@ -228,9 +228,16 @@ export async function linkWithRedirect(
  * @public
  */
 export async function getRedirectResult(
+  auth: externs.Auth,
+  resolver?: externs.PopupRedirectResolver,
+): Promise<externs.UserCredential | null> {
+  return _getRedirectResult(auth, resolver, false);
+}
+
+export async function _getRedirectResult(
   auth: externs.Auth,
   resolverExtern?: externs.PopupRedirectResolver,
-  bypassAuthState = false
+  bypassAuthState = false,
 ): Promise<externs.UserCredential | null> {
   const authInternal = _castAuth(auth);
   const resolver = _withDefaultResolver(authInternal, resolverExtern);

packages-exp/auth-exp/test/helpers/mock_popup_redirect_resolver.ts

@@ -49,5 +49,9 @@ export function makeMockPopupRedirectResolver(
     }
 
     _redirectPersistence?: Persistence;
+
+    async _completeRedirectFn(): Promise<void> {
+
+    }
   };
 }

packages-exp/auth-exp/test/integration/flows/phone.test.ts

@@ -107,7 +107,8 @@ describe('Integration test: phone auth', () => {
 
     await unlink(user, ProviderId.PHONE);
     expect(auth.currentUser!.uid).to.eq(anonId);
-    expect(auth.currentUser!.isAnonymous).to.be.true;
+    // Is anonymous stays false even after unlinking
+    expect(auth.currentUser!.isAnonymous).to.be.false;
     expect(auth.currentUser!.phoneNumber).to.be.null;
   });