diff --git a/frontend/src/index.test.ts b/frontend/src/index.test.ts index e09bc96a8d..843630e4ee 100644 --- a/frontend/src/index.test.ts +++ b/frontend/src/index.test.ts @@ -2,6 +2,7 @@ import { expect, fixture } from "@open-wc/testing"; import { html } from "lit"; import { restore, stub } from "sinon"; +import { APIController } from "./controllers/api"; import { NavigateController } from "./controllers/navigate"; import { NotifyController } from "./controllers/notify"; import { type AppSettings } from "./utils/app"; @@ -46,20 +47,20 @@ const mockAppSettings: AppSettings = { salesEmail: "", supportEmail: "", localesEnabled: ["en", "es"], + numBrowsersPerInstance: 1, }; describe("browsertrix-app", () => { beforeEach(() => { AppStateService.resetAll(); - AuthService.broadcastChannel = new BroadcastChannel(AuthService.storageKey); window.sessionStorage.clear(); window.localStorage.clear(); stub(window.history, "pushState"); stub(NotifyController.prototype, "toast"); + stub(APIController.prototype, "fetch"); }); afterEach(() => { - AuthService.broadcastChannel.close(); restore(); }); @@ -69,7 +70,7 @@ describe("browsertrix-app", () => { }); it("don't block render if settings aren't defined", async () => { - stub(AuthService, "initSessionStorage").returns( + stub(AuthService.prototype, "initSessionStorage").returns( Promise.resolve({ headers: { Authorization: "_fake_headers_" }, tokenExpiresAt: 0, @@ -85,16 +86,13 @@ describe("browsertrix-app", () => { }); it("renders 404 when not in org", async () => { - stub(AuthService, "initSessionStorage").returns( - Promise.resolve({ - headers: { Authorization: "_fake_headers_" }, - tokenExpiresAt: 0, - username: "test-auth@example.com", - }), + stub(AuthService.prototype, "initSessionStorage").returns( + Promise.resolve(mockAuth), ); // @ts-expect-error checkFreshness is private stub(AuthService.prototype, "checkFreshness"); + AppStateService.updateAuth(mockAuth); AppStateService.updateUser( formatAPIUser({ ...mockAPIUser, @@ -117,16 +115,13 @@ describe("browsertrix-app", () => { role: 10, }; - stub(AuthService, "initSessionStorage").returns( - Promise.resolve({ - headers: { Authorization: "_fake_headers_" }, - tokenExpiresAt: 0, - username: "test-auth@example.com", - }), + stub(AuthService.prototype, "initSessionStorage").returns( + Promise.resolve(mockAuth), ); // @ts-expect-error checkFreshness is private stub(AuthService.prototype, "checkFreshness"); + AppStateService.updateAuth(mockAuth); AppStateService.updateUser( formatAPIUser({ ...mockAPIUser, @@ -142,7 +137,9 @@ describe("browsertrix-app", () => { }); it("renders log in when not authenticated", async () => { - stub(AuthService, "initSessionStorage").returns(Promise.resolve(null)); + stub(AuthService.prototype, "initSessionStorage").returns( + Promise.resolve(null), + ); // @ts-expect-error checkFreshness is private stub(AuthService.prototype, "checkFreshness"); stub(NavigateController, "createNavigateEvent").callsFake( @@ -158,33 +155,12 @@ describe("browsertrix-app", () => { expect(el.shadowRoot?.querySelector("btrix-log-in")).to.exist; }); - // TODO move tests to AuthService - it("sets auth state from session storage", async () => { - stub(AuthService.prototype, "startFreshnessCheck").callsFake(() => {}); - stub(window.sessionStorage, "getItem").callsFake((key) => { - if (key === "btrix.auth") - return JSON.stringify({ - headers: { Authorization: "_fake_headers_" }, - tokenExpiresAt: 0, - username: "test-auth@example.com", - }); - return null; - }); - const el = await fixture(""); - - expect(el.authService.authState).to.eql({ - headers: { Authorization: "_fake_headers_" }, - tokenExpiresAt: 0, - username: "test-auth@example.com", - }); - }); - it("sets user info", async () => { stub(App.prototype, "getUserInfo").callsFake(async () => Promise.resolve(mockAPIUser), ); stub(AuthService.prototype, "startFreshnessCheck").callsFake(() => {}); - stub(AuthService, "initSessionStorage").callsFake(async () => + stub(AuthService.prototype, "initSessionStorage").callsFake(async () => Promise.resolve({ headers: { Authorization: "_fake_headers_" }, tokenExpiresAt: 0, @@ -203,7 +179,7 @@ describe("browsertrix-app", () => { Promise.resolve(mockAPIUser), ); stub(AuthService.prototype, "startFreshnessCheck").callsFake(() => {}); - stub(AuthService, "initSessionStorage").callsFake(async () => + stub(AuthService.prototype, "initSessionStorage").callsFake(async () => Promise.resolve({ headers: { Authorization: "_fake_headers_" }, tokenExpiresAt: 0, @@ -227,6 +203,7 @@ describe("browsertrix-app", () => { slug: id, role: 10, }; + AppStateService.updateAuth(mockAuth); AppStateService.updateUser( formatAPIUser({ ...mockAPIUser, @@ -235,12 +212,8 @@ describe("browsertrix-app", () => { ); stub(App.prototype, "getLocationPathname").callsFake(() => `/orgs/${id}`); stub(AuthService.prototype, "startFreshnessCheck").callsFake(() => {}); - stub(AuthService, "initSessionStorage").callsFake(async () => - Promise.resolve({ - headers: { Authorization: "_fake_headers_" }, - tokenExpiresAt: 0, - username: "test-auth@example.com", - }), + stub(AuthService.prototype, "initSessionStorage").callsFake(async () => + Promise.resolve(mockAuth), ); const el = await fixture(""); diff --git a/frontend/src/index.ts b/frontend/src/index.ts index f159b684da..14e360349a 100644 --- a/frontend/src/index.ts +++ b/frontend/src/index.ts @@ -26,7 +26,6 @@ import type { UserInfo, UserOrg } from "./types/user"; import { pageView, type AnalyticsTrackProps } from "./utils/analytics"; import { type ViewState } from "./utils/APIRouter"; import AuthService, { - type AuthEventDetail, type LoggedInEventDetail, type NeedLoginEventDetail, } from "./utils/AuthService"; @@ -34,7 +33,6 @@ import AuthService, { import { BtrixElement } from "@/classes/BtrixElement"; import type { NavigateEventDetail } from "@/controllers/navigate"; import type { NotifyEventDetail } from "@/controllers/notify"; -import { type Auth } from "@/types/auth"; import { translatedLocales, type TranslatedLocaleEnum, @@ -139,14 +137,13 @@ export class App extends BtrixElement { async connectedCallback() { let authState: AuthService["authState"] = null; try { - authState = await AuthService.initSessionStorage(); + authState = await this.authService.initSessionStorage(); } catch (e) { console.debug(e); } + this.syncViewState(); - if (authState) { - this.authService.saveLogin(authState); - } + if (authState && !this.userInfo) { void this.fetchAndUpdateUserInfo(); } @@ -162,8 +159,6 @@ export class App extends BtrixElement { window.addEventListener("popstate", () => { this.syncViewState(); }); - - this.startSyncBrowserTabs(); } private attachUserGuideListeners() { @@ -1085,6 +1080,7 @@ export class App extends BtrixElement { private clearUser() { this.authService.logout(); + this.authService.finalize(); this.authService = new AuthService(); AppStateService.resetUser(); } @@ -1130,26 +1126,6 @@ export class App extends BtrixElement { }); } - private startSyncBrowserTabs() { - AuthService.broadcastChannel.addEventListener( - "message", - ({ data }: { data: AuthEventDetail }) => { - if (data.name === "auth_storage") { - if (data.value !== AuthService.storage.getItem()) { - if (data.value) { - this.authService.saveLogin(JSON.parse(data.value) as Auth); - void this.fetchAndUpdateUserInfo(); - this.syncViewState(); - } else { - this.clearUser(); - this.routeTo(urlForName("login")); - } - } - } - }, - ); - } - private clearSelectedOrg() { AppStateService.updateOrgSlug(null); } diff --git a/frontend/src/utils/AuthService.test.ts b/frontend/src/utils/AuthService.test.ts index 21e4953a5a..58d2134ab6 100644 --- a/frontend/src/utils/AuthService.test.ts +++ b/frontend/src/utils/AuthService.test.ts @@ -4,32 +4,41 @@ import { restore, stub } from "sinon"; import AuthService from "./AuthService"; import { AppStateService } from "./state"; +import { APIController } from "@/controllers/api"; + describe("AuthService", () => { beforeEach(() => { AppStateService.resetAll(); - AuthService.broadcastChannel = new BroadcastChannel(AuthService.storageKey); + window.sessionStorage.clear(); window.sessionStorage.clear(); stub(window.history, "pushState"); + stub(APIController.prototype, "fetch"); + stub(AuthService.prototype, "refresh"); }); afterEach(() => { - AuthService.broadcastChannel.close(); restore(); }); - describe("AuthService.initSessionStorage()", () => { + describe("initSessionStorage()", () => { + let authService = new AuthService(); + + beforeEach(() => { + authService = new AuthService(); + }); + it("returns auth in session storage", async () => { stub(window.sessionStorage, "getItem").returns( JSON.stringify({ headers: { Authorization: "_fake_headers_" }, - tokenExpiresAt: "_fake_tokenExpiresAt_", + tokenExpiresAt: 1111, username: "test-auth@example.com", }), ); - const result = await AuthService.initSessionStorage(); + const result = await authService.initSessionStorage(); expect(result).to.deep.equal({ headers: { Authorization: "_fake_headers_" }, - tokenExpiresAt: "_fake_tokenExpiresAt_", + tokenExpiresAt: 1111, username: "test-auth@example.com", }); }); @@ -41,22 +50,43 @@ describe("AuthService", () => { name: "responding_auth", auth: { headers: { Authorization: "_fake_headers_from_tab_" }, - tokenExpiresAt: "_fake_tokenExpiresAt_from_tab_", + tokenExpiresAt: 9999, username: "test-auth@example.com_from_tab_", }, }); }); - const result = await AuthService.initSessionStorage(); + const result = await authService.initSessionStorage(); expect(result).to.deep.equal({ headers: { Authorization: "_fake_headers_from_tab_" }, - tokenExpiresAt: "_fake_tokenExpiresAt_from_tab_", + tokenExpiresAt: 9999, username: "test-auth@example.com_from_tab_", }); otherTabChannel.close(); }); + it("saves auth in session storage", 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_to_save_" }, + tokenExpiresAt: 9999, + username: "test-auth@example.com_from_tab_to_save_", + }, + }); + }); + await authService.initSessionStorage(); + expect(authService.authState).to.deep.equal({ + headers: { Authorization: "_fake_headers_from_tab_to_save_" }, + tokenExpiresAt: 9999, + username: "test-auth@example.com_from_tab_to_save_", + }); + otherTabChannel.close(); + }); it("resolves without stored auth or another tab", async () => { stub(window.sessionStorage, "getItem"); - const result = await AuthService.initSessionStorage(); + const result = await authService.initSessionStorage(); expect(result).to.equal(null); }); }); @@ -84,26 +114,6 @@ describe("AuthService", () => { expect(window.sessionStorage.setItem).to.have.been.called; }); - it("posts to the broadcast channel", () => { - stub(AuthService.storage, "getItem").returns(""); - stub(AuthService.broadcastChannel, "postMessage"); - stub(window.sessionStorage, "setItem"); - - const authValue = JSON.stringify({ - headers: { Authorization: self.crypto.randomUUID() }, - tokenExpiresAt: Date.now(), - username: "test-auth@example.com", - }); - AuthService.storage.setItem(authValue); - - expect( - AuthService.broadcastChannel.postMessage, - ).to.have.been.calledWith({ - name: "auth_storage", - value: authValue, - }); - }); - it("does not store the same value", () => { stub(AuthService.storage, "getItem").returns(JSON.stringify(mockAuth)); stub(window.sessionStorage, "setItem"); diff --git a/frontend/src/utils/AuthService.ts b/frontend/src/utils/AuthService.ts index 4d627edcc5..960a2a0c01 100644 --- a/frontend/src/utils/AuthService.ts +++ b/frontend/src/utils/AuthService.ts @@ -1,4 +1,5 @@ import { APIError } from "./api"; +import { STORAGE_KEY_PREFIX } from "./persist"; import { urlForName } from "./router"; import appState, { AppStateService } from "./state"; @@ -36,15 +37,7 @@ type AuthResponseEventDetail = { auth: AuthState; }; -export type AuthStorageEventDetail = { - name: "auth_storage"; - value: string | null; -}; - -export type AuthEventDetail = - | AuthRequestEventDetail - | AuthResponseEventDetail - | AuthStorageEventDetail; +export type AuthEventDetail = AuthRequestEventDetail | AuthResponseEventDetail; export interface AuthEventMap { "btrix-need-login": CustomEvent; @@ -63,8 +56,6 @@ export default class AuthService { static loggedInEvent: keyof AuthEventMap = "btrix-logged-in"; static logOutEvent: keyof AuthEventMap = "btrix-log-out"; static needLoginEvent: keyof AuthEventMap = "btrix-need-login"; - - static broadcastChannel = new BroadcastChannel(AuthService.storageKey); static storage = { getItem() { return window.sessionStorage.getItem(AuthService.storageKey); @@ -73,22 +64,17 @@ export default class AuthService { const oldValue = AuthService.storage.getItem(); if (oldValue === newValue) return; window.sessionStorage.setItem(AuthService.storageKey, newValue); - AuthService.broadcastChannel.postMessage({ - name: "auth_storage", - value: newValue, - } as AuthStorageEventDetail); }, removeItem() { const oldValue = AuthService.storage.getItem(); if (!oldValue) return; window.sessionStorage.removeItem(AuthService.storageKey); - AuthService.broadcastChannel.postMessage({ - name: "auth_storage", - value: null, - } as AuthStorageEventDetail); }, }; + private broadcastChannel?: BroadcastChannel; + private readonly finalizeController = new AbortController(); + get authState() { return appState.auth; } @@ -192,23 +178,18 @@ export default class AuthService { * Retrieve or set auth data from shared session * and set up session syncing */ - static async initSessionStorage(): Promise { - const authState = - AuthService.getCurrentTabAuth() || - (await AuthService.getSharedSessionAuth()); + async initSessionStorage(): Promise { + let authState = AuthService.getCurrentTabAuth(); - AuthService.broadcastChannel.addEventListener( - "message", - ({ data }: MessageEvent) => { - if (data.name === "requesting_auth") { - // A new tab/window opened and is requesting shared auth - AuthService.broadcastChannel.postMessage({ - name: "responding_auth", - auth: AuthService.getCurrentTabAuth(), - } as AuthResponseEventDetail); - } - }, - ); + if (!authState) { + authState = await this.getSharedSessionAuth(); + } + + if (authState) { + this.saveLogin(authState); + } else { + AppStateService.updateAuth(null); + } return authState; } @@ -226,20 +207,20 @@ export default class AuthService { /** * Retrieve shared session from another tab/window **/ - private static async getSharedSessionAuth(): Promise { + private async getSharedSessionAuth(): Promise { const broadcastPromise = new Promise((resolve) => { // Check if there's any authenticated tabs - AuthService.broadcastChannel.postMessage({ + this.broadcastChannel?.postMessage({ name: "requesting_auth", - } as AuthRequestEventDetail); + } satisfies AuthRequestEventDetail); // Wait for another tab to respond const cb = ({ data }: MessageEvent) => { if (data.name === "responding_auth") { - AuthService.broadcastChannel.removeEventListener("message", cb); + this.broadcastChannel?.removeEventListener("message", cb); resolve(data.auth); } }; - AuthService.broadcastChannel.addEventListener("message", cb); + this.broadcastChannel?.addEventListener("message", cb); }); // Ensure that `getSharedSessionAuth` is resolved within a reasonable // timeframe, even if another window/tab doesn't respond: @@ -266,30 +247,63 @@ export default class AuthService { } constructor() { + const { signal } = this.finalizeController; + + this.broadcastChannel = new BroadcastChannel(AuthService.storageKey); + // Only have freshness check run in visible tab(s) - document.addEventListener("visibilitychange", () => { - if (!this.authState) return; - if (document.visibilityState === "visible") { - this.startFreshnessCheck(); - } else { - this.cancelFreshnessCheck(); - } - }); + document.addEventListener( + "visibilitychange", + () => { + if (!this.authState) return; + if (document.visibilityState === "visible") { + this.startFreshnessCheck(); + } else { + this.cancelFreshnessCheck(); + } + }, + { signal }, + ); + + // Check for storage event so that the auth token can be removed from + // session storage if any other other tab is logged out; otherwise, + // the token will be refreshed until the original expiration time + window.addEventListener( + "storage", + (e: StorageEvent) => { + if (e.key !== `${STORAGE_KEY_PREFIX}.loggedIn`) return; + + if (this.authState && e.oldValue === "true") { + this.revoke(); + } + }, + { signal }, + ); + } + + finalize() { + this.finalizeController.abort(); + this.stopSharingSession(); } saveLogin(auth: Auth) { this.persist(auth); this.startFreshnessCheck(); + this.startSharingSession(); } logout() { this.cancelFreshnessCheck(); this.revoke(); + this.stopSharingSession(); } startFreshnessCheck() { window.clearTimeout(this.timerId); - void this.checkFreshness(); + + if (document.visibilityState === "visible") { + void this.checkFreshness(); + } } private cancelFreshnessCheck() { @@ -302,9 +316,36 @@ export default class AuthService { AuthService.storage.removeItem(); } + private startSharingSession() { + this.broadcastChannel?.addEventListener( + "message", + ({ data }: MessageEvent) => { + if (data.name === "requesting_auth") { + const auth = AuthService.getCurrentTabAuth(); + + if (auth) { + // A new tab/window opened and is requesting shared auth + this.broadcastChannel?.postMessage({ + name: "responding_auth", + auth: AuthService.getCurrentTabAuth(), + } satisfies AuthResponseEventDetail); + } + } + }, + ); + } + + private stopSharingSession() { + this.broadcastChannel?.close(); + this.broadcastChannel = undefined; + } + persist(auth: Auth) { this.authState = auth; - AuthService.storage.setItem(JSON.stringify(auth)); + + const authStr = JSON.stringify(auth); + + AuthService.storage.setItem(authStr); } private async checkFreshness() { diff --git a/frontend/src/utils/persist.ts b/frontend/src/utils/persist.ts index bd8ee0bc68..dbfcb255d2 100644 --- a/frontend/src/utils/persist.ts +++ b/frontend/src/utils/persist.ts @@ -7,7 +7,7 @@ import type { StateVar, } from "lit-shared-state"; -const STORAGE_KEY_PREFIX = "btrix.app"; +export const STORAGE_KEY_PREFIX = "btrix.app"; type ExpiringValue = { value: unknown; diff --git a/frontend/src/utils/state.test.ts b/frontend/src/utils/state.test.ts index cfc488e27e..bb42492dde 100644 --- a/frontend/src/utils/state.test.ts +++ b/frontend/src/utils/state.test.ts @@ -30,6 +30,7 @@ describe("state", () => { const mockAuth = null; AppStateService.updateAuth(mockAuth); expect(AppStateService.appState.auth).to.equal(mockAuth); + expect(AppStateService.appState.loggedIn).to.equal(false); }); it("updates valid auth", () => { @@ -40,6 +41,7 @@ describe("state", () => { }; AppStateService.updateAuth(mockAuth); expect(AppStateService.appState.auth).to.equal(mockAuth); + expect(AppStateService.appState.loggedIn).to.equal(true); }); it("does not update invalid auth", () => { diff --git a/frontend/src/utils/state.ts b/frontend/src/utils/state.ts index 28b08c185a..9b744b5d56 100644 --- a/frontend/src/utils/state.ts +++ b/frontend/src/utils/state.ts @@ -35,11 +35,17 @@ export function makeAppStateService() { @options(persist(window.localStorage)) userPreferences: UserPreferences | null = null; - // TODO persist here + // TODO persist here instead of in AuthService auth: Auth | null = null; + // Store logged-in flag in local storage so that tabs know whether to + // check auth again when focused. + // This can get out of sync with `auth` if any other tabs log in/out. + @options(persist(window.localStorage)) + loggedIn: boolean | null = null; + // Store user's org slug preference in local storage in order to redirect - // to the most recently visited org on next log in + // to the most recently visited org on next log in. // TODO move to `userPreferences` @options(persist(window.localStorage)) orgSlug: string | null = null; @@ -96,11 +102,13 @@ export function makeAppStateService() { appState.settings = settings; } + @transaction() @unlock() updateAuth(authState: AppState["auth"]) { authSchema.nullable().parse(authState); appState.auth = authState; + appState.loggedIn = Boolean(authState); } @transaction() @@ -181,6 +189,7 @@ export function makeAppStateService() { private _resetUser() { appState.auth = null; + appState.loggedIn = null; appState.userInfo = null; appState.userPreferences = null; appState.orgSlug = null;