OAuth credential sync and app integration enhancements

[ShashankFC]

Test 8nn

Summary by CodeRabbit

• New Features

  • Added app credential sharing and synchronization to enable seamless credential management and real-time updates across connected applications
  • Improved OAuth token refresh mechanism for enhanced authentication reliability and consistency across all supported integrations

• Chores

  • Reorganized internal OAuth utility infrastructure

_{✏️ Tip: You can customize this high-level summary in your review settings.}

nn---n*Replicated from [ai-code-review-evaluation/cal.com-coderabbit#8](https://github.com/ai-code-review-evaluation/cal.com-coderabbit/pull/8)*

[ShashankFC]

@cubic-dev-ai review this pull request

[cubic-dev-ai]

@cubic-dev-ai review this pull request

@ShashankFC I have started the AI code review. It will take a few minutes to complete.

[cubic-dev-ai]

14 issues found across 40 files

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="packages/app-store/googlecalendar/lib/CalendarService.ts">

<violation number="1" location="packages/app-store/googlecalendar/lib/CalendarService.ts:97">
P0: Bug: `parseRefreshTokenResponse` returns a Zod `SafeParseReturnType` object (`{ success, data, error }`) instead of the parsed data. The original code used `googleCredentialSchema.parse()` which returns the data directly. Storing this SafeParseResult wrapper in the database will corrupt the credential, since subsequent reads via `googleCredentialSchema.parse(credential.key)` will fail — the key will have a `success` and `data` wrapper around the actual credential fields.

You should access `.data` on the return value, or the utility should be fixed to return `refreshTokenResponse.data` instead of `refreshTokenResponse`.</violation>
</file>

<file name="packages/app-store/_utils/oauth/refreshOAuthTokens.ts">

<violation number="1" location="packages/app-store/_utils/oauth/refreshOAuthTokens.ts:8">
P1: The `fetch` call to the credential sync endpoint has no error handling. Network failures will throw unhandled exceptions, and HTTP error responses (4xx/5xx) will be silently passed to callers. Consider wrapping in try/catch and checking `response.ok` before returning.</violation>

<violation number="2" location="packages/app-store/_utils/oauth/refreshOAuthTokens.ts:15">
P1: Inconsistent return types between the two branches. The credential sync branch returns a raw `fetch` `Response`, but the `refreshFunction()` branch returns whatever the callback returns (e.g., `HubspotToken` for HubSpot). Callers like HubSpot directly access `.expiryDate` and `.expiresIn` on the result, which will fail at runtime when credential sync is enabled because `Response` doesn't have those properties. The sync response should be parsed (e.g., `await response.json()`) to match the expected shape of each caller.</violation>
</file>

<file name="apps/web/pages/api/webhook/app-credential.ts">

<violation number="1" location="apps/web/pages/api/webhook/app-credential.ts:17">
P2: Missing HTTP method check. This webhook handler accepts any HTTP method (GET, DELETE, PUT, etc.). It should be restricted to POST only to follow REST conventions and prevent unintended invocations.</violation>

<violation number="2" location="apps/web/pages/api/webhook/app-credential.ts:31">
P1: No error handling around `schema.parse()`, `symmetricDecrypt()`, and `JSON.parse()`. All three can throw, resulting in unhandled 500 errors with potentially sensitive internal details leaked in the response. Wrap in try-catch or use `.safeParse()` for the Zod validation.</violation>

<violation number="3" location="apps/web/pages/api/webhook/app-credential.ts:58">
P1: Falling back to an empty string (`""`) when `CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY` is missing is dangerous. AES-256 requires a 32-byte key, so this will throw a confusing `Invalid key length` error at runtime. Fail fast with an explicit check instead.</violation>
</file>

<file name="packages/lib/constants.ts">

<violation number="1" location="packages/lib/constants.ts:104">
P2: Missing `!!` coercion — `APP_CREDENTIAL_SHARING_ENABLED` will be `string | undefined` instead of `boolean`. This is inconsistent with the established pattern for similar constants in this file (e.g., `IS_STRIPE_ENABLED` uses `!!(...)`). An `_ENABLED` constant should be a boolean.</violation>
</file>

<file name="packages/app-store/_utils/oauth/parseRefreshTokenResponse.ts">

<violation number="1" location="packages/app-store/_utils/oauth/parseRefreshTokenResponse.ts:8">
P0: Bug: `[z.string().toString()]` does not create a dynamic/catch-all key in `z.object()` — it evaluates to a literal string property name (the stringified Zod type). This schema will either reject valid OAuth token responses or strip all fields except `access_token`. To allow arbitrary additional properties through the schema, use `.passthrough()` on the `z.object()`.</violation>

<violation number="2" location="packages/app-store/_utils/oauth/parseRefreshTokenResponse.ts:26">
P1: Bug: Setting `refresh_token` to the literal string `"refresh_token"` when it's missing will cause downstream OAuth refresh failures. This dummy value will be sent as the actual refresh token to the OAuth provider, which will reject it. If the response genuinely lacks a refresh token (e.g., some OAuth flows), consider preserving the existing refresh token from the credential rather than substituting a dummy value.</violation>
</file>

<file name="packages/app-store/zoho-bigin/lib/CalendarService.ts">

<violation number="1" location="packages/app-store/zoho-bigin/lib/CalendarService.ts:93">
P1: Bug: `credentialId` (credential's own ID) is passed as the third argument to `refreshOAuthTokens`, but the function expects a `userId` (the owning user's ID). Other integrations (google-calendar, hubspot) correctly pass `credential.userId`. When credential sharing is enabled, this sends the wrong identifier to the sync endpoint, causing incorrect credential resolution.</violation>
</file>

<file name="packages/app-store/salesforce/lib/CalendarService.ts">

<violation number="1" location="packages/app-store/salesforce/lib/CalendarService.ts:86">
P1: Bug: `response.statusText` is unreliable for determining request success. In Node.js 18+ native fetch (especially over HTTP/2), `statusText` may be empty even for error responses. Use `!response.ok` instead, which reliably checks if the status code is in the 200-299 range.</violation>

<violation number="2" location="packages/app-store/salesforce/lib/CalendarService.ts:96">
P0: Bug: Refreshed token is saved to DB but the current `jsforce.Connection` still uses the stale `credentialKey.access_token` and `credentialKey.instance_url`. The newly refreshed values from `accessTokenParsed.data` should be used for the connection. Consider updating the variables used by the Connection constructor, e.g.:
```js
return new jsforce.Connection({
  ...
  instanceUrl: accessTokenParsed.data.instance_url,
  accessToken: accessTokenParsed.data.access_token,
  ...
});
```</violation>
</file>

<file name="packages/app-store/larkcalendar/lib/CalendarService.ts">

<violation number="1" location="packages/app-store/larkcalendar/lib/CalendarService.ts:67">
P1: Potential runtime error when `APP_CREDENTIAL_SHARING_ENABLED` is true: the sync endpoint response is passed to `handleLarkError<RefreshTokenResp>()`, which expects Lark-specific fields (`refresh_token`, `refresh_expires_in`, etc.). The sync endpoint comment states it should only return "access token and expiry date", which would miss the `refresh_token` and `refresh_expires_in` fields needed downstream. Consider handling the sync endpoint response separately before passing it to the Lark-specific parser.</violation>
</file>

<file name="packages/app-store/zoho-bigin/api/add.ts">

<violation number="1" location="packages/app-store/zoho-bigin/api/add.ts:17">
P2: Hardcoded slug `"zoho-bigin"` is inconsistent with the rest of this file, which reads the slug dynamically via `appConfig.slug` (e.g., line 12). Prefer the previous dynamic approach to keep the code DRY and avoid a silent mismatch if the slug ever changes.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

[cubic-dev-ai]

P0: Bug: parseRefreshTokenResponse returns a Zod SafeParseReturnType object ({ success, data, error }) instead of the parsed data. The original code used googleCredentialSchema.parse() which returns the data directly. Storing this SafeParseResult wrapper in the database will corrupt the credential, since subsequent reads via googleCredentialSchema.parse(credential.key) will fail — the key will have a success and data wrapper around the actual credential fields.

You should access .data on the return value, or the utility should be fixed to return refreshTokenResponse.data instead of refreshTokenResponse.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At packages/app-store/googlecalendar/lib/CalendarService.ts, line 97:

<comment>Bug: `parseRefreshTokenResponse` returns a Zod `SafeParseReturnType` object (`{ success, data, error }`) instead of the parsed data. The original code used `googleCredentialSchema.parse()` which returns the data directly. Storing this SafeParseResult wrapper in the database will corrupt the credential, since subsequent reads via `googleCredentialSchema.parse(credential.key)` will fail — the key will have a `success` and `data` wrapper around the actual credential fields.

You should access `.data` on the return value, or the utility should be fixed to return `refreshTokenResponse.data` instead of `refreshTokenResponse`.</comment>

<file context>
@@ -81,11 +83,18 @@ export default class GoogleCalendarService implements Calendar {
         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 },
</file context>

[cubic-dev-ai]

P0: Bug: [z.string().toString()] does not create a dynamic/catch-all key in z.object() — it evaluates to a literal string property name (the stringified Zod type). This schema will either reject valid OAuth token responses or strip all fields except access_token. To allow arbitrary additional properties through the schema, use .passthrough() on the z.object().

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At packages/app-store/_utils/oauth/parseRefreshTokenResponse.ts, line 8:

<comment>Bug: `[z.string().toString()]` does not create a dynamic/catch-all key in `z.object()` — it evaluates to a literal string property name (the stringified Zod type). This schema will either reject valid OAuth token responses or strip all fields except `access_token`. To allow arbitrary additional properties through the schema, use `.passthrough()` on the `z.object()`.</comment>

<file context>
@@ -0,0 +1,32 @@
+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(),
</file context>

[cubic-dev-ai]

P0: Bug: Refreshed token is saved to DB but the current jsforce.Connection still uses the stale credentialKey.access_token and credentialKey.instance_url. The newly refreshed values from accessTokenParsed.data should be used for the connection. Consider updating the variables used by the Connection constructor, e.g.:

return new jsforce.Connection({
  ...
  instanceUrl: accessTokenParsed.data.instance_url,
  accessToken: accessTokenParsed.data.access_token,
  ...
});

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At packages/app-store/salesforce/lib/CalendarService.ts, line 96:

<comment>Bug: Refreshed token is saved to DB but the current `jsforce.Connection` still uses the stale `credentialKey.access_token` and `credentialKey.instance_url`. The newly refreshed values from `accessTokenParsed.data` should be used for the connection. Consider updating the variables used by the Connection constructor, e.g.:
```js
return new jsforce.Connection({
  ...
  instanceUrl: accessTokenParsed.data.instance_url,
  accessToken: accessTokenParsed.data.access_token,
  ...
});
```</comment>

<file context>
@@ -60,6 +72,32 @@ export default class SalesforceCalendarService implements Calendar {
+      return Promise.reject(new Error("Invalid refreshed tokens were returned"));
+    }
+
+    await prisma.credential.update({
+      where: { id: credential.id },
+      data: { key: { ...accessTokenParsed.data, refresh_token: credentialKey.refresh_token } },
</file context>

[cubic-dev-ai]

P1: The fetch call to the credential sync endpoint has no error handling. Network failures will throw unhandled exceptions, and HTTP error responses (4xx/5xx) will be silently passed to callers. Consider wrapping in try/catch and checking response.ok before returning.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At packages/app-store/_utils/oauth/refreshOAuthTokens.ts, line 8:

<comment>The `fetch` call to the credential sync endpoint has no error handling. Network failures will throw unhandled exceptions, and HTTP error responses (4xx/5xx) will be silently passed to callers. Consider wrapping in try/catch and checking `response.ok` before returning.</comment>

<file context>
@@ -0,0 +1,22 @@
+  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({
</file context>

[cubic-dev-ai]

P1: Inconsistent return types between the two branches. The credential sync branch returns a raw fetch Response, but the refreshFunction() branch returns whatever the callback returns (e.g., HubspotToken for HubSpot). Callers like HubSpot directly access .expiryDate and .expiresIn on the result, which will fail at runtime when credential sync is enabled because Response doesn't have those properties. The sync response should be parsed (e.g., await response.json()) to match the expected shape of each caller.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At packages/app-store/_utils/oauth/refreshOAuthTokens.ts, line 15:

<comment>Inconsistent return types between the two branches. The credential sync branch returns a raw `fetch` `Response`, but the `refreshFunction()` branch returns whatever the callback returns (e.g., `HubspotToken` for HubSpot). Callers like HubSpot directly access `.expiryDate` and `.expiresIn` on the result, which will fail at runtime when credential sync is enabled because `Response` doesn't have those properties. The sync response should be parsed (e.g., `await response.json()`) to match the expected shape of each caller.</comment>

<file context>
@@ -0,0 +1,22 @@
+        appSlug,
+      }),
+    });
+    return response;
+  } else {
+    const response = await refreshFunction();
</file context>

[cubic-dev-ai]

P1: Bug: response.statusText is unreliable for determining request success. In Node.js 18+ native fetch (especially over HTTP/2), statusText may be empty even for error responses. Use !response.ok instead, which reliably checks if the status code is in the 200-299 range.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At packages/app-store/salesforce/lib/CalendarService.ts, line 86:

<comment>Bug: `response.statusText` is unreliable for determining request success. In Node.js 18+ native fetch (especially over HTTP/2), `statusText` may be empty even for error responses. Use `!response.ok` instead, which reliably checks if the status code is in the 200-299 range.</comment>

<file context>
@@ -60,6 +72,32 @@ export default class SalesforceCalendarService implements Calendar {
+      }),
+    });
+
+    if (response.statusText !== "OK") throw new HttpError({ statusCode: 400, message: response.statusText });
+
+    const accessTokenJson = await response.json();
</file context>

