Skip to content

OAuth credential sync and app integration enhancements #4

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
akshayutture-augment wants to merge 1 commit into
base: oauth-security-base
Choose a base branch
from
Open

OAuth credential sync and app integration enhancements #4

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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
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(),
});
Comment on lines +9 to +15

@coderabbitai coderabbitai Bot Nov 21, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

Use safeParse for body validation and harden key decryption/parsing

Two related robustness issues:

  • appCredentialWebhookRequestBodySchema.parse(req.body) will throw on invalid input, returning a 500 instead of a clear 4xx.
  • JSON.parse(symmetricDecrypt(...)) is unguarded and passes "" as a fallback encryption key, which can also throw and end up as 500s.

A more defensive pattern:

-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(),
-});
+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(),
+});
@@
-  const reqBody = appCredentialWebhookRequestBodySchema.parse(req.body);
+  const parsedBody = appCredentialWebhookRequestBodySchema.safeParse(req.body);
+  if (!parsedBody.success) {
+    return res.status(400).json({ message: "Invalid request body" });
+  }
+  const reqBody = parsedBody.data;
@@
-  // Decrypt the keys
-  const keys = JSON.parse(
-    symmetricDecrypt(reqBody.keys, process.env.CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY || "")
-  );
+  // Decrypt the keys
+  const encryptionKey = process.env.CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY;
+  if (!encryptionKey) {
+    return res.status(500).json({ message: "Encryption key misconfigured" });
+  }
+
+  let keys;
+  try {
+    const decrypted = symmetricDecrypt(reqBody.keys, encryptionKey);
+    keys = JSON.parse(decrypted);
+  } catch {
+    return res.status(400).json({ message: "Invalid keys payload" });
+  }

This keeps malformed bodies and invalid/mis‑encrypted keys from bubbling up as unhandled exceptions and gives clients clear 4xx/5xx feedback.

Also applies to: 31-32, 56-59, 72-92

I'll verify the review comment by examining the actual file and code patterns.


---

🏁 Script executed:

```shell
#!/bin/bash
# Fetch and examine the webhook file at the specified lines
cat apps/web/pages/api/webhook/app-credential.ts | head -100

Length of output: 2782

Use safeParse for body validation and harden key decryption/parsing

Two related robustness issues are confirmed in the code:

  • Line 31: appCredentialWebhookRequestBodySchema.parse(req.body) will throw on invalid input, returning a 500 instead of a clear 4xx.
  • Lines 56–59: JSON.parse(symmetricDecrypt(reqBody.keys, process.env.CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY || "")) is unguarded and passes "" as a fallback encryption key, which can also throw and result in 500s instead of clear error responses.

A more defensive pattern:

-  const reqBody = appCredentialWebhookRequestBodySchema.parse(req.body);
+  const parsedBody = appCredentialWebhookRequestBodySchema.safeParse(req.body);
+  if (!parsedBody.success) {
+    return res.status(400).json({ message: "Invalid request body" });
+  }
+  const reqBody = parsedBody.data;
-  // Decrypt the keys
-  const keys = JSON.parse(
-    symmetricDecrypt(reqBody.keys, process.env.CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY || "")
-  );
+  // Decrypt the keys
+  const encryptionKey = process.env.CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY;
+  if (!encryptionKey) {
+    return res.status(500).json({ message: "Encryption key misconfigured" });
+  }
+
+  let keys;
+  try {
+    const decrypted = symmetricDecrypt(reqBody.keys, encryptionKey);
+    keys = JSON.parse(decrypted);
+  } catch {
+    return res.status(400).json({ message: "Invalid keys payload" });
+  }

This keeps malformed bodies and invalid/mis-encrypted keys from bubbling up as unhandled exceptions and gives clients clear 4xx/5xx feedback.

Committable suggestion skipped: line range outside the PR's diff.

/** */
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

@coderabbitai coderabbitai Bot Nov 21, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

❓ Verification inconclusive

Add HTTP method guard and make webhook secret check header‑case safe

Right now the handler:

  • Accepts any HTTP method (no 405 guard).
  • Reads req.headers[process.env.CALCOM_WEBHOOK_HEADER_NAME || "calcom-webhook-secret"] directly. Node/Next lowercase header keys, so if the env header name contains uppercase characters, the lookup will fail and every request will be rejected.

You can make this stricter and more robust as follows:

 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" });
-  }
+  if (req.method !== "POST") {
+    res.setHeader("Allow", "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" });
+  }
+
+  const webhookHeaderName = (process.env.CALCOM_WEBHOOK_HEADER_NAME || "calcom-webhook-secret").toLowerCase();
+  const providedSecret = req.headers[webhookHeaderName];
+  const expectedSecret = process.env.CALCOM_WEBHOOK_SECRET;
+
+  // Check that the webhook secret matches
+  if (
+    !expectedSecret ||
+    (Array.isArray(providedSecret)
+      ? !providedSecret.includes(expectedSecret)
+      : providedSecret !== expectedSecret)
+  ) {
+    return res.status(403).json({ message: "Invalid webhook secret" });
+  }

This keeps the check case‑insensitive, deals with string | string[], and returns 405 for non‑POST methods.

I'll verify the key claims in this review by searching for documentation on how Node.js and Next.js handle HTTP headers.

Based on my verification, the review comment identifies valid and important issues. Here is the rewritten review:

Add HTTP method guard and make webhook secret check header-case safe

The handler currently:

  • Accepts any HTTP method (no 405 guard)
  • Reads req.headers[process.env.CALCOM_WEBHOOK_HEADER_NAME || "calcom-webhook-secret"] directly. Node has taken the pragmatic approach of lowercasing all incoming headers, so if the env header name contains uppercase characters, the lookup will fail and every request will be rejected.

Suggested improvements:

 export default async function handler(req: NextApiRequest, res: NextApiResponse) {
+  if (req.method !== "POST") {
+    res.setHeader("Allow", "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" });
   }
 
+  const webhookHeaderName = (process.env.CALCOM_WEBHOOK_HEADER_NAME || "calcom-webhook-secret").toLowerCase();
+  const providedSecret = req.headers[webhookHeaderName];
+  const expectedSecret = process.env.CALCOM_WEBHOOK_SECRET;
+
   // Check that the webhook secret matches
   if (
-    req.headers[process.env.CALCOM_WEBHOOK_HEADER_NAME || "calcom-webhook-secret"] !==
-    process.env.CALCOM_WEBHOOK_SECRET
+    !expectedSecret ||
+    (Array.isArray(providedSecret)
+      ? !providedSecret.includes(expectedSecret)
+      : providedSecret !== expectedSecret)
   ) {
     return res.status(403).json({ message: "Invalid webhook secret" });
   }

This makes the check case-insensitive, handles the string | string[] case for headers, and returns 405 with an Allow header as required for method not allowed responses.

🤖 Prompt for AI Agents 
In apps/web/pages/api/webhook/app-credential.ts around lines 17 to 29, add an
HTTP method guard and make the webhook-secret header check case- and type-safe:
only allow the intended method (e.g., POST) and if other methods are received
return 405 with an "Allow" header; derive the header key from
process.env.CALCOM_WEBHOOK_HEADER_NAME || "calcom-webhook-secret" and normalize
it to lower-case before accessing req.headers (Node lowercases incoming
headers), then handle the header value possibly being a string[] by selecting
the first element (or treating undefined appropriately) and compare that string
to process.env.CALCOM_WEBHOOK_SECRET.


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

// 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 || "")
);

// 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(),
});

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;
};

export default parseRefreshTokenResponse;
Comment on lines +1 to +32

@coderabbitai coderabbitai Bot Nov 21, 2025

Copy link
Copy Markdown

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

Fix parseRefreshTokenResponse schema and return type; current version will mis-parse and corrupt stored keys

There are a few serious issues here:

  1. minimumTokenResponseSchema is effectively unusable

Using computed keys like:

[z.string().toString()]: z.number(),
[z.string().optional().toString()]: z.unknown().optional(),

produces literal property names like "[object Object]", not an index signature. No real token response will have such keys, so minimumTokenResponseSchema.safeParse(response) will always fail when APP_CREDENTIAL_SHARING_ENABLED is true, causing "Invalid refreshed tokens were returned" even for valid responses.

  1. The helper returns the entire safeParse result, not the parsed token

Callers such as the Google calendar service pass the returned value directly into prisma.credential.update({ data: { key } }). With the current implementation, key becomes { success: true, data: { ... } } instead of the credential object expected by the corresponding Zod schema, so subsequent schema.parse(credential.key) calls will throw.

  1. Hard‑coding refresh_token = "refresh_token" is dangerous

If a response genuinely omits refresh_token and the caller persists the returned object, you will overwrite any existing real refresh token with the literal string "refresh_token", breaking future refreshes.

At minimum, this helper should:

  • Use a realistic “minimal token” schema (access token + optional numeric expiry fields), allowing passthrough for unknown properties.
  • Return the parsed data object, not the safeParse wrapper.
  • Leave refresh_token handling to the caller (or at least not fabricate a placeholder string).

A safer implementation could look like this:

 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(),
-});
-
-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;
-};
+const minimumTokenResponseSchema = z
+  .object({
+    access_token: z.string(),
+    // Common expiry fields used by providers; both optional.
+    expires_in: z.number().optional(),
+    expiry_date: z.number().optional(),
+    refresh_token: z.string().optional(),
+  })
+  // Allow provider‑specific extras without failing validation.
+  .passthrough();
+
+const parseRefreshTokenResponse = (response: unknown, schema: z.ZodTypeAny) => {
+  const tokenSchema =
+    APP_CREDENTIAL_SHARING_ENABLED && process.env.CALCOM_CREDENTIAL_SYNC_ENDPOINT
+      ? minimumTokenResponseSchema
+      : schema;
+
+  const parsed = tokenSchema.safeParse(response);
+
+  if (!parsed.success) {
+    throw new Error("Invalid refreshed tokens were returned");
+  }
+
+  const data = parsed.data as any;
+
+  // Do not fabricate a refresh_token here; callers can decide how to handle
+  // missing refresh tokens (e.g., by reusing the previous one).
+
+  return data;
+};

With this change, callers will receive a plain token object (matching either the provider schema or the minimal schema), and existing usages like the Google calendar service can safely persist it as credential.key without corrupting the stored structure.

🏁 Script executed:

#!/bin/bash
# After applying the change, run TypeScript build/tests focusing on OAuth flows.
pnpm test --filter "app-store..." -- --runInBand || pnpm test -- --runInBand

Length of output: 385

Let me verify the codebase and search for how this function is being used.

🏁 Script executed:

#!/bin/bash
# Verify current implementation and find all call sites
echo "=== Current implementation ==="
cat packages/app-store/_utils/oauth/parseRefreshTokenResponse.ts 2>/dev/null || echo "File not found"

echo -e "\n=== Call sites of parseRefreshTokenResponse ==="
rg -n "parseRefreshTokenResponse" --type ts --type tsx -g "!node_modules" -C 2

Length of output: 1138

🏁 Script executed:

#!/bin/bash
# Try running tests with yarn (which the project uses)
cd /repo && yarn test --help 2>&1 | head -20

Length of output: 55

Let me search for call sites with corrected syntax:

🏁 Script executed:

#!/bin/bash
# Find all call sites of parseRefreshTokenResponse
echo "=== Searching for parseRefreshTokenResponse imports and usage ==="
rg -n "parseRefreshTokenResponse" -g "!node_modules" -C 3

echo -e "\n=== Checking if APP_CREDENTIAL_SHARING_ENABLED is used anywhere ==="
rg -n "APP_CREDENTIAL_SHARING_ENABLED" -g "!node_modules"

echo -e "\n=== Checking CALCOM_CREDENTIAL_SYNC_ENDPOINT usage ==="
rg -n "CALCOM_CREDENTIAL_SYNC_ENDPOINT" -g "!node_modules"

Length of output: 8800

🏁 Script executed:

#!/bin/bash
# Get full context around Google Calendar usage
echo "=== Google Calendar usage context ==="
rg -n "parseRefreshTokenResponse" packages/app-store/googlecalendar/lib/CalendarService.ts -A 10 -B 5

echo -e "\n=== Salesforce usage context ==="
rg -n "parseRefreshTokenResponse" packages/app-store/salesforce/lib/CalendarService.ts -A 10 -B 5

echo -e "\n=== Office365 usage context ==="
rg -n "parseRefreshTokenResponse" packages/app-store/office365calendar/lib/CalendarService.ts -A 10 -B 5

Length of output: 3713

🏁 Script executed:

#!/bin/bash
# Get Zoom usage context
echo "=== Zoom usage context ==="
rg -n "parseRefreshTokenResponse" packages/app-store/zoomvideo/lib/VideoApiAdapter.ts -A 15 -B 5

# Check what happens when credentials are read from database
echo -e "\n=== Check how credentials are used after stored ==="
rg -n "googleCredentialSchema" packages/app-store/googlecalendar/lib/CalendarService.ts -B 2 -A 2

Length of output: 2987

🏁 Script executed:

#!/bin/bash
# Verify what happens when Google credentials are parsed after being stored with wrapper
echo "=== Google credential schema definition ==="
cat packages/app-store/googlecalendar/lib/googleCredentialSchema.ts 2>/dev/null || rg -n "googleCredentialSchema" packages/app-store/googlecalendar/lib/ -A 5 -B 2

# Check if there's any defensive parsing or error handling
echo -e "\n=== Check error handling around line 75 ==="
rg -n "googleCredentialSchema.parse" packages/app-store/googlecalendar/lib/CalendarService.ts -A 3 -B 3

Length of output: 591

Fix parseRefreshTokenResponse implementation and update all callers; current code causes Google Calendar failures and proposed fix is incomplete

The review correctly identifies three critical issues:

  1. minimumTokenResponseSchema computed keys are broken: [z.string().toString()]: z.number() produces literal property names like "[object Object]", not a pattern. The schema will never validate real token responses, and any call will throw "Invalid refreshed tokens were returned".

  2. Google Calendar stores corrupted data: At line 97, parseRefreshTokenResponse returns a Zod SafeParseReturnType object { success: true, data: {...} } instead of just the data. This gets stored directly at line 98-100 via prisma.credential.update({data: {key}}). When credentials are later parsed at line 75 with googleCredentialSchema.parse(credential.key), it fails because the stored object has success and data properties instead of the expected {scope, token_type, expiry_date, access_token, refresh_token}.

  3. Hard-coding refresh_token = "refresh_token" is dangerous: If a response omits refresh_token and gets persisted, you overwrite the stored token with a literal placeholder string.

Critical note on the proposed fix: The suggestion to return parsed.data directly will fix Google Calendar but will break Zoom (line 104-111), Salesforce (line 90-98), and Office365 (line 263-264), all of which check the .success property and access .data. The proposed diff does not update these three call sites, so following it would introduce new failures.

A proper fix requires either:

  • Returning .data directly AND updating Zoom/Salesforce/Office365 to not check .success, or
  • Keeping SafeParseReturnType return and fixing only the schema + Google Calendar call site

At minimum, minimumTokenResponseSchema must use .passthrough() or explicit optional fields like expires_in, expiry_date, and refresh_token, not computed keys.

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) => {

@coderabbitai coderabbitai Bot Nov 21, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Replace any return type with a proper type definition.

The function signature uses any for both the refreshFunction parameter and return type, which eliminates type safety. This makes it difficult to catch type errors at compile time and reduces code maintainability.

Consider defining an interface for the token response:

+interface TokenRefreshResponse {
+  access_token: string;
+  expiry_date: number;
+  refresh_token?: string;
+  [key: string]: any;
+}
+
-const refreshOAuthTokens = async (refreshFunction: () => any, appSlug: string, userId: number | null) => {
+const refreshOAuthTokens = async (
+  refreshFunction: () => Promise<Response>,
+  appSlug: string,
+  userId: number | null
+): Promise<Response> => {

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In packages/app-store/_utils/oauth/refreshOAuthTokens.ts around line 3, the
function uses `any` for the refreshFunction parameter and its return type;
replace `any` with a concrete type by defining an interface (e.g. TokenResponse
with accessToken, refreshToken, expiresIn, scope, etc.) or a generic like <T
extends TokenResponse> and update the refreshFunction signature to return
Promise<T> (or T) and the refreshOAuthTokens return type to Promise<T> (or T).
Update callers to pass functions that match the new type or specify the generic,
and import/export the interface so the token shape is consistently enforced
across the codebase.

// 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;
Comment on lines +8 to +15

@coderabbitai coderabbitai Bot Nov 21, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Add error handling and timeout for the credential sync endpoint call.

The fetch request to CALCOM_CREDENTIAL_SYNC_ENDPOINT lacks error handling and timeout configuration, which could lead to:

  • Unhandled network failures or HTTP errors
  • Indefinite hanging if the endpoint is unresponsive
  • Poor user experience during OAuth flows

Apply this diff to add proper error handling and timeout:

-    const response = await fetch(process.env.CALCOM_CREDENTIAL_SYNC_ENDPOINT, {
-      method: "POST",
-      body: new URLSearchParams({
-        calcomUserId: userId.toString(),
-        appSlug,
-      }),
-    });
-    return response;
+    try {
+      const controller = new AbortController();
+      const timeoutId = setTimeout(() => controller.abort(), 10000); // 10s timeout
+      
+      const response = await fetch(process.env.CALCOM_CREDENTIAL_SYNC_ENDPOINT, {
+        method: "POST",
+        body: new URLSearchParams({
+          calcomUserId: userId.toString(),
+          appSlug,
+        }),
+        signal: controller.signal,
+      });
+      
+      clearTimeout(timeoutId);
+      
+      if (!response.ok) {
+        console.error(`Credential sync failed: ${response.status} ${response.statusText}`);
+        // Fall back to local refresh on sync endpoint failure
+        return await refreshFunction();
+      }
+      
+      return response;
+    } catch (error) {
+      console.error('Credential sync endpoint error:', error);
+      // Fall back to local refresh on network error
+      return await refreshFunction();
+    }

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In packages/app-store/_utils/oauth/refreshOAuthTokens.ts around lines 8 to 15,
the POST to CALCOM_CREDENTIAL_SYNC_ENDPOINT lacks timeout and error handling;
wrap the fetch in a try/catch, validate the environment variable first, create
an AbortController with a sensible timeout (e.g. 5s) and pass its signal to
fetch, check response.ok and throw or return a clear error if the status is not
2xx, ensure you clear the timeout on success, and surface or log the caught
error so callers can handle failures instead of hanging indefinitely.

} else {
const response = await refreshFunction();
return response;
}
};

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";
Comment on lines +26 to 27

@coderabbitai coderabbitai Bot Nov 21, 2025

Copy link
Copy Markdown

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

Confirm refreshOAuthTokens returns a HubspotToken object in this path

Here refreshOAuthTokens is expected to return a plain HubspotToken (used directly for expiresIn, accessToken, etc.), whereas other integrations (e.g., Zoho, Webex) appear to expect an HTTP response–like shape with .data.

Please verify that, for the "hubspot" provider, refreshOAuthTokens normalizes its result to the HubspotToken structure rather than an Axios-style { data } wrapper; otherwise this code will break at runtime.

Also applies to: 175-205

🏁 Script executed:

#!/bin/bash
# Quickly inspect the implementation of refreshOAuthTokens and confirm branching per provider.
fd 'refreshOAuthTokens.ts' -a
if [ -f "packages/app-store/_utils/oauth/refreshOAuthTokens.ts" ]; then
  sed -n '1,260p' packages/app-store/_utils/oauth/refreshOAuthTokens.ts
fi

Length of output: 923

I need to examine how the return value from refreshOAuthTokens is being used in HubSpot's CalendarService to understand the impact.

I need to inspect the actual HubSpot CalendarService implementation to see how the return value from refreshOAuthTokens is being used.

[/run_scripts]

🏁 Script executed:

#!/bin/bash
# Inspect HubSpot CalendarService refreshAccessToken method (lines 175-205)
if [ -f "packages/app-store/hubspot/lib/CalendarService.ts" ]; then
  sed -n '175,205p' packages/app-store/hubspot/lib/CalendarService.ts
fi

Length of output: 1011

I need to understand what tokensApi.createToken() returns and compare with other providers to confirm the issue.

[/run_scripts]

🏁 Script executed:

#!/bin/bash
# Search for tokensApi.createToken usage and check other provider implementations
rg -n "tokensApi\.createToken" --type=ts -A2 -B2 packages/app-store/hubspot/

# Also check other providers for pattern comparison
echo "=== Zoho pattern ==="
rg -n "refreshOAuthTokens" packages/app-store/zoho/ --type=ts -A5 -B2 | head -30

echo "=== Webex pattern ==="
rg -n "refreshOAuthTokens" packages/app-store/webex/ --type=ts -A5 -B2 | head -30

Length of output: 2499

I need to see how Webex actually handles the response to understand the expected pattern.

[/run_scripts]

🏁 Script executed:

#!/bin/bash
# See how Webex uses the response from refreshOAuthTokens
sed -n '60,85p' packages/app-store/webex/lib/VideoApiAdapter.ts

# Also check if there's any .json() or .data extraction in Webex
rg -n "response\." packages/app-store/webex/lib/VideoApiAdapter.ts -A2 -B1 | head -40

Length of output: 2576

The verification confirms the issue raised in the review comment. Let me check the HubSpot token type definition to be thorough.

[/run_scripts]

🏁 Script executed:

#!/bin/bash
# Verify HubspotToken type definition
cat packages/app-store/hubspot/api/callback.ts | grep -A10 "type HubspotToken"

# Also verify tokensApi.createToken return type by checking the import
rg -n "hubspotClient" packages/app-store/hubspot/lib/CalendarService.ts -B5 | head -20

Length of output: 615

Confirm refreshOAuthTokens returns inconsistent types that will cause runtime failure in HubSpot

The review comment is correct. refreshOAuthTokens has two return paths:

  1. When APP_CREDENTIAL_SHARING_ENABLED is true: returns a Response object from fetch()
  2. When disabled: returns the result from the callback function (tokensApi.createToken())

The HubSpot code at lines 191–192 directly accesses .expiresIn and .accessToken without extracting from a Response object. This means:

  • If the sync endpoint is enabled, the code will crash with "Cannot read properties of undefined" because Response objects don't have these fields
  • If the sync endpoint is disabled, it works because tokensApi.createToken() returns the token object directly

Unlike Webex (which calls handleWebexResponse() to extract .json() from the Response), HubSpot has no such handler. The wrapper function needs to return a consistent type or HubSpot's code needs to handle the Response case.


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
Loading