Skip to content

Commit 73aec54

Browse files
avolkoviavolkovi
committed
Add tests for AuthImpl._onStorageEvent and fix getIdToken to trigger
correctly
1 parent 7eea870 commit 73aec54

File tree

12 files changed

+407
-157
lines changed

12 files changed

+407
-157
lines changed

‎packages-exp/auth-exp/demo/src/index.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1641,7 +1641,7 @@ function initApp() {
16411641
log('Initializing app...');
16421642
app = initializeApp(config);
16431643
auth = initializeAuth(app, {
1644-
persistence: browserSessionPersistence,
1644+
persistence: indexedDBLocalPersistence,
16451645
popupRedirectResolver: browserPopupRedirectResolver
16461646
});
16471647

‎packages-exp/auth-exp/index.webworker.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ import { ClientPlatform } from './src/core/util/version';
2323
// Core functionality shared by all clients
2424
export * from './src';
2525

26+
export { indexedDBLocalPersistence } from './src/platform_browser/persistence/indexed_db';
27+
2628
export const initializeAuth = _initializeAuthForClientPlatform(
2729
ClientPlatform.WORKER
2830
);

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

Lines changed: 122 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,14 @@ import * as chaiAsPromised from 'chai-as-promised';
2222
import * as sinon from 'sinon';
2323
import * as sinonChai from 'sinon-chai';
2424
import { testUser } from '../../../test/helpers/mock_auth';
25+
import { Auth } from '../../model/auth';
2526
import { User } from '../../model/user';
2627
import { Persistence } from '../persistence';
2728
import { inMemoryPersistence } from '../persistence/in_memory';
2829
import { _getInstance } from '../util/instantiator';
2930
import * as navigator from '../util/navigator';
3031
import { ClientPlatform } from '../util/version';
3132
import { _castAuth, _initializeAuthForClientPlatform } from './auth_impl';
32-
import { Auth } from '../../model/auth';
3333

3434
use(sinonChai);
3535
use(chaiAsPromised);
@@ -254,4 +254,125 @@ describe('core/auth/auth_impl', () => {
254254
});
255255
});
256256
});
257+
258+
describe('#_onStorageEvent', () => {
259+
let authStateCallback: sinon.SinonSpy;
260+
let idTokenCallback: sinon.SinonSpy;
261+
262+
beforeEach(async () => {
263+
authStateCallback = sinon.spy();
264+
idTokenCallback = sinon.spy();
265+
auth._onAuthStateChanged(authStateCallback);
266+
auth._onIdTokenChanged(idTokenCallback);
267+
await auth.updateCurrentUser(null); // force event handlers to clear out
268+
authStateCallback.resetHistory();
269+
idTokenCallback.resetHistory();
270+
});
271+
272+
context('previously logged out', () => {
273+
context('still logged out', () => {
274+
it('should do nothing', async () => {
275+
await auth._onStorageEvent();
276+
277+
expect(authStateCallback).not.to.have.been.called;
278+
expect(idTokenCallback).not.to.have.been.called;
279+
});
280+
});
281+
282+
context('now logged in', () => {
283+
let user: User;
284+
285+
beforeEach(() => {
286+
user = testUser(auth, 'uid');
287+
persistenceStub.get.returns(Promise.resolve(user.toJSON()));
288+
});
289+
290+
it('should update the current user', async () => {
291+
await auth._onStorageEvent();
292+
293+
expect(auth.currentUser?.toJSON()).to.eql(user.toJSON());
294+
expect(authStateCallback).to.have.been.called;
295+
expect(idTokenCallback).to.have.been.called;
296+
});
297+
});
298+
});
299+
300+
context('previously logged in', () => {
301+
let user: User;
302+
303+
beforeEach(async () => {
304+
user = testUser(auth, 'uid', undefined, true);
305+
await auth.updateCurrentUser(user);
306+
authStateCallback.resetHistory();
307+
idTokenCallback.resetHistory();
308+
});
309+
310+
context('now logged out', () => {
311+
beforeEach(() => {
312+
persistenceStub.get.returns(Promise.resolve(null));
313+
});
314+
315+
it('should log out', async () => {
316+
await auth._onStorageEvent();
317+
318+
expect(auth.currentUser).to.be.null;
319+
expect(authStateCallback).to.have.been.called;
320+
expect(idTokenCallback).to.have.been.called;
321+
});
322+
});
323+
324+
context('still logged in as same user', () => {
325+
it('should do nothing if nothing changed', async () => {
326+
persistenceStub.get.returns(Promise.resolve(user.toJSON()));
327+
328+
await auth._onStorageEvent();
329+
330+
expect(auth.currentUser?.toJSON()).to.eql(user.toJSON());
331+
expect(authStateCallback).not.to.have.been.called;
332+
expect(idTokenCallback).not.to.have.been.called;
333+
});
334+
335+
it('should update fields if they have changed', async () => {
336+
const userObj = user.toJSON();
337+
userObj['displayName'] = 'other-name';
338+
persistenceStub.get.returns(Promise.resolve(userObj));
339+
340+
await auth._onStorageEvent();
341+
342+
expect(auth.currentUser?.uid).to.eq(user.uid);
343+
expect(auth.currentUser?.displayName).to.eq('other-name');
344+
expect(authStateCallback).not.to.have.been.called;
345+
expect(idTokenCallback).not.to.have.been.called;
346+
});
347+
348+
it('should update tokens if they have changed', async () => {
349+
const userObj = user.toJSON();
350+
(userObj['stsTokenManager'] as any)['accessToken'] = 'new-access-token';
351+
persistenceStub.get.returns(Promise.resolve(userObj));
352+
353+
await auth._onStorageEvent();
354+
355+
expect(auth.currentUser?.uid).to.eq(user.uid);
356+
expect(auth.currentUser?.stsTokenManager.accessToken).to.eq('new-access-token');
357+
expect(authStateCallback).not.to.have.been.called;
358+
expect(idTokenCallback).to.have.been.called;
359+
});
360+
});
361+
362+
context('now logged in as different user', () => {
363+
it('should re-login as the new user', async () => {
364+
const newUser = testUser(auth, 'other-uid', undefined, true);
365+
persistenceStub.get.returns(Promise.resolve(newUser.toJSON()));
366+
367+
await auth._onStorageEvent();
368+
369+
console.log(auth.currentUser?.toJSON());
370+
console.log(newUser.toJSON());
371+
expect(auth.currentUser?.toJSON()).to.eql(newUser.toJSON());
372+
expect(authStateCallback).to.have.been.called;
373+
expect(idTokenCallback).to.have.been.called;
374+
});
375+
});
376+
});
377+
});
257378
});

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

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,11 @@ export class AuthImplCompat<T extends User> implements Auth {
108108
async _onStorageEvent(): Promise<void> {
109109
const user = await this.persistenceManager!.getCurrentUser();
110110

111+
if (!this.currentUser && !user) {
112+
// No change, do nothing (was signed out and remained signed out).
113+
return;
114+
}
115+
111116
// If the same user is to be synchronized.
112117
if (this.currentUser && user && this.currentUser.uid === user.uid) {
113118
// Data update, simply copy data changes.
@@ -116,16 +121,13 @@ export class AuthImplCompat<T extends User> implements Auth {
116121
// notifyAuthListeners_.
117122
await this.currentUser.getIdToken();
118123
return;
119-
} else if (!this.currentUser && !user) {
120-
// No change, do nothing (was signed out and remained signed out).
121-
return;
122-
} else {
123-
// Update current Auth state. Either a new login or logout.
124-
await this.updateCurrentUser(user);
125-
// TODO: If a new user is signed in, enabled popup and redirect on that user.
126-
// Notify external Auth changes of Auth change event.
127-
this.notifyAuthListeners();
128124
}
125+
126+
// Update current Auth state. Either a new login or logout.
127+
await this.updateCurrentUser(user);
128+
// TODO: If a new user is signed in, enabled popup and redirect on that user.
129+
// Notify external Auth changes of Auth change event.
130+
this.notifyAuthListeners();
129131
}
130132

131133
_createUser(params: UserParameters): T {

‎packages-exp/auth-exp/src/core/persistence/persistence_user_manager.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ export class PersistenceUserManager {
5151
config.apiKey,
5252
name
5353
);
54-
this.persistence.addListener(this.fullUserKey, auth._onStorageEvent);
54+
this.persistence.addListener(this.fullUserKey, auth._onStorageEvent.bind(auth));
5555
}
5656

5757
setCurrentUser(user: User): Promise<void> {

‎packages-exp/auth-exp/src/core/user/token_manager.test.ts

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -110,11 +110,7 @@ describe('core/user/token_manager', () => {
110110
expect(stsTokenManager.refreshToken).to.eq('new-refresh-token');
111111
expect(stsTokenManager.expirationTime).to.eq(now + 3_600_000);
112112

113-
expect(tokens).to.eql({
114-
accessToken: 'new-access-token',
115-
refreshToken: 'new-refresh-token',
116-
wasRefreshed: true
117-
});
113+
expect(tokens).to.eql('new-access-token');
118114
});
119115

120116
it('refreshes the token if token is expired', async () => {
@@ -130,11 +126,7 @@ describe('core/user/token_manager', () => {
130126
expect(stsTokenManager.refreshToken).to.eq('new-refresh-token');
131127
expect(stsTokenManager.expirationTime).to.eq(now + 3_600_000);
132128

133-
expect(tokens).to.eql({
134-
accessToken: 'new-access-token',
135-
refreshToken: 'new-refresh-token',
136-
wasRefreshed: true
137-
});
129+
expect(tokens).to.eql('new-access-token');
138130
});
139131
});
140132

@@ -162,11 +154,7 @@ describe('core/user/token_manager', () => {
162154
});
163155

164156
const tokens = (await stsTokenManager.getToken(auth))!;
165-
expect(tokens).to.eql({
166-
accessToken: 'token',
167-
refreshToken: 'refresh',
168-
wasRefreshed: false
169-
});
157+
expect(tokens).to.eql('token');
170158
});
171159
});
172160