Suggested change

	if (response.statusText !== "OK") throw new HttpError({ statusCode: 400, message: response.statusText });
	if (!response.ok) throw new HttpError({ statusCode: response.status, message: `Salesforce token refresh failed: ${response.status}` });

[cubic-dev-ai]

P1: Potential runtime error when APP_CREDENTIAL_SHARING_ENABLED is true: the sync endpoint response is passed to handleLarkError<RefreshTokenResp>(), which expects Lark-specific fields (refresh_token, refresh_expires_in, etc.). The sync endpoint comment states it should only return "access token and expiry date", which would miss the refresh_token and refresh_expires_in fields needed downstream. Consider handling the sync endpoint response separately before passing it to the Lark-specific parser.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At packages/app-store/larkcalendar/lib/CalendarService.ts, line 67:

<comment>Potential runtime error when `APP_CREDENTIAL_SHARING_ENABLED` is true: the sync endpoint response is passed to `handleLarkError<RefreshTokenResp>()`, which expects Lark-specific fields (`refresh_token`, `refresh_expires_in`, etc.). The sync endpoint comment states it should only return "access token and expiry date", which would miss the `refresh_token` and `refresh_expires_in` fields needed downstream. Consider handling the sync endpoint response separately before passing it to the Lark-specific parser.</comment>

