Skip to content

Commit 4983f4d

Browse files
authored
[Auth] Fix errors when cookies are disabled in Chrome (compat layer) (#5923)
* Fix bug in compat layer when cookies are disabled * Formatting * Changeset * Add delay to (negative) tests
1 parent 2d04af9 commit 4983f4d

File tree

4 files changed

+94
-11
lines changed

4 files changed

+94
-11
lines changed

.changeset/gold-geckos-carry.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
"@firebase/auth-compat": patch
3+
"@firebase/auth": patch
4+
---
5+
6+
Fix errors in compatibility layer when cookies are fully disabled in Chrome

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

Lines changed: 70 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,14 @@ import * as sinon from 'sinon';
2323
import sinonChai from 'sinon-chai';
2424
import { Auth } from './auth';
2525
import { CompatPopupRedirectResolver } from './popup_redirect';
26+
import * as platform from './platform';
2627

2728
use(sinonChai);
2829

30+
function delay(ms: number): Promise<void> {
31+
return new Promise(resolve => setTimeout(resolve, ms));
32+
}
33+
2934
// For the most part, the auth methods just call straight through. Some parts
3035
// of the auth compat layer are more complicated: these tests cover those
3136
describe('auth compat', () => {
@@ -45,7 +50,7 @@ describe('auth compat', () => {
4550
});
4651

4752
afterEach(() => {
48-
sinon.restore;
53+
sinon.restore();
4954
});
5055

5156
it('saves the persistence into session storage if available', async () => {
@@ -75,6 +80,40 @@ describe('auth compat', () => {
7580
}
7681
});
7782

83+
it('does not save persistence if property throws DOMException', async () => {
84+
if (typeof self !== 'undefined') {
85+
sinon.stub(platform, '_getSelfWindow').returns({
86+
get sessionStorage(): Storage {
87+
throw new DOMException('Nope!');
88+
}
89+
} as unknown as Window);
90+
const setItemSpy = sinon.spy(sessionStorage, 'setItem');
91+
sinon.stub(underlyingAuth, '_getPersistence').returns('TEST');
92+
sinon
93+
.stub(underlyingAuth, '_initializationPromise')
94+
.value(Promise.resolve());
95+
sinon.stub(
96+
exp._getInstance<exp.PopupRedirectResolverInternal>(
97+
CompatPopupRedirectResolver
98+
),
99+
'_openRedirect'
100+
);
101+
providerStub.isInitialized.returns(true);
102+
providerStub.getImmediate.returns(underlyingAuth);
103+
const authCompat = new Auth(
104+
app,
105+
providerStub as unknown as Provider<'auth'>
106+
);
107+
// eslint-disable-next-line @typescript-eslint/no-floating-promises
108+
await authCompat.signInWithRedirect(new exp.GoogleAuthProvider());
109+
await delay(50);
110+
expect(setItemSpy).not.to.have.been.calledWith(
111+
'firebase:persistence:api-key:undefined',
112+
'TEST'
113+
);
114+
}
115+
});
116+
78117
it('pulls the persistence and sets as the main persitsence if set', () => {
79118
if (typeof self !== 'undefined') {
80119
sessionStorage.setItem(
@@ -98,5 +137,35 @@ describe('auth compat', () => {
98137
});
99138
}
100139
});
140+
141+
it('does not die if sessionStorage errors', async () => {
142+
if (typeof self !== 'undefined') {
143+
sinon.stub(platform, '_getSelfWindow').returns({
144+
get sessionStorage(): Storage {
145+
throw new DOMException('Nope!');
146+
}
147+
} as unknown as Window);
148+
sessionStorage.setItem(
149+
'firebase:persistence:api-key:undefined',
150+
'none'
151+
);
152+
providerStub.isInitialized.returns(false);
153+
providerStub.initialize.returns(underlyingAuth);
154+
new Auth(app, providerStub as unknown as Provider<'auth'>);
155+
// eslint-disable-next-line @typescript-eslint/no-floating-promises
156+
await delay(50);
157+
expect(providerStub.initialize).to.have.been.calledWith({
158+
options: {
159+
popupRedirectResolver: CompatPopupRedirectResolver,
160+
persistence: [
161+
exp.indexedDBLocalPersistence,
162+
exp.browserLocalPersistence,
163+
exp.browserSessionPersistence,
164+
exp.inMemoryPersistence
165+
]
166+
}
167+
});
168+
}
169+
});
101170
});
102171
});

packages/auth-compat/src/persistence.ts

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717

1818
import * as exp from '@firebase/auth/internal';
1919
import { isIndexedDBAvailable, isNode, isReactNative } from '@firebase/util';
20-
import { _isWebStorageSupported, _isWorker } from './platform';
20+
import { _getSelfWindow, _isWebStorageSupported, _isWorker } from './platform';
2121

2222
export const Persistence = {
2323
LOCAL: 'local',
@@ -84,29 +84,28 @@ export async function _savePersistenceForRedirect(
8484
auth: exp.AuthInternal
8585
): Promise<void> {
8686
await auth._initializationPromise;
87-
88-
const win = getSelfWindow();
87+
const session = getSessionStorageIfAvailable();
8988
const key = exp._persistenceKeyName(
9089
PERSISTENCE_KEY,
9190
auth.config.apiKey,
9291
auth.name
9392
);
94-
if (win?.sessionStorage) {
95-
win.sessionStorage.setItem(key, auth._getPersistence());
93+
if (session) {
94+
session.setItem(key, auth._getPersistence());
9695
}
9796
}
9897

9998
export function _getPersistencesFromRedirect(
10099
apiKey: string,
101100
appName: string
102101
): exp.Persistence[] {
103-
const win = getSelfWindow();
104-
if (!win?.sessionStorage) {
102+
const session = getSessionStorageIfAvailable();
103+
if (!session) {
105104
return [];
106105
}
107106

108107
const key = exp._persistenceKeyName(PERSISTENCE_KEY, apiKey, appName);
109-
const persistence = win.sessionStorage.getItem(key);
108+
const persistence = session.getItem(key);
110109

111110
switch (persistence) {
112111
case Persistence.NONE:
@@ -120,6 +119,11 @@ export function _getPersistencesFromRedirect(
120119
}
121120
}
122121

123-
function getSelfWindow(): Window | null {
124-
return typeof window !== 'undefined' ? window : null;
122+
/** Returns session storage, or null if the property access errors */
123+
function getSessionStorageIfAvailable(): Storage | null {
124+
try {
125+
return _getSelfWindow()?.sessionStorage || null;
126+
} catch (e) {
127+
return null;
128+
}
125129
}

packages/auth-compat/src/platform.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,3 +171,7 @@ export async function _isCordova(): Promise<boolean> {
171171
});
172172
});
173173
}
174+
175+
export function _getSelfWindow(): Window | null {
176+
return typeof window !== 'undefined' ? window : null;
177+
}

0 commit comments

Comments
 (0)