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 <hariom@cal.com> * chore: retrigger CI checks Co-Authored-By: hariom@cal.com <hariom@cal.com> * chore: remove temporary CI retrigger file Co-Authored-By: hariom@cal.com <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 <hariom@cal.com> Co-authored-by: Hariom Balhara <hariombalhara@gmail.com>
2025-08-08 19:12:36 +05:30
parent 6fe6540bc6
commit 4d662d31a1
2 changed files with 553 additions and 21 deletions
@@ -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> = {}): 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<string, string>) => {
+  Object.entries(paramValues).forEach(([key, value]) => {
+    expect(result.get(key)).toBe(value);
+  });
+};
+
+// Helper to assert URL search params
+const assertUrlSearchParams = (url: string, expectedParams: Record<string, string | null>) => {
+  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");
+    });
  });
 });
@@ -13,20 +13,47 @@ import { getSafe } from "./getSafe";
 export function getNewSearchParams(args: {
  query: Record<string, string | null | undefined | boolean>;
  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) => {