Skip to content

[WIP] Fix auth infinite loop with multi-tabs#262

Open
mikkopaderes wants to merge 2 commits into
masterfrom
fix-auth-restoration-with-multi-tabs
Open

[WIP] Fix auth infinite loop with multi-tabs#262
mikkopaderes wants to merge 2 commits into
masterfrom
fix-auth-restoration-with-multi-tabs

Conversation

@mikkopaderes
Copy link
Copy Markdown
Owner

No description provided.

@mikkopaderes mikkopaderes self-assigned this Nov 25, 2022
@mikkopaderes mikkopaderes changed the title Fix auth infinite loop with multi-tabs [WIP] Fix auth infinite loop with multi-tabs Nov 27, 2022
@roomman
Copy link
Copy Markdown

roomman commented Nov 28, 2022

Hi @mikkopaderes, you may already have spotted this issue but I was playing about with the "cherryPicker" idea today, using the latest commit on this PR.

  public async restore(): Promise<AuthenticatedData> {
    return new Promise((resolve, reject) => {
      const auth = getAuth();

      const unsubscribe = onAuthStateChanged(
        auth,
        async (user) => {
          unsubscribe();

          if (user) {
            resolve({ user: parseCherryPickedUser(user) });
          } else {
            getRedirectResult(auth)
              .then((credential) => {
                if (credential) {
                  resolve({ user: credential.user });
                } else {
                  reject();
                }
              })
              .catch(() => {
                reject();
              });
          }
        },
        () => {
          unsubscribe();
          reject();
        }
      );
    });
  }

I opened a session and logged in fine. I then opened two additional tabs on the same route. I logged out in one and all three sessions were invalidated. Then I attempted to login again but that was immediately invalidated by the others. I tried again and all three sessions fired up.

I logged everything out and discovered that on the initial attempt, onAuthStateChanged and getRedirectResult in the second and third sessions is returning null, so the promise is rejected.

@mikkopaderes
Copy link
Copy Markdown
Owner Author

@roomman yeah, noticed that which is what's keeping this PR from being merged. There's some relevant info here so I'm trying to see if that would work for us.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants