From 62f246dbd35a2f7752c282e94d6521a6ba6dab52 Mon Sep 17 00:00:00 2001 From: Andrew Polk Date: Fri, 17 Jul 2026 15:56:19 -0700 Subject: [PATCH 1/2] Harden repeated login and auth state handling --- src/authentication/firebase/firebase.ts | 15 ++++- src/components/User/UserMenu.tsx | 82 +++++++++++++------------ src/connection/LoggedInUser.ts | 24 +++++--- src/connection/ParseServerConnection.ts | 69 +++++++++++++++++---- 4 files changed, 130 insertions(+), 60 deletions(-) diff --git a/src/authentication/firebase/firebase.ts b/src/authentication/firebase/firebase.ts index 115f55b2..8057fa72 100644 --- a/src/authentication/firebase/firebase.ts +++ b/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" ); diff --git a/src/components/User/UserMenu.tsx b/src/components/User/UserMenu.tsx index d87fbaeb..4e815501 100644 --- a/src/components/User/UserMenu.tsx +++ b/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 = (props) => { const [loggedInUser, setLoggedInUser] = useState< ReturnType["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 = (props) => { setShowTroubleshootingStuff, ] = useShowTroubleshootingStuff(); + useEffect(() => { + loggedInUserRef.current = loggedInUser; + }, [loggedInUser]); + /*useEffect(() => { firebase .auth() @@ -85,44 +90,43 @@ export const UserMenu: React.FunctionComponent = (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) { diff --git a/src/connection/LoggedInUser.ts b/src/connection/LoggedInUser.ts index c4bb8c86..ae4c0dbe 100644 --- a/src/connection/LoggedInUser.ts +++ b/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,11 +47,14 @@ 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; } @@ -59,11 +62,14 @@ 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; } diff --git a/src/connection/ParseServerConnection.ts b/src/connection/ParseServerConnection.ts index b01cb4a6..8746f187 100644 --- a/src/connection/ParseServerConnection.ts +++ b/src/connection/ParseServerConnection.ts @@ -19,6 +19,9 @@ const connectionsByDataSource: Record = { [DataSource.Local]: createParseConnection(DataSource.Local), }; +let activeParseLoginPromise: Promise | 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((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((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,7 +192,7 @@ export async function connectParseServer( } }) .catch((err) => { - failedToLoginInToParseServer(); + failedToLoginInToParseServer(!alreadyHadSession); reject(err); }); }) @@ -168,20 +200,35 @@ export async function connectParseServer( 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() { + +function failedToLoginInToParseServer(showAlert: boolean) { 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." - ); + if (showAlert) { + 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. From da1d44f6b1e60c3aa075fb614d1968b60a28ccf7 Mon Sep 17 00:00:00 2001 From: Andrew Polk Date: Fri, 17 Jul 2026 16:21:53 -0700 Subject: [PATCH 2/2] Suppress spurious Sentry report on benign repeated-login failure When the user already has an active Parse session, a failed repeat login is benign, so we already skip the user-facing alert. Skip the Sentry exception in that same case too, so an expected condition isn't reported as an error that could mask genuine login failures. (Greptile P2.) Co-Authored-By: Claude Opus 4.8 (1M context) --- src/connection/ParseServerConnection.ts | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/connection/ParseServerConnection.ts b/src/connection/ParseServerConnection.ts index 8746f187..f9f062d3 100644 --- a/src/connection/ParseServerConnection.ts +++ b/src/connection/ParseServerConnection.ts @@ -219,12 +219,16 @@ export async function connectParseServer( } function failedToLoginInToParseServer(showAlert: boolean) { - Sentry.captureException( - new Error( - "Login to parse server failed after successful firebase login" - ) - ); + // 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." );