‎packages-exp/auth-exp/src/core/user/token_manager.ts

Lines changed: 7 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -15,26 +15,20 @@
1515
* limitations under the License.
1616
*/
1717

18+
import { FinalizeMfaResponse } from '../../api/authentication/mfa';
1819
import { requestStsToken } from '../../api/authentication/token';
20+
import { AuthCore } from '../../model/auth';
1921
import { IdTokenResponse } from '../../model/id_token';
20-
import { AUTH_ERROR_FACTORY, AuthErrorCode } from '../errors';
22+
import { AuthErrorCode } from '../errors';
2123
import { PersistedBlob } from '../persistence';
2224
import { assert, debugFail } from '../util/assert';
23-
import { FinalizeMfaResponse } from '../../api/authentication/mfa';
24-
import { AuthCore } from '../../model/auth';
2525

2626
/**
2727
* The number of milliseconds before the official expiration time of a token
2828
* to refresh that token, to provide a buffer for RPCs to complete.
2929
*/
3030
export const TOKEN_REFRESH_BUFFER_MS = 30_000;
3131

32-
export interface Tokens {
33-
accessToken: string;
34-
refreshToken: string | null;
35-
wasRefreshed: boolean;
36-
}
37-
3832
export class StsTokenManager {
3933
refreshToken: string | null = null;
4034
accessToken: string | null = null;
@@ -57,31 +51,18 @@ export class StsTokenManager {
5751
);
5852
}
5953

