From f7d72935d47c6a7aeec5f9dcaf78d52b0bc5c7e4 Mon Sep 17 00:00:00 2001 From: Pavithra Ramesh Date: Mon, 20 Mar 2023 16:13:37 -0700 Subject: [PATCH 1/2] Wait for upto 8s for IDP sign in to finish, after popup is closed. In some cases (Fiefox or mobile or if opener is an iframe), the popup is closed by the oauth helper code right after sign in has completed at the IDP. The IDP response still needs to be processed by the SDK + signInWithIdp API call needs to be invoked. This can take upto 7s, if there is a blocking function configured. Increase the poller timeout to 8s, so it does not reject the sign in with popup-closed-by-user error. increase timeout for the aborted sign in test. --- .../auth/src/platform_browser/strategies/popup.ts | 11 ++++++++--- .../auth/test/integration/webdriver/popup.test.ts | 7 ++++--- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/packages/auth/src/platform_browser/strategies/popup.ts b/packages/auth/src/platform_browser/strategies/popup.ts index 5ed2727967b..c41cd43342a 100644 --- a/packages/auth/src/platform_browser/strategies/popup.ts +++ b/packages/auth/src/platform_browser/strategies/popup.ts @@ -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); @@ -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( diff --git a/packages/auth/test/integration/webdriver/popup.test.ts b/packages/auth/test/integration/webdriver/popup.test.ts index 05dc267a36f..145d9431bf0 100644 --- a/packages/auth/test/integration/webdriver/popup.test.ts +++ b/packages/auth/test/integration/webdriver/popup.test.ts @@ -192,7 +192,7 @@ browserDescribe('Popup IdP tests', driver => { await widget.fillEmail('bob@bob.test'); 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); @@ -210,7 +210,7 @@ browserDescribe('Popup IdP tests', driver => { await widget.fillEmail('bob@bob.test'); 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); @@ -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. }); }); From 4efa91a3331a2164441b1e6b9503a974e91304a8 Mon Sep 17 00:00:00 2001 From: Pavithra Ramesh Date: Mon, 20 Mar 2023 16:34:35 -0700 Subject: [PATCH 2/2] Add changeset. --- .changeset/dry-cats-lick.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/dry-cats-lick.md diff --git a/.changeset/dry-cats-lick.md b/.changeset/dry-cats-lick.md new file mode 100644 index 00000000000..a8de7967549 --- /dev/null +++ b/.changeset/dry-cats-lick.md @@ -0,0 +1,5 @@ +--- +'@firebase/auth': patch +--- + +Increase the popup poller timeout to 8s to support blocking functions + Firefox