Skip to content

Commit 1ac3c9d

Browse files
authored
Implement beforeAuthStateChanged() Auth middleware function (#6151)
1 parent 9c6808f commit 1ac3c9d

32 files changed

+909
-26
lines changed

.changeset/light-poets-eat.md

+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
'@firebase/auth': minor
3+
'@firebase/auth-compat': patch
4+
---
5+
6+
Add `beforeAuthStateChanged()` middleware function which allows the user to provide callbacks that are run before an auth state change
7+
sets a new user.

common/api-review/auth.api.md

+4
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ export function applyActionCode(auth: Auth, oobCode: string): Promise<void>;
8181
// @public
8282
export interface Auth {
8383
readonly app: FirebaseApp;
84+
beforeAuthStateChanged(callback: (user: User | null) => void | Promise<void>, onAbort?: () => void): Unsubscribe;
8485
readonly config: Config;
8586
readonly currentUser: User | null;
8687
readonly emulatorConfig: EmulatorConfig | null;
@@ -241,6 +242,9 @@ export interface AuthSettings {
241242
appVerificationDisabledForTesting: boolean;
242243
}
243244

245+
// @public
246+
export function beforeAuthStateChanged(auth: Auth, callback: (user: User | null) => void | Promise<void>, onAbort?: () => void): Unsubscribe;
247+
244248
// @public
245249
export const browserLocalPersistence: Persistence;
246250

packages/auth-compat/src/popup_redirect.test.ts

+1
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,7 @@ describe('popup_redirect/CompatPopupRedirectResolver', () => {
175175

176176
class FakeResolver implements exp.PopupRedirectResolverInternal {
177177
_completeRedirectFn = async (): Promise<null> => null;
178+
_overrideRedirectResult = (): void => {};
178179
_redirectPersistence = exp.inMemoryPersistence;
179180
_shouldInitProactively = true;
180181

packages/auth-compat/src/popup_redirect.ts

+1
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ export class CompatPopupRedirectResolver
3838
resolver: exp.PopupRedirectResolver,
3939
bypassAuthState: boolean
4040
) => Promise<exp.UserCredential | null> = exp._getRedirectResult;
41+
_overrideRedirectResult = exp._overrideRedirectResult;
4142

4243
async _initialize(auth: exp.AuthImpl): Promise<exp.EventManager> {
4344
await this.selectUnderlyingResolver();

packages/auth/internal/index.ts

+1
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ export { TaggedWithTokenResponse } from '../src/model/id_token';
4545
export { _fail, _assert } from '../src/core/util/assert';
4646
export { AuthPopup } from '../src/platform_browser/util/popup';
4747
export { _getRedirectResult } from '../src/platform_browser/strategies/redirect';
48+
export { _overrideRedirectResult } from '../src/core/strategies/redirect';
4849
export { cordovaPopupRedirectResolver } from '../src/platform_cordova/popup_redirect/popup_redirect';
4950
export { FetchProvider } from '../src/core/util/fetch_provider';
5051
export { SAMLAuthCredential } from '../src/core/credentials/saml';

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

+75-1
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import * as reload from '../user/reload';
3434
import { AuthImpl, DefaultConfig } from './auth_impl';
3535
import { _initializeAuthInstance } from './initialize';
3636
import { ClientPlatform } from '../util/version';
37+
import { AuthErrorCode } from '../errors';
3738

3839
use(sinonChai);
3940
use(chaiAsPromised);
@@ -138,6 +139,11 @@ describe('core/auth/auth_impl', () => {
138139
expect(persistenceStub._remove).to.have.been.called;
139140
expect(auth.currentUser).to.be.null;
140141
});
142+
it('is blocked if a beforeAuthStateChanged callback throws', async () => {
143+
await auth._updateCurrentUser(testUser(auth, 'test'));
144+
auth.beforeAuthStateChanged(sinon.stub().throws());
145+
await expect(auth.signOut()).to.be.rejectedWith(AuthErrorCode.LOGIN_BLOCKED);
146+
});
141147
});
142148

143149
describe('#useDeviceLanguage', () => {
@@ -208,20 +214,24 @@ describe('core/auth/auth_impl', () => {
208214
let user: UserInternal;
209215
let authStateCallback: sinon.SinonSpy;
210216
let idTokenCallback: sinon.SinonSpy;
217+
let beforeAuthCallback: sinon.SinonSpy;
211218

212219
beforeEach(() => {
213220
user = testUser(auth, 'uid');
214221
authStateCallback = sinon.spy();
215222
idTokenCallback = sinon.spy();
223+
beforeAuthCallback = sinon.spy();
216224
});
217225

218226
context('initially currentUser is null', () => {
219227
beforeEach(async () => {
220228
auth.onAuthStateChanged(authStateCallback);
221229
auth.onIdTokenChanged(idTokenCallback);
230+
auth.beforeAuthStateChanged(beforeAuthCallback);
222231
await auth._updateCurrentUser(null);
223232
authStateCallback.resetHistory();
224233
idTokenCallback.resetHistory();
234+
beforeAuthCallback.resetHistory();
225235
});
226236

227237
it('onAuthStateChange triggers on log in', async () => {
@@ -233,15 +243,22 @@ describe('core/auth/auth_impl', () => {
233243
await auth._updateCurrentUser(user);
234244
expect(idTokenCallback).to.have.been.calledWith(user);
235245
});
246+
247+
it('beforeAuthStateChanged triggers on log in', async () => {
248+
await auth._updateCurrentUser(user);
249+
expect(beforeAuthCallback).to.have.been.calledWith(user);
250+
});
236251
});
237252

238253
context('initially currentUser is user', () => {
239254
beforeEach(async () => {
240255
auth.onAuthStateChanged(authStateCallback);
241256
auth.onIdTokenChanged(idTokenCallback);
257+
auth.beforeAuthStateChanged(beforeAuthCallback);
242258
await auth._updateCurrentUser(user);
243259
authStateCallback.resetHistory();
244260
idTokenCallback.resetHistory();
261+
beforeAuthCallback.resetHistory();
245262
});
246263

247264
it('onAuthStateChange triggers on log out', async () => {
@@ -254,6 +271,11 @@ describe('core/auth/auth_impl', () => {
254271
expect(idTokenCallback).to.have.been.calledWith(null);
255272
});
256273

274+
it('beforeAuthStateChanged triggers on log out', async () => {
275+
await auth._updateCurrentUser(null);
276+
expect(beforeAuthCallback).to.have.been.calledWith(null);
277+
});
278+
257279
it('onAuthStateChange does not trigger for user props change', async () => {
258280
user.photoURL = 'blah';
259281
await auth._updateCurrentUser(user);
@@ -300,21 +322,61 @@ describe('core/auth/auth_impl', () => {
300322
expect(cb1).to.have.been.calledWith(user);
301323
expect(cb2).to.have.been.calledWith(user);
302324
});
325+
326+
it('beforeAuthStateChange works for multiple listeners', async () => {
327+
const cb1 = sinon.spy();
328+
const cb2 = sinon.spy();
329+
auth.beforeAuthStateChanged(cb1);
330+
auth.beforeAuthStateChanged(cb2);
331+
await auth._updateCurrentUser(null);
332+
cb1.resetHistory();
333+
cb2.resetHistory();
334+
335+
await auth._updateCurrentUser(user);
336+
expect(cb1).to.have.been.calledWith(user);
337+
expect(cb2).to.have.been.calledWith(user);
338+
});
339+
340+
it('_updateCurrentUser throws if a beforeAuthStateChange callback throws', async () => {
341+
await auth._updateCurrentUser(null);
342+
const cb1 = sinon.stub().throws();
343+
const cb2 = sinon.spy();
344+
auth.beforeAuthStateChanged(cb1);
345+
auth.beforeAuthStateChanged(cb2);
346+
347+
await expect(auth._updateCurrentUser(user)).to.be.rejectedWith(AuthErrorCode.LOGIN_BLOCKED);
348+
expect(cb2).not.to.be.called;
349+
});
350+
351+
it('_updateCurrentUser throws if a beforeAuthStateChange callback rejects', async () => {
352+
await auth._updateCurrentUser(null);
353+
const cb1 = sinon.stub().rejects();
354+
const cb2 = sinon.spy();
355+
auth.beforeAuthStateChanged(cb1);
356+
auth.beforeAuthStateChanged(cb2);
357+
358+
await expect(auth._updateCurrentUser(user)).to.be.rejectedWith(AuthErrorCode.LOGIN_BLOCKED);
359+
expect(cb2).not.to.be.called;
360+
});
303361
});
304362
});
305363

306364
describe('#_onStorageEvent', () => {
307365
let authStateCallback: sinon.SinonSpy;
308366
let idTokenCallback: sinon.SinonSpy;
367+
let beforeStateCallback: sinon.SinonSpy;
309368

310369
beforeEach(async () => {
311370
authStateCallback = sinon.spy();
312371
idTokenCallback = sinon.spy();
372+
beforeStateCallback = sinon.spy();
313373
auth.onAuthStateChanged(authStateCallback);
314374
auth.onIdTokenChanged(idTokenCallback);
375+
auth.beforeAuthStateChanged(beforeStateCallback);
315376
await auth._updateCurrentUser(null); // force event handlers to clear out
316377
authStateCallback.resetHistory();
317378
idTokenCallback.resetHistory();
379+
beforeStateCallback.resetHistory();
318380
});
319381

320382
context('previously logged out', () => {
@@ -324,6 +386,7 @@ describe('core/auth/auth_impl', () => {
324386

325387
expect(authStateCallback).not.to.have.been.called;
326388
expect(idTokenCallback).not.to.have.been.called;
389+
expect(beforeStateCallback).not.to.have.been.called;
327390
});
328391
});
329392

@@ -341,6 +404,8 @@ describe('core/auth/auth_impl', () => {
341404
expect(auth.currentUser?.toJSON()).to.eql(user.toJSON());
342405
expect(authStateCallback).to.have.been.called;
343406
expect(idTokenCallback).to.have.been.called;
407+
// This should never be called on a storage event.
408+
expect(beforeStateCallback).not.to.have.been.called;
344409
});
345410
});
346411
});
@@ -353,6 +418,7 @@ describe('core/auth/auth_impl', () => {
353418
await auth._updateCurrentUser(user);
354419
authStateCallback.resetHistory();
355420
idTokenCallback.resetHistory();
421+
beforeStateCallback.resetHistory();
356422
});
357423

358424
context('now logged out', () => {
@@ -366,6 +432,8 @@ describe('core/auth/auth_impl', () => {
366432
expect(auth.currentUser).to.be.null;
367433
expect(authStateCallback).to.have.been.called;
368434
expect(idTokenCallback).to.have.been.called;
435+
// This should never be called on a storage event.
436+
expect(beforeStateCallback).not.to.have.been.called;
369437
});
370438
});
371439

@@ -378,6 +446,7 @@ describe('core/auth/auth_impl', () => {
378446
expect(auth.currentUser?.toJSON()).to.eql(user.toJSON());
379447
expect(authStateCallback).not.to.have.been.called;
380448
expect(idTokenCallback).not.to.have.been.called;
449+
expect(beforeStateCallback).not.to.have.been.called;
381450
});
382451

383452
it('should update fields if they have changed', async () => {
@@ -391,6 +460,7 @@ describe('core/auth/auth_impl', () => {
391460
expect(auth.currentUser?.displayName).to.eq('other-name');
392461
expect(authStateCallback).not.to.have.been.called;
393462
expect(idTokenCallback).not.to.have.been.called;
463+
expect(beforeStateCallback).not.to.have.been.called;
394464
});
395465

396466
it('should update tokens if they have changed', async () => {
@@ -407,6 +477,8 @@ describe('core/auth/auth_impl', () => {
407477
).to.eq('new-access-token');
408478
expect(authStateCallback).not.to.have.been.called;
409479
expect(idTokenCallback).to.have.been.called;
480+
// This should never be called on a storage event.
481+
expect(beforeStateCallback).not.to.have.been.called;
410482
});
411483
});
412484

@@ -420,6 +492,8 @@ describe('core/auth/auth_impl', () => {
420492
expect(auth.currentUser?.toJSON()).to.eql(newUser.toJSON());
421493
expect(authStateCallback).to.have.been.called;
422494
expect(idTokenCallback).to.have.been.called;
495+
// This should never be called on a storage event.
496+
expect(beforeStateCallback).not.to.have.been.called;
423497
});
424498
});
425499
});
@@ -461,7 +535,7 @@ describe('core/auth/auth_impl', () => {
461535
});
462536
});
463537

464-
context ('#_getAdditionalHeaders', () => {
538+
context('#_getAdditionalHeaders', () => {
465539
it('always adds the client version', async () => {
466540
expect(await auth._getAdditionalHeaders()).to.eql({
467541
'X-Client-Version': 'v',

0 commit comments

Comments
 (0)