fix: require state verification on all app installs

packages/app-store/_utils/oauth/decodeOAuthState.test.ts

@@ -0,0 +1,159 @@
+import { createHmac, randomUUID } from "node:crypto";
+
+import { describe, expect, it, vi, afterEach } from "vitest";
+
+import { decodeOAuthState } from "./decodeOAuthState";
+
+const TEST_SECRET = "test-nextauth-secret";
+const TEST_USER_ID = 42;
+
+function buildRequest({
+  state,
+  userId,
+}: {
+  state?: Record<string, unknown> | string;
+  userId?: number | null;
+}): Parameters<typeof decodeOAuthState>[0] {
+  const query: Record<string, string> = {};
+  if (state !== undefined) {
+    query.state = typeof state === "string" ? state : JSON.stringify(state);
+  }
+  return {
+    query,
+    session: userId !== null ? { user: { id: userId ?? TEST_USER_ID } } : undefined,
+  } as Parameters<typeof decodeOAuthState>[0];
+}
+
+function signNonce(nonce: string, userId: number, secret: string = TEST_SECRET): string {
+  return createHmac("sha256", secret).update(`${nonce}:${userId}`).digest("hex");
+}
+
+function buildStateWithValidNonce(
+  extra: Record<string, unknown> = {}
+): Record<string, unknown> & { nonce: string; nonceHash: string } {
+  const nonce = randomUUID();
+  const nonceHash = signNonce(nonce, TEST_USER_ID);
+  return { returnTo: "/apps", onErrorReturnTo: "/error", fromApp: true, nonce, nonceHash, ...extra };
+}
+
+afterEach(() => {
+  vi.restoreAllMocks();
+});
+
+describe("decodeOAuthState", () => {
+  describe("when state query param is missing or not a string", () => {
+    it("returns undefined for missing state", () => {
+      const req = buildRequest({ state: undefined });
+      delete (req.query as Record<string, unknown>).state;
+      expect(decodeOAuthState(req)).toBeUndefined();
+    });
+
+    it("returns undefined for non-string state", () => {
+      const req = { query: { state: 123 }, session: { user: { id: TEST_USER_ID } } } as unknown as Parameters<
+        typeof decodeOAuthState
+      >[0];
+      expect(decodeOAuthState(req)).toBeUndefined();
+    });
+  });
+
+  describe("all apps require nonce verification (no exemptions)", () => {
+    it.each(["basecamp3", "webex", "tandem", "stripe", "dub"])(
+      "rejects %s-shaped state without nonce",
+      () => {
+        vi.stubEnv("NEXTAUTH_SECRET", TEST_SECRET);
+        const stateObj = { returnTo: "/apps", onErrorReturnTo: "/error", fromApp: true };
+        const req = buildRequest({ state: stateObj, userId: TEST_USER_ID });
+        expect(decodeOAuthState(req)).toBeUndefined();
+      }
+    );
+
+    it.each(["basecamp3", "webex", "tandem", "stripe", "dub"])(
+      "accepts %s-shaped state with valid nonce",
+      () => {
+        vi.stubEnv("NEXTAUTH_SECRET", TEST_SECRET);
+        const stateObj = buildStateWithValidNonce();
+        const req = buildRequest({ state: stateObj, userId: TEST_USER_ID });
+        expect(decodeOAuthState(req)).toEqual(stateObj);
+      }
+    );
+  });
+
+  describe("mandatory nonce verification (non-exempt apps)", () => {
+    it("returns undefined when nonce is missing", () => {
+      vi.stubEnv("NEXTAUTH_SECRET", TEST_SECRET);
+      const stateObj = { returnTo: "/apps", onErrorReturnTo: "/error", fromApp: true };
+      const req = buildRequest({ state: stateObj, userId: TEST_USER_ID });
+      expect(decodeOAuthState(req)).toBeUndefined();
+    });
+
+    it("returns undefined when nonceHash is missing", () => {
+      vi.stubEnv("NEXTAUTH_SECRET", TEST_SECRET);
+      const stateObj = { returnTo: "/apps", onErrorReturnTo: "/error", fromApp: true, nonce: randomUUID() };
+      const req = buildRequest({ state: stateObj, userId: TEST_USER_ID });
+      expect(decodeOAuthState(req)).toBeUndefined();
+    });
+
+    it("returns undefined when userId is missing", () => {
+      vi.stubEnv("NEXTAUTH_SECRET", TEST_SECRET);
+      const stateObj = buildStateWithValidNonce();
+      const req = buildRequest({ state: stateObj, userId: null });
+      expect(decodeOAuthState(req)).toBeUndefined();
+    });
+
+    it("returns undefined when NEXTAUTH_SECRET is not set", () => {
+      vi.stubEnv("NEXTAUTH_SECRET", "");
+      const stateObj = buildStateWithValidNonce();
+      const req = buildRequest({ state: stateObj, userId: TEST_USER_ID });
+      expect(decodeOAuthState(req)).toBeUndefined();
+    });
+
+    it("returns state when nonce and HMAC are valid", () => {
+      vi.stubEnv("NEXTAUTH_SECRET", TEST_SECRET);
+      const stateObj = buildStateWithValidNonce();
+      const req = buildRequest({ state: stateObj, userId: TEST_USER_ID });
+      const result = decodeOAuthState(req);
+      expect(result).toEqual(stateObj);
+    });
+
+    it("returns undefined when nonceHash is tampered", () => {
+      vi.stubEnv("NEXTAUTH_SECRET", TEST_SECRET);
+      const stateObj = buildStateWithValidNonce();
+      stateObj.nonceHash = "deadbeef".repeat(8);
+      const req = buildRequest({ state: stateObj, userId: TEST_USER_ID });
+      expect(decodeOAuthState(req)).toBeUndefined();
+    });
+
+    it("returns undefined when nonce is tampered", () => {
+      vi.stubEnv("NEXTAUTH_SECRET", TEST_SECRET);
+      const stateObj = buildStateWithValidNonce();
+      stateObj.nonce = randomUUID();
+      const req = buildRequest({ state: stateObj, userId: TEST_USER_ID });
+      expect(decodeOAuthState(req)).toBeUndefined();
+    });
+
+    it("returns undefined when signed with a different userId", () => {
+      vi.stubEnv("NEXTAUTH_SECRET", TEST_SECRET);
+      const nonce = randomUUID();
+      const nonceHash = signNonce(nonce, 999);
+      const stateObj = { returnTo: "/apps", onErrorReturnTo: "/error", fromApp: true, nonce, nonceHash };
+      const req = buildRequest({ state: stateObj, userId: TEST_USER_ID });
+      expect(decodeOAuthState(req)).toBeUndefined();
+    });
+
+    it("returns undefined when signed with a different secret", () => {
+      vi.stubEnv("NEXTAUTH_SECRET", TEST_SECRET);
+      const nonce = randomUUID();
+      const nonceHash = createHmac("sha256", "wrong-secret").update(`${nonce}:${TEST_USER_ID}`).digest("hex");
+      const stateObj = { returnTo: "/apps", onErrorReturnTo: "/error", fromApp: true, nonce, nonceHash };
+      const req = buildRequest({ state: stateObj, userId: TEST_USER_ID });
+      expect(decodeOAuthState(req)).toBeUndefined();
+    });
+
+    it("prevents CSRF bypass by stripping nonce fields from non-exempt app", () => {
+      vi.stubEnv("NEXTAUTH_SECRET", TEST_SECRET);
+      const stateObj = { returnTo: "/apps", onErrorReturnTo: "/error", fromApp: true };
+      const req = buildRequest({ state: stateObj, userId: TEST_USER_ID });
+      expect(decodeOAuthState(req)).toBeUndefined();
+    });
+  });
+});

