Firebase Auth: the pendingRedirect sessionStorage key signInWithRedirect creates can be cleared in race condition

[WestonThayer]

[REQUIRED] Describe your environment

• Operating System version: MacOS 12.6
• Browser version: Chrome 107
• Firebase SDK version: 9.14.0
• Firebase Product: auth

[REQUIRED] Describe the problem

Steps to reproduce:

I've set up a test Firebase instance and a codesandbox for easy reproduction. All I did to the Firebase instance was to create a web app, initialize auth, and add codesandbox as a trusted domain.

1. Load https://codesandbox.io/s/firebase-auth-pending-redirect-bug-16oin0 and open the output in a new tab
2. Open the devtools console and make sure to persist logs
3. Click the button

Actual: the console log will contain window.sessionStorage\n[].

Expected: the console log contains window.sessionStorage\n['firebase:pendingRedirect:AIzaSyA8msq271ik2ryYJw99pZreiAWmErgBqww:[DEFAULT]'].

Notes

When the button is clicked, that's the first time .getAuth() is called. getAuth() apparently has an internal async task queue. One of the async tasks added to that queue is initializeCurrentUser, which tries to process a pending redirect.

But before that async task can complete, our code calls signInWithRedirect(), which ends up creating window.sessionStorage["firebase:pendingRedirect:AIzaSyA8msq271ik2ryYJw99pZreiAWmErgBqww:[DEFAULT]"] = "true" before initializeCurrentUser runs.

When initializeCurrentUser does run a few seconds later, it manages to delete the sessionStorage key before the browser redirects to the next page.

The impact of this is pretty much Firebase Auth w/Provider: Calling getRedirectResult() always returns NULL user #3115, when the user does finally get redirected back to your app, they aren't signed in because the sessionStorage key doesn't exist.

The Firebase SDK appears to assume that getAuth() will never be called for the first time in close proximity to signInWithRedirect. I worry some other bugs might be lurking as a result of that assumption, but this is the issue facing our app.

[jbalidiong]

Hi @WestonThayer, thanks for bringing this to our attention. I was able to replicate the behavior. Let me check this with our Auth team or bring someone here that can provide more context about it. I’ll update this thread if I have any information to share.

[andylnkd]

Thanks, I am seeing a lot of page refreshes happen as well. I think its very closely related to this issue.

[prameshj]

Thanks for filing this @WestonThayer .. I am curious why the auth initialize is also done as part of a button click? If auth initialize happens at the time of loading and signInWithRedirect is called in an event handler (like button click), that will reduce the chance of this happening.

Having said that, I think we can await authInternal._initializationPromise; in

firebase-js-sdk/packages/auth/src/platform_browser/strategies/redirect.ts

Line 93 in 1625f7a

_assertInstanceOf(auth, provider, FederatedAuthProvider);

like we do in getRedirectResult -

firebase-js-sdk/packages/auth/src/platform_browser/strategies/redirect.ts

Line 258 in 1625f7a

await _castAuth(auth)._initializationPromise;

cc @sam-gc

[WestonThayer]

Thanks for the investigation all.

I am curious why the auth initialize is also done as part of a button click?

Moving initializeApp outside the click handler doesn't fix the issue, here's a forked codesandbox: https://codesandbox.io/s/firebase-auth-pending-redirect-bug-initoutside-16oin0.

The issue is the getAuth call being inside the click handler, which I would think is pretty common to have getAuth called "just in time" instead of having a singleton for the entire application? Nothing in the documentation made me think I was supposed to store its result in a singleton.

---

To expand on how we hit this in a real world example, we have a Next.js app, and for the most part each route is statically rendered to HTML, then hydrated & enriched on the client. For most routes, we set up an onAuthStateChanged on page load and update the UI accordingly, but we don't do that for /sign-in.

That choice was rooted in the dilemma of deciding what /sign-in's initial HTML should be. It couldn't know whether there was already a user signed in without implementing a custom cookie solution and SSRing the page. So the choice was between...

• the initial HTML being a blank/loading state, then waiting for onAuthStateChanged to tell the user they were already signed in or display the sign in form, and...
• the initial HTML being the sign in form, and properly handling the case where a user is already signed in, but is now requesting a new sign in

We went with the latter approach, which I can now understand might be at odds with Firebase Auth's design — especially since native apps don't really have the notion of supporting a /sign-in route that needs to be handled whether the user is signed in or out (I imagine a native app would always check for onAuthStateChanged on startup and only render a sign in button if the user isn't signed in).

In any case, because /sign-in doesn't request onAuthStateChanged on page load, the first call to getAuth doesn't happen until the user has already supplied their email and we've determined that they should be sent into an SSO flow.

[prameshj]

Thanks for the additional context.

the initial HTML being a blank/loading state, then waiting for onAuthStateChanged to tell the user they were already signed in or display the sign in form, and...

This is the pattern I was more familiar with and also what we do in the demo app -

firebase-js-sdk/packages/auth/demo/src/index.js

Line 1654 in 7e237cd

function initApp() {

I opened an internal bug to investigate the race condition issue - b/261039380

[prameshj]

I think I am able to repro the issue with the following changes to the demo app - https://github.com/firebase/firebase-js-sdk/compare/redirect-race2

If I await on the initialization promise in signInWithRedirect, the login upon demo app init is successful. Without that, it fails.

I will do a closer review of the code + add unit tests and send a PR.

[thomsn]

Work around rn with the signInWithPopup

[ricardogoncalves]

Any update on this issue?

[ricardogoncalves]

Not sure if this will help investigate the issue, but I see this error before Google redirects back to the app: XMLHttpRequest cannot load https://signaler-pa.googleapis.com/punctual/multi-watch/channel?VER=8&gsessionid=w2E6bB_0sjvW36er1TNLRWSqkNjliktW6RziP0Egrgg&key=AIzaSyBK4Jek59hl05JpbITG8AHxRTXZfrATQn8&RID=rpc&SID=qj7NFHeHzk5NR-aob5OTGQ&CI=0&AID=3&TYPE=xmlhttp&zx=ayv6j2i9e2vs&t=2 due to access control checks