60-
async getToken(auth: AuthCore, forceRefresh = false): Promise<Tokens | null> {
54+
async getToken(auth: AuthCore, forceRefresh = false): Promise<string | null> {
6155
if (!forceRefresh && this.accessToken && !this.isExpired) {
62-
return {
63-
accessToken: this.accessToken,
64-
refreshToken: this.refreshToken,
65-
wasRefreshed: false
66-
};
67-
}
68-
69-
if (this.accessToken && !this.refreshToken) {
70-
throw AUTH_ERROR_FACTORY.create(AuthErrorCode.TOKEN_EXPIRED, {
71-
appName: auth.name
72-
});
56+
return this.accessToken;
7357
}
7458

7559
if (!this.refreshToken) {
60+
assert(!this.accessToken, AuthErrorCode.TOKEN_EXPIRED, { appName: auth.name });
7661
return null;
7762
}
7863

7964
await this.refresh(auth, this.refreshToken);
80-
return {
81-
accessToken: this.accessToken!,
82-
refreshToken: this.refreshToken,
83-
wasRefreshed: true
84-
};
65+
return this.accessToken;
8566
}
8667

8768
clearRefreshToken(): void {

‎packages-exp/auth-exp/src/core/user/user_impl.ts

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,8 @@ export class UserImpl implements User {
4848
// For the user object, provider is always Firebase.
4949
readonly providerId = externs.ProviderId.FIREBASE;
5050
stsTokenManager: StsTokenManager;
51+
// Last known accessToken so we know when it changes
52+
private accessToken: string | null;
5153

5254
uid: string;
5355
auth: Auth;
@@ -69,6 +71,7 @@ export class UserImpl implements User {
6971
this.uid = uid;
7072
this.auth = auth;
7173
this.stsTokenManager = stsTokenManager;
74+
this.accessToken = stsTokenManager.accessToken;
7275
this.displayName = opt.displayName || null;
7376
this.email = opt.email || null;
7477
this.phoneNumber = opt.phoneNumber || null;
@@ -78,12 +81,11 @@ export class UserImpl implements User {
7881
}
7982

8083
async getIdToken(forceRefresh?: boolean): Promise<string> {
81-
const tokens = await this.stsTokenManager.getToken(this.auth, forceRefresh);
82-
assert(tokens, AuthErrorCode.INTERNAL_ERROR, { appName: this.auth.name });
84+
const accessToken = await this.stsTokenManager.getToken(this.auth, forceRefresh);
85+
assert(accessToken, AuthErrorCode.INTERNAL_ERROR, { appName: this.auth.name });
8386

84-
const { accessToken, wasRefreshed } = tokens;
85-
86-
if (wasRefreshed) {
87+
if (this.accessToken !== accessToken) {
88+
this.accessToken = accessToken;
8789
await this.auth._persistUserIfCurrent(this);
8890
this.auth._notifyListenersIfCurrent(this);
8991
}

0 commit comments

Comments
 (0)