Harden repeated login and auth state handling

src/authentication/firebase/firebase.ts

@@ -3,7 +3,11 @@
 // Note, currently using the "compat" version of firebase v9, which doesn't support treeshaking. No reason, just a TODO to upgrade to full v9 API.
 // See https://firebase.google.com/docs/web/modular-upgrade
 import firebase from "firebase/compat/app";
-import { connectParseServer } from "../../connection/ParseServerConnection";
+import {
+    connectParseServer,
+    hasActiveParseSession,
+} from "../../connection/ParseServerConnection";
+import { LoggedInUser } from "../../connection/LoggedInUser";
 import { getCookie } from "../../Utilities";
 import { isLogoutMode } from "../authentication";
 
@@ -98,6 +102,15 @@ async function getFirebaseAuthInternal() {
                 //     console.log("ConnectParseServer resolved with " + result)
                 // )
                 .catch((err) => {
+                    if (
+                        LoggedInUser.current?.email === user.email &&
+                        hasActiveParseSession(user.email!)
+                    ) {
+                        console.log(
+                            "*** Ignoring Parse login error because an existing Parse session is still active"
+                        );
+                        return;
+                    }
                     console.log(
                         "*** Signing out of firebase because of an error connecting to ParseServer"
                     );

src/components/User/UserMenu.tsx

@@ -1,6 +1,6 @@
 import { css } from "@emotion/react";
 
-import React, { useState, useEffect } from "react";
+import React, { useState, useEffect, useRef } from "react";
 import { Button, Menu, MenuItem } from "@material-ui/core";
 import loginIcon from "../../assets/NoUser.svg";
 // Note, currently using the "compat" version of firebase v9, which doesn't support treeshaking. No reason, just a TODO to upgrade to full v9 API.
@@ -45,6 +45,7 @@ export const UserMenu: React.FunctionComponent<IUserMenuProps> = (props) => {
     const [loggedInUser, setLoggedInUser] = useState<
         ReturnType<typeof firebase.auth>["currentUser"]
     >(null);
+    const loggedInUserRef = useRef(loggedInUser);
     const user = LoggedInUser.current;
 
     const history = useHistory(); // used to jump to My Books
@@ -57,6 +58,10 @@ export const UserMenu: React.FunctionComponent<IUserMenuProps> = (props) => {
         setShowTroubleshootingStuff,
     ] = useShowTroubleshootingStuff();
 
+    useEffect(() => {
+        loggedInUserRef.current = loggedInUser;
+    }, [loggedInUser]);
+
     /*useEffect(() => {
         firebase
             .auth()
@@ -85,44 +90,43 @@ export const UserMenu: React.FunctionComponent<IUserMenuProps> = (props) => {
             });
     }, []);*/
 
-    useEffect(
-        () =>
-            firebaseAuthStateChanged(() => {
-                const kSecondsPerDay = 24 * 60 * 60;
-                // If someone is now logged in, and it wasn't who we previously had logged
-                // in (or more likely no one was previously logged in), report login
-                if (
-                    firebase.auth()?.currentUser?.email &&
-                    loggedInUser?.email !== firebase.auth()?.currentUser?.email
-                ) {
-                    setCookie("loggedIn", "true", {
-                        maxAge: 360 * kSecondsPerDay,
-                        path: "/",
-                    });
-                    // In previous blorg, we tracked the user name, but we're avoiding PII now.
-                    track("Log In", {});
-                }
-                // If no one is now logged in and someone was, report logout.
-                // Review: we won't (and probably can't?) get a report when the browser
-                // or tab shuts down and similar.
-                if (
-                    !firebase.auth()?.currentUser?.email &&
-                    loggedInUser?.email
-                ) {
-                    setCookie("loggedIn", "false", {
-                        maxAge: 360 * kSecondsPerDay,
-                        path: "/",
-                    });
-                    track("Log Out", {});
-                }
-                setLoggedInUser(firebase.auth().currentUser);
-                // console.log(
-                //     "$$$$$$$$$$$$ onAuthStateChanged " +
-                //         firebase.auth().currentUser
-                // );
-            }),
-        [loggedInUser, setCookie]
-    );
+    useEffect(() => {
+        firebaseAuthStateChanged(() => {
+            const kSecondsPerDay = 24 * 60 * 60;
+            const currentUser = firebase.auth().currentUser;
+            const previousLoggedInUser = loggedInUserRef.current;
+
+            // If someone is now logged in, and it wasn't who we previously had logged
+            // in (or more likely no one was previously logged in), report login
+            if (
+                currentUser?.email &&
+                previousLoggedInUser?.email !== currentUser.email
+            ) {
+                setCookie("loggedIn", "true", {
+                    maxAge: 360 * kSecondsPerDay,
+                    path: "/",
+                });
+                // In previous blorg, we tracked the user name, but we're avoiding PII now.
+                track("Log In", {});
+            }
+            // If no one is now logged in and someone was, report logout.
+            // Review: we won't (and probably can't?) get a report when the browser
+            // or tab shuts down and similar.
+            if (!currentUser?.email && previousLoggedInUser?.email) {
+                setCookie("loggedIn", "false", {
+                    maxAge: 360 * kSecondsPerDay,
+                    path: "/",
+                });
+                track("Log Out", {});
+            }
+            loggedInUserRef.current = currentUser;
+            setLoggedInUser(currentUser);
+            // console.log(
+            //     "$$$$$$$$$$$$ onAuthStateChanged " +
+            //         firebase.auth().currentUser
+            // );
+        });
+    }, [setCookie]);
     useEffect(() => {
         getCurrentUser().then((currentUser) => {
             if (currentUser !== null) {

src/connection/LoggedInUser.ts

@@ -1,5 +1,5 @@
 import { observable, makeObservable, autorun } from "mobx";
-import { useState } from "react";
+import { useEffect, useState } from "react";
 
 // This is currently just a subset of what ParseServer returns,
 // so don't go renaming anything
@@ -47,23 +47,29 @@ export const LoggedInUser: UserHolder = new UserHolder();
 // OR USE A MORE SPECIFIC HOOK LIKE useGetUserIsModerator().
 export function useGetLoggedInUser(): User | undefined {
     const [user, setUser] = useState(LoggedInUser.current);
-    autorun(() => {
-        if (LoggedInUser.current !== user) {
+    useEffect(() => {
+        const dispose = autorun(() => {
             setUser(LoggedInUser.current);
-        }
-    });
+        });
+
+        return dispose;
+    }, []);
+
     return user;
 }
 
 export function useGetUserIsModerator(): boolean | undefined {
     const [isModerator, setUserIsModerator] = useState(
         LoggedInUser.current?.moderator
     );
-    autorun(() => {
-        if (LoggedInUser.current?.moderator !== isModerator) {
+    useEffect(() => {
+        const dispose = autorun(() => {
             setUserIsModerator(LoggedInUser.current?.moderator);
-        }
-    });
+        });
+
+        return dispose;
+    }, []);
+
     return isModerator;
 }
 

src/connection/ParseServerConnection.ts

@@ -19,6 +19,9 @@ const connectionsByDataSource: Record<DataSource, IParseConnection> = {
     [DataSource.Local]: createParseConnection(DataSource.Local),
 };
 
+let activeParseLoginPromise: Promise<any> | undefined;
+let activeParseLoginEmail: string | undefined;
+
 export function getConnection(): IParseConnection {
     const result = connectionsByDataSource[getDataSource()];
 
@@ -42,6 +45,25 @@ export function getConnection(): IParseConnection {
     return result;
 }
 
+function getHeadersForParseLogin(connection: IParseConnection) {
+    const headers = { ...connection.headers };
+    delete headers["X-Parse-Session-Token"];
+    return headers;
+}
+
+export function hasActiveParseSession(emailAddress?: string): boolean {
+    const currentUser = LoggedInUser.current;
+    if (!currentUser) {
+        return false;
+    }
+
+    if (emailAddress && currentUser.email !== emailAddress) {
+        return false;
+    }
+
+    return !!getConnection().headers["X-Parse-Session-Token"];
+}
+
 // This should only be called when there is a current user logged in.
 // It attempts to retrieve a role with name moderator and this user as
 // one of its users. If it gets one, this establishes that this user
@@ -84,8 +106,18 @@ export async function connectParseServer(
     // We only pass it through to the editor login POST; Parse itself doesn't use it.
     photoUrl?: string | null
 ) {
-    return new Promise<any>((resolve, reject) => {
-        const connection = getConnection();
+    if (
+        activeParseLoginPromise !== undefined &&
+        activeParseLoginEmail === emailAddress
+    ) {
+        return activeParseLoginPromise;
+    }
+
+    const connection = getConnection();
+    const loginHeaders = getHeadersForParseLogin(connection);
+    const alreadyHadSession = hasActiveParseSession(emailAddress);
+
+    const loginPromise = new Promise<any>((resolve, reject) => {
         // Run a cloud code function (bloomLink) which,
         // if this is a new Firebase user with the email of a known parse server user, will link them.
         // It will do nothing if
@@ -102,7 +134,7 @@ export async function connectParseServer(
                 },
 
                 {
-                    headers: connection.headers,
+                    headers: loginHeaders,
                 }
             )
             .then(() => {
@@ -120,7 +152,7 @@ export async function connectParseServer(
                         },
 
                         {
-                            headers: connection.headers,
+                            headers: loginHeaders,
                         }
                     )
                     .then((usersResult) => {
@@ -148,7 +180,7 @@ export async function connectParseServer(
                             resolve(usersResult.data);
                             checkIfUserIsModerator();
                         } else {
-                            failedToLoginInToParseServer();
+                            failedToLoginInToParseServer(!alreadyHadSession);
                             // Reject the Promise returned by `connectParseServer()`.
                             // This lets callers stop the login flow (for example, `firebase.ts` catches this and
                             // signs the user out of Firebase) rather than silently continuing.
@@ -160,28 +192,47 @@ export async function connectParseServer(
                         }
                     })
                     .catch((err) => {
-                        failedToLoginInToParseServer();
+                        failedToLoginInToParseServer(!alreadyHadSession);
                         reject(err);
                     });
             })
             .catch((err) => {
                 console.log(
                     "The `Bloom Link` call failed:" + JSON.stringify(err)
                 );
-                failedToLoginInToParseServer();
+                failedToLoginInToParseServer(!alreadyHadSession);
                 reject(err);
             });
     });
+
+    const trackedPromise = loginPromise.finally(() => {
+        if (activeParseLoginPromise === trackedPromise) {
+            activeParseLoginPromise = undefined;
+            activeParseLoginEmail = undefined;
+        }
+    });
+
+    activeParseLoginPromise = trackedPromise;
+    activeParseLoginEmail = emailAddress;
+
+    return trackedPromise;
 }
-function failedToLoginInToParseServer() {
-    Sentry.captureException(
-        new Error(
-            "Login to parse server failed after successful firebase login"
-        )
-    );
-    alert(
-        "Oops, something went wrong when trying to log you into our database."
-    );
+
+function failedToLoginInToParseServer(showAlert: boolean) {
+    // Only report/alert when this is a "real" login failure. When the user already
+    // had an active Parse session (showAlert === false), a failed repeat login is
+    // benign: we neither nag the user with an alert nor report a spurious error to
+    // Sentry (which would be noise that could mask genuine login failures).
+    if (showAlert) {
+        Sentry.captureException(
+            new Error(
+                "Login to parse server failed after successful firebase login"
+            )
+        );
+        alert(
+            "Oops, something went wrong when trying to log you into our database."
+        );
+    }
 }
 // Remove the parse session header when the user logs out.
 // This is probably redundant since currently the logout process reloads the whole page.