fix: Embed Plus Org - Adds missing embed route, also fixes infinite redirection for migrated user in embed (#12071)

2023-10-26 13:42:05 +05:30 · 2023-10-26 13:42:05 +05:30 · bf8580fa88
parent c7b1e4dfa1
commit bf8580fa88
11 changed files with 461 additions and 7 deletions
--- a/apps/web/lib/getTemporaryOrgRedirect.test.ts
+++ b/apps/web/lib/getTemporaryOrgRedirect.test.ts
@ -0,0 +1,162 @@
+import prismaMock from "../../../tests/libs/__mocks__/prismaMock";
+
+import { describe, it, expect } from "vitest";
+
+import { RedirectType } from "@calcom/prisma/client";
+
+import { getTemporaryOrgRedirect } from "./getTemporaryOrgRedirect";
+
+function mockARedirectInDB({
+  toUrl,
+  slug,
+  redirectType,
+}: {
+  toUrl: string;
+  slug: string;
+  redirectType: RedirectType;
+}) {
+  // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+  //@ts-ignore
+  prismaMock.tempOrgRedirect.findUnique.mockImplementation(({ where }) => {
+    return new Promise((resolve) => {
+      if (
+        where.from_type_fromOrgId.type === redirectType &&
+        where.from_type_fromOrgId.from === slug &&
+        where.from_type_fromOrgId.fromOrgId === 0
+      ) {
+        resolve({ toUrl });
+      } else {
+        resolve(null);
+      }
+    });
+  });
+}
+
+describe("getTemporaryOrgRedirect", () => {
+  it("should generate event-type URL without existing query params", async () => {
+    mockARedirectInDB({ slug: "slug", toUrl: "https://calcom.cal.com", redirectType: RedirectType.User });
+    const redirect = await getTemporaryOrgRedirect({
+      slug: "slug",
+      redirectType: RedirectType.User,
+      eventTypeSlug: "30min",
+      currentQuery: {},
+    });
+
+    expect(redirect).toEqual({
+      redirect: {
+        permanent: false,
+        destination: "https://calcom.cal.com/30min",
+      },
+    });
+  });
+
+  it("should generate event-type URL with existing query params", async () => {
+    mockARedirectInDB({ slug: "slug", toUrl: "https://calcom.cal.com", redirectType: RedirectType.User });
+
+    const redirect = await getTemporaryOrgRedirect({
+      slug: "slug",
+      redirectType: RedirectType.User,
+      eventTypeSlug: "30min",
+      currentQuery: {
+        abc: "1",
+      },
+    });
+
+    expect(redirect).toEqual({
+      redirect: {
+        permanent: false,
+        destination: "https://calcom.cal.com/30min?abc=1",
+      },
+    });
+  });
+
+  it("should generate User URL with existing query params", async () => {
+    mockARedirectInDB({ slug: "slug", toUrl: "https://calcom.cal.com", redirectType: RedirectType.User });
+
+    const redirect = await getTemporaryOrgRedirect({
+      slug: "slug",
+      redirectType: RedirectType.User,
+      eventTypeSlug: null,
+      currentQuery: {
+        abc: "1",
+      },
+    });
+
+    expect(redirect).toEqual({
+      redirect: {
+        permanent: false,
+        destination: "https://calcom.cal.com?abc=1",
+      },
+    });
+  });
+
+  it("should generate Team Profile URL with existing query params", async () => {
+    mockARedirectInDB({
+      slug: "seeded-team",
+      toUrl: "https://calcom.cal.com",
+      redirectType: RedirectType.Team,
+    });
+
+    const redirect = await getTemporaryOrgRedirect({
+      slug: "seeded-team",
+      redirectType: RedirectType.Team,
+      eventTypeSlug: null,
+      currentQuery: {
+        abc: "1",
+      },
+    });
+
+    expect(redirect).toEqual({
+      redirect: {
+        permanent: false,
+        destination: "https://calcom.cal.com?abc=1",
+      },
+    });
+  });
+
+  it("should generate Team Event URL with existing query params", async () => {
+    mockARedirectInDB({
+      slug: "seeded-team",
+      toUrl: "https://calcom.cal.com",
+      redirectType: RedirectType.Team,
+    });
+
+    const redirect = await getTemporaryOrgRedirect({
+      slug: "seeded-team",
+      redirectType: RedirectType.Team,
+      eventTypeSlug: "30min",
+      currentQuery: {
+        abc: "1",
+      },
+    });
+
+    expect(redirect).toEqual({
+      redirect: {
+        permanent: false,
+        destination: "https://calcom.cal.com/30min?abc=1",
+      },
+    });
+  });
+
+  it("should generate Team Event URL without query params", async () => {
+    mockARedirectInDB({
+      slug: "seeded-team",
+      toUrl: "https://calcom.cal.com",
+      redirectType: RedirectType.Team,
+    });
+
+    const redirect = await getTemporaryOrgRedirect({
+      slug: "seeded-team",
+      redirectType: RedirectType.Team,
+      eventTypeSlug: "30min",
+      currentQuery: {},
+    });
+
+    expect(redirect).toEqual({
+      redirect: {
+        permanent: false,
+        destination: "https://calcom.cal.com/30min",
+      },
+    });
+  });
+});
--- a/apps/web/lib/getTemporaryOrgRedirect.ts
+++ b/apps/web/lib/getTemporaryOrgRedirect.ts
@ -1,3 +1,6 @@
+import type { ParsedUrlQuery } from "querystring";
+import { stringify } from "querystring";
+
 import logger from "@calcom/lib/logger";
 import { safeStringify } from "@calcom/lib/safeStringify";
 import type { RedirectType } from "@calcom/prisma/client";
@ -7,10 +10,12 @@ export const getTemporaryOrgRedirect = async ({
  slug,
  redirectType,
  eventTypeSlug,
+  currentQuery,
 }: {
  slug: string;
  redirectType: RedirectType;
  eventTypeSlug: string | null;
+  currentQuery: ParsedUrlQuery;
 }) => {
  const prisma = (await import("@calcom/prisma")).default;
  log.debug(
@ -33,10 +38,12 @@ export const getTemporaryOrgRedirect = async ({

  if (redirect) {
    log.debug(`Redirecting ${slug} to ${redirect.toUrl}`);
+    const newDestinationWithoutQuery = eventTypeSlug ? `${redirect.toUrl}/${eventTypeSlug}` : redirect.toUrl;
+    const currentQueryString = stringify(currentQuery);
    return {
      redirect: {
        permanent: false,
-        destination: eventTypeSlug ? `${redirect.toUrl}/${eventTypeSlug}` : redirect.toUrl,
+        destination: `${newDestinationWithoutQuery}${currentQueryString ? `?${currentQueryString}` : ""}`,
      },
    } as const;
  }
--- a/apps/web/lib/withEmbedSsr.test.ts
+++ b/apps/web/lib/withEmbedSsr.test.ts
@ -0,0 +1,258 @@
+import type { Request, Response } from "express";
+import type { Redirect } from "next";
+import type { NextApiRequest, NextApiResponse } from "next";
+import { createMocks } from "node-mocks-http";
+import { describe, expect, it } from "vitest";
+
+import withEmbedSsr from "./withEmbedSsr";
+
+export type CustomNextApiRequest = NextApiRequest & Request;
+
+export type CustomNextApiResponse = NextApiResponse & Response;
+export function createMockNextJsRequest(...args: Parameters<typeof createMocks>) {
+  return createMocks<CustomNextApiRequest, CustomNextApiResponse>(...args);
+}
+
+function getServerSidePropsFnGenerator(
+  config:
+    | { redirectUrl: string }
+    | { props: Record<string, unknown> }
+    | {
+        notFound: true;
+      }
+) {
+  if ("redirectUrl" in config)
+    return async () => {
+      return {
+        redirect: {
+          permanent: false,
+          destination: config.redirectUrl,
+        } satisfies Redirect,
+      };
+    };
+
+  if ("props" in config)
+    return async () => {
+      return {
+        props: config.props,
+      };
+    };
+
+  if ("notFound" in config)
+    return async () => {
+      return {
+        notFound: true as const,
+      };
+    };
+
+  throw new Error("Invalid config");
+}
+
+function getServerSidePropsContextArg({
+  embedRelatedParams,
+}: {
+  embedRelatedParams?: Record<string, string>;
+}) {
+  return {
+    ...createMockNextJsRequest(),
+    query: {
+      ...embedRelatedParams,
+    },
+    resolvedUrl: "/MOCKED_RESOLVED_URL",
+  };
+}
+
+describe("withEmbedSsr", () => {
+  describe("when gSSP returns redirect", () => {
+    describe("when redirect destination is relative, should add /embed to end of the path", () => {
+      it("should add layout and embed params from the current query", async () => {
+        const withEmbedGetSsr = withEmbedSsr(
+          getServerSidePropsFnGenerator({
+            redirectUrl: "/reschedule",
+          })
+        );
+
+        const ret = await withEmbedGetSsr(
+          getServerSidePropsContextArg({
+            embedRelatedParams: {
+              layout: "week_view",
+              embed: "namespace1",
+            },
+          })
+        );
+        expect(ret).toEqual({
+          redirect: {
+            destination: "/reschedule/embed?layout=week_view&embed=namespace1",
+            permanent: false,
+          },
+        });
+      });
+
+      it("should add layout and embed params without losing query params that were in redirect", async () => {
+        const withEmbedGetSsr = withEmbedSsr(
+          getServerSidePropsFnGenerator({
+            redirectUrl: "/reschedule?redirectParam=1",
+          })
+        );
+
+        const ret = await withEmbedGetSsr(
+          getServerSidePropsContextArg({
+            embedRelatedParams: {
+              layout: "week_view",
+              embed: "namespace1",
+            },
+          })
+        );
+        expect(ret).toEqual({
+          redirect: {
+            destination: "/reschedule/embed?redirectParam=1&layout=week_view&embed=namespace1",
+            permanent: false,
+          },
+        });
+      });
+
+      it("should add embed param even when it was empty(i.e. default namespace of embed)", async () => {
+        const withEmbedGetSsr = withEmbedSsr(
+          getServerSidePropsFnGenerator({
+            redirectUrl: "/reschedule?redirectParam=1",
+          })
+        );
+
+        const ret = await withEmbedGetSsr(
+          getServerSidePropsContextArg({
+            embedRelatedParams: {
+              layout: "week_view",
+              embed: "",
+            },
+          })
+        );
+        expect(ret).toEqual({
+          redirect: {
+            destination: "/reschedule/embed?redirectParam=1&layout=week_view&embed=",
+            permanent: false,
+          },
+        });
+      });
+    });
+
+    describe("when redirect destination is absolute, should add /embed to end of the path", () => {
+      it("should add layout and embed params from the current query when destination URL is HTTPS", async () => {
+        const withEmbedGetSsr = withEmbedSsr(
+          getServerSidePropsFnGenerator({
+            redirectUrl: "https://calcom.cal.local/owner",
+          })
+        );
+
+        const ret = await withEmbedGetSsr(
+          getServerSidePropsContextArg({
+            embedRelatedParams: {
+              layout: "week_view",
+              embed: "namespace1",
+            },
+          })
+        );
+
+        expect(ret).toEqual({
+          redirect: {
+            destination: "https://calcom.cal.local/owner/embed?layout=week_view&embed=namespace1",
+            permanent: false,
+          },
+        });
+      });
+      it("should add layout and embed params from the current query when destination URL is HTTP", async () => {
+        const withEmbedGetSsr = withEmbedSsr(
+          getServerSidePropsFnGenerator({
+            redirectUrl: "http://calcom.cal.local/owner",
+          })
+        );
+
+        const ret = await withEmbedGetSsr(
+          getServerSidePropsContextArg({
+            embedRelatedParams: {
+              layout: "week_view",
+              embed: "namespace1",
+            },
+          })
+        );
+
+        expect(ret).toEqual({
+          redirect: {
+            destination: "http://calcom.cal.local/owner/embed?layout=week_view&embed=namespace1",
+            permanent: false,
+          },
+        });
+      });
+      it("should correctly identify a URL as non absolute URL if protocol is missing", async () => {
+        const withEmbedGetSsr = withEmbedSsr(
+          getServerSidePropsFnGenerator({
+            redirectUrl: "httpcalcom.cal.local/owner",
+          })
+        );
+
+        const ret = await withEmbedGetSsr(
+          getServerSidePropsContextArg({
+            embedRelatedParams: {
+              layout: "week_view",
+              embed: "namespace1",
+            },
+          })
+        );
+
+        expect(ret).toEqual({
+          redirect: {
+            // FIXME: Note that it is adding a / in the beginning of the path, which might be fine for now, but could be an issue
+            destination: "/httpcalcom.cal.local/owner/embed?layout=week_view&embed=namespace1",
+            permanent: false,
+          },
+        });
+      });
+    });
+  });
+
+  describe("when gSSP returns props", () => {
+    it("should add isEmbed=true prop", async () => {
+      const withEmbedGetSsr = withEmbedSsr(
+        getServerSidePropsFnGenerator({
+          props: {
+            prop1: "value1",
+          },
+        })
+      );
+      const ret = await withEmbedGetSsr(
+        getServerSidePropsContextArg({
+          embedRelatedParams: {
+            layout: "week_view",
+            embed: "",
+          },
+        })
+      );
+      expect(ret).toEqual({
+        props: {
+          prop1: "value1",
+          isEmbed: true,
+        },
+      });
+    });
+  });
+
+  describe("when gSSP doesn't have props or redirect ", () => {
+    it("should return the result from gSSP as is", async () => {
+      const withEmbedGetSsr = withEmbedSsr(
+        getServerSidePropsFnGenerator({
+          notFound: true,
+        })
+      );
+
+      const ret = await withEmbedGetSsr(
+        getServerSidePropsContextArg({
+          embedRelatedParams: {
+            layout: "week_view",
+            embed: "",
+          },
+        })
+      );
+
+      expect(ret).toEqual({ notFound: true });
+    });
+  });
+});
--- a/apps/web/lib/withEmbedSsr.tsx
+++ b/apps/web/lib/withEmbedSsr.tsx
@ -1,5 +1,7 @@
 import type { GetServerSideProps, GetServerSidePropsContext, GetServerSidePropsResult } from "next";

+import { WEBAPP_URL } from "@calcom/lib/constants";
+
 export type EmbedProps = {
  isEmbed?: boolean;
 };
@ -11,14 +13,25 @@ export default function withEmbedSsr(getServerSideProps: GetServerSideProps) {
    const layout = context.query.layout;

    if ("redirect" in ssrResponse) {
-      // Use a dummy URL https://base as the fallback base URL so that URL parsing works for relative URLs as well.
-      const destinationUrlObj = new URL(ssrResponse.redirect.destination, "https://base");
+      const destinationUrl = ssrResponse.redirect.destination;
+      let urlPrefix = "";

+      // Get the URL parsed from URL so that we can reliably read pathname and searchParams from it.
+      const destinationUrlObj = new URL(ssrResponse.redirect.destination, WEBAPP_URL);
+
+      // If it's a complete URL, use the origin as the prefix to ensure we redirect to the same domain.
+      if (destinationUrl.search(/^(http:|https:).*/) !== -1) {
+        urlPrefix = destinationUrlObj.origin;
+      } else {
+        // Don't use any prefix for relative URLs to ensure we stay on the same domain
+        urlPrefix = "";
+      }
+
+      const destinationQueryStr = destinationUrlObj.searchParams.toString();
      // Make sure that redirect happens to /embed page and pass on embed query param as is for preserving Cal JS API namespace
-      const newDestinationUrl = `${
-        destinationUrlObj.pathname
-      }/embed?${destinationUrlObj.searchParams.toString()}&layout=${layout}&embed=${embed}`;
-
+      const newDestinationUrl = `${urlPrefix}${destinationUrlObj.pathname}/embed?${
+        destinationQueryStr ? `${destinationQueryStr}&` : ""
+      }layout=${layout}&embed=${embed}`;
      return {
        ...ssrResponse,
        redirect: {
--- a/apps/web/pages/[user].tsx
+++ b/apps/web/pages/[user].tsx
@ -343,6 +343,7 @@ export const getServerSideProps: GetServerSideProps<UserPageProps> = async (cont
      slug: usernameList[0],
      redirectType: RedirectType.User,
      eventTypeSlug: null,
+      currentQuery: context.query,
    });

    if (redirect) {
--- a/apps/web/pages/[user]/[type].tsx
+++ b/apps/web/pages/[user]/[type].tsx
@ -160,6 +160,7 @@ async function getUserPageProps(context: GetServerSidePropsContext) {
      slug: usernames[0],
      redirectType: RedirectType.User,
      eventTypeSlug: slug,
+      currentQuery: context.query,
    });

    if (redirect) {
--- a/apps/web/pages/org/[orgSlug]/[user]/embed.tsx
+++ b/apps/web/pages/org/[orgSlug]/[user]/embed.tsx
@ -0,0 +1,7 @@
+import withEmbedSsr from "@lib/withEmbedSsr";
+
+import { getServerSideProps as _getServerSideProps } from "../[user]";
+
+export { default } from "../[user]";
+
+export const getServerSideProps = withEmbedSsr(_getServerSideProps);
--- a/apps/web/pages/team/[slug].tsx
+++ b/apps/web/pages/team/[slug].tsx
@ -299,6 +299,7 @@ export const getServerSideProps = async (context: GetServerSidePropsContext) =>
      slug: slug,
      redirectType: RedirectType.Team,
      eventTypeSlug: null,
+      currentQuery: context.query,
    });

    if (redirect) {
--- a/apps/web/pages/team/[slug]/[type].tsx
+++ b/apps/web/pages/team/[slug]/[type].tsx
@ -85,6 +85,7 @@ export const getServerSideProps = async (context: GetServerSidePropsContext) =>
      slug: teamSlug,
      redirectType: RedirectType.Team,
      eventTypeSlug: meetingSlug,
+      currentQuery: context.query,
    });

    if (redirect) {
--- a/apps/web/playwright/booking-seats.e2e.ts
+++ b/apps/web/playwright/booking-seats.e2e.ts
@ -435,6 +435,8 @@ test.describe("Reschedule for booking with seats", () => {

    await page.locator('[data-testid="confirm_cancel"]').click();

+    await page.waitForLoadState("networkidle");
+
    const oldBooking = await prisma.booking.findFirst({
      where: { uid: booking.uid },
      select: {
--- a/packages/prisma/schema.prisma
+++ b/packages/prisma/schema.prisma
@ -991,6 +991,7 @@ model TempOrgRedirect {
  // 0 would mean it is non org
  fromOrgId Int
  type      RedirectType
+  // It doesn't have any query params
  toUrl     String
  enabled   Boolean      @default(true)
  createdAt DateTime     @default(now())