From 4d662d31a18b953df4b2b7e39bd7bc5b47053d30 Mon Sep 17 00:00:00 2001 From: "devin-ai-integration[bot]" <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Fri, 8 Aug 2025 19:12:36 +0530 Subject: [PATCH] feat: filter embed query params from booking success redirects (#22028) * feat: filter embed query params from booking success redirects - Add filterEmbedParams option to getNewSearchParams function - Filter out embed, layout, embedType, and ui.color-scheme params when redirecting to external pages - Add comprehensive unit tests for bookingSuccessRedirect covering: - External redirects with and without parameter forwarding - Embed parameter filtering functionality - Internal redirects to booking pages - Booking parameter extraction - All tests pass and type checking succeeds Fixes #20469 Co-Authored-By: hariom@cal.com * chore: retrigger CI checks Co-Authored-By: hariom@cal.com * chore: remove temporary CI retrigger file Co-Authored-By: hariom@cal.com * Handle a case --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: hariom@cal.com Co-authored-by: Hariom Balhara --- packages/lib/bookingSuccessRedirect.test.ts | 533 +++++++++++++++++++- packages/lib/bookingSuccessRedirect.ts | 41 +- 2 files changed, 553 insertions(+), 21 deletions(-) diff --git a/packages/lib/bookingSuccessRedirect.test.ts b/packages/lib/bookingSuccessRedirect.test.ts index d406d8bdfc..9ab1da7932 100644 --- a/packages/lib/bookingSuccessRedirect.test.ts +++ b/packages/lib/bookingSuccessRedirect.test.ts @@ -1,27 +1,528 @@ -import { describe, expect, it } from "vitest"; +import { describe, it, expect, vi, beforeEach, test } from "vitest"; -import { getNewSearchParams } from "./bookingSuccessRedirect"; +import { useIsEmbed } from "@calcom/embed-core/embed-iframe"; +import { useCompatSearchParams } from "@calcom/lib/hooks/useCompatSearchParams"; +import { navigateInTopWindow } from "@calcom/lib/navigateInTopWindow"; + +import { useBookingSuccessRedirect, getNewSearchParams } from "./bookingSuccessRedirect"; + +const mockPush = vi.fn(); + +vi.mock("next/navigation", () => ({ + useRouter: () => ({ + push: mockPush, + }), +})); + +vi.mock("@calcom/lib/navigateInTopWindow", () => ({ + navigateInTopWindow: vi.fn(), +})); + +vi.mock("@calcom/lib/hooks/useCompatSearchParams", () => ({ + useCompatSearchParams: vi.fn(), +})); + +vi.mock("@calcom/embed-core/embed-iframe", () => ({ + useIsEmbed: vi.fn(), +})); + +// Test data constants +const EMBED_PARAMS = ["embed", "layout", "embedType", "ui.color-scheme"]; +const WEBAPP_PARAMS = ["overlayCalendar"]; + +// Helper function to create a mock booking with sensible defaults +type MockBooking = Pick< + BookingResponse, + | "uid" + | "title" + | "description" + | "startTime" + | "endTime" + | "location" + | "attendees" + | "user" + | "responses" +>; + +const createMockBooking = (overrides: Partial = {}): MockBooking => ({ + uid: "test-booking-uid", + title: "Test Meeting", + description: "Test Description", + startTime: new Date("2024-01-01T10:00:00Z"), + endTime: new Date("2024-01-01T11:00:00Z"), + location: "Zoom", + attendees: [ + { + id: 1, + name: "John Doe", + email: "john@example.com", + timeZone: "UTC", + locale: "en", + phoneNumber: "+1234567890", + bookingId: 1, + noShow: false, + }, + ], + user: { + email: null, + name: "Jane Host", + timeZone: "UTC", + username: "jane-host", + }, + responses: { + name: "John Doe", + phone: "+1234567890", + }, + ...overrides, +}); + +// Helper to assert multiple params are null +const assertParamsNull = (result: URLSearchParams, params: string[]) => { + params.forEach((param) => expect(result.get(param)).toBeNull()); +}; + +// Helper to assert multiple params have specific values +const assertParamsEqual = (result: URLSearchParams, paramValues: Record) => { + Object.entries(paramValues).forEach(([key, value]) => { + expect(result.get(key)).toBe(value); + }); +}; + +// Helper to assert URL search params +const assertUrlSearchParams = (url: string, expectedParams: Record) => { + const urlObject = new URL(url); + Object.entries(expectedParams).forEach(([key, value]) => { + if (value === null) { + expect(urlObject.searchParams.get(key)).toBeNull(); + } else { + expect(urlObject.searchParams.get(key)).toBe(value); + } + }); +}; describe("getNewSearchParams", () => { - it("should remove 'embed' param when isEmbed is true", () => { - const searchParams = new URLSearchParams({ embed: "true", foo: "bar" }); - const query = { bookingId: "123" }; + describe("filtering internal params", () => { + describe("when filterInternalParams is true", () => { + test.each([ + { + name: "filters embed params from searchParams", + searchParams: { + embed: "true", + layout: "month_view", + embedType: "inline", + "ui.color-scheme": "dark", + foo: "bar", + }, + query: { bookingId: "123" }, + expectedNull: ["embed", "layout", "embedType", "ui.color-scheme"], + expectedPresent: { foo: "bar", bookingId: "123" }, + }, + { + name: "filters webapp params from searchParams", + searchParams: { overlayCalendar: "true", foo: "bar" }, + query: { bookingId: "123" }, + expectedNull: ["overlayCalendar"], + expectedPresent: { foo: "bar", bookingId: "123" }, + }, + { + name: "filters both embed and webapp params from searchParams", + searchParams: { embed: "true", layout: "month_view", overlayCalendar: "true", foo: "bar" }, + query: { bookingId: "123" }, + expectedNull: ["embed", "layout", "overlayCalendar"], + expectedPresent: { foo: "bar", bookingId: "123" }, + }, + ])("$name", ({ searchParams, query, expectedNull, expectedPresent }) => { + const result = getNewSearchParams({ + query, + searchParams: new URLSearchParams(searchParams), + filterInternalParams: true, + }); - const result = getNewSearchParams({ query, searchParams, isEmbed: true }); + assertParamsNull(result, expectedNull); + assertParamsEqual(result, expectedPresent); + }); - expect(result.get("embed")).toBeNull(); - expect(result.get("foo")).toBe("bar"); - expect(result.get("bookingId")).toBe("123"); + test.each([ + { + name: "filters embed params from query object", + query: { + bookingId: "789", + embed: "namespace", + layout: "month_view", + embedType: "inline", + "ui.color-scheme": "dark", + test: "value", + }, + expectedNull: ["embed", "layout", "embedType", "ui.color-scheme"], + expectedPresent: { bookingId: "789", test: "value" }, + }, + { + name: "filters webapp params from query object", + query: { + bookingId: "789", + overlayCalendar: "true", + test: "value", + }, + expectedNull: ["overlayCalendar"], + expectedPresent: { bookingId: "789", test: "value" }, + }, + ])("$name", ({ query, expectedNull, expectedPresent }) => { + const result = getNewSearchParams({ query, filterInternalParams: true }); + + assertParamsNull(result, expectedNull); + assertParamsEqual(result, expectedPresent); + }); + }); + + describe("when filterInternalParams is false", () => { + test.each([ + { + name: "preserves embed params", + searchParams: { embed: "true", layout: "month_view", foo: "bar" }, + query: { bookingId: "456" }, + expectedPresent: { embed: "true", layout: "month_view", foo: "bar", bookingId: "456" }, + }, + { + name: "preserves webapp params", + searchParams: { overlayCalendar: "true", foo: "bar" }, + query: { bookingId: "456" }, + expectedPresent: { overlayCalendar: "true", foo: "bar", bookingId: "456" }, + }, + ])("$name", ({ searchParams, query, expectedPresent }) => { + const result = getNewSearchParams({ + query, + searchParams: new URLSearchParams(searchParams), + filterInternalParams: false, + }); + + assertParamsEqual(result, expectedPresent); + }); + }); + + describe("when filterInternalParams is not provided", () => { + it("defaults to false and preserves all params", () => { + const searchParams = new URLSearchParams({ + embed: "true", + overlayCalendar: "true", + foo: "bar", + }); + const query = { bookingId: "123" }; + + const result = getNewSearchParams({ query, searchParams }); + + assertParamsEqual(result, { + embed: "true", + overlayCalendar: "true", + foo: "bar", + bookingId: "123", + }); + }); + }); }); - it("should keep 'embed' param when isEmbed is false", () => { - const searchParams = new URLSearchParams({ embed: "true", foo: "bar" }); - const query = { bookingId: "456" }; + describe("edge cases", () => { + it("handles null values in query", () => { + const query = { + bookingId: "123", + nullParam: null, + undefinedParam: undefined, + validParam: "value", + }; - const result = getNewSearchParams({ query, searchParams, isEmbed: false }); + const result = getNewSearchParams({ query }); - expect(result.get("embed")).toBe("true"); - expect(result.get("foo")).toBe("bar"); - expect(result.get("bookingId")).toBe("456"); + expect(result.get("bookingId")).toBe("123"); + expect(result.get("nullParam")).toBeNull(); + expect(result.get("undefinedParam")).toBeNull(); + expect(result.get("validParam")).toBe("value"); + }); + + it("handles empty strings in query", () => { + const query = { + emptyString: "", + bookingId: "123", + }; + + const result = getNewSearchParams({ query }); + + expect(result.get("emptyString")).toBe(""); + expect(result.get("bookingId")).toBe("123"); + }); + + it("handles boolean values in query", () => { + const query = { + boolTrue: true, + boolFalse: false, + }; + + const result = getNewSearchParams({ query }); + + expect(result.get("boolTrue")).toBe("true"); + expect(result.get("boolFalse")).toBe("false"); + }); + + it("handles missing searchParams", () => { + const query = { bookingId: "123", test: "value" }; + + const result = getNewSearchParams({ query, filterInternalParams: true }); + + assertParamsEqual(result, { bookingId: "123", test: "value" }); + }); + + it("handles empty query object", () => { + const searchParams = new URLSearchParams({ foo: "bar" }); + + const result = getNewSearchParams({ query: {}, searchParams }); + + assertParamsEqual(result, { foo: "bar" }); + }); + + it("query params are appended after searchParams when keys conflict", () => { + const searchParams = new URLSearchParams({ shared: "fromSearchParams", foo: "bar" }); + const query = { shared: "fromQuery", baz: "qux" }; + + const result = getNewSearchParams({ query, searchParams }); + + // When using append, both values are kept with the first one returned by get() + expect(result.get("shared")).toBe("fromSearchParams"); + expect(result.getAll("shared")).toEqual(["fromSearchParams", "fromQuery"]); + expect(result.get("foo")).toBe("bar"); + expect(result.get("baz")).toBe("qux"); + }); + }); +}); + +describe("useBookingSuccessRedirect", () => { + beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(useCompatSearchParams).mockReturnValue(new URLSearchParams() as any); + vi.mocked(useIsEmbed).mockReturnValue(false); + }); + + const mockBooking = createMockBooking(); + + describe("external redirects", () => { + describe("without forwardParamsSuccessRedirect", () => { + it("redirects to external URL without any parameters", () => { + const bookingSuccessRedirect = useBookingSuccessRedirect(); + + bookingSuccessRedirect({ + successRedirectUrl: "https://example.com/success", + forwardParamsSuccessRedirect: false, + query: { test: "value", embed: "namespace" }, + booking: mockBooking, + }); + + expect(navigateInTopWindow).toHaveBeenCalledWith("https://example.com/success"); + }); + }); + + describe("with forwardParamsSuccessRedirect", () => { + test.each([ + { + name: "filters out embed params from query", + query: { + test: "value", + embed: "namespace", + layout: "month_view", + embedType: "inline", + "ui.color-scheme": "dark", + }, + searchParams: "", + expectedPresent: { test: "value", uid: "test-booking-uid" }, + expectedNull: ["embed", "layout", "embedType", "ui.color-scheme"], + }, + { + name: "filters out webapp params from query", + query: { + test: "value", + overlayCalendar: "true", + }, + searchParams: "", + expectedPresent: { test: "value", uid: "test-booking-uid" }, + expectedNull: ["overlayCalendar"], + }, + { + name: "filters out embed params from searchParams", + query: { additional: "param" }, + searchParams: "embed=namespace&layout=month_view&embedType=inline&ui.color-scheme=dark&test=value", + expectedPresent: { test: "value", additional: "param", uid: "test-booking-uid" }, + expectedNull: ["embed", "layout", "embedType", "ui.color-scheme"], + }, + { + name: "filters out webapp params from searchParams", + query: { additional: "param" }, + searchParams: "overlayCalendar=true&test=value", + expectedPresent: { test: "value", additional: "param", uid: "test-booking-uid" }, + expectedNull: ["overlayCalendar"], + }, + ])("$name", ({ query, searchParams, expectedPresent, expectedNull }) => { + if (searchParams) { + vi.mocked(useCompatSearchParams).mockReturnValue(new URLSearchParams(searchParams) as any); + } + + const bookingSuccessRedirect = useBookingSuccessRedirect(); + + bookingSuccessRedirect({ + successRedirectUrl: "https://example.com/success", + forwardParamsSuccessRedirect: true, + query, + booking: mockBooking, + }); + + const calledUrl = vi.mocked(navigateInTopWindow).mock.calls[0][0]; + const url = new URL(calledUrl); + + // Check expected present params (allow for additional booking params) + Object.entries(expectedPresent).forEach(([key, value]) => { + expect(url.searchParams.get(key)).toBe(value); + }); + + // Check expected null params + expectedNull.forEach((param) => { + expect(url.searchParams.get(param)).toBeNull(); + }); + }); + + it("includes cal.rerouting param from searchParams", () => { + vi.mocked(useCompatSearchParams).mockReturnValue(new URLSearchParams("cal.rerouting=true") as any); + + const bookingSuccessRedirect = useBookingSuccessRedirect(); + + bookingSuccessRedirect({ + successRedirectUrl: "https://example.com/success", + forwardParamsSuccessRedirect: true, + query: { test: "value" }, + booking: mockBooking, + }); + + const calledUrl = vi.mocked(navigateInTopWindow).mock.calls[0][0]; + assertUrlSearchParams(calledUrl, { "cal.rerouting": "true" }); + }); + }); + }); + + describe("internal redirects to booking page", () => { + describe("URL structure", () => { + test.each([ + { isEmbed: false, expectedPath: "/booking/test-booking-uid?" }, + { isEmbed: true, expectedPath: "/booking/test-booking-uid/embed?" }, + ])("redirects to $expectedPath when isEmbed is $isEmbed", ({ isEmbed, expectedPath }) => { + vi.mocked(useIsEmbed).mockReturnValue(isEmbed); + + const bookingSuccessRedirect = useBookingSuccessRedirect(); + + bookingSuccessRedirect({ + successRedirectUrl: null, + forwardParamsSuccessRedirect: false, + query: { test: "value" }, + booking: mockBooking, + }); + + expect(mockPush).toHaveBeenCalledWith(expect.stringContaining(expectedPath)); + }); + }); + + describe("param preservation for internal navigation", () => { + it("preserves ALL params including embed and webapp params (no filtering)", () => { + vi.mocked(useIsEmbed).mockReturnValue(false); + + const bookingSuccessRedirect = useBookingSuccessRedirect(); + + bookingSuccessRedirect({ + successRedirectUrl: null, + forwardParamsSuccessRedirect: false, + query: { + test: "value", + embed: "namespace", + layout: "month_view", + embedType: "inline", + "ui.color-scheme": "dark", + overlayCalendar: "true", + }, + booking: mockBooking, + }); + + const calledUrl = mockPush.mock.calls[0][0]; + + // ALL params should be preserved for internal navigation + expect(calledUrl).toContain("test=value"); + expect(calledUrl).toContain("embed=namespace"); + expect(calledUrl).toContain("layout=month_view"); + expect(calledUrl).toContain("embedType=inline"); + expect(calledUrl).toContain("ui.color-scheme=dark"); + expect(calledUrl).toContain("overlayCalendar=true"); + }); + + test.each([ + { + name: "includes flag.coep from searchParams", + searchParams: "flag.coep=true", + expectedParam: "flag.coep=true", + }, + { + name: "includes cal.rerouting from searchParams", + searchParams: "cal.rerouting=true", + expectedParam: "cal.rerouting=true", + }, + ])("$name", ({ searchParams, expectedParam }) => { + vi.mocked(useCompatSearchParams).mockReturnValue(new URLSearchParams(searchParams) as any); + + const bookingSuccessRedirect = useBookingSuccessRedirect(); + + bookingSuccessRedirect({ + successRedirectUrl: null, + forwardParamsSuccessRedirect: false, + query: { test: "value" }, + booking: mockBooking, + }); + + const calledUrl = mockPush.mock.calls[0][0]; + expect(calledUrl).toContain(expectedParam); + }); + }); + }); + + describe("edge cases", () => { + it("handles null successRedirectUrl correctly", () => { + const bookingSuccessRedirect = useBookingSuccessRedirect(); + + bookingSuccessRedirect({ + successRedirectUrl: null, + forwardParamsSuccessRedirect: true, + query: { test: "value" }, + booking: mockBooking, + }); + + expect(mockPush).toHaveBeenCalled(); + expect(navigateInTopWindow).not.toHaveBeenCalled(); + }); + + it("handles undefined successRedirectUrl correctly", () => { + const bookingSuccessRedirect = useBookingSuccessRedirect(); + + bookingSuccessRedirect({ + successRedirectUrl: undefined as any, + forwardParamsSuccessRedirect: true, + query: { test: "value" }, + booking: mockBooking, + }); + + expect(mockPush).toHaveBeenCalled(); + expect(navigateInTopWindow).not.toHaveBeenCalled(); + }); + + it("handles empty query object", () => { + const bookingSuccessRedirect = useBookingSuccessRedirect(); + + bookingSuccessRedirect({ + successRedirectUrl: "https://example.com/success", + forwardParamsSuccessRedirect: false, + query: {}, + booking: mockBooking, + }); + + expect(navigateInTopWindow).toHaveBeenCalledWith("https://example.com/success"); + }); }); }); diff --git a/packages/lib/bookingSuccessRedirect.ts b/packages/lib/bookingSuccessRedirect.ts index 7142d89caa..1f3a8d3e3c 100644 --- a/packages/lib/bookingSuccessRedirect.ts +++ b/packages/lib/bookingSuccessRedirect.ts @@ -13,20 +13,47 @@ import { getSafe } from "./getSafe"; export function getNewSearchParams(args: { query: Record; searchParams?: URLSearchParams; - isEmbed?: boolean; + filterInternalParams?: boolean; }) { - const { query, searchParams, isEmbed } = args; - const newSearchParams = new URLSearchParams(searchParams); + const { query, searchParams, filterInternalParams = false } = args; + const newSearchParams = new URLSearchParams(); - if (isEmbed) { - newSearchParams.delete("embed"); + // Embed-specific params + const embedParams = new Set(["embed", "layout", "embedType", "ui.color-scheme"]); + + // Webapp-specific params + const webappParams = new Set(["overlayCalendar"]); + + // Add non-excluded params from searchParams if provided + if (searchParams) { + searchParams.forEach((value, key) => { + if (shouldExcludeParam(key)) { + return; + } + newSearchParams.append(key, value); + }); } + + // Add params from query, filtering excluded params Object.entries(query).forEach(([key, value]) => { if (value === null || value === undefined) { return; } + + if (shouldExcludeParam(key)) { + return; + } + newSearchParams.append(key, String(value)); }); + + function shouldExcludeParam(key: string) { + if (filterInternalParams) { + return embedParams.has(key) || webappParams.has(key); + } + return false; + } + return newSearchParams; } @@ -185,6 +212,9 @@ export const useBookingSuccessRedirect = () => { const bookingExtraParams = getBookingRedirectExtraParams(booking); + // Filter internal Cal.com params when redirecting to external URLs. + // - It prevents leaking internal state. + // - Certain websites might break due to the presence of certain params e.g. Wordpress has different meaning for `embed` param and an embed param passed by Cal.com breaks a wordpress webpage const newSearchParams = getNewSearchParams({ query: { ...query, @@ -192,6 +222,7 @@ export const useBookingSuccessRedirect = () => { isEmbed, }, searchParams: new URLSearchParams(searchParams.toString()), + filterInternalParams: true, }); newSearchParams.forEach((value, key) => {