Skip to content
Jump to bottom

Automated Test: oauth-security-enhanced #363

Closed
wants to merge 1 commit into from
+375 −119
Closed

Automated Test: oauth-security-enhanced #363

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 14 additions & 1 deletion .env.example
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,19 @@ AUTH_BEARER_TOKEN_VERCEL=
E2E_TEST_APPLE_CALENDAR_EMAIL=""
E2E_TEST_APPLE_CALENDAR_PASSWORD=""

# - APP CREDENTIAL SYNC ***********************************************************************************
# Used for self-hosters that are implementing Cal.com into their applications that already have certain integrations
# Under settings/admin/apps ensure that all app secrets are set the same as the parent application
# You can use: `openssl rand -base64 32` to generate one
CALCOM_WEBHOOK_SECRET=""
# This is the header name that will be used to verify the webhook secret. Should be in lowercase
CALCOM_WEBHOOK_HEADER_NAME="calcom-webhook-secret"
CALCOM_CREDENTIAL_SYNC_ENDPOINT=""
# Key should match on Cal.com and your application
# must be 32 bytes for AES256 encryption algorithm
# You can use: `openssl rand -base64 24` to generate one
CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY=""

# - OIDC E2E TEST *******************************************************************************************

# Ensure this ADMIN EMAIL is present in the SAML_ADMINS list
Expand All @@ -243,4 +256,4 @@ E2E_TEST_OIDC_PROVIDER_DOMAIN=
E2E_TEST_OIDC_USER_EMAIL=
E2E_TEST_OIDC_USER_PASSWORD=

# ***********************************************************************************************************
# ***********************************************************************************************************
93 changes: 93 additions & 0 deletions apps/web/pages/api/webhook/app-credential.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
import type { NextApiRequest, NextApiResponse } from "next";
import z from "zod";

import { appStoreMetadata } from "@calcom/app-store/appStoreMetaData";
import { APP_CREDENTIAL_SHARING_ENABLED } from "@calcom/lib/constants";
import { symmetricDecrypt } from "@calcom/lib/crypto";
import prisma from "@calcom/prisma";

