Skip to content

[Auth] Fix halted initialization when proactive popup resolver init fails #5777

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Dec 2, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/red-actors-care.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@firebase/auth": patch
---

Fix errors during Auth initialization when the network is unavailable
5 changes: 4 additions & 1 deletion packages/auth/src/core/auth/auth_impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,10 @@ export class AuthImpl implements AuthInternal, _FirebaseService {
// Initialize the resolver early if necessary (only applicable to web:
// this will cause the iframe to load immediately in certain cases)
if (this._popupRedirectResolver?._shouldInitProactively) {
await this._popupRedirectResolver._initialize(this);
// If this fails, don't halt auth loading
try {
await this._popupRedirectResolver._initialize(this);
} catch (e) { /* Ignore the error */ }
}

await this.initializeCurrentUser(popupRedirectResolver);
Expand Down
10 changes: 10 additions & 0 deletions packages/auth/src/platform_browser/auth.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,16 @@ describe('core/auth/initializeAuth', () => {
expect(resolverInternal._initialize).to.have.been.called;
});

it('does not halt init if resolver fails', async () => {
const popupRedirectResolver = makeMockPopupRedirectResolver();
const resolverInternal: PopupRedirectResolverInternal = _getInstance(
popupRedirectResolver
);
sinon.stub(resolverInternal, '_shouldInitProactively').value(true);
sinon.stub(resolverInternal, '_initialize').rejects(new Error());
await expect(initAndWait(inMemoryPersistence, popupRedirectResolver)).not.to.be.rejected;
});

it('reloads non-redirect users', async () => {
sinon
.stub(_getInstance<PersistenceInternal>(inMemoryPersistence), '_get')
Expand Down
9 changes: 8 additions & 1 deletion packages/auth/src/platform_browser/iframe/gapi.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,14 @@ use(chaiAsPromised);
describe('platform_browser/iframe/gapi', () => {
let library: typeof gapi;
let auth: TestAuth;
let loadJsStub: sinon.SinonStub;
function onJsLoad(globalLoadFnName: string): void {
_window().gapi = library as typeof gapi;
_window()[globalLoadFnName]();
}

beforeEach(async () => {
sinon.stub(js, '_loadJS').callsFake(url => {
loadJsStub = sinon.stub(js, '_loadJS').callsFake(url => {
onJsLoad(url.split('onload=')[1]);
return Promise.resolve(new Event('load'));
});
Expand Down Expand Up @@ -134,4 +135,10 @@ describe('platform_browser/iframe/gapi', () => {
);
expect(_loadGapi(auth)).not.to.eq(firstAttempt);
});

it('rejects if gapi itself does not load', async () => {
const error = new Error();
loadJsStub.rejects(error);
await expect(_loadGapi(auth)).to.be.rejectedWith(error);
});
});
2 changes: 1 addition & 1 deletion packages/auth/src/platform_browser/iframe/gapi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ function loadGapi(auth: AuthInternal): Promise<gapi.iframes.Context> {
}
};
// Load GApi loader.
return js._loadJS(`https://apis.google.com/js/api.js?onload=${cbName}`);
return js._loadJS(`https://apis.google.com/js/api.js?onload=${cbName}`).catch(e => reject(e));
}
}).catch(error => {
// Reset cached promise to allow for retrial.
Expand Down
29 changes: 21 additions & 8 deletions packages/auth/src/platform_browser/popup_redirect.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,15 +54,26 @@ describe('platform_browser/popup_redirect', () => {
let auth: TestAuth;
let onIframeMessage: (event: GapiAuthEvent) => Promise<void>;
let iframeSendStub: sinon.SinonStub;
let loadGapiStub: sinon.SinonStub;

beforeEach(async () => {
auth = await testAuth();
resolver = new (browserPopupRedirectResolver as SingletonInstantiator<PopupRedirectResolverInternal>)();

sinon.stub(validateOrigin, '_validateOrigin').returns(Promise.resolve());
iframeSendStub = sinon.stub();
loadGapiStub = sinon.stub(gapiLoader, '_loadGapi');
setGapiStub();

sinon.stub(gapiLoader, '_loadGapi').returns(
sinon.stub(authWindow._window(), 'gapi').value({
iframes: {
CROSS_ORIGIN_IFRAMES_FILTER: 'cross-origin-iframes-filter'
}
});
});

function setGapiStub(): void {
loadGapiStub.returns(
Promise.resolve(({
open: () =>
Promise.resolve({
Expand All @@ -74,13 +85,7 @@ describe('platform_browser/popup_redirect', () => {
})
} as unknown) as gapi.iframes.Context)
);

sinon.stub(authWindow._window(), 'gapi').value({
iframes: {
CROSS_ORIGIN_IFRAMES_FILTER: 'cross-origin-iframes-filter'
}
});
});
}

afterEach(() => {
sinon.restore();
Expand Down Expand Up @@ -241,6 +246,14 @@ describe('platform_browser/popup_redirect', () => {
expect(resolver._initialize(secondAuth)).to.eq(secondPromise);
});

it('clears the cache if the initialize fails', async () => {
const error = new Error();
loadGapiStub.rejects(error);
await expect(resolver._initialize(auth)).to.be.rejectedWith(error);
setGapiStub(); // Reset the gapi load stub
await expect(resolver._initialize(auth)).not.to.be.rejected;
});

it('iframe event goes through to the manager', async () => {
const manager = (await resolver._initialize(auth)) as AuthEventManager;
sinon.stub(manager, 'onEvent').returns(true);
Expand Down
7 changes: 7 additions & 0 deletions packages/auth/src/platform_browser/popup_redirect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,13 @@ class BrowserPopupRedirectResolver implements PopupRedirectResolverInternal {

const promise = this.initAndGetManager(auth);
this.eventManagers[key] = { promise };

// If the promise is rejected, the key should be removed so that the
// operation can be retried later.
promise.catch(() => {
delete this.eventManagers[key];
});

return promise;
}

Expand Down