From ac6214ab10fc97c6f92cf59dfdf011a31faca827 Mon Sep 17 00:00:00 2001 From: sua yoo Date: Thu, 26 Jun 2025 18:03:07 -0700 Subject: [PATCH 01/10] only share auth if present --- frontend/src/utils/AuthService.ts | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/frontend/src/utils/AuthService.ts b/frontend/src/utils/AuthService.ts index 4d627edcc5..23a444be42 100644 --- a/frontend/src/utils/AuthService.ts +++ b/frontend/src/utils/AuthService.ts @@ -201,11 +201,15 @@ export default class AuthService { "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); + const auth = AuthService.getCurrentTabAuth(); + + if (auth) { + // A new tab/window opened and is requesting shared auth + AuthService.broadcastChannel.postMessage({ + name: "responding_auth", + auth: AuthService.getCurrentTabAuth(), + } as AuthResponseEventDetail); + } } }, ); From bf3a4ba0457ef36ddef86f72bd3740932edfb0d4 Mon Sep 17 00:00:00 2001 From: sua yoo Date: Tue, 16 Sep 2025 15:40:06 -0700 Subject: [PATCH 02/10] only check freshness if visible --- frontend/src/utils/AuthService.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/frontend/src/utils/AuthService.ts b/frontend/src/utils/AuthService.ts index 23a444be42..3a67e70aef 100644 --- a/frontend/src/utils/AuthService.ts +++ b/frontend/src/utils/AuthService.ts @@ -293,7 +293,10 @@ export default class AuthService { startFreshnessCheck() { window.clearTimeout(this.timerId); - void this.checkFreshness(); + + if (document.visibilityState === "visible") { + void this.checkFreshness(); + } } private cancelFreshnessCheck() { From 409066982c92932e3ee01d8e1227d77c74ec8085 Mon Sep 17 00:00:00 2001 From: sua yoo Date: Tue, 16 Sep 2025 16:22:29 -0700 Subject: [PATCH 03/10] switch to instance --- frontend/src/index.ts | 48 ++++++++++++++---------------- frontend/src/utils/AuthService.ts | 49 ++++++++++++++++++------------- 2 files changed, 51 insertions(+), 46 deletions(-) diff --git a/frontend/src/index.ts b/frontend/src/index.ts index f159b684da..0f41cb74b8 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,12 @@ 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(); } @@ -163,7 +159,7 @@ export class App extends BtrixElement { this.syncViewState(); }); - this.startSyncBrowserTabs(); + // this.startSyncBrowserTabs(); } private attachUserGuideListeners() { @@ -1130,25 +1126,25 @@ 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 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.ts b/frontend/src/utils/AuthService.ts index 3a67e70aef..4d4e485a93 100644 --- a/frontend/src/utils/AuthService.ts +++ b/frontend/src/utils/AuthService.ts @@ -57,14 +57,13 @@ const FRESHNESS_TIMER_INTERVAL = 60 * 1000 * 5; export default class AuthService { private timerId?: number; + private readonly broadcastChannel?: BroadcastChannel; static storageKey = "btrix.auth"; static unsupportedAuthErrorCode = "UNSUPPORTED_AUTH_TYPE"; 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,19 +72,11 @@ 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); }, }; @@ -192,12 +183,11 @@ export default class AuthService { * Retrieve or set auth data from shared session * and set up session syncing */ - static async initSessionStorage(): Promise { + async initSessionStorage(): Promise { const authState = - AuthService.getCurrentTabAuth() || - (await AuthService.getSharedSessionAuth()); + AuthService.getCurrentTabAuth() || (await this.getSharedSessionAuth()); - AuthService.broadcastChannel.addEventListener( + this.broadcastChannel?.addEventListener( "message", ({ data }: MessageEvent) => { if (data.name === "requesting_auth") { @@ -205,7 +195,7 @@ export default class AuthService { if (auth) { // A new tab/window opened and is requesting shared auth - AuthService.broadcastChannel.postMessage({ + this.broadcastChannel?.postMessage({ name: "responding_auth", auth: AuthService.getCurrentTabAuth(), } as AuthResponseEventDetail); @@ -214,6 +204,10 @@ export default class AuthService { }, ); + if (authState) { + this.saveLogin(authState); + } + return authState; } @@ -230,20 +224,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); // 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: @@ -270,6 +264,8 @@ export default class AuthService { } constructor() { + this.broadcastChannel = new BroadcastChannel(AuthService.storageKey); + // Only have freshness check run in visible tab(s) document.addEventListener("visibilitychange", () => { if (!this.authState) return; @@ -307,11 +303,24 @@ export default class AuthService { private revoke() { this.authState = null; AuthService.storage.removeItem(); + + this.broadcastChannel?.postMessage({ + name: "auth_storage", + value: null, + } as AuthStorageEventDetail); } persist(auth: Auth) { this.authState = auth; - AuthService.storage.setItem(JSON.stringify(auth)); + + const authStr = JSON.stringify(auth); + + AuthService.storage.setItem(authStr); + + this.broadcastChannel?.postMessage({ + name: "auth_storage", + value: authStr, + } as AuthStorageEventDetail); } private async checkFreshness() { From da64524b84a139ee95e198fcbf7678f6bd6e5a80 Mon Sep 17 00:00:00 2001 From: sua yoo Date: Tue, 16 Sep 2025 17:10:26 -0700 Subject: [PATCH 04/10] close broadcast channel when finished --- frontend/src/index.ts | 22 ------- frontend/src/utils/AuthService.ts | 95 ++++++++++++++++++++----------- 2 files changed, 61 insertions(+), 56 deletions(-) diff --git a/frontend/src/index.ts b/frontend/src/index.ts index 0f41cb74b8..ac5817c1ef 100644 --- a/frontend/src/index.ts +++ b/frontend/src/index.ts @@ -158,8 +158,6 @@ export class App extends BtrixElement { window.addEventListener("popstate", () => { this.syncViewState(); }); - - // this.startSyncBrowserTabs(); } private attachUserGuideListeners() { @@ -1126,26 +1124,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.ts b/frontend/src/utils/AuthService.ts index 4d4e485a93..9f022f8a2d 100644 --- a/frontend/src/utils/AuthService.ts +++ b/frontend/src/utils/AuthService.ts @@ -1,3 +1,6 @@ +import { nanoid } from "nanoid"; +import { z } from "zod"; + import { APIError } from "./api"; import { urlForName } from "./router"; import appState, { AppStateService } from "./state"; @@ -5,6 +8,8 @@ import appState, { AppStateService } from "./state"; import type { APIUser } from "@/index"; import type { Auth } from "@/types/auth"; +const sessionIdSchema = z.string().nanoid(); +type SessionId = z.infer; type AuthState = Auth | null; type JWT = { user_id: string; @@ -27,24 +32,28 @@ export type LogOutEventDetail = { redirect?: boolean; }; -type AuthRequestEventDetail = { +type AuthMessage = { + responderId?: SessionId; + requesterId?: SessionId; +}; + +type AuthRequestEventDetail = AuthMessage & { name: "requesting_auth"; }; -type AuthResponseEventDetail = { +type AuthResponseEventDetail = AuthMessage & { name: "responding_auth"; auth: AuthState; }; -export type AuthStorageEventDetail = { - name: "auth_storage"; - value: string | null; +export type AuthReceivedEventDetail = AuthMessage & { + name: "auth_received"; }; export type AuthEventDetail = | AuthRequestEventDetail | AuthResponseEventDetail - | AuthStorageEventDetail; + | AuthReceivedEventDetail; export interface AuthEventMap { "btrix-need-login": CustomEvent; @@ -57,7 +66,8 @@ const FRESHNESS_TIMER_INTERVAL = 60 * 1000 * 5; export default class AuthService { private timerId?: number; - private readonly broadcastChannel?: BroadcastChannel; + private readonly sessionId: SessionId = nanoid(); + private broadcastChannel?: BroadcastChannel; static storageKey = "btrix.auth"; static unsupportedAuthErrorCode = "UNSUPPORTED_AUTH_TYPE"; @@ -187,23 +197,6 @@ export default class AuthService { const authState = AuthService.getCurrentTabAuth() || (await this.getSharedSessionAuth()); - 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(), - } as AuthResponseEventDetail); - } - } - }, - ); - if (authState) { this.saveLogin(authState); } @@ -228,12 +221,20 @@ export default class AuthService { const broadcastPromise = new Promise((resolve) => { // Check if there's any authenticated tabs this.broadcastChannel?.postMessage({ + requesterId: this.sessionId, name: "requesting_auth", - } as AuthRequestEventDetail); + } satisfies AuthRequestEventDetail); // Wait for another tab to respond const cb = ({ data }: MessageEvent) => { if (data.name === "responding_auth") { this.broadcastChannel?.removeEventListener("message", cb); + + // Confirm receipt + this.broadcastChannel?.postMessage({ + requesterId: this.sessionId, + responderId: data.responderId, + name: "auth_received", + } satisfies AuthReceivedEventDetail); resolve(data.auth); } }; @@ -280,11 +281,13 @@ export default class AuthService { saveLogin(auth: Auth) { this.persist(auth); this.startFreshnessCheck(); + this.startSharingSession(); } logout() { this.cancelFreshnessCheck(); this.revoke(); + this.stopSharingSession(); } startFreshnessCheck() { @@ -303,11 +306,40 @@ export default class AuthService { private revoke() { this.authState = null; AuthService.storage.removeItem(); + } - this.broadcastChannel?.postMessage({ - name: "auth_storage", - value: null, - } as AuthStorageEventDetail); + /** + * Listens to broadcast channel events until confirmaion is received + * that the newest window or tab is logged. Once the request is met + * this broadcast channel is closed to prevent having too many + * destinations, which may slow down postMessage performance. + */ + private startSharingSession() { + this.broadcastChannel?.addEventListener( + "message", + ({ data }: MessageEvent) => { + if (data.name === "auth_received") { + this.stopSharingSession(); + } + if (data.name === "requesting_auth") { + const auth = AuthService.getCurrentTabAuth(); + + if (auth) { + // A new tab/window opened and is requesting shared auth + this.broadcastChannel?.postMessage({ + responderId: this.sessionId, + name: "responding_auth", + auth: AuthService.getCurrentTabAuth(), + } satisfies AuthResponseEventDetail); + } + } + }, + ); + } + + private stopSharingSession() { + this.broadcastChannel?.close(); + this.broadcastChannel = undefined; } persist(auth: Auth) { @@ -316,11 +348,6 @@ export default class AuthService { const authStr = JSON.stringify(auth); AuthService.storage.setItem(authStr); - - this.broadcastChannel?.postMessage({ - name: "auth_storage", - value: authStr, - } as AuthStorageEventDetail); } private async checkFreshness() { From 672a5e709682db166c5e7929b1b8870945e94843 Mon Sep 17 00:00:00 2001 From: sua yoo Date: Tue, 16 Sep 2025 19:06:21 -0700 Subject: [PATCH 05/10] handle log out --- frontend/src/index.ts | 1 + frontend/src/utils/AuthService.ts | 74 ++++++++++++++++++++----------- frontend/src/utils/persist.ts | 2 +- frontend/src/utils/state.ts | 9 +++- 4 files changed, 57 insertions(+), 29 deletions(-) diff --git a/frontend/src/index.ts b/frontend/src/index.ts index ac5817c1ef..54e2dbfc5d 100644 --- a/frontend/src/index.ts +++ b/frontend/src/index.ts @@ -1079,6 +1079,7 @@ export class App extends BtrixElement { private clearUser() { this.authService.logout(); + this.authService.finalize(); this.authService = new AuthService(); AppStateService.resetUser(); } diff --git a/frontend/src/utils/AuthService.ts b/frontend/src/utils/AuthService.ts index 9f022f8a2d..89bdf2f3be 100644 --- a/frontend/src/utils/AuthService.ts +++ b/frontend/src/utils/AuthService.ts @@ -1,15 +1,11 @@ -import { nanoid } from "nanoid"; -import { z } from "zod"; - import { APIError } from "./api"; +import { STORAGE_KEY_PREFIX } from "./persist"; import { urlForName } from "./router"; import appState, { AppStateService } from "./state"; import type { APIUser } from "@/index"; import type { Auth } from "@/types/auth"; -const sessionIdSchema = z.string().nanoid(); -type SessionId = z.infer; type AuthState = Auth | null; type JWT = { user_id: string; @@ -32,21 +28,16 @@ export type LogOutEventDetail = { redirect?: boolean; }; -type AuthMessage = { - responderId?: SessionId; - requesterId?: SessionId; -}; - -type AuthRequestEventDetail = AuthMessage & { +type AuthRequestEventDetail = { name: "requesting_auth"; }; -type AuthResponseEventDetail = AuthMessage & { +type AuthResponseEventDetail = { name: "responding_auth"; auth: AuthState; }; -export type AuthReceivedEventDetail = AuthMessage & { +export type AuthReceivedEventDetail = { name: "auth_received"; }; @@ -66,8 +57,6 @@ const FRESHNESS_TIMER_INTERVAL = 60 * 1000 * 5; export default class AuthService { private timerId?: number; - private readonly sessionId: SessionId = nanoid(); - private broadcastChannel?: BroadcastChannel; static storageKey = "btrix.auth"; static unsupportedAuthErrorCode = "UNSUPPORTED_AUTH_TYPE"; @@ -90,6 +79,9 @@ export default class AuthService { }, }; + private broadcastChannel?: BroadcastChannel; + private readonly finalizeController = new AbortController(); + get authState() { return appState.auth; } @@ -221,7 +213,6 @@ export default class AuthService { const broadcastPromise = new Promise((resolve) => { // Check if there's any authenticated tabs this.broadcastChannel?.postMessage({ - requesterId: this.sessionId, name: "requesting_auth", } satisfies AuthRequestEventDetail); // Wait for another tab to respond @@ -231,10 +222,9 @@ export default class AuthService { // Confirm receipt this.broadcastChannel?.postMessage({ - requesterId: this.sessionId, - responderId: data.responderId, name: "auth_received", } satisfies AuthReceivedEventDetail); + resolve(data.auth); } }; @@ -265,17 +255,48 @@ 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 }, + ); + + /** + * Assume another tab has logged out if `orgSlug` preference is removed + * See FIXME note in state.ts + */ + window.addEventListener( + "storage", + (e: StorageEvent) => { + if (!this.authState) return; + + if ( + e.key === `${STORAGE_KEY_PREFIX}.orgSlug` && + e.oldValue && + !e.newValue + ) { + this.revoke(); + } + }, + { signal }, + ); + } + + finalize() { + this.finalizeController.abort(); + this.stopSharingSession(); } saveLogin(auth: Auth) { @@ -327,7 +348,6 @@ export default class AuthService { if (auth) { // A new tab/window opened and is requesting shared auth this.broadcastChannel?.postMessage({ - responderId: this.sessionId, name: "responding_auth", auth: AuthService.getCurrentTabAuth(), } satisfies AuthResponseEventDetail); 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.ts b/frontend/src/utils/state.ts index 28b08c185a..c9ef9fdbbe 100644 --- a/frontend/src/utils/state.ts +++ b/frontend/src/utils/state.ts @@ -39,7 +39,14 @@ export function makeAppStateService() { auth: Auth | 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. + // + // FIXME Since the org slug preference is removed on log out, AuthService + // currently checks whether `orgSlug` is being removed in a `storage` + // event to determine whether another tab has logged out. + // It's not the cleanest solution to use `orgSlug` as a cross-tab logout + // event, so we may want to refactor this in the future. + // // TODO move to `userPreferences` @options(persist(window.localStorage)) orgSlug: string | null = null; From cb320a7c79e062d9046c632c3f0a31a8d8232a15 Mon Sep 17 00:00:00 2001 From: sua yoo Date: Tue, 16 Sep 2025 19:21:20 -0700 Subject: [PATCH 06/10] remove receipt logic --- frontend/src/utils/AuthService.ts | 26 ++------------------------ 1 file changed, 2 insertions(+), 24 deletions(-) diff --git a/frontend/src/utils/AuthService.ts b/frontend/src/utils/AuthService.ts index 89bdf2f3be..cbaee86c22 100644 --- a/frontend/src/utils/AuthService.ts +++ b/frontend/src/utils/AuthService.ts @@ -37,14 +37,7 @@ type AuthResponseEventDetail = { auth: AuthState; }; -export type AuthReceivedEventDetail = { - name: "auth_received"; -}; - -export type AuthEventDetail = - | AuthRequestEventDetail - | AuthResponseEventDetail - | AuthReceivedEventDetail; +export type AuthEventDetail = AuthRequestEventDetail | AuthResponseEventDetail; export interface AuthEventMap { "btrix-need-login": CustomEvent; @@ -189,7 +182,7 @@ export default class AuthService { const authState = AuthService.getCurrentTabAuth() || (await this.getSharedSessionAuth()); - if (authState) { + if (!this.authState && authState) { this.saveLogin(authState); } @@ -219,12 +212,6 @@ export default class AuthService { const cb = ({ data }: MessageEvent) => { if (data.name === "responding_auth") { this.broadcastChannel?.removeEventListener("message", cb); - - // Confirm receipt - this.broadcastChannel?.postMessage({ - name: "auth_received", - } satisfies AuthReceivedEventDetail); - resolve(data.auth); } }; @@ -329,19 +316,10 @@ export default class AuthService { AuthService.storage.removeItem(); } - /** - * Listens to broadcast channel events until confirmaion is received - * that the newest window or tab is logged. Once the request is met - * this broadcast channel is closed to prevent having too many - * destinations, which may slow down postMessage performance. - */ private startSharingSession() { this.broadcastChannel?.addEventListener( "message", ({ data }: MessageEvent) => { - if (data.name === "auth_received") { - this.stopSharingSession(); - } if (data.name === "requesting_auth") { const auth = AuthService.getCurrentTabAuth(); From f56dc231f7af8b9bc34a9f99c0af184177dfa508 Mon Sep 17 00:00:00 2001 From: sua yoo Date: Tue, 16 Sep 2025 19:31:39 -0700 Subject: [PATCH 07/10] update tests --- frontend/src/index.test.ts | 18 +++++------ frontend/src/utils/AuthService.test.ts | 45 +++++++++----------------- 2 files changed, 24 insertions(+), 39 deletions(-) diff --git a/frontend/src/index.test.ts b/frontend/src/index.test.ts index e09bc96a8d..bd5fa81a88 100644 --- a/frontend/src/index.test.ts +++ b/frontend/src/index.test.ts @@ -51,7 +51,6 @@ const mockAppSettings: AppSettings = { describe("browsertrix-app", () => { beforeEach(() => { AppStateService.resetAll(); - AuthService.broadcastChannel = new BroadcastChannel(AuthService.storageKey); window.sessionStorage.clear(); window.localStorage.clear(); stub(window.history, "pushState"); @@ -59,7 +58,6 @@ describe("browsertrix-app", () => { }); afterEach(() => { - AuthService.broadcastChannel.close(); restore(); }); @@ -69,7 +67,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,7 +83,7 @@ describe("browsertrix-app", () => { }); it("renders 404 when not in org", async () => { - stub(AuthService, "initSessionStorage").returns( + stub(AuthService.prototype, "initSessionStorage").returns( Promise.resolve({ headers: { Authorization: "_fake_headers_" }, tokenExpiresAt: 0, @@ -117,7 +115,7 @@ describe("browsertrix-app", () => { role: 10, }; - stub(AuthService, "initSessionStorage").returns( + stub(AuthService.prototype, "initSessionStorage").returns( Promise.resolve({ headers: { Authorization: "_fake_headers_" }, tokenExpiresAt: 0, @@ -142,7 +140,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( @@ -184,7 +184,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, @@ -203,7 +203,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, @@ -235,7 +235,7 @@ describe("browsertrix-app", () => { ); stub(App.prototype, "getLocationPathname").callsFake(() => `/orgs/${id}`); stub(AuthService.prototype, "startFreshnessCheck").callsFake(() => {}); - stub(AuthService, "initSessionStorage").callsFake(async () => + stub(AuthService.prototype, "initSessionStorage").callsFake(async () => Promise.resolve({ headers: { Authorization: "_fake_headers_" }, tokenExpiresAt: 0, diff --git a/frontend/src/utils/AuthService.test.ts b/frontend/src/utils/AuthService.test.ts index 21e4953a5a..edb2cce9c8 100644 --- a/frontend/src/utils/AuthService.test.ts +++ b/frontend/src/utils/AuthService.test.ts @@ -7,29 +7,34 @@ import { AppStateService } from "./state"; describe("AuthService", () => { beforeEach(() => { AppStateService.resetAll(); - AuthService.broadcastChannel = new BroadcastChannel(AuthService.storageKey); + window.sessionStorage.clear(); window.sessionStorage.clear(); stub(window.history, "pushState"); }); 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 +46,22 @@ 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("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 +89,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"); From ffe5ffc0b5ca8289cfa35222ae3ae4b2606398c8 Mon Sep 17 00:00:00 2001 From: sua yoo Date: Wed, 17 Sep 2025 13:25:59 -0700 Subject: [PATCH 08/10] store logged in state in local storage --- frontend/src/utils/AuthService.ts | 23 +++++++++++------------ frontend/src/utils/state.test.ts | 2 ++ frontend/src/utils/state.ts | 17 +++++++++-------- 3 files changed, 22 insertions(+), 20 deletions(-) diff --git a/frontend/src/utils/AuthService.ts b/frontend/src/utils/AuthService.ts index cbaee86c22..094e11b447 100644 --- a/frontend/src/utils/AuthService.ts +++ b/frontend/src/utils/AuthService.ts @@ -182,8 +182,12 @@ export default class AuthService { const authState = AuthService.getCurrentTabAuth() || (await this.getSharedSessionAuth()); - if (!this.authState && authState) { - this.saveLogin(authState); + if (authState) { + if (!this.authState) { + this.saveLogin(authState); + } + } else { + AppStateService.updateAuth(null); } return authState; @@ -260,20 +264,15 @@ export default class AuthService { { signal }, ); - /** - * Assume another tab has logged out if `orgSlug` preference is removed - * See FIXME note in state.ts - */ + // 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 (!this.authState) return; + if (e.key !== `${STORAGE_KEY_PREFIX}.loggedIn`) return; - if ( - e.key === `${STORAGE_KEY_PREFIX}.orgSlug` && - e.oldValue && - !e.newValue - ) { + if (this.authState && e.oldValue === "true") { this.revoke(); } }, 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 c9ef9fdbbe..e3cca26907 100644 --- a/frontend/src/utils/state.ts +++ b/frontend/src/utils/state.ts @@ -35,18 +35,16 @@ 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 + @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. - // - // FIXME Since the org slug preference is removed on log out, AuthService - // currently checks whether `orgSlug` is being removed in a `storage` - // event to determine whether another tab has logged out. - // It's not the cleanest solution to use `orgSlug` as a cross-tab logout - // event, so we may want to refactor this in the future. - // // TODO move to `userPreferences` @options(persist(window.localStorage)) orgSlug: string | null = null; @@ -103,11 +101,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() @@ -188,6 +188,7 @@ export function makeAppStateService() { private _resetUser() { appState.auth = null; + appState.loggedIn = null; appState.userInfo = null; appState.userPreferences = null; appState.orgSlug = null; From 138e1a96d996af08413cd8d8a7067fe1ae30cf2d Mon Sep 17 00:00:00 2001 From: sua yoo Date: Wed, 17 Sep 2025 13:59:28 -0700 Subject: [PATCH 09/10] fix tests --- frontend/src/index.test.ts | 45 ++++++-------------------- frontend/src/index.ts | 1 + frontend/src/utils/AuthService.test.ts | 25 ++++++++++++++ frontend/src/utils/AuthService.ts | 13 ++++---- 4 files changed, 42 insertions(+), 42 deletions(-) diff --git a/frontend/src/index.test.ts b/frontend/src/index.test.ts index bd5fa81a88..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,6 +47,7 @@ const mockAppSettings: AppSettings = { salesEmail: "", supportEmail: "", localesEnabled: ["en", "es"], + numBrowsersPerInstance: 1, }; describe("browsertrix-app", () => { @@ -55,6 +57,7 @@ describe("browsertrix-app", () => { window.localStorage.clear(); stub(window.history, "pushState"); stub(NotifyController.prototype, "toast"); + stub(APIController.prototype, "fetch"); }); afterEach(() => { @@ -84,15 +87,12 @@ describe("browsertrix-app", () => { it("renders 404 when not in org", async () => { stub(AuthService.prototype, "initSessionStorage").returns( - Promise.resolve({ - headers: { Authorization: "_fake_headers_" }, - tokenExpiresAt: 0, - username: "test-auth@example.com", - }), + Promise.resolve(mockAuth), ); // @ts-expect-error checkFreshness is private stub(AuthService.prototype, "checkFreshness"); + AppStateService.updateAuth(mockAuth); AppStateService.updateUser( formatAPIUser({ ...mockAPIUser, @@ -116,15 +116,12 @@ describe("browsertrix-app", () => { }; stub(AuthService.prototype, "initSessionStorage").returns( - Promise.resolve({ - headers: { Authorization: "_fake_headers_" }, - tokenExpiresAt: 0, - username: "test-auth@example.com", - }), + Promise.resolve(mockAuth), ); // @ts-expect-error checkFreshness is private stub(AuthService.prototype, "checkFreshness"); + AppStateService.updateAuth(mockAuth); AppStateService.updateUser( formatAPIUser({ ...mockAPIUser, @@ -158,27 +155,6 @@ 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), @@ -227,6 +203,7 @@ describe("browsertrix-app", () => { slug: id, role: 10, }; + AppStateService.updateAuth(mockAuth); AppStateService.updateUser( formatAPIUser({ ...mockAPIUser, @@ -236,11 +213,7 @@ describe("browsertrix-app", () => { stub(App.prototype, "getLocationPathname").callsFake(() => `/orgs/${id}`); stub(AuthService.prototype, "startFreshnessCheck").callsFake(() => {}); stub(AuthService.prototype, "initSessionStorage").callsFake(async () => - Promise.resolve({ - headers: { Authorization: "_fake_headers_" }, - tokenExpiresAt: 0, - username: "test-auth@example.com", - }), + Promise.resolve(mockAuth), ); const el = await fixture(""); diff --git a/frontend/src/index.ts b/frontend/src/index.ts index 54e2dbfc5d..14e360349a 100644 --- a/frontend/src/index.ts +++ b/frontend/src/index.ts @@ -141,6 +141,7 @@ export class App extends BtrixElement { } catch (e) { console.debug(e); } + this.syncViewState(); if (authState && !this.userInfo) { diff --git a/frontend/src/utils/AuthService.test.ts b/frontend/src/utils/AuthService.test.ts index edb2cce9c8..58d2134ab6 100644 --- a/frontend/src/utils/AuthService.test.ts +++ b/frontend/src/utils/AuthService.test.ts @@ -4,12 +4,16 @@ import { restore, stub } from "sinon"; import AuthService from "./AuthService"; import { AppStateService } from "./state"; +import { APIController } from "@/controllers/api"; + describe("AuthService", () => { beforeEach(() => { AppStateService.resetAll(); window.sessionStorage.clear(); window.sessionStorage.clear(); stub(window.history, "pushState"); + stub(APIController.prototype, "fetch"); + stub(AuthService.prototype, "refresh"); }); afterEach(() => { @@ -59,6 +63,27 @@ describe("AuthService", () => { }); 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(); diff --git a/frontend/src/utils/AuthService.ts b/frontend/src/utils/AuthService.ts index 094e11b447..f405733b88 100644 --- a/frontend/src/utils/AuthService.ts +++ b/frontend/src/utils/AuthService.ts @@ -179,15 +179,16 @@ export default class AuthService { * and set up session syncing */ async initSessionStorage(): Promise { - const authState = - AuthService.getCurrentTabAuth() || (await this.getSharedSessionAuth()); + let authState = AuthService.getCurrentTabAuth(); - if (authState) { - if (!this.authState) { + if (!authState) { + authState = await this.getSharedSessionAuth(); + + if (authState) { this.saveLogin(authState); + } else { + AppStateService.updateAuth(null); } - } else { - AppStateService.updateAuth(null); } return authState; From 3604b9ee0c846d4036418e3cc9eb9ad30d1fd1ba Mon Sep 17 00:00:00 2001 From: sua yoo Date: Wed, 17 Sep 2025 14:08:55 -0700 Subject: [PATCH 10/10] aways save login during init --- frontend/src/utils/AuthService.ts | 10 +++++----- frontend/src/utils/state.ts | 3 ++- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/frontend/src/utils/AuthService.ts b/frontend/src/utils/AuthService.ts index f405733b88..960a2a0c01 100644 --- a/frontend/src/utils/AuthService.ts +++ b/frontend/src/utils/AuthService.ts @@ -183,12 +183,12 @@ export default class AuthService { if (!authState) { authState = await this.getSharedSessionAuth(); + } - if (authState) { - this.saveLogin(authState); - } else { - AppStateService.updateAuth(null); - } + if (authState) { + this.saveLogin(authState); + } else { + AppStateService.updateAuth(null); } return authState; diff --git a/frontend/src/utils/state.ts b/frontend/src/utils/state.ts index e3cca26907..9b744b5d56 100644 --- a/frontend/src/utils/state.ts +++ b/frontend/src/utils/state.ts @@ -39,7 +39,8 @@ export function makeAppStateService() { auth: Auth | null = null; // Store logged-in flag in local storage so that tabs know whether to - // check auth again when focused + // 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;