const appCredentialWebhookRequestBodySchema = z.object({
// UserId of the cal.com user
userId: z.number().int(),
appSlug: z.string(),
// Keys should be AES256 encrypted with the CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY
keys: z.string(),
});
/** */
export default async function handler(req: NextApiRequest, res: NextApiResponse) {
// Check that credential sharing is enabled
if (!APP_CREDENTIAL_SHARING_ENABLED) {
return res.status(403).json({ message: "Credential sharing is not enabled" });
}

// Check that the webhook secret matches
if (
req.headers[process.env.CALCOM_WEBHOOK_HEADER_NAME || "calcom-webhook-secret"] !==
process.env.CALCOM_WEBHOOK_SECRET
) {
return res.status(403).json({ message: "Invalid webhook secret" });
}
Comment on lines +17 to +29
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Missing HTTP method restriction — endpoint accepts any method.

This webhook handler should only accept POST. Without a method guard, GET/DELETE/etc. requests fall through to the body-parsing logic, resulting in confusing 500 errors instead of a clean 405 Method Not Allowed.

Additionally, the webhook-secret comparison on line 25 uses !==, which is susceptible to timing attacks. Use timingSafeEqual from Node's crypto module instead.

🛡️ Proposed fix 
+import { timingSafeEqual } from "crypto";
+
 export default async function handler(req: NextApiRequest, res: NextApiResponse) {
+  if (req.method !== "POST") {
+    return res.status(405).json({ message: "Method not allowed" });
+  }
+
   // Check that credential sharing is enabled
   if (!APP_CREDENTIAL_SHARING_ENABLED) {
     return res.status(403).json({ message: "Credential sharing is not enabled" });
   }
 
   // Check that the webhook secret matches
-  if (
-    req.headers[process.env.CALCOM_WEBHOOK_HEADER_NAME || "calcom-webhook-secret"] !==
-    process.env.CALCOM_WEBHOOK_SECRET
-  ) {
+  const expectedSecret = process.env.CALCOM_WEBHOOK_SECRET || "";
+  const receivedSecret = String(
+    req.headers[process.env.CALCOM_WEBHOOK_HEADER_NAME || "calcom-webhook-secret"] || ""
+  );
+  if (
+    expectedSecret.length !== receivedSecret.length ||
+    !timingSafeEqual(Buffer.from(expectedSecret), Buffer.from(receivedSecret))
+  ) {
     return res.status(403).json({ message: "Invalid webhook secret" });
   }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export default async function handler(req: NextApiRequest, res: NextApiResponse) {
// Check that credential sharing is enabled
if (!APP_CREDENTIAL_SHARING_ENABLED) {
return res.status(403).json({ message: "Credential sharing is not enabled" });
}
// Check that the webhook secret matches
if (
req.headers[process.env.CALCOM_WEBHOOK_HEADER_NAME || "calcom-webhook-secret"] !==
process.env.CALCOM_WEBHOOK_SECRET
) {
return res.status(403).json({ message: "Invalid webhook secret" });
}
import { timingSafeEqual } from "crypto";
export default async function handler(req: NextApiRequest, res: NextApiResponse) {
if (req.method !== "POST") {
return res.status(405).json({ message: "Method not allowed" });
}
// Check that credential sharing is enabled
if (!APP_CREDENTIAL_SHARING_ENABLED) {
return res.status(403).json({ message: "Credential sharing is not enabled" });
}
// Check that the webhook secret matches
const expectedSecret = process.env.CALCOM_WEBHOOK_SECRET || "";
const receivedSecret = String(
req.headers[process.env.CALCOM_WEBHOOK_HEADER_NAME || "calcom-webhook-secret"] || ""
);
if (
expectedSecret.length !== receivedSecret.length ||
!timingSafeEqual(Buffer.from(expectedSecret), Buffer.from(receivedSecret))
) {
return res.status(403).json({ message: "Invalid webhook secret" });
}
🤖 Prompt for AI Agents 
In `@apps/web/pages/api/webhook/app-credential.ts` around lines 17 - 29, Add an
explicit HTTP method guard at the top of the handler (export default async
function handler) to only allow POST and return 405 for other methods; keep the
existing APP_CREDENTIAL_SHARING_ENABLED check. Replace the direct !== comparison
of the incoming header (req.headers[process.env.CALCOM_WEBHOOK_HEADER_NAME ||
"calcom-webhook-secret"]) against process.env.CALCOM_WEBHOOK_SECRET with a
timing-safe comparison using Node's crypto.timingSafeEqual: coerce both values
to Buffers, ensure equal length before calling timingSafeEqual (treat unequal
lengths as a failed match), and handle missing header or missing secret by
returning 403.


const reqBody = appCredentialWebhookRequestBodySchema.parse(req.body);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Unhandled ZodError from .parse() will surface as a 500.

appCredentialWebhookRequestBodySchema.parse(req.body) throws a ZodError on invalid input. Without a try/catch (or using .safeParse()), the caller receives a raw 500 instead of a structured 400 response.

♻️ Proposed fix using safeParse 
-  const reqBody = appCredentialWebhookRequestBodySchema.parse(req.body);
+  const parsed = appCredentialWebhookRequestBodySchema.safeParse(req.body);
+  if (!parsed.success) {
+    return res.status(400).json({ message: "Invalid request body", error: parsed.error.flatten() });
+  }
+  const reqBody = parsed.data;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const reqBody = appCredentialWebhookRequestBodySchema.parse(req.body);
const parsed = appCredentialWebhookRequestBodySchema.safeParse(req.body);
if (!parsed.success) {
return res.status(400).json({ message: "Invalid request body", error: parsed.error.flatten() });
}
const reqBody = parsed.data;
🤖 Prompt for AI Agents 
In `@apps/web/pages/api/webhook/app-credential.ts` at line 31, The code calls
appCredentialWebhookRequestBodySchema.parse(req.body) which throws a ZodError
and yields a 500; change this to use
appCredentialWebhookRequestBodySchema.safeParse(req.body) (or wrap parse in
try/catch) in the API handler so validation failures are detected, and when
validation fails return a 400 with the validation errors instead of letting the
exception bubble; update the variable assignment (reqBody) to only be set when
parsing succeeds and use res.status(400).json(...) to send the structured error
response.


// Check that the user exists
const user = await prisma.user.findUnique({ where: { id: reqBody.userId } });

if (!user) {
return res.status(404).json({ message: "User not found" });
}

const app = await prisma.app.findUnique({
where: { slug: reqBody.appSlug },
select: { slug: true },
});

if (!app) {
return res.status(404).json({ message: "App not found" });
}

// Search for the app's slug and type
const appMetadata = appStoreMetadata[app.slug as keyof typeof appStoreMetadata];

if (!appMetadata) {
return res.status(404).json({ message: "App not found. Ensure that you have the correct app slug" });
}

// Decrypt the keys
const keys = JSON.parse(
symmetricDecrypt(reqBody.keys, process.env.CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY || "")
);
Comment on lines +56 to +59
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Decryption and JSON parse are unguarded — any failure becomes a 500.

If CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY is unset (falls back to ""), or the payload is tampered with, symmetricDecrypt or JSON.parse will throw. Wrap this in a try/catch and return a 400 with a meaningful message.

♻️ Proposed fix 
   // Decrypt the keys
-  const keys = JSON.parse(
-    symmetricDecrypt(reqBody.keys, process.env.CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY || "")
-  );
+  let keys;
+  try {
+    keys = JSON.parse(
+      symmetricDecrypt(reqBody.keys, process.env.CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY || "")
+    );
+  } catch (e) {
+    return res.status(400).json({ message: "Failed to decrypt or parse credential keys" });
+  }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Decrypt the keys
const keys = JSON.parse(
symmetricDecrypt(reqBody.keys, process.env.CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY || "")
);
// Decrypt the keys
let keys;
try {
keys = JSON.parse(
symmetricDecrypt(reqBody.keys, process.env.CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY || "")
);
} catch (e) {
return res.status(400).json({ message: "Failed to decrypt or parse credential keys" });
}
🤖 Prompt for AI Agents 
In `@apps/web/pages/api/webhook/app-credential.ts` around lines 56 - 59, Wrap the
decryption/parse of reqBody.keys (the call to symmetricDecrypt(...) and
JSON.parse(...)) in a try/catch inside the API handler so failures return a 400
with a clear message instead of throwing a 500; specifically detect if
process.env.CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY is empty or if
symmetricDecrypt/JSON.parse throws, catch the error, log or include the error
message for debugging, and respond with res.status(400).json({ error: "Invalid
encrypted keys" /* plus brief detail */ }) rather than allowing the exception to
bubble.


// Can't use prisma upsert as we don't know the id of the credential
const appCredential = await prisma.credential.findFirst({
where: {
userId: reqBody.userId,
appId: appMetadata.slug,
},
select: {
id: true,
},
});

if (appCredential) {
await prisma.credential.update({
where: {
id: appCredential.id,
},
data: {
key: keys,
},
});
return res.status(200).json({ message: `Credentials updated for userId: ${reqBody.userId}` });
} else {
await prisma.credential.create({
data: {
key: keys,
userId: reqBody.userId,
appId: appMetadata.slug,
type: appMetadata.type,
},
});
return res.status(200).json({ message: `Credentials created for userId: ${reqBody.userId}` });
}
}
4 changes: 2 additions & 2 deletions ...-store/_utils/createOAuthAppCredential.ts → .../_utils/oauth/createOAuthAppCredential.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ import type { NextApiRequest } from "next";
import { HttpError } from "@calcom/lib/http-error";
import prisma from "@calcom/prisma";

import { decodeOAuthState } from "./decodeOAuthState";
import { throwIfNotHaveAdminAccessToTeam } from "./throwIfNotHaveAdminAccessToTeam";
import { decodeOAuthState } from "../oauth/decodeOAuthState";
import { throwIfNotHaveAdminAccessToTeam } from "../throwIfNotHaveAdminAccessToTeam";

/**
* This function is used to create app credentials for either a user or a team
Expand Down
2 changes: 1 addition & 1 deletion ...ages/app-store/_utils/decodeOAuthState.ts → ...pp-store/_utils/oauth/decodeOAuthState.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type { NextApiRequest } from "next";

import type { IntegrationOAuthCallbackState } from "../types";
import type { IntegrationOAuthCallbackState } from "../../types";

export function decodeOAuthState(req: NextApiRequest) {
if (typeof req.query.state !== "string") {
Expand Down
2 changes: 1 addition & 1 deletion ...ages/app-store/_utils/encodeOAuthState.ts → ...pp-store/_utils/oauth/encodeOAuthState.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type { NextApiRequest } from "next";

import type { IntegrationOAuthCallbackState } from "../types";
import type { IntegrationOAuthCallbackState } from "../../types";

export function encodeOAuthState(req: NextApiRequest) {
if (typeof req.query.state !== "string") {
Expand Down
32 changes: 32 additions & 0 deletions packages/app-store/_utils/oauth/parseRefreshTokenResponse.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { z } from "zod";

import { APP_CREDENTIAL_SHARING_ENABLED } from "@calcom/lib/constants";

const minimumTokenResponseSchema = z.object({
access_token: z.string(),
// Assume that any property with a number is the expiry
[z.string().toString()]: z.number(),
// Allow other properties in the token response
[z.string().optional().toString()]: z.unknown().optional(),
});
Comment on lines +5 to +11
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Test Zod behavior with computed property keys and key stripping
cat <<'EOF' > /tmp/test_zod.js
const { z } = require("zod");
console.log("=== Testing z.string().toString() ===");
console.log("z.string().toString() =>", z.string().toString());
console.log("z.string().optional().toString() =>", z.string().optional().toString());

console.log("\n=== Testing z.object with computed keys ===");
const schema = z.object({
  access_token: z.string(),
  [z.string().toString()]: z.number(),
  [z.string().optional().toString()]: z.unknown().optional(),
});

console.log("Schema keys:", Object.keys(schema.shape));

console.log("\n=== Testing safeParse with unknown keys ===");
const input = { access_token: "abc", refresh_token: "def", expires_in: 3600, scope: "read" };
const result = schema.safeParse(input);
console.log("Parse success:", result.success);
if (result.success) {
  console.log("Parsed data keys:", Object.keys(result.data));
  console.log("Parsed data:", JSON.stringify(result.data, null, 2));
} else {
  console.log("Error:", result.error.message);
}

console.log("\n=== Testing with .passthrough() ===");
const passthroughSchema = z.object({
  access_token: z.string(),
}).passthrough();
const result2 = passthroughSchema.safeParse(input);
console.log("Parse success:", result2.success);
if (result2.success) {
  console.log("Parsed data keys:", Object.keys(result2.data));
  console.log("Parsed data:", JSON.stringify(result2.data, null, 2));
}
EOF
cd /tmp && npm init -y --silent 2>/dev/null && npm install zod --silent 2>/dev/null && node test_zod.js

Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror

Length of output: 975

🏁 Script executed:

# Check the parseRefreshTokenResponse.ts file for the Google Calendar issue
grep -n "parseRefreshTokenResponse" packages/app-store/_utils/oauth/parseRefreshTokenResponse.ts | head -5
cat -n packages/app-store/_utils/oauth/parseRefreshTokenResponse.ts | head -35

Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror

Length of output: 1469

🏁 Script executed:

# Search for usage of parseRefreshTokenResponse in Google Calendar and other apps
rg "parseRefreshTokenResponse" --type typescript -B 2 -A 5 packages/app-store/ | grep -E "(googlecalendar|zoomvideo|office365calendar)" -A 8 | head -60

Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror

Length of output: 124

🏁 Script executed:

# Search for parseRefreshTokenResponse usage in the codebase
rg "parseRefreshTokenResponse" -B 2 -A 8 packages/app-store/ | head -150

Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror

Length of output: 9924

🏁 Script executed:

# Double-check what the literal computed key string is
cat <<'EOF' > /tmp/check_key.js
const key = "[object Object]";
console.log("Literal key string:", key);
console.log("Key from z.string().toString():", JSON.stringify(key));

// Verify the schema shape
const { z } = require("zod");
const schema = z.object({
  access_token: z.string(),
  [z.string().toString()]: z.number(),
});
console.log("Schema shape keys:", Object.keys(schema.shape));
EOF
cd /tmp && npm install zod --silent 2>/dev/null && node check_key.js

Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror

Length of output: 232

minimumTokenResponseSchema computed keys do not work as intended.

z.string().toString() evaluates to the literal string "[object Object]" (JavaScript's default toString() representation), creating fixed property names rather than wildcard matchers. This means the schema defines literal keys like "[object Object]" instead of "any string key → number" patterns. Since z.object() strips unrecognized keys by default, when this schema is used (i.e., when APP_CREDENTIAL_SHARING_ENABLED is true), the parsed output will lose properties like refresh_token, expires_in, expiry_date, scope, etc.

To retain all properties while only requiring access_token, use .passthrough():

Proposed fix 
-const minimumTokenResponseSchema = z.object({
-  access_token: z.string(),
-  //   Assume that any property with a number is the expiry
-  [z.string().toString()]: z.number(),
-  //   Allow other properties in the token response
-  [z.string().optional().toString()]: z.unknown().optional(),
-});
+const minimumTokenResponseSchema = z
+  .object({
+    access_token: z.string(),
+  })
+  .passthrough();
🤖 Prompt for AI Agents 
In `@packages/app-store/_utils/oauth/parseRefreshTokenResponse.ts` around lines 5
- 11, minimumTokenResponseSchema currently uses computed keys like
[z.string().toString()] which evaluate to literal "[object Object]" and don't
create wildcard matchers; replace the object definition with a schema that only
asserts required fields and allows all other properties to pass through — e.g.
change minimumTokenResponseSchema to z.object({ access_token: z.string()
}).passthrough() (remove the computed-key lines) so properties like
refresh_token, expires_in, expiry_date, scope, etc. are preserved.


const parseRefreshTokenResponse = (response: any, schema: z.ZodTypeAny) => {
let refreshTokenResponse;
if (APP_CREDENTIAL_SHARING_ENABLED && process.env.CALCOM_CREDENTIAL_SYNC_ENDPOINT) {
refreshTokenResponse = minimumTokenResponseSchema.safeParse(response);
} else {
refreshTokenResponse = schema.safeParse(response);
}

if (!refreshTokenResponse.success) {
throw new Error("Invalid refreshed tokens were returned");
}

if (!refreshTokenResponse.data.refresh_token) {
refreshTokenResponse.data.refresh_token = "refresh_token";
}

return refreshTokenResponse;
Comment on lines +21 to +29
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Return type is SafeParseReturnType, but callers expect inconsistent things.

The function throws on parse failure (Line 22) but returns the full SafeParseReturnType (with .success, .data, .error properties). This creates two issues across callers:

  1. Google Calendar (line 97) passes the return value directly as key to prisma.credential.update, storing { success: true, data: {...} } instead of the actual credential data — this corrupts the stored credential.
  2. Zoom (line 108) checks !parsedToken.success after this function already threw on failure — dead code that can never be reached.

Since the function already throws on failure, it should return just .data:

Proposed fix 
   if (!refreshTokenResponse.success) {
     throw new Error("Invalid refreshed tokens were returned");
   }
 
   if (!refreshTokenResponse.data.refresh_token) {
     refreshTokenResponse.data.refresh_token = "refresh_token";
   }
 
-  return refreshTokenResponse;
+  return refreshTokenResponse.data;
 };
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (!refreshTokenResponse.success) {
throw new Error("Invalid refreshed tokens were returned");
}
if (!refreshTokenResponse.data.refresh_token) {
refreshTokenResponse.data.refresh_token = "refresh_token";
}
return refreshTokenResponse;
if (!refreshTokenResponse.success) {
throw new Error("Invalid refreshed tokens were returned");
}
if (!refreshTokenResponse.data.refresh_token) {
refreshTokenResponse.data.refresh_token = "refresh_token";
}
return refreshTokenResponse.data;
🤖 Prompt for AI Agents 
In `@packages/app-store/_utils/oauth/parseRefreshTokenResponse.ts` around lines 21
- 29, parseRefreshTokenResponse currently throws on parse failure but returns
the full SafeParseReturnType object, which leads callers (e.g., Google Calendar
code that passes the return value as the `key` to prisma.credential.update and
Zoom code that checks `parsedToken.success`) to misuse the shape; change
parseRefreshTokenResponse to throw on failure and return only the parsed `.data`
(the credential token object) instead of the entire SafeParseReturnType, then
update callers that expect the full object (notably the Google Calendar flow
that writes to prisma.credential.update and the Zoom flow that redundantly
checks `parsedToken.success`) to treat the function as returning the credential
data directly (remove the dead `!parsedToken.success` check and pass the
returned data as the `key`/credential payload).

};

export default parseRefreshTokenResponse;
22 changes: 22 additions & 0 deletions packages/app-store/_utils/oauth/refreshOAuthTokens.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import { APP_CREDENTIAL_SHARING_ENABLED } from "@calcom/lib/constants";

const refreshOAuthTokens = async (refreshFunction: () => any, appSlug: string, userId: number | null) => {
// Check that app syncing is enabled and that the credential belongs to a user
if (APP_CREDENTIAL_SHARING_ENABLED && process.env.CALCOM_CREDENTIAL_SYNC_ENDPOINT && userId) {
// Customize the payload based on what your endpoint requires
// The response should only contain the access token and expiry date
const response = await fetch(process.env.CALCOM_CREDENTIAL_SYNC_ENDPOINT, {
method: "POST",
body: new URLSearchParams({
calcomUserId: userId.toString(),
appSlug,
}),
});
Comment on lines +8 to +14
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if CALCOM_WEBHOOK_SECRET is used anywhere for outbound requests
echo "=== Usage of CALCOM_WEBHOOK_SECRET ==="
rg -n "CALCOM_WEBHOOK_SECRET" --type=ts -C3
echo ""
echo "=== Usage of CALCOM_WEBHOOK_HEADER_NAME ==="
rg -n "CALCOM_WEBHOOK_HEADER_NAME" --type=ts -C3

Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror

Length of output: 1695

🏁 Script executed:

# Check .env.example for these variables
echo "=== .env.example definitions ==="
if [ -f .env.example ]; then
  grep -n "CALCOM_WEBHOOK_SECRET\|CALCOM_WEBHOOK_HEADER_NAME" .env.example
else
  echo ".env.example not found"
fi
echo ""

# Find and check the actual refreshOAuthTokens.ts file
echo "=== Full context of refreshOAuthTokens.ts ==="
find . -name "refreshOAuthTokens.ts" -type f

Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror

Length of output: 309

🏁 Script executed:

# Check how CALCOM_CREDENTIAL_SYNC_ENDPOINT is used
echo "=== Usage of CALCOM_CREDENTIAL_SYNC_ENDPOINT ==="
rg -n "CALCOM_CREDENTIAL_SYNC_ENDPOINT" --type=ts -C5

Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror

Length of output: 2683

🏁 Script executed:

cat -n packages/app-store/_utils/oauth/refreshOAuthTokens.ts

Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror

Length of output: 1094

🏁 Script executed:

# Check for any documentation or comments about CALCOM_CREDENTIAL_SYNC_ENDPOINT
rg -n "CALCOM_CREDENTIAL_SYNC_ENDPOINT" --type=ts --type=md -B2 -A2

Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror

Length of output: 1699

🏁 Script executed:

# Check how the webhook endpoint at app-credential.ts is structured (see if it's meant to receive this request)
cat -n apps/web/pages/api/webhook/app-credential.ts | head -50

Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror

Length of output: 2123

Add authentication header to credential sync request.

The POST to CALCOM_CREDENTIAL_SYNC_ENDPOINT lacks the webhook secret header. The codebase establishes a clear authentication pattern in apps/web/pages/api/webhook/app-credential.ts that validates requests using CALCOM_WEBHOOK_HEADER_NAME and CALCOM_WEBHOOK_SECRET (defined in .env.example lines 237–239). The outbound credential sync request should include this same header to authenticate the call.

Proposed fix — include the webhook secret header 
     const response = await fetch(process.env.CALCOM_CREDENTIAL_SYNC_ENDPOINT, {
       method: "POST",
+      headers: {
+        [process.env.CALCOM_WEBHOOK_HEADER_NAME || "calcom-webhook-secret"]:
+          process.env.CALCOM_WEBHOOK_SECRET || "",
+      },
       body: new URLSearchParams({
         calcomUserId: userId.toString(),
         appSlug,
       }),
     });
🤖 Prompt for AI Agents 
In `@packages/app-store/_utils/oauth/refreshOAuthTokens.ts` around lines 8 - 14,
The POST to CALCOM_CREDENTIAL_SYNC_ENDPOINT in refreshOAuthTokens.ts must
include the webhook auth header used by the receiver; modify the fetch call that
builds the request (the const response = await fetch(...) block) to add a
headers object with the header name taken from
process.env.CALCOM_WEBHOOK_HEADER_NAME and the secret from
process.env.CALCOM_WEBHOOK_SECRET so the outbound request matches the validation
in app-credential.ts; ensure you read both env vars and include the header when
making the POST.

return response;
} else {
const response = await refreshFunction();
return response;
}
Comment on lines +3 to +19
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Return type divergence between the two code paths may break callers.

When APP_CREDENTIAL_SHARING_ENABLED is true, this returns a Response from CALCOM_CREDENTIAL_SYNC_ENDPOINT. The callers (e.g., Webex's handleWebexResponse, Lark's handleLarkError<RefreshTokenResp>) then parse the response body using provider-specific schemas. The external sync endpoint would need to return a body conforming to each provider's expected schema, which is fragile and undocumented.

Additionally:

  • No error handling on the fetch to the sync endpoint — network errors will throw unhandled.
  • The () => any parameter type loses all type safety. Consider () => Promise<Response> since all callers pass fetch calls.
Suggested improvements 
-const refreshOAuthTokens = async (refreshFunction: () => any, appSlug: string, userId: number | null) => {
+const refreshOAuthTokens = async (
+  refreshFunction: () => Promise<Response>,
+  appSlug: string,
+  userId: number | null
+): Promise<Response> => {
   // Check that app syncing is enabled and that the credential belongs to a user
   if (APP_CREDENTIAL_SHARING_ENABLED && process.env.CALCOM_CREDENTIAL_SYNC_ENDPOINT && userId) {
-    // Customize the payload based on what your endpoint requires
-    // The response should only contain the access token and expiry date
     const response = await fetch(process.env.CALCOM_CREDENTIAL_SYNC_ENDPOINT, {
       method: "POST",
       body: new URLSearchParams({
         calcomUserId: userId.toString(),
         appSlug,
       }),
     });
+    if (!response.ok) {
+      throw new Error(`Credential sync endpoint returned ${response.status}`);
+    }
     return response;
   } else {
-    const response = await refreshFunction();
-    return response;
+    return refreshFunction();
   }
 };
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const refreshOAuthTokens = async (refreshFunction: () => any, appSlug: string, userId: number | null) => {
// Check that app syncing is enabled and that the credential belongs to a user
if (APP_CREDENTIAL_SHARING_ENABLED && process.env.CALCOM_CREDENTIAL_SYNC_ENDPOINT && userId) {
// Customize the payload based on what your endpoint requires
// The response should only contain the access token and expiry date
const response = await fetch(process.env.CALCOM_CREDENTIAL_SYNC_ENDPOINT, {
method: "POST",
body: new URLSearchParams({
calcomUserId: userId.toString(),
appSlug,
}),
});
return response;
} else {
const response = await refreshFunction();
return response;
}
const refreshOAuthTokens = async (
refreshFunction: () => Promise<Response>,
appSlug: string,
userId: number | null
): Promise<Response> => {
// Check that app syncing is enabled and that the credential belongs to a user
if (APP_CREDENTIAL_SHARING_ENABLED && process.env.CALCOM_CREDENTIAL_SYNC_ENDPOINT && userId) {
const response = await fetch(process.env.CALCOM_CREDENTIAL_SYNC_ENDPOINT, {
method: "POST",
body: new URLSearchParams({
calcomUserId: userId.toString(),
appSlug,
}),
});
if (!response.ok) {
throw new Error(`Credential sync endpoint returned ${response.status}`);
}
return response;
} else {
return refreshFunction();
}
};
🤖 Prompt for AI Agents 
In `@packages/app-store/_utils/oauth/refreshOAuthTokens.ts` around lines 3 - 19,
refreshOAuthTokens currently returns different types depending on
APP_CREDENTIAL_SHARING_ENABLED (raw fetch Response vs provider-specific
refreshFunction result), causing caller breakage; change the function to have a
consistent return shape (make it generic or return a normalized
RefreshTokenResp-like object) and update the signature to accept
refreshFunction: () => Promise<Response> (or a typed Promise<T>), call the sync
endpoint inside a try/catch to handle network errors, parse and validate the
sync endpoint JSON and map it into the provider-expected schema (or reject with
a clear error) so callers like handleWebexResponse and
handleLarkError<RefreshTokenResp> always receive the same, typed object; ensure
errors from fetch or validation are thrown as meaningful exceptions.

};

export default refreshOAuthTokens;
2 changes: 1 addition & 1 deletion packages/app-store/googlecalendar/api/add.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ import type { NextApiRequest, NextApiResponse } from "next";

import { WEBAPP_URL_FOR_OAUTH } from "@calcom/lib/constants";

import { encodeOAuthState } from "../../_utils/encodeOAuthState";
import getAppKeysFromSlug from "../../_utils/getAppKeysFromSlug";
import { encodeOAuthState } from "../../_utils/oauth/encodeOAuthState";

const scopes = [
"https://www.googleapis.com/auth/calendar.readonly",
Expand Down
2 changes: 1 addition & 1 deletion packages/app-store/googlecalendar/api/callback.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ import { WEBAPP_URL_FOR_OAUTH, CAL_URL } from "@calcom/lib/constants";
import { getSafeRedirectUrl } from "@calcom/lib/getSafeRedirectUrl";
import prisma from "@calcom/prisma";

import { decodeOAuthState } from "../../_utils/decodeOAuthState";
import getAppKeysFromSlug from "../../_utils/getAppKeysFromSlug";
import getInstalledAppPath from "../../_utils/getInstalledAppPath";
import { decodeOAuthState } from "../../_utils/oauth/decodeOAuthState";

let client_id = "";
let client_secret = "";
Expand Down
13 changes: 11 additions & 2 deletions packages/app-store/googlecalendar/lib/CalendarService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import type {
} from "@calcom/types/Calendar";
import type { CredentialPayload } from "@calcom/types/Credential";

import parseRefreshTokenResponse from "../../_utils/oauth/parseRefreshTokenResponse";
import refreshOAuthTokens from "../../_utils/oauth/refreshOAuthTokens";
import { getGoogleAppKeys } from "./getGoogleAppKeys";
import { googleCredentialSchema } from "./googleCredentialSchema";

Expand Down Expand Up @@ -81,11 +83,18 @@ export default class GoogleCalendarService implements Calendar {

const refreshAccessToken = async (myGoogleAuth: Awaited<ReturnType<typeof getGoogleAuth>>) => {
try {
const { res } = await myGoogleAuth.refreshToken(googleCredentials.refresh_token);
const res = await refreshOAuthTokens(
async () => {
const fetchTokens = await myGoogleAuth.refreshToken(googleCredentials.refresh_token);
return fetchTokens.res;
},
"google-calendar",
credential.userId
);
const token = res?.data;
googleCredentials.access_token = token.access_token;
googleCredentials.expiry_date = token.expiry_date;
const key = googleCredentialSchema.parse(googleCredentials);
const key = parseRefreshTokenResponse(googleCredentials, googleCredentialSchema);
await prisma.credential.update({
where: { id: credential.id },
data: { key },
Expand Down
2 changes: 1 addition & 1 deletion packages/app-store/hubspot/api/add.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ import type { NextApiRequest, NextApiResponse } from "next";

import { WEBAPP_URL } from "@calcom/lib/constants";

import { encodeOAuthState } from "../../_utils/encodeOAuthState";
import getAppKeysFromSlug from "../../_utils/getAppKeysFromSlug";
import { encodeOAuthState } from "../../_utils/oauth/encodeOAuthState";

const scopes = ["crm.objects.contacts.read", "crm.objects.contacts.write"];

Expand Down
4 changes: 2 additions & 2 deletions packages/app-store/hubspot/api/callback.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ import type { NextApiRequest, NextApiResponse } from "next";
import { WEBAPP_URL } from "@calcom/lib/constants";
import { getSafeRedirectUrl } from "@calcom/lib/getSafeRedirectUrl";

import createOAuthAppCredential from "../../_utils/createOAuthAppCredential";
import { decodeOAuthState } from "../../_utils/decodeOAuthState";
import getAppKeysFromSlug from "../../_utils/getAppKeysFromSlug";
import getInstalledAppPath from "../../_utils/getInstalledAppPath";
import createOAuthAppCredential from "../../_utils/oauth/createOAuthAppCredential";
import { decodeOAuthState } from "../../_utils/oauth/decodeOAuthState";

let client_id = "";
let client_secret = "";
Expand Down
20 changes: 13 additions & 7 deletions packages/app-store/hubspot/lib/CalendarService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import type {
import type { CredentialPayload } from "@calcom/types/Credential";

import getAppKeysFromSlug from "../../_utils/getAppKeysFromSlug";
import refreshOAuthTokens from "../../_utils/oauth/refreshOAuthTokens";
import type { HubspotToken } from "../api/callback";

const hubspotClient = new hubspot.Client();
Expand Down Expand Up @@ -173,13 +174,18 @@ export default class HubspotCalendarService implements Calendar {

const refreshAccessToken = async (refreshToken: string) => {
try {
const hubspotRefreshToken: HubspotToken = await hubspotClient.oauth.tokensApi.createToken(
"refresh_token",
undefined,
WEBAPP_URL + "/api/integrations/hubspot/callback",
this.client_id,
this.client_secret,
refreshToken
const hubspotRefreshToken: HubspotToken = await refreshOAuthTokens(
async () =>
await hubspotClient.oauth.tokensApi.createToken(
"refresh_token",
undefined,
WEBAPP_URL + "/api/integrations/hubspot/callback",
this.client_id,
this.client_secret,
refreshToken
),
"hubspot",
credential.userId
);

// set expiry date as offset from current time.
Expand Down
2 changes: 1 addition & 1 deletion packages/app-store/larkcalendar/api/add.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ import { z } from "zod";
import { WEBAPP_URL } from "@calcom/lib/constants";
import { defaultHandler, defaultResponder } from "@calcom/lib/server";

import { encodeOAuthState } from "../../_utils/encodeOAuthState";
import getAppKeysFromSlug from "../../_utils/getAppKeysFromSlug";
import { encodeOAuthState } from "../../_utils/oauth/encodeOAuthState";
import { LARK_HOST } from "../common";

const larkKeysSchema = z.object({
Expand Down
2 changes: 1 addition & 1 deletion packages/app-store/larkcalendar/api/callback.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ import logger from "@calcom/lib/logger";
import { defaultHandler, defaultResponder } from "@calcom/lib/server";
import prisma from "@calcom/prisma";

import { decodeOAuthState } from "../../_utils/decodeOAuthState";
import getInstalledAppPath from "../../_utils/getInstalledAppPath";
import { decodeOAuthState } from "../../_utils/oauth/decodeOAuthState";
import { LARK_HOST } from "../common";
import { getAppAccessToken } from "../lib/AppAccessToken";
import type { LarkAuthCredentials } from "../types/LarkCalendar";
Expand Down
28 changes: 17 additions & 11 deletions packages/app-store/larkcalendar/lib/CalendarService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import type {
} from "@calcom/types/Calendar";
import type { CredentialPayload } from "@calcom/types/Credential";

import refreshOAuthTokens from "../../_utils/oauth/refreshOAuthTokens";
import { handleLarkError, isExpired, LARK_HOST } from "../common";
import type {
CreateAttendeesResp,
Expand Down Expand Up @@ -63,17 +64,22 @@ export default class LarkCalendarService implements Calendar {
}
try {
const appAccessToken = await getAppAccessToken();
const resp = await fetch(`${this.url}/authen/v1/refresh_access_token`, {
method: "POST",
headers: {
Authorization: `Bearer ${appAccessToken}`,
"Content-Type": "application/json; charset=utf-8",
},
body: JSON.stringify({
grant_type: "refresh_token",
refresh_token: refreshToken,
}),
});
const resp = await refreshOAuthTokens(
async () =>
await fetch(`${this.url}/authen/v1/refresh_access_token`, {
method: "POST",
headers: {
Authorization: `Bearer ${appAccessToken}`,
"Content-Type": "application/json; charset=utf-8",
},
body: JSON.stringify({
grant_type: "refresh_token",
refresh_token: refreshToken,
}),
}),
"lark-calendar",
credential.userId
);

const data = await handleLarkError<RefreshTokenResp>(resp, this.log);
this.log.debug(
Expand Down
2 changes: 1 addition & 1 deletion packages/app-store/office365calendar/api/add.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ import { stringify } from "querystring";

import { WEBAPP_URL } from "@calcom/lib/constants";

import { encodeOAuthState } from "../../_utils/encodeOAuthState";
import getAppKeysFromSlug from "../../_utils/getAppKeysFromSlug";
import { encodeOAuthState } from "../../_utils/oauth/encodeOAuthState";

const scopes = ["User.Read", "Calendars.Read", "Calendars.ReadWrite", "offline_access"];

Expand Down
2 changes: 1 addition & 1 deletion packages/app-store/office365calendar/api/callback.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ import { WEBAPP_URL } from "@calcom/lib/constants";
import { getSafeRedirectUrl } from "@calcom/lib/getSafeRedirectUrl";
import prisma from "@calcom/prisma";

import { decodeOAuthState } from "../../_utils/decodeOAuthState";
import getAppKeysFromSlug from "../../_utils/getAppKeysFromSlug";
import getInstalledAppPath from "../../_utils/getInstalledAppPath";
import { decodeOAuthState } from "../../_utils/oauth/decodeOAuthState";

const scopes = ["offline_access", "Calendars.Read", "Calendars.ReadWrite"];

Expand Down