Skip to content
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

Wait for upto 8s for IDP sign in to finish, after popup is closed. #7140

Merged
merged 2 commits into from
Apr 14, 2023
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/dry-cats-lick.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@firebase/auth': patch
---

Increase the popup poller timeout to 8s to support blocking functions + Firefox
11 changes: 8 additions & 3 deletions packages/auth/src/platform_browser/strategies/popup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,13 @@ import { FederatedAuthProvider } from '../../core/providers/federated';
import { getModularInstance } from '@firebase/util';

/*
* The event timeout is the same on mobile and desktop, no need for Delay.
* The event timeout is the same on mobile and desktop, no need for Delay. Set this to 8s since
* blocking functions can take upto 7s to complete sign in, as documented in:
* https://cloud.google.com/identity-platform/docs/blocking-functions#understanding_blocking_functions
* https://firebase.google.com/docs/auth/extend-with-blocking-functions#understanding_blocking_functions
*/
export const enum _Timeout {
AUTH_EVENT = 2000
AUTH_EVENT = 8000
}
export const _POLL_WINDOW_CLOSE_TIMEOUT = new Delay(2000, 10000);

Expand Down Expand Up @@ -282,7 +285,9 @@ class PopupOperation extends AbstractPopupRedirectOperation {
if (this.authWindow?.window?.closed) {
// Make sure that there is sufficient time for whatever action to
// complete. The window could have closed but the sign in network
// call could still be in flight.
// call could still be in flight. This is specifically true for
// Firefox or if the opener is in an iframe, in which case the oauth
// helper closes the popup.
this.pollId = window.setTimeout(() => {
this.pollId = null;
this.reject(
Expand Down
7 changes: 4 additions & 3 deletions packages/auth/test/integration/webdriver/popup.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ browserDescribe('Popup IdP tests', driver => {
await widget.fillEmail('[email protected]');
await widget.clickSignIn();

// On redirect, check that the signed in user is different
// On return to main window, check that the signed in user is different
await driver.selectMainWindow();
const curUser = await driver.getUserSnapshot();
expect(curUser.uid).not.to.eq(anonUser.uid);
Expand All @@ -210,7 +210,7 @@ browserDescribe('Popup IdP tests', driver => {
await widget.fillEmail('[email protected]');
await widget.clickSignIn();

// On redirect, check that the signed in user is upgraded
// On return to main window, check that the signed in user is upgraded
await driver.selectMainWindow();
const curUser = await driver.getUserSnapshot();
expect(curUser.uid).to.eq(anonUser.uid);
Expand Down Expand Up @@ -396,6 +396,7 @@ browserDescribe('Popup IdP tests', driver => {
user = await driver.getUserSnapshot();
expect(user.uid).to.eq(user1.uid);
expect(user.email).to.eq(user1.email);
}).timeout(12_000); // Test takes a while due to the closed-by-user errors
}).timeout(25_000); // Test takes a while due to the closed-by-user errors. Each closed-by-user
// takes 8s to timeout, and we have 2 instances.
});
});