From cc6beedfb0fd93bdc6759eb8b6cf6a862ab06445 Mon Sep 17 00:00:00 2001 From: Hariom Balhara Date: Fri, 25 Jul 2025 09:14:54 +0530 Subject: [PATCH] fix: App Crash inside Embed when Cross Domain cookies are disabled in browser(Firefox) (#22679) * Fix: embed crash when third party cookies are disabled * :Fix: embed crash when third party cookies are disabled * fix missing import --- apps/web/pages/_applyThemeForDocument.test.ts | 157 ++++++++++++++++++ apps/web/pages/_applyThemeForDocument.ts | 59 +++++++ apps/web/pages/_document.tsx | 33 +--- .../src/rules/avoid-web-storage.ts | 82 +++++++++ packages/lib/hooks/useTheme.tsx | 4 +- 5 files changed, 303 insertions(+), 32 deletions(-) create mode 100644 apps/web/pages/_applyThemeForDocument.test.ts create mode 100644 apps/web/pages/_applyThemeForDocument.ts diff --git a/apps/web/pages/_applyThemeForDocument.test.ts b/apps/web/pages/_applyThemeForDocument.test.ts new file mode 100644 index 0000000000..446d89f3b3 --- /dev/null +++ b/apps/web/pages/_applyThemeForDocument.test.ts @@ -0,0 +1,157 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; + +import { applyTheme } from "./_applyThemeForDocument"; + +describe("applyTheme", () => { + let reconstructedFn: () => void; + let mockLocalStorage: Storage; + let mockRequestAnimationFrame: typeof window.requestAnimationFrame; + let mockDocument: Document; + let addedClasses: string[] = []; + + beforeEach(() => { + // Create the function from stringified version + const stringifiedFunction = applyTheme.toString(); + reconstructedFn = new Function(`return ${stringifiedFunction}`)(); + + // Mock localStorage + mockLocalStorage = { + getItem: vi.fn(() => null), + setItem: vi.fn(), + removeItem: vi.fn(), + clear: vi.fn(), + key: vi.fn(() => null), + length: 0, + }; + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + global.localStorage = mockLocalStorage; + + // Mock requestAnimationFrame + mockRequestAnimationFrame = vi.fn((cb) => { + cb(0); + return 0; + }); + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + global.window = { location: { pathname: "/" } }; + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + global.requestAnimationFrame = mockRequestAnimationFrame; + + // Mock document + addedClasses = []; + mockDocument = { + body: { + classList: { + add: vi.fn((className: string) => { + addedClasses.push(className); + }), + }, + }, + } as unknown as Document; + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + global.document = mockDocument; + + // Mock console.error + global.console.error = vi.fn(); + }); + + afterEach(() => { + vi.clearAllMocks(); + }); + + it("should be reconstructable from toString() and executable", () => { + expect(() => reconstructedFn()).not.toThrow(); + }); + + it("should not add any theme when no app-theme is found", () => { + reconstructedFn(); + + expect(mockRequestAnimationFrame).not.toHaveBeenCalled(); + expect(addedClasses).toHaveLength(0); + }); + + it("should handle localStorage errors gracefully", () => { + // Make localStorage throw an error + mockLocalStorage.getItem = vi.fn(() => { + throw new Error("localStorage not available"); + }); + mockLocalStorage.key = vi.fn(() => { + throw new Error("localStorage not available"); + }); + Object.defineProperty(mockLocalStorage, "length", { + get: () => { + throw new Error("localStorage not available"); + }, + }); + + expect(() => reconstructedFn()).not.toThrow(); + expect(addedClasses).toHaveLength(0); + }); + + it("should apply booking theme when app-theme exists and on booking page", () => { + const username = "testuser"; + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + global.window.location.pathname = `/${username}/meeting`; + + mockLocalStorage.key = vi.fn((index: number) => { + if (index === 0) return `booking-theme:${username}`; + return null; + }); + mockLocalStorage.getItem = vi.fn((key: string) => { + if (key === "app-theme") return "light"; + if (key === `booking-theme:${username}`) return "dark"; + return null; + }); + Object.defineProperty(mockLocalStorage, "length", { + get: () => 1, + configurable: true, + }); + + reconstructedFn(); + + expect(addedClasses).toContain("dark"); + }); + + it("should apply app theme when not on booking page", () => { + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + global.window.location.pathname = "/some-other-page"; + + mockLocalStorage.key = vi.fn((index: number) => { + if (index === 0) return "booking-theme:testuser"; + return null; + }); + mockLocalStorage.getItem = vi.fn((key: string) => { + if (key === "app-theme") return "light"; + if (key === "booking-theme:testuser") return "dark"; + return null; + }); + Object.defineProperty(mockLocalStorage, "length", { + get: () => 1, + configurable: true, + }); + + reconstructedFn(); + + expect(addedClasses).toContain("light"); + }); + + it("should apply app theme when app-theme exists but no booking theme", () => { + mockLocalStorage.getItem = vi.fn((key: string) => { + if (key === "app-theme") return "dark"; + return null; + }); + Object.defineProperty(mockLocalStorage, "length", { + get: () => 0, + configurable: true, + }); + + reconstructedFn(); + + expect(addedClasses).toContain("dark"); + }); +}); diff --git a/apps/web/pages/_applyThemeForDocument.ts b/apps/web/pages/_applyThemeForDocument.ts new file mode 100644 index 0000000000..500497db38 --- /dev/null +++ b/apps/web/pages/_applyThemeForDocument.ts @@ -0,0 +1,59 @@ +// Theme application function - will be stringified and injected. So, it must not use anything from the closure +export const applyTheme = function () { + try { + // This utility is a replica of @calcom/lib/webstorage.ts but we can reuse it because applyTheme is stringified and injected, so we can't have deps here + const safeLocalStorage = { + getItem: function (key: string) { + try { + // eslint-disable-next-line @calcom/eslint/avoid-web-storage + return localStorage.getItem(key); + } catch (e) { + return null; + } + }, + key: function (index: number) { + try { + // eslint-disable-next-line @calcom/eslint/avoid-web-storage + return localStorage.key(index); + } catch (e) { + return null; + } + }, + getLength: function () { + try { + // eslint-disable-next-line @calcom/eslint/avoid-web-storage + return localStorage.length; + } catch (e) { + return 0; + } + }, + }; + + const appTheme = safeLocalStorage.getItem("app-theme"); + if (!appTheme) return; + + let bookingTheme: string | null = null; + let username: string | undefined; + for (let i = 0; i < safeLocalStorage.getLength(); i++) { + const key = safeLocalStorage.key(i); + if (key && key.startsWith("booking-theme:")) { + bookingTheme = safeLocalStorage.getItem(key); + username = key.split("booking-theme:")[1]; + break; + } + } + + const onReady = () => { + const isBookingPage = username && window.location.pathname.slice(1).startsWith(username); + if (document.body) { + document.body.classList.add(isBookingPage ? bookingTheme || appTheme : appTheme); + } else { + requestAnimationFrame(onReady); + } + }; + + requestAnimationFrame(onReady); + } catch (e) { + console.error("Error applying theme:", e); + } +}; diff --git a/apps/web/pages/_document.tsx b/apps/web/pages/_document.tsx index 95bbd69ef4..d73a99d929 100644 --- a/apps/web/pages/_document.tsx +++ b/apps/web/pages/_document.tsx @@ -9,6 +9,8 @@ import { IS_PRODUCTION } from "@calcom/lib/constants"; import { csp } from "@lib/csp"; +import { applyTheme } from "./_applyThemeForDocument"; + type Props = Record & DocumentProps & { newLocale: string }; function setHeader(ctx: NextPageContext, name: string, value: string) { try { @@ -70,36 +72,7 @@ class MyDocument extends Document { dangerouslySetInnerHTML={{ __html: ` window.calNewLocale = "${newLocale}"; - (function applyTheme() { - try { - const appTheme = localStorage.getItem('app-theme'); - if (!appTheme) return; - - let bookingTheme, username; - for (let i = 0; i < localStorage.length; i++) { - const key = localStorage.key(i); - if (key.startsWith('booking-theme:')) { - bookingTheme = localStorage.getItem(key); - username = key.split("booking-theme:")[1]; - break; - } - } - - const onReady = () => { - const isBookingPage = username && window.location.pathname.slice(1).startsWith(username); - - if (document.body) { - document.body.classList.add(isBookingPage ? bookingTheme : appTheme); - } else { - requestAnimationFrame(onReady); - } - }; - - requestAnimationFrame(onReady); - } catch (e) { - console.error('Error applying theme:', e); - } - })(); + (${applyTheme.toString()})(); `, }} /> diff --git a/packages/eslint-plugin/src/rules/avoid-web-storage.ts b/packages/eslint-plugin/src/rules/avoid-web-storage.ts index 57c7d561c7..5e4e7d404b 100644 --- a/packages/eslint-plugin/src/rules/avoid-web-storage.ts +++ b/packages/eslint-plugin/src/rules/avoid-web-storage.ts @@ -3,10 +3,27 @@ import { ESLintUtils } from "@typescript-eslint/utils"; const createRule = ESLintUtils.RuleCreator((name) => `https://developer.cal.com/eslint/rule/${name}`); const rule = createRule({ create(context) { + // Track imported names from @calcom/lib/webstorage + const safeImportedNames = new Set(); + return { + ImportDeclaration(node) { + // Check if this is an import from the safe webstorage module + if (node.source.value === "@calcom/lib/webstorage") { + node.specifiers.forEach((specifier) => { + if (specifier.type === "ImportSpecifier") { + // Track the local name used for the import + safeImportedNames.add(specifier.local.name); + } + }); + } + }, + CallExpression(node) { const webStorages = ["localStorage", "sessionStorage"]; const callee = node.callee; + + // Check for window.localStorage or window.sessionStorage if ( // Can't figure out how to fix this TS issue // eslint-disable-next-line @typescript-eslint/ban-ts-comment @@ -23,6 +40,71 @@ const rule = createRule({ messageId: "possible-issue-with-embed", }); } + + // Check for direct localStorage or sessionStorage usage + if ( + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + callee.type === "MemberExpression" && + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + callee.object?.type === "Identifier" && + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + webStorages.includes(callee.object?.name) && + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + !safeImportedNames.has(callee.object?.name) + ) { + return context.report({ + node: node, + loc: node.loc, + messageId: "possible-issue-with-embed", + }); + } + }, + + // Also check for property access like localStorage.length + MemberExpression(node) { + const webStorages = ["localStorage", "sessionStorage"]; + + // Check for direct property access on localStorage/sessionStorage + if ( + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + node.object?.type === "Identifier" && + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + webStorages.includes(node.object?.name) && + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + !safeImportedNames.has(node.object?.name) + ) { + return context.report({ + node: node, + loc: node.loc, + messageId: "possible-issue-with-embed", + }); + } + + // Check for window.localStorage/sessionStorage property access + if ( + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + node.object?.type === "MemberExpression" && + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + node.object?.object?.name === "window" && + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + webStorages.includes(node.object?.property?.name) + ) { + return context.report({ + node: node, + loc: node.loc, + messageId: "possible-issue-with-embed", + }); + } }, }; }, diff --git a/packages/lib/hooks/useTheme.tsx b/packages/lib/hooks/useTheme.tsx index e58b661818..ee8a507919 100644 --- a/packages/lib/hooks/useTheme.tsx +++ b/packages/lib/hooks/useTheme.tsx @@ -2,6 +2,7 @@ import { useTheme as useNextTheme } from "next-themes"; import { useEffect } from "react"; import { useEmbedTheme } from "@calcom/embed-core/embed-iframe"; +import { localStorage } from "@calcom/lib/webstorage"; /** * It should be called once per route if you intend to use a theme different from `system` theme. `system` theme is automatically supported using @@ -12,8 +13,7 @@ import { useEmbedTheme } from "@calcom/embed-core/embed-iframe"; // eslint-disable-next-line @typescript-eslint/ban-types export default function useTheme(themeToSet: "system" | (string & {}) | undefined | null, getOnly = false) { if (typeof window !== "undefined") { - const themeFromLocalStorage = - typeof window !== "undefined" && window.localStorage ? localStorage.getItem("app-theme") : null; + const themeFromLocalStorage = localStorage.getItem("app-theme"); themeToSet = themeToSet ?? themeFromLocalStorage ?? "system"; } const { resolvedTheme, setTheme, forcedTheme, theme: activeTheme } = useNextTheme();