<file context>
@@ -63,17 +64,22 @@ export default class LarkCalendarService implements Calendar {
-          refresh_token: refreshToken,
-        }),
-      });
+      const resp = await refreshOAuthTokens(
+        async () =>
+          await fetch(`${this.url}/authen/v1/refresh_access_token`, {
</file context>

[cubic-dev-ai]

P2: Missing HTTP method check. This webhook handler accepts any HTTP method (GET, DELETE, PUT, etc.). It should be restricted to POST only to follow REST conventions and prevent unintended invocations.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/pages/api/webhook/app-credential.ts, line 17:

<comment>Missing HTTP method check. This webhook handler accepts any HTTP method (GET, DELETE, PUT, etc.). It should be restricted to POST only to follow REST conventions and prevent unintended invocations.</comment>

<file context>
@@ -0,0 +1,93 @@
+  keys: z.string(),
+});
+/** */
+export default async function handler(req: NextApiRequest, res: NextApiResponse) {
+  // Check that credential sharing is enabled
+  if (!APP_CREDENTIAL_SHARING_ENABLED) {
</file context>

[cubic-dev-ai]

P2: Missing !! coercion — APP_CREDENTIAL_SHARING_ENABLED will be string | undefined instead of boolean. This is inconsistent with the established pattern for similar constants in this file (e.g., IS_STRIPE_ENABLED uses !!(...)). An _ENABLED constant should be a boolean.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At packages/lib/constants.ts, line 104:

<comment>Missing `!!` coercion — `APP_CREDENTIAL_SHARING_ENABLED` will be `string | undefined` instead of `boolean`. This is inconsistent with the established pattern for similar constants in this file (e.g., `IS_STRIPE_ENABLED` uses `!!(...)`). An `_ENABLED` constant should be a boolean.</comment>

<file context>
@@ -99,3 +99,6 @@ export const ORGANIZATION_MIN_SEATS = 30;
 export const CALCOM_VERSION = process.env.NEXT_PUBLIC_CALCOM_VERSION as string;
+
+export const APP_CREDENTIAL_SHARING_ENABLED =
+  process.env.CALCOM_WEBHOOK_SECRET && process.env.CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY;
</file context>

Suggested change

	process.env.CALCOM_WEBHOOK_SECRET && process.env.CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY;
	!!(process.env.CALCOM_WEBHOOK_SECRET && process.env.CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY);

[cubic-dev-ai]

P2: Hardcoded slug "zoho-bigin" is inconsistent with the rest of this file, which reads the slug dynamically via appConfig.slug (e.g., line 12). Prefer the previous dynamic approach to keep the code DRY and avoid a silent mismatch if the slug ever changes.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At packages/app-store/zoho-bigin/api/add.ts, line 17:

<comment>Hardcoded slug `"zoho-bigin"` is inconsistent with the rest of this file, which reads the slug dynamically via `appConfig.slug` (e.g., line 12). Prefer the previous dynamic approach to keep the code DRY and avoid a silent mismatch if the slug ever changes.</comment>

<file context>
@@ -14,7 +14,7 @@ export default async function handler(req: NextApiRequest, res: NextApiResponse)
     if (!clientId) return res.status(400).json({ message: "Zoho Bigin client_id missing." });
 
-    const redirectUri = WEBAPP_URL + `/api/integrations/${appConfig.slug}/callback`;
+    const redirectUri = WEBAPP_URL + `/api/integrations/zoho-bigin/callback`;
 
     const authUrl = axios.getUri({
</file context>

Suggested change

	const redirectUri = WEBAPP_URL + `/api/integrations/zoho-bigin/callback`;
	const redirectUri = WEBAPP_URL + `/api/integrations/${appConfig.slug}/callback`;