Auth middleware (beforeAuthStateChanged) (#6068)

Author: hsubox76

common/api-review/auth.api.md

@@ -81,6 +81,7 @@ export function applyActionCode(auth: Auth, oobCode: string): Promise<void>;
 // @public
 export interface Auth {
     readonly app: FirebaseApp;
+    beforeAuthStateChanged(callback: (user: User | null) => void | Promise<void>): Unsubscribe;
     readonly config: Config;
     readonly currentUser: User | null;
     readonly emulatorConfig: EmulatorConfig | null;

packages/auth/src/core/auth/auth_impl.test.ts

@@ -34,6 +34,7 @@ import * as reload from '../user/reload';
 import { AuthImpl, DefaultConfig } from './auth_impl';
 import { _initializeAuthInstance } from './initialize';
 import { ClientPlatform } from '../util/version';
+import { AuthErrorCode } from '../errors';
 
 use(sinonChai);
 use(chaiAsPromised);
@@ -138,6 +139,11 @@ describe('core/auth/auth_impl', () => {
       expect(persistenceStub._remove).to.have.been.called;
       expect(auth.currentUser).to.be.null;
     });
+    it('is blocked if a beforeAuthStateChanged callback throws', async () => {
+      await auth._updateCurrentUser(testUser(auth, 'test'));
+      auth.beforeAuthStateChanged(sinon.stub().throws());
+      await expect(auth.signOut()).to.be.rejectedWith(AuthErrorCode.LOGIN_BLOCKED);
+    });
   });
 
   describe('#useDeviceLanguage', () => {
@@ -208,20 +214,24 @@ describe('core/auth/auth_impl', () => {
       let user: UserInternal;
       let authStateCallback: sinon.SinonSpy;
       let idTokenCallback: sinon.SinonSpy;
+      let beforeAuthCallback: sinon.SinonSpy;
 
       beforeEach(() => {
         user = testUser(auth, 'uid');
         authStateCallback = sinon.spy();
         idTokenCallback = sinon.spy();
+        beforeAuthCallback = sinon.spy();
       });
 
       context('initially currentUser is null', () => {
         beforeEach(async () => {
           auth.onAuthStateChanged(authStateCallback);
           auth.onIdTokenChanged(idTokenCallback);
+          auth.beforeAuthStateChanged(beforeAuthCallback);
           await auth._updateCurrentUser(null);
           authStateCallback.resetHistory();
           idTokenCallback.resetHistory();
+          beforeAuthCallback.resetHistory();
         });
 
         it('onAuthStateChange triggers on log in', async () => {
@@ -233,15 +243,22 @@ describe('core/auth/auth_impl', () => {
           await auth._updateCurrentUser(user);
           expect(idTokenCallback).to.have.been.calledWith(user);
         });
+
+        it('beforeAuthStateChanged triggers on log in', async () => {
+          await auth._updateCurrentUser(user);
+          expect(beforeAuthCallback).to.have.been.calledWith(user);
+        });
       });
 
       context('initially currentUser is user', () => {
         beforeEach(async () => {
           auth.onAuthStateChanged(authStateCallback);
           auth.onIdTokenChanged(idTokenCallback);
+          auth.beforeAuthStateChanged(beforeAuthCallback);
           await auth._updateCurrentUser(user);
           authStateCallback.resetHistory();
           idTokenCallback.resetHistory();
+          beforeAuthCallback.resetHistory();
         });
 
         it('onAuthStateChange triggers on log out', async () => {
@@ -254,6 +271,11 @@ describe('core/auth/auth_impl', () => {
           expect(idTokenCallback).to.have.been.calledWith(null);
         });
 
+        it('beforeAuthStateChanged triggers on log out', async () => {
+          await auth._updateCurrentUser(null);
+          expect(beforeAuthCallback).to.have.been.calledWith(null);
+        });
+
         it('onAuthStateChange does not trigger for user props change', async () => {
           user.photoURL = 'blah';
           await auth._updateCurrentUser(user);
@@ -300,21 +322,61 @@ describe('core/auth/auth_impl', () => {
         expect(cb1).to.have.been.calledWith(user);
         expect(cb2).to.have.been.calledWith(user);
       });
+
+      it('beforeAuthStateChange works for multiple listeners', async () => {
+        const cb1 = sinon.spy();
+        const cb2 = sinon.spy();
+        auth.beforeAuthStateChanged(cb1);
+        auth.beforeAuthStateChanged(cb2);
+        await auth._updateCurrentUser(null);
+        cb1.resetHistory();
+        cb2.resetHistory();
+
+        await auth._updateCurrentUser(user);
+        expect(cb1).to.have.been.calledWith(user);
+        expect(cb2).to.have.been.calledWith(user);
+      });
+
+      it('_updateCurrentUser throws if a beforeAuthStateChange callback throws', async () => {
+        await auth._updateCurrentUser(null);
+        const cb1 = sinon.stub().throws();
+        const cb2 = sinon.spy();
+        auth.beforeAuthStateChanged(cb1);
+        auth.beforeAuthStateChanged(cb2);
+
+        await expect(auth._updateCurrentUser(user)).to.be.rejectedWith(AuthErrorCode.LOGIN_BLOCKED);
+        expect(cb2).not.to.be.called;
+      });
+
+      it('_updateCurrentUser throws if a beforeAuthStateChange callback rejects', async () => {
+        await auth._updateCurrentUser(null);
+        const cb1 = sinon.stub().rejects();
+        const cb2 = sinon.spy();
+        auth.beforeAuthStateChanged(cb1);
+        auth.beforeAuthStateChanged(cb2);
+
+        await expect(auth._updateCurrentUser(user)).to.be.rejectedWith(AuthErrorCode.LOGIN_BLOCKED);
+        expect(cb2).not.to.be.called;
+      });
     });
   });
 
   describe('#_onStorageEvent', () => {
     let authStateCallback: sinon.SinonSpy;
     let idTokenCallback: sinon.SinonSpy;
+    let beforeStateCallback: sinon.SinonSpy;
 
     beforeEach(async () => {
       authStateCallback = sinon.spy();
       idTokenCallback = sinon.spy();
+      beforeStateCallback = sinon.spy();
       auth.onAuthStateChanged(authStateCallback);
       auth.onIdTokenChanged(idTokenCallback);
+      auth.beforeAuthStateChanged(beforeStateCallback);
       await auth._updateCurrentUser(null); // force event handlers to clear out
       authStateCallback.resetHistory();
       idTokenCallback.resetHistory();
+      beforeStateCallback.resetHistory();
     });
 
     context('previously logged out', () => {
@@ -324,6 +386,7 @@ describe('core/auth/auth_impl', () => {
 
           expect(authStateCallback).not.to.have.been.called;
           expect(idTokenCallback).not.to.have.been.called;
+          expect(beforeStateCallback).not.to.have.been.called;
         });
       });
 
@@ -341,6 +404,8 @@ describe('core/auth/auth_impl', () => {
           expect(auth.currentUser?.toJSON()).to.eql(user.toJSON());
           expect(authStateCallback).to.have.been.called;
           expect(idTokenCallback).to.have.been.called;
+          // This should never be called on a storage event.
+          expect(beforeStateCallback).not.to.have.been.called;
         });
       });
     });
@@ -353,6 +418,7 @@ describe('core/auth/auth_impl', () => {
         await auth._updateCurrentUser(user);
         authStateCallback.resetHistory();
         idTokenCallback.resetHistory();
+        beforeStateCallback.resetHistory();
       });
 
       context('now logged out', () => {
@@ -366,6 +432,8 @@ describe('core/auth/auth_impl', () => {
           expect(auth.currentUser).to.be.null;
           expect(authStateCallback).to.have.been.called;
           expect(idTokenCallback).to.have.been.called;
+          // This should never be called on a storage event.
+          expect(beforeStateCallback).not.to.have.been.called;
         });
       });
 
@@ -378,6 +446,7 @@ describe('core/auth/auth_impl', () => {
           expect(auth.currentUser?.toJSON()).to.eql(user.toJSON());
           expect(authStateCallback).not.to.have.been.called;
           expect(idTokenCallback).not.to.have.been.called;
+          expect(beforeStateCallback).not.to.have.been.called;
         });
 
         it('should update fields if they have changed', async () => {
@@ -391,6 +460,7 @@ describe('core/auth/auth_impl', () => {
           expect(auth.currentUser?.displayName).to.eq('other-name');
           expect(authStateCallback).not.to.have.been.called;
           expect(idTokenCallback).not.to.have.been.called;
+          expect(beforeStateCallback).not.to.have.been.called;
         });
 
         it('should update tokens if they have changed', async () => {
@@ -407,6 +477,8 @@ describe('core/auth/auth_impl', () => {
           ).to.eq('new-access-token');
           expect(authStateCallback).not.to.have.been.called;
           expect(idTokenCallback).to.have.been.called;
+          // This should never be called on a storage event.
+          expect(beforeStateCallback).not.to.have.been.called;
         });
       });
 
@@ -420,6 +492,8 @@ describe('core/auth/auth_impl', () => {
           expect(auth.currentUser?.toJSON()).to.eql(newUser.toJSON());
           expect(authStateCallback).to.have.been.called;
           expect(idTokenCallback).to.have.been.called;
+          // This should never be called on a storage event.
+          expect(beforeStateCallback).not.to.have.been.called;
         });
       });
     });
@@ -461,7 +535,7 @@ describe('core/auth/auth_impl', () => {
     });
   });
 
-  context ('#_getAdditionalHeaders', () => {
+  context('#_getAdditionalHeaders', () => {
     it('always adds the client version', async () => {
       expect(await auth._getAdditionalHeaders()).to.eql({
         'X-Client-Version': 'v',

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

@@ -79,6 +79,7 @@ export class AuthImpl implements AuthInternal, _FirebaseService {
   private redirectPersistenceManager?: PersistenceUserManager;
   private authStateSubscription = new Subscription<User>(this);
   private idTokenSubscription = new Subscription<User>(this);
+  private beforeStateQueue: Array<(user: User | null) => Promise<void>> = [];
   private redirectUser: UserInternal | null = null;
   private isProactiveRefreshEnabled = false;
 
@@ -183,7 +184,8 @@ export class AuthImpl implements AuthInternal, _FirebaseService {
     }
 
     // Update current Auth state. Either a new login or logout.
-    await this._updateCurrentUser(user);
+    // Skip blocking callbacks, they should not apply to a change in another tab.
+    await this._updateCurrentUser(user, /* skipBeforeStateCallbacks */ true);
   }
 
   private async initializeCurrentUser(
@@ -225,6 +227,14 @@ export class AuthImpl implements AuthInternal, _FirebaseService {
     _assert(this._popupRedirectResolver, this, AuthErrorCode.ARGUMENT_ERROR);
     await this.getOrInitRedirectPersistenceManager();
 
+    // At this point in the flow, this is a redirect user. Run blocking
+    // middleware callbacks before setting the user.
+    try {
+      await this._runBeforeStateCallbacks(storedUser);
+    } catch(e) {
+      return;
+    }
+
     // If the redirect user's event ID matches the current user's event ID,
     // DO NOT reload the current user, otherwise they'll be cleared from storage.
     // This is important for the reauthenticateWithRedirect() flow.
@@ -315,7 +325,7 @@ export class AuthImpl implements AuthInternal, _FirebaseService {
     return this._updateCurrentUser(user && user._clone(this));
   }
 
-  async _updateCurrentUser(user: User | null): Promise<void> {
+  async _updateCurrentUser(user: User | null, skipBeforeStateCallbacks: boolean = false): Promise<void> {
     if (this._deleted) {
       return;
     }
@@ -327,19 +337,41 @@ export class AuthImpl implements AuthInternal, _FirebaseService {
       );
     }
 
+    if (!skipBeforeStateCallbacks) {
+      await this._runBeforeStateCallbacks(user);
+    }
+
     return this.queue(async () => {
       await this.directlySetCurrentUser(user as UserInternal | null);
       this.notifyAuthListeners();
     });
   }
 
+  async _runBeforeStateCallbacks(user: User | null): Promise<void> {
+    if (this.currentUser === user) {
+      return;
+    }
+    try {
+      for (const beforeStateCallback of this.beforeStateQueue) {
+        await beforeStateCallback(user);
+      }
+    } catch (e) {
+      throw this._errorFactory.create(
+        AuthErrorCode.LOGIN_BLOCKED, { originalMessage: e.message });
+    }
+  }
+
   async signOut(): Promise<void> {
+    // Run first, to block _setRedirectUser() if any callbacks fail.
+    await this._runBeforeStateCallbacks(null);
     // Clear the redirect user when signOut is called
     if (this.redirectPersistenceManager || this._popupRedirectResolver) {
       await this._setRedirectUser(null);
     }
 
-    return this._updateCurrentUser(null);
+    // Prevent callbacks from being called again in _updateCurrentUser, as
+    // they were already called in the first line.
+    return this._updateCurrentUser(null, /* skipBeforeStateCallbacks */ true);
   }
 
   setPersistence(persistence: Persistence): Promise<void> {
@@ -373,6 +405,32 @@ export class AuthImpl implements AuthInternal, _FirebaseService {
     );
   }
 
+  beforeAuthStateChanged(
+    callback: (user: User | null) => void | Promise<void>
+  ): Unsubscribe {
+    // The callback could be sync or async. Wrap it into a
+    // function that is always async.
+    const wrappedCallback =
+      (user: User | null): Promise<void> => new Promise((resolve, reject) => {
+        try {
+          const result = callback(user);
+          // Either resolve with existing promise or wrap a non-promise
+          // return value into a promise.
+          resolve(result);
+        } catch (e) {
+          // Sync callback throws.
+          reject(e);
+        }
+      });
+    this.beforeStateQueue.push(wrappedCallback);
+    const index = this.beforeStateQueue.length - 1;
+    return () => {
+      // Unsubscribe. Replace with no-op. Do not remove from array, or it will disturb
+      // indexing of other elements.
+      this.beforeStateQueue[index] = () => Promise.resolve();
+    };
+  }
+
   onIdTokenChanged(
     nextOrObserver: NextOrObserver<User>,
     error?: ErrorFn,
@@ -431,7 +489,7 @@ export class AuthImpl implements AuthInternal, _FirebaseService {
     // Make sure we've cleared any pending persistence actions if we're not in
     // the initializer
     if (this._isInitialized) {
-      await this.queue(async () => {});
+      await this.queue(async () => { });
     }
 
     if (this._currentUser?._redirectEventId === id) {
@@ -502,7 +560,7 @@ export class AuthImpl implements AuthInternal, _FirebaseService {
     completed?: CompleteFn
   ): Unsubscribe {
     if (this._deleted) {
-      return () => {};
+      return () => { };
     }
 
     const cb =
@@ -618,7 +676,7 @@ class Subscription<T> {
     observer => (this.observer = observer)
   );
 
-  constructor(readonly auth: AuthInternal) {}
+  constructor(readonly auth: AuthInternal) { }
 
   get next(): NextFn<T | null> {
     _assert(this.observer, this.auth, AuthErrorCode.INTERNAL_ERROR);