Fix app not rendering with bad auth storage states (#597)

* render even if session store throws

* handle after timeout

* remove localstorage key

* update tests

frontend/src/index.test.ts

@@ -1,33 +1,18 @@
 import { spy, stub, mock, restore } from "sinon";
 import { fixture, expect } from "@open-wc/testing";
-// import { expect } from "@esm-bundle/chai";
 
 import AuthService from "./utils/AuthService";
 import { App } from "./index";
 
 describe("browsertrix-app", () => {
   beforeEach(() => {
-    stub(window.sessionStorage, "setItem");
-    stub(App.prototype, "getUserInfo").callsFake(() =>
-      Promise.resolve({
-        id: "test_id",
-        email: "test-user@example.com",
-        name: "Test User",
-        is_verified: false,
-        is_superuser: false,
-        orgs: [
-          {
-            id: "test_org_id",
-            name: "test org",
-            role: 10,
-            email: "test@org.org",
-          },
-        ],
-      })
-    );
+    AuthService.broadcastChannel = new BroadcastChannel(AuthService.storageKey);
+    window.sessionStorage.clear();
+    stub(window.history, "pushState");
   });
 
   afterEach(() => {
+    AuthService.broadcastChannel.close();
     restore();
   });
 
@@ -36,6 +21,24 @@ describe("browsertrix-app", () => {
     expect(el).instanceOf(App);
   });
 
+  it("renders home when authenticated", async () => {
+    stub(AuthService, "initSessionStorage").returns(
+      Promise.resolve({
+        headers: { Authorization: "_fake_headers_" },
+        tokenExpiresAt: 0,
+        username: "test-auth@example.com",
+      })
+    );
+    const el = await fixture("<browsertrix-app></browsertrix-app>");
+    expect(el).lightDom.descendants("btrix-home");
+  });
+
+  it("renders when `AuthService.initSessionStorage` rejects", async () => {
+    stub(AuthService, "initSessionStorage").returns(Promise.reject());
+    const el = await fixture("<browsertrix-app></browsertrix-app>");
+    expect(el).lightDom.descendants("btrix-home");
+  });
+
   // TODO move tests to AuthService
   it("sets auth state from session storage", async () => {
     stub(AuthService.prototype, "startFreshnessCheck");
@@ -58,6 +61,23 @@ describe("browsertrix-app", () => {
   });
 
   it("sets user info", async () => {
+    stub(App.prototype, "getUserInfo").callsFake(() =>
+      Promise.resolve({
+        id: "test_id",
+        email: "test-user@example.com",
+        name: "Test User",
+        is_verified: false,
+        is_superuser: false,
+        orgs: [
+          {
+            id: "test_org_id",
+            name: "test org",
+            role: 10,
+            email: "test@org.org",
+          },
+        ],
+      })
+    );
     stub(AuthService.prototype, "startFreshnessCheck");
     stub(window.sessionStorage, "getItem").callsFake((key) => {
       if (key === "btrix.auth")

frontend/src/index.ts

@@ -12,7 +12,7 @@ import type { OrgTab } from "./pages/org";
 import type { NotifyEvent, NavigateEvent } from "./utils/LiteElement";
 import LiteElement, { html } from "./utils/LiteElement";
 import APIRouter from "./utils/APIRouter";
-import AuthService from "./utils/AuthService";
+import AuthService, { AuthState } from "./utils/AuthService";
 import type { LoggedInEvent } from "./utils/AuthService";
 import type { ViewState } from "./utils/APIRouter";
 import type { CurrentUser, UserOrg } from "./types/user";
@@ -91,7 +91,12 @@ export class App extends LiteElement {
   private selectedOrgId?: string;
 
   async connectedCallback() {
-    const authState = await AuthService.initSessionStorage();
+    let authState: AuthState = null;
+    try {
+      authState = await AuthService.initSessionStorage();
+    } catch (e: any) {
+      console.debug(e);
+    }
     this.syncViewState();
     if (this.viewState.route === "org") {
       this.selectedOrgId = this.viewState.params.orgId;

frontend/src/utils/AuthService.test.ts

@@ -0,0 +1,61 @@
+import { spy, stub, mock, restore } from "sinon";
+import { fixture, expect } from "@open-wc/testing";
+
+import AuthService from "./AuthService";
+
+describe("AuthService", () => {
+  beforeEach(() => {
+    AuthService.broadcastChannel = new BroadcastChannel(AuthService.storageKey);
+    window.sessionStorage.clear();
+    stub(window.history, "pushState");
+  });
+
+  afterEach(() => {
+    AuthService.broadcastChannel.close();
+    restore();
+  });
+
+  describe("#initSessionStorage", () => {
+    it("returns auth in session storage", async () => {
+      stub(window.sessionStorage, "getItem").returns(
+        JSON.stringify({
+          headers: { Authorization: "_fake_headers_" },
+          tokenExpiresAt: "_fake_tokenExpiresAt_",
+          username: "test-auth@example.com",
+        })
+      );
+      const result = await AuthService.initSessionStorage();
+      expect(result).to.deep.equal({
+        headers: { Authorization: "_fake_headers_" },
+        tokenExpiresAt: "_fake_tokenExpiresAt_",
+        username: "test-auth@example.com",
+      });
+    });
+    it("returns auth from another tab", async () => {
+      stub(window.sessionStorage, "getItem");
+      const otherTabChannel = new BroadcastChannel(AuthService.storageKey);
+      otherTabChannel.addEventListener("message", () => {
+        otherTabChannel.postMessage({
+          name: "responding_auth",
+          auth: {
+            headers: { Authorization: "_fake_headers_from_tab_" },
+            tokenExpiresAt: "_fake_tokenExpiresAt_from_tab_",
+            username: "test-auth@example.com_from_tab_",
+          },
+        });
+      });
+      const result = await AuthService.initSessionStorage();
+      expect(result).to.deep.equal({
+        headers: { Authorization: "_fake_headers_from_tab_" },
+        tokenExpiresAt: "_fake_tokenExpiresAt_from_tab_",
+        username: "test-auth@example.com_from_tab_",
+      });
+      otherTabChannel.close();
+    });
+    it("resolves without stored auth or another tab", async () => {
+      stub(window.sessionStorage, "getItem");
+      const result = await AuthService.initSessionStorage();
+      expect(result).to.equal(null);
+    });
+  });
+});

frontend/src/utils/AuthService.ts

@@ -27,10 +27,6 @@ export interface LoggedInEvent<T = LoggedInEventDetail> extends CustomEvent {
   readonly detail: T;
 }
 
-type HasAuthStorageData = {
-  auth: boolean;
-};
-
 type AuthRequestEventData = {
   name: "requesting_auth";
 };
@@ -171,10 +167,6 @@ export default class AuthService {
       }
     );
 
-    window.addEventListener("beforeunload", () => {
-      window.localStorage.removeItem(AuthService.storageKey);
-    });
-
     return authState;
   }
 
@@ -192,26 +184,43 @@ export default class AuthService {
    * Retrieve shared session from another tab/window
    **/
   private static async getSharedSessionAuth(): Promise<AuthState> {
-    return new Promise((resolve) => {
+    const broadcastPromise = new Promise((resolve) => {
       // Check if there's any authenticated tabs
-      const value = window.localStorage.getItem(AuthService.storageKey);
-      if (value && (JSON.parse(value) as HasAuthStorageData).auth) {
-        // Ask for auth
-        AuthService.broadcastChannel.postMessage(<AuthRequestEventData>{
-          name: "requesting_auth",
-        });
-        // Wait for another tab to respond
-        const cb = ({ data }: any) => {
-          if (data.name === "responding_auth") {
-            AuthService.broadcastChannel.removeEventListener("message", cb);
-            resolve(data.auth);
-          }
-        };
-        AuthService.broadcastChannel.addEventListener("message", cb);
-      } else {
-        resolve(null);
-      }
+      AuthService.broadcastChannel.postMessage(<AuthRequestEventData>{
+        name: "requesting_auth",
+      });
+      // Wait for another tab to respond
+      const cb = ({ data }: any) => {
+        if (data.name === "responding_auth") {
+          AuthService.broadcastChannel.removeEventListener("message", cb);
+          resolve(data.auth);
+        }
+      };
+      AuthService.broadcastChannel.addEventListener("message", cb);
     });
+    // Ensure that `getSharedSessionAuth` is resolved within a reasonable
+    // timeframe, even if another window/tab doesn't respond:
+    const timeoutPromise = new Promise((resolve) => {
+      window.setTimeout(() => {
+        resolve(null);
+      }, 10);
+    });
+
+    return Promise.race([broadcastPromise, timeoutPromise]).then(
+      (value: any) => {
+        try {
+          if (value.username && value.headers && value.tokenExpiresAt) {
+            return value;
+          }
+        } catch {
+          return null;
+        }
+      },
+      (error) => {
+        console.debug(error);
+        return null;
+      }
+    );
   }
 
   constructor() {
@@ -227,16 +236,11 @@ export default class AuthService {
   }
 
   saveLogin(auth: Auth) {
-    window.localStorage.setItem(
-      AuthService.storageKey,
-      JSON.stringify(<HasAuthStorageData>{ auth: true })
-    );
     this.persist(auth);
     this.startFreshnessCheck();
   }
 
   logout() {
-    window.localStorage.removeItem(AuthService.storageKey);
     this.cancelFreshnessCheck();
     this.revoke();
   }