Skip to content

Commit 14c151b

Browse files
volneiclaudedevin-ai-integration[bot]
authored
fix: add CSRF protection to OAuth callback via HMAC-signed nonce (calcom#28083)
* fix: add CSRF protection to OAuth callback via HMAC-signed nonce The OAuth state parameter was used only for passing application data (returnTo, teamId) with no cryptographic binding to the user session. An attacker could authorize their own account on a provider, capture the authorization code, and trick a logged-in user into visiting the callback URL to link the attacker's account to the victim's Cal.com profile. Changes: - encodeOAuthState: generate a random nonce and HMAC-sign it with NEXTAUTH_SECRET + userId, injecting both into the OAuth state - decodeOAuthState: verify the HMAC on callback using timingSafeEqual; skip verification when nonce is absent (backwards compatible with apps that don't yet use encodeOAuthState) - Stripe callback: replace raw state.returnTo redirect with getSafeRedirectUrl to prevent open redirect, remove redundant getReturnToValueFromQueryState, add missing return on access_denied Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: make CSRF nonce verification mandatory with allowlist for exempt apps Makes nonce/HMAC verification mandatory by default in decodeOAuthState, preventing attackers from bypassing CSRF protection by omitting nonce fields from the state parameter. Apps not yet migrated to encodeOAuthState (stripe, basecamp3, dub, webex, tandem) are explicitly allowlisted and pass their slug to decodeOAuthState to skip verification. Addresses review feedback (identified by cubic) about the conditional check being trivially bypassable. Co-Authored-By: unknown <> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
1 parent 1aae57d commit 14c151b

8 files changed

Lines changed: 52 additions & 30 deletions

File tree

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,35 @@
1+
import { createHmac, timingSafeEqual } from "node:crypto";
2+
import process from "node:process";
13
import type { NextApiRequest } from "next";
2-
34
import type { IntegrationOAuthCallbackState } from "../../types";
45

5-
export function decodeOAuthState(req: NextApiRequest) {
6+
const NONCE_EXEMPT_APPS = new Set(["stripe", "basecamp3", "dub", "webex", "tandem"]);
7+
8+
export function decodeOAuthState(req: NextApiRequest, appSlug?: string) {
69
if (typeof req.query.state !== "string") {
710
return undefined;
811
}
912
const state: IntegrationOAuthCallbackState = JSON.parse(req.query.state);
1013

14+
if (appSlug && NONCE_EXEMPT_APPS.has(appSlug)) {
15+
return state;
16+
}
17+
18+
if (!state.nonce || !state.nonceHash) {
19+
return undefined;
20+
}
21+
22+
const userId = req.session?.user?.id;
23+
if (!userId || !process.env.NEXTAUTH_SECRET) {
24+
return undefined;
25+
}
26+
const expected = createHmac("sha256", process.env.NEXTAUTH_SECRET)
27+
.update(`${state.nonce}:${userId}`)
28+
.digest();
29+
const actual = Buffer.from(state.nonceHash, "hex");
30+
if (expected.length !== actual.length || !timingSafeEqual(expected, actual)) {
31+
return undefined;
32+
}
33+
1134
return state;
1235
}
Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
1+
import { createHmac, randomUUID } from "node:crypto";
2+
import process from "node:process";
13
import type { NextApiRequest } from "next";
2-
34
import type { IntegrationOAuthCallbackState } from "../../types";
45

56
export function encodeOAuthState(req: NextApiRequest) {
@@ -8,5 +9,13 @@ export function encodeOAuthState(req: NextApiRequest) {
89
}
910
const state: IntegrationOAuthCallbackState = JSON.parse(req.query.state);
1011

12+
const userId = req.session?.user?.id;
13+
if (userId && process.env.NEXTAUTH_SECRET) {
14+
state.nonce = randomUUID();
15+
state.nonceHash = createHmac("sha256", process.env.NEXTAUTH_SECRET)
16+
.update(`${state.nonce}:${userId}`)
17+
.digest("hex");
18+
}
19+
1120
return JSON.stringify(state);
1221
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ export default async function handler(req: NextApiRequest, res: NextApiResponse)
8787
},
8888
});
8989

90-
const state = decodeOAuthState(req);
90+
const state = decodeOAuthState(req, "basecamp3");
9191

9292
res.redirect(getInstalledAppPath({ variant: appConfig.variant, slug: appConfig.slug }));
9393
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import { dubAppKeysSchema } from "../lib/utils";
1313
export default async function handler(req: NextApiRequest, res: NextApiResponse) {
1414
const { code } = req.query;
1515

16-
const state = decodeOAuthState(req);
16+
const state = decodeOAuthState(req, "dub");
1717

1818
if (typeof code !== "string") {
1919
if (state?.onErrorReturnTo || state?.returnTo) {
Lines changed: 11 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,36 +1,23 @@
1-
import type { NextApiRequest, NextApiResponse } from "next";
21
import { stringify } from "node:querystring";
3-
2+
import { getSafeRedirectUrl } from "@calcom/lib/getSafeRedirectUrl";
43
import type { Prisma } from "@calcom/prisma/client";
5-
4+
import type { NextApiRequest, NextApiResponse } from "next";
65
import getInstalledAppPath from "../../_utils/getInstalledAppPath";
76
import createOAuthAppCredential from "../../_utils/oauth/createOAuthAppCredential";
87
import { decodeOAuthState } from "../../_utils/oauth/decodeOAuthState";
98
import type { StripeData } from "../lib/server";
109
import stripe from "../lib/server";
1110

12-
function getReturnToValueFromQueryState(req: NextApiRequest) {
13-
let returnTo = "";
14-
try {
15-
returnTo = JSON.parse(`${req.query.state}`).returnTo;
16-
} catch (error) {
17-
console.info("No 'returnTo' in req.query.state");
18-
}
19-
return returnTo;
20-
}
21-
2211
export default async function handler(req: NextApiRequest, res: NextApiResponse) {
2312
const { code, error, error_description } = req.query;
24-
const state = decodeOAuthState(req);
13+
const state = decodeOAuthState(req, "stripe");
2514

2615
if (error) {
27-
// User cancels flow
2816
if (error === "access_denied") {
29-
state?.onErrorReturnTo ? res.redirect(state.onErrorReturnTo) : res.redirect("/apps/installed/payment");
17+
return res.redirect(getSafeRedirectUrl(state?.onErrorReturnTo) ?? "/apps/installed/payment");
3018
}
3119
const query = stringify({ error, error_description });
32-
res.redirect(`/apps/installed?${query}`);
33-
return;
20+
return res.redirect(`/apps/installed?${query}`);
3421
}
3522

3623
if (!req.session?.user?.id) {
@@ -43,9 +30,9 @@ export default async function handler(req: NextApiRequest, res: NextApiResponse)
4330
});
4431

4532
const data: StripeData = { ...response, default_currency: "" };
46-
if (response["stripe_user_id"]) {
47-
const account = await stripe.accounts.retrieve(response["stripe_user_id"]);
48-
data["default_currency"] = account.default_currency;
33+
if (response.stripe_user_id) {
34+
const account = await stripe.accounts.retrieve(response.stripe_user_id);
35+
data.default_currency = account.default_currency;
4936
}
5037

5138
await createOAuthAppCredential(
@@ -54,6 +41,7 @@ export default async function handler(req: NextApiRequest, res: NextApiResponse)
5441
req
5542
);
5643

57-
const returnTo = getReturnToValueFromQueryState(req);
58-
res.redirect(returnTo || getInstalledAppPath({ variant: "payment", slug: "stripe" }));
44+
res.redirect(
45+
getSafeRedirectUrl(state?.returnTo) ?? getInstalledAppPath({ variant: "payment", slug: "stripe" })
46+
);
5947
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ export default async function handler(req: NextApiRequest, res: NextApiResponse)
1515
}
1616

1717
const code = req.query.code as string;
18-
const state = decodeOAuthState(req);
18+
const state = decodeOAuthState(req, "tandem");
1919

2020
let clientId = "";
2121
let clientSecret = "";

packages/app-store/types.d.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ export type IntegrationOAuthCallbackState = {
1212
installGoogleVideo?: boolean;
1313
teamId?: number;
1414
defaultInstall?: boolean;
15+
nonce?: string;
16+
nonceHash?: string;
1517
};
1618

1719
export type CredentialOwner = {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import { getWebexAppKeys } from "../lib/getWebexAppKeys";
1313
export default async function handler(req: NextApiRequest, res: NextApiResponse) {
1414
const { code } = req.query;
1515
const { client_id, client_secret } = await getWebexAppKeys();
16-
const state = decodeOAuthState(req);
16+
const state = decodeOAuthState(req, "webex");
1717

1818
/** @link https://developer.webex.com/docs/integrations#getting-an-access-token **/
1919

0 commit comments

Comments
 (0)