Skip to content

Commit 1d03b1f

Browse files
sam-gcavolkovi
authored andcommitted
Add persistence for redirect users (#3410)
* Updates to auth for persistence redirect user * Add tests for persistence * Formatting * Removing erroneous console.logs * Formatting * Updates to the auth init flow * Formatting * Fix typo * Formatting * Rogue console.logwq * Formatting * PR feedback * Formatting
1 parent 38b6b5e commit 1d03b1f

File tree

8 files changed

+214
-17
lines changed

8 files changed

+214
-17
lines changed

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

Lines changed: 109 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
*/
1717

1818
import { expect, use } from 'chai';
19+
import * as chaiAsPromised from 'chai-as-promised';
1920
import * as sinon from 'sinon';
2021
import * as sinonChai from 'sinon-chai';
2122

@@ -26,10 +27,16 @@ import { FirebaseError } from '@firebase/util';
2627
import { testUser } from '../../../test/mock_auth';
2728
import { Auth } from '../../model/auth';
2829
import { User } from '../../model/user';
30+
import { browserPopupRedirectResolver } from '../../platform_browser/popup_redirect';
31+
import { AUTH_ERROR_FACTORY, AuthErrorCode } from '../errors';
2932
import { Persistence } from '../persistence';
30-
import { browserLocalPersistence } from '../persistence/browser';
33+
import {
34+
browserLocalPersistence,
35+
browserSessionPersistence
36+
} from '../persistence/browser';
3137
import { inMemoryPersistence } from '../persistence/in_memory';
3238
import { PersistenceUserManager } from '../persistence/persistence_user_manager';
39+
import * as reload from '../user/reload';
3340
import { _getInstance } from '../util/instantiator';
3441
import * as navigator from '../util/navigator';
3542
import { _getClientVersion, ClientPlatform } from '../util/version';
@@ -41,6 +48,7 @@ import {
4148
} from './auth_impl';
4249

4350
use(sinonChai);
51+
use(chaiAsPromised);
4452

4553
const FAKE_APP: FirebaseApp = {
4654
name: 'test-app',
@@ -286,14 +294,23 @@ describe('core/auth/initializeAuth', () => {
286294

287295
describe('persistence manager creation', () => {
288296
let createManagerStub: sinon.SinonSpy;
297+
let reloadStub: sinon.SinonStub;
298+
289299
beforeEach(() => {
290300
createManagerStub = sinon.spy(PersistenceUserManager, 'create');
301+
reloadStub = sinon
302+
.stub(reload, '_reloadWithoutSaving')
303+
.returns(Promise.resolve());
291304
});
292305

293306
async function initAndWait(
294-
persistence: externs.Persistence | externs.Persistence[]
307+
persistence: externs.Persistence | externs.Persistence[],
308+
popupRedirectResolver?: externs.PopupRedirectResolver
295309
): Promise<Auth> {
296-
const auth = initializeAuth(FAKE_APP, { persistence });
310+
const auth = initializeAuth(FAKE_APP, {
311+
persistence,
312+
popupRedirectResolver
313+
});
297314
// Auth initializes async. We can make sure the initialization is
298315
// flushed by awaiting a method on the queue.
299316
await auth.setPersistence(inMemoryPersistence);
@@ -326,6 +343,95 @@ describe('core/auth/initializeAuth', () => {
326343
]);
327344
});
328345

346+
it('does not reload redirect users', async () => {
347+
const user = testUser({}, 'uid');
348+
user._redirectEventId = 'event-id';
349+
sinon
350+
.stub(_getInstance<Persistence>(inMemoryPersistence), 'get')
351+
.returns(Promise.resolve(user.toPlainObject()));
352+
sinon
353+
.stub(_getInstance<Persistence>(browserSessionPersistence), 'get')
354+
.returns(Promise.resolve(user.toPlainObject()));
355+
await initAndWait(inMemoryPersistence);
356+
expect(reload._reloadWithoutSaving).not.to.have.been.called;
357+
});
358+
359+
it('reloads non-redirect users', async () => {
360+
sinon
361+
.stub(_getInstance<Persistence>(inMemoryPersistence), 'get')
362+
.returns(Promise.resolve(testUser({}, 'uid').toPlainObject()));
363+
sinon
364+
.stub(_getInstance<Persistence>(browserSessionPersistence), 'get')
365+
.returns(Promise.resolve(null));
366+
367+
await initAndWait(inMemoryPersistence);
368+
expect(reload._reloadWithoutSaving).to.have.been.called;
369+
});
370+
371+
it('Does not reload if the event ids match', async () => {
372+
const user = testUser({}, 'uid');
373+
user._redirectEventId = 'event-id';
374+
375+
sinon
376+
.stub(_getInstance<Persistence>(inMemoryPersistence), 'get')
377+
.returns(Promise.resolve(user.toPlainObject()));
378+
sinon
379+
.stub(_getInstance<Persistence>(browserSessionPersistence), 'get')
380+
.returns(Promise.resolve(user.toPlainObject()));
381+
382+
await initAndWait(inMemoryPersistence, browserPopupRedirectResolver);
383+
expect(reload._reloadWithoutSaving).not.to.have.been.called;
384+
});
385+
386+
it('Reloads if the event ids do not match', async () => {
387+
const user = testUser({}, 'uid');
388+
user._redirectEventId = 'event-id';
389+
390+
sinon
391+
.stub(_getInstance<Persistence>(inMemoryPersistence), 'get')
392+
.returns(Promise.resolve(user.toPlainObject()));
393+
394+
user._redirectEventId = 'some-other-id';
395+
sinon
396+
.stub(_getInstance<Persistence>(browserSessionPersistence), 'get')
397+
.returns(Promise.resolve(user.toPlainObject()));
398+
399+
await initAndWait(inMemoryPersistence, browserPopupRedirectResolver);
400+
expect(reload._reloadWithoutSaving).to.have.been.called;
401+
});
402+
403+
it('Nulls out the current user if reload fails', async () => {
404+
const stub = sinon.stub(_getInstance<Persistence>(inMemoryPersistence));
405+
stub.get.returns(Promise.resolve(testUser({}, 'uid').toPlainObject()));
406+
stub.remove.returns(Promise.resolve());
407+
reloadStub.returns(
408+
Promise.reject(
409+
AUTH_ERROR_FACTORY.create(AuthErrorCode.TOKEN_EXPIRED, {
410+
appName: 'app'
411+
})
412+
)
413+
);
414+
415+
await initAndWait(inMemoryPersistence);
416+
expect(stub.remove).to.have.been.called;
417+
});
418+
419+
it('Keeps current user if reload fails with network error', async () => {
420+
const stub = sinon.stub(_getInstance<Persistence>(inMemoryPersistence));
421+
stub.get.returns(Promise.resolve(testUser({}, 'uid').toPlainObject()));
422+
stub.remove.returns(Promise.resolve());
423+
reloadStub.returns(
424+
Promise.reject(
425+
AUTH_ERROR_FACTORY.create(AuthErrorCode.NETWORK_REQUEST_FAILED, {
426+
appName: 'app'
427+
})
428+
)
429+
);
430+
431+
await initAndWait(inMemoryPersistence);
432+
expect(stub.remove).not.to.have.been.called;
433+
});
434+
329435
it('sets auth name and config', async () => {
330436
const auth = await initAndWait(inMemoryPersistence);
331437
expect(auth.name).to.eq(FAKE_APP.name);

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

Lines changed: 79 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,15 @@ import {
2929
} from '@firebase/util';
3030

3131
import { Auth, Dependencies } from '../../model/auth';
32+
import { PopupRedirectResolver } from '../../model/popup_redirect';
3233
import { User } from '../../model/user';
3334
import { AuthErrorCode } from '../errors';
3435
import { Persistence } from '../persistence';
35-
import { PersistenceUserManager } from '../persistence/persistence_user_manager';
36+
import {
37+
_REDIRECT_USER_KEY_NAME,
38+
PersistenceUserManager
39+
} from '../persistence/persistence_user_manager';
40+
import { _reloadWithoutSaving } from '../user/reload';
3641
import { assert } from '../util/assert';
3742
import { _getInstance } from '../util/instantiator';
3843
import { _getUserLanguage } from '../util/navigator';
@@ -50,8 +55,10 @@ export class AuthImpl implements Auth {
5055
currentUser: User | null = null;
5156
private operations = Promise.resolve();
5257
private persistenceManager?: PersistenceUserManager;
58+
private redirectPersistenceManager?: PersistenceUserManager;
5359
private authStateSubscription = new Subscription<User>(this);
5460
private idTokenSubscription = new Subscription<User>(this);
61+
private redirectUser: User | null = null;
5562
_isInitialized = false;
5663

5764
// Tracks the last notified UID for state change listeners to prevent
@@ -68,25 +75,73 @@ export class AuthImpl implements Auth {
6875
) {}
6976

7077
_initializeWithPersistence(
71-
persistenceHierarchy: Persistence[]
78+
persistenceHierarchy: Persistence[],
79+
popupRedirectResolver?: externs.PopupRedirectResolver
7280
): Promise<void> {
7381
return this.queue(async () => {
7482
this.persistenceManager = await PersistenceUserManager.create(
7583
this,
7684
persistenceHierarchy
7785
);
7886

79-
const storedUser = await this.persistenceManager.getCurrentUser();
80-
// TODO: Check redirect user, if not redirect user, call refresh on stored user
81-
if (storedUser) {
82-
await this.directlySetCurrentUser(storedUser);
83-
}
87+
await this.initializeCurrentUser(popupRedirectResolver);
8488

8589
this._isInitialized = true;
8690
this.notifyAuthListeners();
8791
});
8892
}
8993

94+
private async initializeCurrentUser(
95+
popupRedirectResolver?: externs.PopupRedirectResolver
96+
): Promise<void> {
97+
const storedUser = await this.assertedPersistence.getCurrentUser();
98+
if (!storedUser) {
99+
return this.directlySetCurrentUser(storedUser);
100+
}
101+
102+
if (!storedUser._redirectEventId) {
103+
// This isn't a redirect user, we can reload and bail
104+
return this.reloadAndSetCurrentUserOrClear(storedUser);
105+
}
106+
107+
assert(popupRedirectResolver, this.name, AuthErrorCode.ARGUMENT_ERROR);
108+
const resolver: PopupRedirectResolver = _getInstance(popupRedirectResolver);
109+
110+
this.redirectPersistenceManager = await PersistenceUserManager.create(
111+
this,
112+
[_getInstance(resolver._redirectPersistence)],
113+
_REDIRECT_USER_KEY_NAME
114+
);
115+
116+
this.redirectUser = await this.redirectPersistenceManager.getCurrentUser();
117+
118+
// If the redirect user's event ID matches the current user's event ID,
119+
// DO NOT reload the current user, otherwise they'll be cleared from storage.
120+
// This is important for the reauthenticateWithRedirect() flow.
121+
if (
122+
this.redirectUser &&
123+
this.redirectUser._redirectEventId === storedUser._redirectEventId
124+
) {
125+
return this.directlySetCurrentUser(storedUser);
126+
}
127+
128+
return this.reloadAndSetCurrentUserOrClear(storedUser);
129+
}
130+
131+
private async reloadAndSetCurrentUserOrClear(user: User): Promise<void> {
132+
try {
133+
await _reloadWithoutSaving(user);
134+
} catch (e) {
135+
if (e.code !== `auth/${AuthErrorCode.NETWORK_REQUEST_FAILED}`) {
136+
// Something's wrong with the user's token. Log them out and remove
137+
// them from storage
138+
return this.directlySetCurrentUser(null);
139+
}
140+
}
141+
142+
return this.directlySetCurrentUser(user);
143+
}
144+
90145
useDeviceLanguage(): void {
91146
this.languageCode = _getUserLanguage();
92147
}
@@ -134,6 +189,22 @@ export class AuthImpl implements Auth {
134189
);
135190
}
136191

192+
async _setRedirectUser(user: User): Promise<void> {
193+
return this.redirectPersistenceManager?.setCurrentUser(user);
194+
}
195+
196+
_redirectUserForId(id: string): User | null {
197+
if (this.currentUser?._redirectEventId === id) {
198+
return this.currentUser;
199+
}
200+
201+
if (this.redirectUser?._redirectEventId === id) {
202+
return this.redirectUser;
203+
}
204+
205+
return null;
206+
}
207+
137208
async _persistUserIfCurrent(user: User): Promise<void> {
138209
if (user === this.currentUser) {
139210
return this.queue(async () => this.directlySetCurrentUser(user));
@@ -238,7 +309,7 @@ export function initializeAuth(
238309
// This promise is intended to float; auth initialization happens in the
239310
// background, meanwhile the auth object may be used by the app.
240311
// eslint-disable-next-line @typescript-eslint/no-floating-promises
241-
auth._initializeWithPersistence(hierarchy);
312+
auth._initializeWithPersistence(hierarchy, deps?.popupRedirectResolver);
242313

243314
return auth;
244315
}

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,15 +23,16 @@ import { _getInstance } from '../util/instantiator';
2323
import { inMemoryPersistence } from './in_memory';
2424

2525
export const _AUTH_USER_KEY_NAME = 'authUser';
26+
export const _REDIRECT_USER_KEY_NAME = 'redirectUser';
2627
export const _PERSISTENCE_KEY_NAME = 'persistence';
27-
const _PERSISTENCE_NAMESPACE = 'firebase';
28+
const PERSISTENCE_NAMESPACE = 'firebase';
2829

2930
function _persistenceKeyName(
3031
key: string,
3132
apiKey: ApiKey,
3233
appName: AppName
3334
): string {
34-
return `${_PERSISTENCE_NAMESPACE}:${key}:${apiKey}:${appName}`;
35+
return `${PERSISTENCE_NAMESPACE}:${key}:${apiKey}:${appName}`;
3536
}
3637

3738
export class PersistenceUserManager {

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

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,8 @@ export class UserImpl implements User {
7373
photoURL: string | null;
7474
isAnonymous: boolean = false;
7575

76+
_redirectEventId?: string;
77+
7678
constructor({ uid, auth, stsTokenManager, ...opt }: UserParameters) {
7779
this.uid = uid;
7880
this.auth = auth;
@@ -169,7 +171,8 @@ export class UserImpl implements User {
169171
displayName: this.displayName || undefined,
170172
email: this.email || undefined,
171173
phoneNumber: this.phoneNumber || undefined,
172-
photoURL: this.phoneNumber || undefined
174+
photoURL: this.phoneNumber || undefined,
175+
_redirectEventId: this._redirectEventId
173176
};
174177
}
175178

@@ -184,7 +187,8 @@ export class UserImpl implements User {
184187
displayName,
185188
email,
186189
phoneNumber,
187-
photoURL
190+
photoURL,
191+
_redirectEventId
188192
} = object;
189193

190194
assert(uid && plainObjectTokenManager, auth.name);
@@ -199,7 +203,8 @@ export class UserImpl implements User {
199203
assertStringOrUndefined(email, auth.name);
200204
assertStringOrUndefined(phoneNumber, auth.name);
201205
assertStringOrUndefined(photoURL, auth.name);
202-
return new UserImpl({
206+
assertStringOrUndefined(_redirectEventId, auth.name);
207+
const user = new UserImpl({
203208
uid,
204209
auth,
205210
stsTokenManager,
@@ -208,6 +213,11 @@ export class UserImpl implements User {
208213
phoneNumber,
209214
photoURL
210215
});
216+
if (_redirectEventId) {
217+
user._redirectEventId = _redirectEventId;
218+
}
219+
220+
return user;
211221
}
212222

213223
/**

packages-exp/auth-exp/src/model/auth.d.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
*/
1717

1818
import * as externs from '@firebase/auth-types-exp';
19-
import { ErrorFn, CompleteFn, Unsubscribe } from '@firebase/util';
19+
import { CompleteFn, ErrorFn, Unsubscribe } from '@firebase/util';
2020

2121
import { User } from './user';
2222

@@ -42,8 +42,11 @@ export interface Auth extends externs.Auth {
4242
): Unsubscribe;
4343
_notifyListenersIfCurrent(user: User): void;
4444
_persistUserIfCurrent(user: User): Promise<void>;
45+
_setRedirectUser(user: User): Promise<void>;
46+
_redirectUserForId(id: string): User | null;
4547
}
4648

4749
export interface Dependencies {
4850
persistence?: externs.Persistence | externs.Persistence[];
51+
popupRedirectResolver?: externs.PopupRedirectResolver;
4952
}

packages-exp/auth-exp/src/model/popup_redirect.d.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,4 +87,5 @@ export interface PopupRedirectResolver extends externs.PopupRedirectResolver {
8787
authType: AuthEventType,
8888
eventId?: string
8989
): Promise<never>;
90+
_redirectPersistence: externs.Persistence;
9091
}

0 commit comments

Comments
 (0)