From 9532f4851511257b4e175be5d78de92189dd8c7b Mon Sep 17 00:00:00 2001 From: sua yoo Date: Tue, 14 Feb 2023 18:35:21 -0800 Subject: [PATCH] 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 | 58 ++++++++++++++-------- frontend/src/index.ts | 9 +++- frontend/src/utils/AuthService.test.ts | 61 ++++++++++++++++++++++++ frontend/src/utils/AuthService.ts | 66 ++++++++++++++------------ 4 files changed, 142 insertions(+), 52 deletions(-) create mode 100644 frontend/src/utils/AuthService.test.ts diff --git a/frontend/src/index.test.ts b/frontend/src/index.test.ts index 33e5fa53..6ec3664a 100644 --- a/frontend/src/index.test.ts +++ b/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(""); + expect(el).lightDom.descendants("btrix-home"); + }); + + it("renders when `AuthService.initSessionStorage` rejects", async () => { + stub(AuthService, "initSessionStorage").returns(Promise.reject()); + const el = await fixture(""); + 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") diff --git a/frontend/src/index.ts b/frontend/src/index.ts index b02e128d..2acdfd31 100644 --- a/frontend/src/index.ts +++ b/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; diff --git a/frontend/src/utils/AuthService.test.ts b/frontend/src/utils/AuthService.test.ts new file mode 100644 index 00000000..91cdd1f3 --- /dev/null +++ b/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); + }); + }); +}); diff --git a/frontend/src/utils/AuthService.ts b/frontend/src/utils/AuthService.ts index 70df8641..b32d3575 100644 --- a/frontend/src/utils/AuthService.ts +++ b/frontend/src/utils/AuthService.ts @@ -27,10 +27,6 @@ export interface LoggedInEvent 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 { - 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({ - 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({ + 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({ auth: true }) - ); this.persist(auth); this.startFreshnessCheck(); } logout() { - window.localStorage.removeItem(AuthService.storageKey); this.cancelFreshnessCheck(); this.revoke(); }