fix: 404 on the public form page for a sub-team form when the owner itself was removed from the org (#18495)

* wip * fix: 404 for a form of a member that has been removed. - Moved existing authorization check to isAuthorizedToViewFormOnOrgDomain function to handle form access control - Add comprehensive test suite for org domain authorization scenarios - Refactor getServerSideProps to use the new authorization function - Improve code organization by separating domain-specific authorization logic Test Coverage: - Non-org domain access - Org member's form access - Sub-team form access within org - Access denial for non-org members/teams
2025-01-07 12:13:19 +00:00
parent 0c69c5dfea
commit 06d8b98b15
4 changed files with 167 additions and 38 deletions
@@ -11,10 +11,10 @@ import { handleResponse } from "@calcom/app-store/routing-forms/lib/handleRespon
 import { findMatchingRoute } from "@calcom/app-store/routing-forms/lib/processRoute";
 import { substituteVariables } from "@calcom/app-store/routing-forms/lib/substituteVariables";
 import { getFieldResponseForJsonLogic } from "@calcom/app-store/routing-forms/lib/transformResponse";
-import { isAuthorizedToViewTheForm } from "@calcom/app-store/routing-forms/pages/routing-link/getServerSideProps";
 import { getUrlSearchParamsToForward } from "@calcom/app-store/routing-forms/pages/routing-link/getUrlSearchParamsToForward";
 import type { FormResponse } from "@calcom/app-store/routing-forms/types/types";
 import { orgDomainConfig } from "@calcom/features/ee/organizations/lib/orgDomains";
+import { isAuthorizedToViewFormOnOrgDomain } from "@calcom/features/routing-forms/lib/isAuthorizedToViewForm";
 import logger from "@calcom/lib/logger";
 import { RoutingFormRepository } from "@calcom/lib/server/repository/routingForm";
 import { TRPCError } from "@calcom/trpc/server";
@@ -70,7 +70,9 @@ export const getServerSideProps = async function getServerSideProps(context: Get
  };
  timeTaken.profileEnrichment = performance.now() - profileEnrichmentStart;

-  if (!isAuthorizedToViewTheForm({ user: formWithUserProfile.user, currentOrgDomain })) {
+  if (
+    !isAuthorizedToViewFormOnOrgDomain({ user: formWithUserProfile.user, currentOrgDomain, team: form.team })
+  ) {
    return {
      notFound: true,
    };
@@ -1,44 +1,10 @@
 import { orgDomainConfig } from "@calcom/features/ee/organizations/lib/orgDomains";
-import type { Prisma } from "@calcom/prisma/client";
-import { userMetadata } from "@calcom/prisma/zod-utils";
+import { isAuthorizedToViewFormOnOrgDomain } from "@calcom/features/routing-forms/lib/isAuthorizedToViewForm";
 import type { AppGetServerSidePropsContext, AppPrisma } from "@calcom/types/AppGetServerSideProps";

 import { enrichFormWithMigrationData } from "../../enrichFormWithMigrationData";
 import { getSerializableForm } from "../../lib/getSerializableForm";

-export function isAuthorizedToViewTheForm({
-  user,
-  currentOrgDomain,
-}: {
-  user: {
-    username: string | null;
-    metadata: Prisma.JsonValue;
-    movedToProfileId: number | null;
-    profile: {
-      organization: { slug: string | null; requestedSlug: string | null } | null;
-    };
-    id: number;
-  };
-  currentOrgDomain: string | null;
-}) {
-  const formUser = {
-    ...user,
-    metadata: userMetadata.parse(user.metadata),
-  };
-  const orgSlug = formUser.profile.organization?.slug ?? formUser.profile.organization?.requestedSlug ?? null;
-
-  if (!currentOrgDomain) {
-    // If not on org domain, let's allow serving any form belong to any organization so that even if the form owner is migrate to an organization, old links for the form keep working
-    return true;
-  } else if (currentOrgDomain !== orgSlug) {
-    // If on org domain,
-    // We don't serve the form that is of another org
-    // We don't serve the form that doesn't belong to any org
-    return false;
-  }
-  return true;
-}
-
 export const getServerSideProps = async function getServerSideProps(
  context: AppGetServerSidePropsContext,
  prisma: AppPrisma
@@ -108,7 +74,9 @@ export const getServerSideProps = async function getServerSideProps(
    user: await UserRepository.enrichUserWithItsProfile({ user: form.user }),
  };

-  if (!isAuthorizedToViewTheForm({ user: formWithUserProfile.user, currentOrgDomain })) {
+  if (
+    !isAuthorizedToViewFormOnOrgDomain({ user: formWithUserProfile.user, currentOrgDomain, team: form.team })
+  ) {
    return {
      notFound: true,
    };
@@ -0,0 +1,113 @@
+import { describe, it, expect } from "vitest";
+
+import { isAuthorizedToViewFormOnOrgDomain } from "./isAuthorizedToViewForm";
+
+const _createUser = (overrides = {}) => ({
+  username: "testuser",
+  metadata: {},
+  movedToProfileId: null,
+  id: 1,
+  ...overrides,
+});
+
+/**
+ * Creates a regular user without organization membership
+ */
+const createRegularUser = (overrides = {}) => ({
+  ..._createUser(overrides),
+  profile: {
+    organization: null,
+  },
+});
+
+/**
+ * Creates a user that is a member of an organization
+ */
+const createOrgMemberUser = ({
+  orgSlug,
+  requestedSlug,
+}: {
+  orgSlug: string;
+  requestedSlug: string | null;
+}) => ({
+  ..._createUser({
+    profile: {
+      organization: { slug: orgSlug, requestedSlug: requestedSlug },
+    },
+  }),
+});
+
+const _createTeam = (overrides = {}) => ({
+  parent: null,
+  ...overrides,
+});
+
+/**
+ * Creates a regular team without organization association
+ */
+const createRegularTeam = (overrides = {}) => _createTeam(overrides);
+
+/**
+ * Creates a sub-team that belongs to an organization
+ */
+const createSubTeam = (orgSlug: string, overrides = {}) =>
+  _createTeam({
+    parent: {
+      slug: orgSlug,
+    },
+    ...overrides,
+  });
+
+describe("isAuthorizedToViewFormOnOrgDomain", () => {
+  it("should allow viewing any form (user or team form) when not on org domain", () => {
+    const result = isAuthorizedToViewFormOnOrgDomain({
+      user: createRegularUser(),
+      currentOrgDomain: null,
+      team: createRegularTeam(),
+    });
+    expect(result).toBe(true);
+  });
+
+  it("should allow viewing org member's form when user belongs to the current org domain", () => {
+    const result = isAuthorizedToViewFormOnOrgDomain({
+      user: createOrgMemberUser({ orgSlug: "test-org", requestedSlug: null }),
+      currentOrgDomain: "test-org",
+    });
+    expect(result).toBe(true);
+  });
+
+  it("should allow viewing sub-team form when the sub team belongs to the current org domain", () => {
+    const result = isAuthorizedToViewFormOnOrgDomain({
+      user: createRegularUser(),
+      currentOrgDomain: "test-org",
+      team: createSubTeam("test-org"),
+    });
+    expect(result).toBe(true);
+  });
+
+  it("should deny viewing form when on org domain but neither user nor sub team belongs to it", () => {
+    const result = isAuthorizedToViewFormOnOrgDomain({
+      user: createOrgMemberUser("different-org"),
+      currentOrgDomain: "test-org",
+      team: createSubTeam("another-org"),
+    });
+    expect(result).toBe(false);
+  });
+
+  it("should handle undefined team parameter on org domain", () => {
+    const result = isAuthorizedToViewFormOnOrgDomain({
+      user: createRegularUser(),
+      currentOrgDomain: "test-org",
+      team: undefined,
+    });
+    expect(result).toBe(false);
+  });
+
+  it("should allow access when user has pending org membership request", () => {
+    const result = isAuthorizedToViewFormOnOrgDomain({
+      user: createOrgMemberUser({ orgSlug: "test-org", requestedSlug: "test-org" }),
+      currentOrgDomain: "test-org",
+    });
+    expect(result).toBe(true);
+  });
+});
@@ -0,0 +1,46 @@
+import type { Prisma } from "@calcom/prisma/client";
+import { userMetadata } from "@calcom/prisma/zod-utils";
+
+type FormUser = {
+  username: string | null;
+  metadata: Prisma.JsonValue;
+  movedToProfileId: number | null;
+  profile: {
+    organization: { slug: string | null; requestedSlug: string | null } | null;
+  };
+  id: number;
+};
+
+type FormTeam = {
+  parent: {
+    slug: string | null;
+  } | null;
+} | null;
+
+export function isAuthorizedToViewFormOnOrgDomain({
+  user,
+  currentOrgDomain,
+  team,
+}: {
+  user: FormUser;
+  currentOrgDomain: string | null;
+  team?: FormTeam;
+}) {
+  const formUser = {
+    ...user,
+    metadata: userMetadata.parse(user.metadata),
+  };
+  const orgSlug = formUser.profile.organization?.slug ?? formUser.profile.organization?.requestedSlug ?? null;
+  const teamOrgSlug = team?.parent?.slug ?? null;
+
+  if (!currentOrgDomain) {
+    // If not on org domain, let's allow serving any form belong to any organization so that even if the form owner is migrate to an organization, old links for the form keep working
+    return true;
+  } else if (currentOrgDomain === orgSlug || currentOrgDomain === teamOrgSlug) {
+    // If on org domain, allow if:
+    // 1. The form belongs to a user who is part of the organization (orgSlug matches)
+    // 2. The form belongs to a team that is part of the organization (teamOrgSlug matches)
+    return true;
+  }
+  return false;
+}