packages/app-store/_utils/oauth/decodeOAuthState.ts

@@ -3,18 +3,12 @@ import process from "node:process";
 import type { NextApiRequest } from "next";
 import type { IntegrationOAuthCallbackState } from "../../types";
 
-const NONCE_EXEMPT_APPS = new Set(["stripe", "basecamp3", "dub", "webex", "tandem"]);
-
-export function decodeOAuthState(req: NextApiRequest, appSlug?: string) {
+export function decodeOAuthState(req: NextApiRequest) {
   if (typeof req.query.state !== "string") {
     return undefined;
   }
   const state: IntegrationOAuthCallbackState = JSON.parse(req.query.state);
 
-  if (appSlug && NONCE_EXEMPT_APPS.has(appSlug)) {
-    return state;
-  }
-
   if (!state.nonce || !state.nonceHash) {
     return undefined;
   }
@@ -28,6 +22,7 @@ export function decodeOAuthState(req: NextApiRequest, appSlug?: string) {
     .digest();
   const actual = Buffer.from(state.nonceHash, "hex");
   if (expected.length !== actual.length || !timingSafeEqual(expected, actual)) {
+    console.warn("[OAuth] State nonce verification failed");
     return undefined;
   }
 

packages/app-store/basecamp3/api/add.ts

@@ -1,29 +1,37 @@
-import type { NextApiRequest } from "next";
 import { stringify } from "node:querystring";
-
 import { WEBAPP_URL } from "@calcom/lib/constants";
+import { HttpError } from "@calcom/lib/http-error";
 import { defaultHandler } from "@calcom/lib/server/defaultHandler";
 import { defaultResponder } from "@calcom/lib/server/defaultResponder";
 import prisma from "@calcom/prisma";
-
+import type { NextApiRequest } from "next";
+import { encodeOAuthState } from "../../_utils/oauth/encodeOAuthState";
 import { getBasecampKeys } from "../lib/getBasecampKeys";
 
 async function handler(req: NextApiRequest) {
+  if (!req.session?.user?.id) {
+    throw new HttpError({ statusCode: 401, message: "You must be logged in to do this" });
+  }
+
   await prisma.user.findFirstOrThrow({
     where: {
-      id: req.session?.user?.id,
+      id: req.session.user.id,
     },
     select: {
       id: true,
     },
   });
 
   const { client_id } = await getBasecampKeys();
+  const state = encodeOAuthState(req);
 
-  const params = {
+  const params: Record<string, string> = {
     type: "web_server",
     client_id,
   };
+  if (state) {
+    params.state = state;
+  }
   const query = stringify(params);
   const url = `https://launchpad.37signals.com/authorization/new?${query}&redirect_uri=${WEBAPP_URL}/api/integrations/basecamp3/callback`;
   return { url };

packages/app-store/basecamp3/api/callback.ts

@@ -1,14 +1,25 @@
-import type { NextApiRequest, NextApiResponse } from "next";
-
 import { WEBAPP_URL } from "@calcom/lib/constants";
+import { getSafeRedirectUrl } from "@calcom/lib/getSafeRedirectUrl";
 import prisma from "@calcom/prisma";
-
+import type { NextApiRequest, NextApiResponse } from "next";
 import getAppKeysFromSlug from "../../_utils/getAppKeysFromSlug";
 import getInstalledAppPath from "../../_utils/getInstalledAppPath";
 import { decodeOAuthState } from "../../_utils/oauth/decodeOAuthState";
 import appConfig from "../config.json";
 
 export default async function handler(req: NextApiRequest, res: NextApiResponse) {
+  const state = decodeOAuthState(req);
+  if (!state) {
+    res.status(403).json({ message: "Invalid or missing OAuth state. Request may have been forged." });
+    return;
+  }
+
+  const userId = req.session?.user.id;
+  if (!userId) {
+    res.status(401).json({ message: "You must be logged in to do this" });
+    return;
+  }
+
   const { code } = req.query;
   const { client_id, client_secret, user_agent } = await getAppKeysFromSlug("basecamp3");
 
@@ -67,14 +78,10 @@ export default async function handler(req: NextApiRequest, res: NextApiResponse)
   }
 
   const authResponseBody = await userAuthResponse.json();
-  const userId = req.session?.user.id;
-  if (!userId) {
-    return res.status(404).json({ message: "No user found" });
-  }
 
   await prisma.user.update({
     where: {
-      id: req.session?.user.id,
+      id: userId,
     },
     data: {
       credentials: {
@@ -87,7 +94,8 @@ export default async function handler(req: NextApiRequest, res: NextApiResponse)
     },
   });
 
-  const state = decodeOAuthState(req, "basecamp3");
-
-  res.redirect(getInstalledAppPath({ variant: appConfig.variant, slug: appConfig.slug }));
+  res.redirect(
+    getSafeRedirectUrl(state?.returnTo) ??
+      getInstalledAppPath({ variant: appConfig.variant, slug: appConfig.slug })
+  );
 }

packages/app-store/tandemvideo/api/add.ts

@@ -1,17 +1,19 @@
-import type { NextApiRequest, NextApiResponse } from "next";
 import { stringify } from "node:querystring";
-
 import { WEBAPP_URL } from "@calcom/lib/constants";
 import prisma from "@calcom/prisma";
-
+import type { NextApiRequest, NextApiResponse } from "next";
 import getAppKeysFromSlug from "../../_utils/getAppKeysFromSlug";
+import { encodeOAuthState } from "../../_utils/oauth/encodeOAuthState";
 
 export default async function handler(req: NextApiRequest, res: NextApiResponse) {
   if (req.method === "GET") {
-    // Get user
+    if (!req.session?.user?.id) {
+      return res.status(401).json({ message: "You must be logged in to do this" });
+    }
+
     await prisma.user.findFirstOrThrow({
       where: {
-        id: req.session?.user?.id,
+        id: req.session.user.id,
       },
       select: {
         id: true,
@@ -27,11 +29,15 @@ export default async function handler(req: NextApiRequest, res: NextApiResponse)
     if (!baseUrl) return res.status(400).json({ message: "Tandem base_url missing." });
 
     const redirect_uri = encodeURI(`${WEBAPP_URL}/api/integrations/tandemvideo/callback`);
+    const state = encodeOAuthState(req);
 
-    const params = {
+    const params: Record<string, string> = {
       client_id: clientId,
       redirect_uri,
     };
+    if (state) {
+      params.state = state;
+    }
     const query = stringify(params);
     const url = `${baseUrl}/oauth/approval?${query}`;
     res.status(200).json({ url });

packages/app-store/tandemvideo/api/callback.ts

@@ -1,8 +1,6 @@
-import type { NextApiRequest, NextApiResponse } from "next";
-
 import { getSafeRedirectUrl } from "@calcom/lib/getSafeRedirectUrl";
 import prisma from "@calcom/prisma";
-
+import type { NextApiRequest, NextApiResponse } from "next";
 import getAppKeysFromSlug from "../../_utils/getAppKeysFromSlug";
 import getInstalledAppPath from "../../_utils/getInstalledAppPath";
 import createOAuthAppCredential from "../../_utils/oauth/createOAuthAppCredential";
@@ -14,8 +12,19 @@ export default async function handler(req: NextApiRequest, res: NextApiResponse)
     return;
   }
 
+  const state = decodeOAuthState(req);
+  if (!state) {
+    res.status(403).json({ message: "Invalid or missing OAuth state. Request may have been forged." });
+    return;
+  }
+
+  const userId = req.session?.user.id;
+  if (!userId) {
+    res.status(401).json({ message: "You must be logged in to do this" });
+    return;
+  }
+
   const code = req.query.code as string;
-  const state = decodeOAuthState(req, "tandem");
 
   let clientId = "";
   let clientSecret = "";
@@ -38,18 +47,13 @@ export default async function handler(req: NextApiRequest, res: NextApiResponse)
 
   const responseBody = await result.json();
 
-  const userId = req.session?.user.id;
-  if (!userId) {
-    return res.status(404).json({ message: "No user found" });
-  }
-
   const existingCredentialTandemVideo = await prisma.credential.findMany({
     select: {
       id: true,
     },
     where: {
       type: "tandem_video",
-      userId: req.session?.user.id,
+      userId,
       appId: "tandem",
     },
   });