Skip to content

Commit 6dacc24

Browse files
authored
[Auth] Make auth resilient against localStorage and sessionStorage permissions errors (#5635)
* Make auth resilient against localStorage and sessionStorage permissions errors * Restore demo code * Add changeset
1 parent f1c38f3 commit 6dacc24

File tree

8 files changed

+139
-7
lines changed

8 files changed

+139
-7
lines changed

.changeset/quiet-wolves-sing.md

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@firebase/auth": patch
3+
---
4+
5+
Make the library resilient against localStorage and sessionStorage permissions errors

packages/auth/src/core/strategies/redirect.test.ts

+25-1
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
import {
1919
AuthError,
20+
Persistence,
2021
PopupRedirectResolver
2122
} from '../../model/public_types';
2223
import { OperationType, ProviderId } from '../../model/enums';
@@ -32,7 +33,7 @@ import {
3233
import { makeMockPopupRedirectResolver } from '../../../test/helpers/mock_popup_redirect_resolver';
3334
import { AuthInternal } from '../../model/auth';
3435
import { AuthEventManager } from '../auth/auth_event_manager';
35-
import { RedirectAction, _clearRedirectOutcomes } from './redirect';
36+
import { RedirectAction, _clearRedirectOutcomes, _getAndClearPendingRedirectStatus } from './redirect';
3637
import {
3738
AuthEvent,
3839
AuthEventType,
@@ -44,6 +45,7 @@ import * as idpTasks from '../strategies/idp';
4445
import { expect, use } from 'chai';
4546
import { AuthErrorCode } from '../errors';
4647
import { RedirectPersistence } from '../../../test/helpers/redirect_persistence';
48+
import { ErroringUnavailablePersistence } from '../../../test/helpers/erroring_unavailable_persistence';
4749

4850
use(sinonChai);
4951

@@ -210,4 +212,26 @@ describe('core/strategies/redirect', () => {
210212
expect(await redirectAction.execute()).to.eq(null);
211213
expect(resolverInstance._initialize).not.to.have.been.called;
212214
});
215+
216+
context('_getAndClearPendingRedirectStatus', () => {
217+
// Do not run these tests in node
218+
if (typeof window === 'undefined') {
219+
return;
220+
}
221+
222+
it('returns false if the key is not set', async () => {
223+
redirectPersistence.hasPendingRedirect = false;
224+
expect(await _getAndClearPendingRedirectStatus(_getInstance(resolver), auth)).to.be.false;
225+
});
226+
227+
it('returns true if the key is found', async () => {
228+
redirectPersistence.hasPendingRedirect = true;
229+
expect(await _getAndClearPendingRedirectStatus(_getInstance(resolver), auth)).to.be.true;
230+
});
231+
232+
it('returns false if sessionStorage is permission denied', async () => {
233+
_getInstance<PopupRedirectResolverInternal>(resolver)._redirectPersistence = ErroringUnavailablePersistence as unknown as Persistence;
234+
expect(await _getAndClearPendingRedirectStatus(_getInstance(resolver), auth)).to.be.false;
235+
});
236+
});
213237
});

packages/auth/src/core/strategies/redirect.ts

+6-2
Original file line numberDiff line numberDiff line change
@@ -118,9 +118,13 @@ export async function _getAndClearPendingRedirectStatus(
118118
auth: AuthInternal
119119
): Promise<boolean> {
120120
const key = pendingRedirectKey(auth);
121+
const persistence = resolverPersistence(resolver);
122+
if (!(await persistence._isAvailable())) {
123+
return false;
124+
}
121125
const hasPendingRedirect =
122-
(await resolverPersistence(resolver)._get(key)) === 'true';
123-
await resolverPersistence(resolver)._remove(key);
126+
(await persistence._get(key)) === 'true';
127+
await persistence._remove(key);
124128
return hasPendingRedirect;
125129
}
126130

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
/**
2+
* @license
3+
* Copyright 2021 Google LLC
4+
*
5+
* Licensed under the Apache License, Version 2.0 (the "License");
6+
* you may not use this file except in compliance with the License.
7+
* You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
18+
19+
import { expect } from 'chai';
20+
import {
21+
PersistenceType
22+
} from '../../core/persistence';
23+
import { BrowserPersistenceClass } from './browser';
24+
25+
// Most tests for this class exist in the tests for the subclasses.
26+
27+
class TestPersistence extends BrowserPersistenceClass {
28+
constructor(storageRetriever: () => Storage) {
29+
super(storageRetriever, PersistenceType.NONE);
30+
}
31+
}
32+
33+
describe('platform_browser/persistence/browser', () => {
34+
it('_isAvailable works if reading the storage accessor throws', async () => {
35+
const browserPersistence = new TestPersistence(() => {
36+
throw new DOMException('no');
37+
});
38+
expect(await browserPersistence._isAvailable()).to.be.false;
39+
});
40+
});

packages/auth/src/platform_browser/persistence/browser.ts

+6-2
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,11 @@ import {
2727

2828
export abstract class BrowserPersistenceClass {
2929
protected constructor(
30-
protected readonly storage: Storage,
30+
protected readonly storageRetriever: () => Storage,
3131
readonly type: PersistenceType
3232
) {}
3333

34-
_isAvailable(this: BrowserPersistenceClass): Promise<boolean> {
34+
_isAvailable(): Promise<boolean> {
3535
try {
3636
if (!this.storage) {
3737
return Promise.resolve(false);
@@ -58,4 +58,8 @@ export abstract class BrowserPersistenceClass {
5858
this.storage.removeItem(key);
5959
return Promise.resolve();
6060
}
61+
62+
protected get storage(): Storage {
63+
return this.storageRetriever();
64+
}
6165
}

packages/auth/src/platform_browser/persistence/local_storage.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ class BrowserLocalPersistence
5151
static type: 'LOCAL' = 'LOCAL';
5252

5353
constructor() {
54-
super(window.localStorage, PersistenceType.LOCAL);
54+
super(() => window.localStorage, PersistenceType.LOCAL);
5555
}
5656

5757
private readonly boundEventHandler = (event: StorageEvent, poll?: boolean): void => this.onStorageEvent(event, poll);

packages/auth/src/platform_browser/persistence/session_storage.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ class BrowserSessionPersistence
3131
static type: 'SESSION' = 'SESSION';
3232

3333
constructor() {
34-
super(window.sessionStorage, PersistenceType.SESSION);
34+
super(() => window.sessionStorage, PersistenceType.SESSION);
3535
}
3636

3737
_addListener(_key: string, _listener: StorageEventListener): void {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
/**
2+
* @license
3+
* Copyright 2020 Google LLC
4+
*
5+
* Licensed under the Apache License, Version 2.0 (the "License");
6+
* you may not use this file except in compliance with the License.
7+
* You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
18+
import { PersistenceInternal, PersistenceType, PersistenceValue } from '../../src/core/persistence';
19+
20+
const PERMISSION_ERROR = typeof window !== 'undefined' ? new DOMException(
21+
'Failed to read this storage class from the Window; access is denied')
22+
: new Error('This is Node.');
23+
24+
/**
25+
* Helper class for mocking completely broken persistence that errors when
26+
* accessed.
27+
*
28+
* When disabling cookies in Chrome entirely, for example, simply reading the
29+
* "localStorage" field in "window" will throw an error, but this can't be
30+
* checked for by calling `'localStorage' in window`. This class simulates a
31+
* situation where _isAvailable works correctly but all other methods fail.
32+
*/
33+
export class ErroringUnavailablePersistence implements PersistenceInternal {
34+
type = PersistenceType.NONE;
35+
async _isAvailable(): Promise<boolean> {
36+
return false;
37+
}
38+
async _set(): Promise<void> {
39+
throw PERMISSION_ERROR;
40+
}
41+
async _get<T extends PersistenceValue>(): Promise<T | null> {
42+
throw PERMISSION_ERROR;
43+
}
44+
async _remove(): Promise<void> {
45+
throw PERMISSION_ERROR;
46+
}
47+
_addListener(): void {
48+
throw PERMISSION_ERROR;
49+
}
50+
_removeListener(): void {
51+
throw PERMISSION_ERROR;
52+
}
53+
_shouldAllowMigration = false;
54+
}
55+

0 commit comments

Comments
 (0)