Merge pull request #606 from BloomBooks/BL14503_NoEmailNoLogin

Don't report successful login more than once or if no email (BL-14503) (#606)

Author: JohnThomson

package.json

@@ -3,7 +3,8 @@
     "private": true,
     "type": "module",
     "scripts": {
-        "dev": "yarn copyInBloomPlayer && vite",
+        "//": "default port is 5173 which is the same for BloomDesktop and can create conflicts",
+        "dev": "yarn copyInBloomPlayer && vite --port 5174",
         "serve": "vite preview",
         "build": "yarn copyInBloomPlayer && vite build",
         "build:ci": "set CI=true&& yarn build",

src/connection/ParseServerConnection.ts

@@ -113,8 +113,7 @@ function checkIfUserIsModerator() {
 
 export async function connectParseServer(
     jwtToken: string,
-    userId: string
-    //,returnParseUser: (user: any) => void
+    emailAddress: string
 ) {
     return new Promise<any>((resolve, reject) => {
         const connection = getConnection();
@@ -130,7 +129,7 @@ export async function connectParseServer(
                 `${connection.url}functions/bloomLink`,
                 {
                     token: jwtToken,
-                    id: userId,
+                    id: emailAddress,
                 },
 
                 {
@@ -144,32 +143,49 @@ export async function connectParseServer(
                         `${connection.url}users`,
                         {
                             authData: {
-                                bloom: { token: jwtToken, id: userId },
+                                bloom: { token: jwtToken, id: emailAddress },
                             },
-                            username: userId,
-                            email: userId, // needed in case we are creating a new user
+                            username: emailAddress,
+                            // Parse requires an `email` field when creating a new _User.
+                            email: emailAddress,
                         },
 
                         {
                             headers: connection.headers,
                         }
                     )
                     .then((usersResult) => {
-                        if (usersResult.data.sessionToken) {
+                        // We require BOTH a session token and an email.
+                        // We don't actually know why we would ever get here without either.
+                        // But we were sending posts to Bloom with a missing email value in the payload,
+                        // which caused Bloom's `/bloom/api/external/login` handler to throw a runtime exception. See BL-14503.
+                        // I don't see any reason to pretend a non-editor login was successful if email
+                        // is missing, either. And it simplifies the code to just check up front.
+                        if (
+                            usersResult.data.sessionToken &&
+                            usersResult.data.email
+                        ) {
                             LoggedInUser.current = new User(usersResult.data);
-                            //Object.assign(CurrentUser, usersResult.data);
                             connection.headers["X-Parse-Session-Token"] =
                                 usersResult.data.sessionToken;
 
                             if (isForEditor()) {
                                 informEditorOfSuccessfulLogin(usersResult.data);
                             }
 
-                            //console.log("Got ParseServer Session ID");
                             resolve(usersResult.data);
-                            //returnParseUser(result.data);
                             checkIfUserIsModerator();
-                        } else failedToLoginInToParseServer();
+                        } else {
+                            failedToLoginInToParseServer();
+                            // 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.
+                            reject(
+                                new Error(
+                                    "Missing sessionToken or email in usersResult.data"
+                                )
+                            );
+                        }
                     })
                     .catch((err) => {
                         failedToLoginInToParseServer();

src/editor.ts

@@ -14,12 +14,35 @@ export function isForEditor() {
     return window.location.pathname.includes("login-for-editor");
 }
 
+// Tracks a sessionToken we've already notified Bloom (editor) about.
+//
+// Why this exists:
+// - In theory, `connectParseServer()` can be triggered more than once during a single browser session
+//   (e.g. auth state callbacks can fire more than once, the user can reload the tab, etc.).
+// - We have observed multiple posts to Bloom for a single "login-for-editor" attempt.
+//   In BL-14503, Bloom's `/bloom/api/external/login` endpoint logged multiple
+//   "External login successful" lines for the same attempt, and also threw when the payload
+//   was missing `email`. Suppressing duplicate notifications on the Blorg side helps reduce
+//   noise and was part of an attempt to bolster error handling.
+//
+// Behavior:
+// - Set before sending to suppress duplicates while the POST is in-flight.
+// - Cleared on failure. I don't think this much matter, but it seems cleaner.
+let notifiedSessionToken: string | undefined;
+
 export function informEditorOfSuccessfulLogin(userData: any) {
+    if (notifiedSessionToken === userData.sessionToken) {
+        return;
+    }
+
     const postData = {
         sessionToken: userData.sessionToken,
         email: userData.email,
         userId: userData.objectId,
     };
+
+    notifiedSessionToken = userData.sessionToken;
+
     axios
         .post(`${getEditorApiUrl()}login`, postData)
         .then(() => {
@@ -29,6 +52,7 @@ export function informEditorOfSuccessfulLogin(userData: any) {
         .catch((err) => {
             LoggedInUser.current!.informEditorResult =
                 IInformEditorResult.Failure;
+            notifiedSessionToken = undefined;
 
             console.error("Unable to inform editor of successful login.");
             console.error(err);