Skip to content

Commit

Permalink
Wait for upto 8s for IDP sign in to finish, after popup is closed. (#…
Browse files Browse the repository at this point in the history
…7140)

* 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.

* Add changeset.
  • Loading branch information
prameshj authored Apr 14, 2023
1 parent ecb4454 commit 1d6771e
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 6 deletions.
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.
});
});

0 comments on commit 1d6771e

Please sign in to comment.