OAuth credential sync and app integration enhancements#78
Conversation
…alcom#11059) * Add credential sync .env variables * Add webhook to send app credentials * Upsert credentials when webhook called * Refresh oauth token from a specific endpoint * Pass appSlug * Add credential encryption * Move oauth helps into a folder * Create parse token response wrapper * Add OAuth helpers to apps * Clean up * Refactor `appDirName` to `appSlug` * Address feedback * Change to safe parse * Remove console.log --------- Co-authored-by: Syed Ali Shahbaz <52925846+alishaz-polymath@users.noreply.github.com> Co-authored-by: Omar López <zomars@me.com>
|
Tenki Code Review - Complete Files Reviewed: 40 By Severity:
This PR introduces a credential sharing feature for OAuth integrations but has multiple critical and high-severity bugs that will cause runtime failures, silent credential corruption, and security vulnerabilities. A missing Files Reviewed (40 files) |
![tenki-reviewer[bot]](https://avatars.githubusercontent.com/in/2900641?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary
This PR refactors OAuth token refresh logic across 14+ calendar and video app integrations to support a credential-sync feature. The changes are structured around three new shared helpers:
parseRefreshTokenResponse.ts— validates token responses with ZodrefreshOAuthTokens.ts— routes requests to either a credential-sync endpoint or normal OAuth providerapp-credential.tswebhook — accepts encrypted credential updates from an external sync service
While the architectural goal is sound, the implementation has 4 critical/high-severity bugs and 8 medium-severity issues that will break production behavior:
🔴 Critical Issues
Missing Import in Salesforce — packages/app-store/salesforce/lib/CalendarService.ts calls prisma.credential.update on line 96 but never imports prisma. This causes a runtime ReferenceError when any Salesforce credential needs refresh.
🔴 High-Severity Bugs
Broken Zod Schema — minimumTokenResponseSchema (lines 8–10) uses computed property keys like [z.string().toString()] which evaluate to the literal string "ZodString" instead of acting as index signatures. Any token response from the credential-sync endpoint will fail validation, breaking the entire credential-sync flow.
Type Mismatch in refreshOAuthTokens — When credential-sync is enabled, the function returns a raw fetch Response object. Callers in ZohoCRM and Zoho-Bigin expect an axios response with a .data property, causing undefined property access and failed token updates.
Unsafe Decryption Fallback — The webhook handler (line 58) uses process.env.CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY || "" as the AES key. If the env var is missing, decryption proceeds with an empty string, either crashing or silently decrypting with a trivially known key.
Timing-Attack on Webhook Secret — The webhook authentication (lines 24–29) uses a plain !== string comparison, allowing character-by-character enumeration via response-time analysis.
🟠 Medium-Severity Issues
- Placeholder refresh token — When the sync endpoint omits
refresh_token, the code stores the literal string"refresh_token"in the database, breaking all future OAuth calls. - Return-type inconsistency —
parseRefreshTokenResponsereturns a Zod wrapper object but callers expect unwrapped data, breaking the Google Calendar integration. - Dead error check — Salesforce's error guard after
parseRefreshTokenResponsecan never execute since the function throws instead of returning failure. - Expiry calculation off by 1000x — ZohoCRM token expiry adds 3600ms (≈3.6 seconds) instead of 1 hour.
- Unhandled Zod errors — Request body validation throws a
ZodErrorthat is never caught, returning 500 instead of 400. - Missing HTTP method check — Webhook accepts GET, DELETE, etc., violating defense-in-depth for credential writes.
- No outbound auth — Sync endpoint requests carry no authentication, allowing DNS poisoning to inject forged tokens.
- Inverted token expiry logic — ZohoCRM's
isTokenValidcheck is backwards, causing expired tokens to be reused.
Recommendations
- Before merge: Fix the 4 critical/high bugs above (import, Zod schema, type mismatch, empty encryption key).
- Security: Use
crypto.timingSafeEqual()for webhook auth, add HTTP method restriction, and require authentication on the sync endpoint. - Testing: Ensure token refresh paths are exercised for Google Calendar, Zoom, Webex, Salesforce, and Zoho integrations with credential-sync enabled.
- Documentation: Document that
CALCOM_CREDENTIAL_SYNC_ENDPOINTmust authenticate requests and always use HTTPS.
| await prisma.credential.update({ | ||
| where: { id: credential.id }, | ||
| data: { key: { ...accessTokenParsed.data, refresh_token: credentialKey.refresh_token } }, | ||
| }); |
There was a problem hiding this comment.
🚨 Missing prisma import in SalesforceCalendarService causes runtime crash (bug)
In packages/app-store/salesforce/lib/CalendarService.ts, prisma.credential.update is called on line 96 to persist refreshed tokens, but prisma is never imported. There is no import prisma from "@calcom/prisma" in the file. This will throw a ReferenceError: prisma is not defined at runtime whenever Salesforce credentials need a token refresh.
💡 Suggestion: Add import prisma from "@calcom/prisma"; to the import block at the top of the file.
📋 Prompt for AI Agents
In packages/app-store/salesforce/lib/CalendarService.ts, add a missing import for prisma. After the existing import block (around line 20), add: import prisma from "@calcom/prisma";. This is required because prisma.credential.update is called on line 96 to persist refreshed tokens, but the module is never imported, causing a ReferenceError at runtime.
| 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(), | ||
| }); |
There was a problem hiding this comment.
🔴 Computed property keys in minimumTokenResponseSchema use Zod schema objects, not string values (bug)
In parseRefreshTokenResponse.ts lines 8 and 10, computed property keys [z.string().toString()] and [z.string().optional().toString()] evaluate to the literal strings 'ZodString' and 'ZodOptional' (the Zod class name returned by .toString()). The intent was to express "any property with a numeric value" and "any other optional property", but Zod's z.object() does not support index signatures via computed keys this way. The schema ends up requiring a property literally named 'ZodString' of type number — which most real token responses would not have — and an optional property named 'ZodOptional'. This means the minimumTokenResponseSchema will fail to validate real token responses in the credential-sharing path, causing parseRefreshTokenResponse to always throw "Invalid refreshed tokens were returned" when APP_CREDENTIAL_SHARING_ENABLED is true and CALCOM_CREDENTIAL_SYNC_ENDPOINT is set.
💡 Suggestion: Replace the broken computed-key approach with a proper Zod schema. Since the goal is to accept any token response that has at least an access_token string, use z.object({ access_token: z.string() }).passthrough(). If enforcing a numeric expiry field is desired, keep only the explicit known keys (e.g. expires_in: z.number().optional()) and use .passthrough() to allow extra fields.
📋 Prompt for AI Agents
In packages/app-store/_utils/oauth/parseRefreshTokenResponse.ts, replace the minimumTokenResponseSchema definition (lines 5-11) with a correct Zod schema. The current schema uses [z.string().toString()]: z.number() and [z.string().optional().toString()]: z.unknown().optional() as computed property keys, but these evaluate to the literal strings 'ZodString' and 'ZodOptional' rather than acting as index signatures. Change the schema to:
const minimumTokenResponseSchema = z.object({
access_token: z.string(),
expires_in: z.number().optional(),
}).passthrough();This correctly requires only access_token, allows an optional numeric expires_in, and passes through any additional properties (like refresh_token from the credential sync endpoint). Remove the broken computed property key lines entirely.
| 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; | ||
| } | ||
| }; |
There was a problem hiding this comment.
🔴 refreshOAuthTokens returns incompatible types: Response vs axios response (bug)
refreshOAuthTokens in packages/app-store/_utils/oauth/refreshOAuthTokens.ts has two branches:
- Credential-sync path (line 8-15): returns a raw
fetchResponseobject. - Normal path (line 17-18): returns whatever the caller's
refreshFunction()returns — an axios response for zohocrm/zoho-bigin, or afetchResponsefor webex/zoom.
In zohocrm/lib/CalendarService.ts (lines 217, 219, 227, 233) and zoho-bigin/lib/CalendarService.ts (lines 96, 98-100, 110), the returned value is used as an axios response (tokenInfo.data.error, tokenInfo.data.access_token, etc.). When APP_CREDENTIAL_SHARING_ENABLED is true and a user ID is present, refreshOAuthTokens returns a raw Response instead — .data will be undefined, causing all subsequent property accesses to silently produce undefined or throw a TypeError at runtime. This means tokens will not be updated in the DB and this.accessToken will be set to undefined.
💡 Suggestion: The function should have a uniform return contract. Either: (a) always return a parsed JSON body (call response.json() in the credential-sync branch before returning), or (b) add a discriminator so callers can detect which branch ran and handle each case. The simplest fix is to parse the Response to JSON inside refreshOAuthTokens before returning it, so every branch returns the same shape of data.
📋 Prompt for AI Agents
In packages/app-store/_utils/oauth/refreshOAuthTokens.ts lines 8-15, the credential-sync branch returns a raw fetch Response object, while all callers (zohocrm, zoho-bigin, webex, zoom) expect an already-resolved response body. Add const json = await response.json(); return json; instead of bare return response; in the credential-sync branch, so the return type is consistent with the normal branch. Also update the function's return type annotation to Promise<any> (or a typed union) to make the contract explicit and catch future divergence at compile time.
| const keys = JSON.parse( | ||
| symmetricDecrypt(reqBody.keys, process.env.CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY || "") | ||
| ); |
There was a problem hiding this comment.
🔴 Decryption falls back to empty string key if env var is unset (crypto)
At line 58 of apps/web/pages/api/webhook/app-credential.ts:
const keys = JSON.parse(
symmetricDecrypt(reqBody.keys, process.env.CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY || "")
);If CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY is absent or empty at runtime, the fallback "" (empty string) is used as the AES-256 decryption key. symmetricDecrypt in packages/lib/crypto.ts converts the key via Buffer.from(key, 'latin1'), producing a zero-byte buffer. Node's crypto.createDecipheriv('aes256', ...) will throw Invalid key length, crashing this request path; worse, in misconfigured deployments it could silently decrypt with a known-empty key.
The correct fix is to fail fast and explicitly at startup if the key is absent, rather than silently falling back to an insecure default.
💡 Suggestion: Reject the request early if the encryption key is missing, rather than falling back to an empty string. Add an explicit guard before calling symmetricDecrypt.
📋 Prompt for AI Agents
In apps/web/pages/api/webhook/app-credential.ts at lines 57-59, replace the symmetricDecrypt call that falls back to an empty string key with an explicit guard that returns a 500 error if the environment variable is absent:
const encryptionKey = process.env.CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY;
if (!encryptionKey) {
return res.status(500).json({ message: "Server misconfiguration: encryption key not set" });
}
const keys = JSON.parse(symmetricDecrypt(reqBody.keys, encryptionKey));This prevents silent use of a zero-length AES key and surfaces misconfiguration clearly.
| 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" }); | ||
| } |
There was a problem hiding this comment.
🔴 Webhook secret compared with non-constant-time string equality (crypto)
In apps/web/pages/api/webhook/app-credential.ts (lines 24-29), the webhook secret is authenticated by a plain JavaScript !== comparison:
if (
req.headers[process.env.CALCOM_WEBHOOK_HEADER_NAME || "calcom-webhook-secret"] !==
process.env.CALCOM_WEBHOOK_SECRET
)This is a timing-attack vector. JavaScript's !== on strings short-circuits as soon as it finds a differing character, so the comparison takes measurably less time for strings that share a longer common prefix with the real secret. An attacker with the ability to make many HTTP requests to this endpoint (which is publicly reachable as a Next.js API route) can statistically enumerate the secret byte-by-byte. This endpoint is high-value: a successful bypass allows writing arbitrary OAuth credentials into any user's account (credential.create / credential.update).
Fix: use crypto.timingSafeEqual() after converting both buffers to the same length, or use a purpose-built constant-time comparison library.
💡 Suggestion: Replace the plain !== comparison with a constant-time comparison using crypto.timingSafeEqual. Both values must be converted to equal-length Buffer instances before comparing.
📋 Prompt for AI Agents
In apps/web/pages/api/webhook/app-credential.ts, replace the webhook-secret comparison at lines 24-29 with a constant-time comparison to prevent timing-based secret enumeration. Import crypto from Node's built-in module and replace the existing if block with:
const incomingSecret = req.headers[process.env.CALCOM_WEBHOOK_HEADER_NAME || "calcom-webhook-secret"] as string | undefined;
const expectedSecret = process.env.CALCOM_WEBHOOK_SECRET || "";
if (
!incomingSecret ||
incomingSecret.length !== expectedSecret.length ||
!crypto.timingSafeEqual(Buffer.from(incomingSecret), Buffer.from(expectedSecret))
) {
return res.status(403).json({ message: "Invalid webhook secret" });
}Also add import crypto from "crypto"; at the top of the file. This prevents an attacker from statistically guessing the secret character-by-character via response-time differences.
| ); | ||
| if (!zohoCrmTokenInfo.data.error) { | ||
| // set expiry date as offset from current time. | ||
| zohoCrmTokenInfo.data.expiryDate = Math.round(Date.now() + 60 * 60); |
There was a problem hiding this comment.
🟠 ZohoCRM token expiry calculation adds seconds instead of milliseconds (bug)
In zohocrm/lib/CalendarService.ts line 219:
zohoCrmTokenInfo.data.expiryDate = Math.round(Date.now() + 60 * 60);Date.now() returns milliseconds. Adding 60 * 60 = 3600 sets the expiry only ~3.6 seconds in the future. The intent (1-hour expiry) requires multiplying by 1000: Date.now() + 60 * 60 * 1000. This causes the token to be treated as always-expired, triggering a new refresh call on every API request.
💡 Suggestion: Change Math.round(Date.now() + 60 * 60) to Math.round(Date.now() + 60 * 60 * 1000) to correctly set expiry 1 hour from now in milliseconds.
| zohoCrmTokenInfo.data.expiryDate = Math.round(Date.now() + 60 * 60); | |
| zohoCrmTokenInfo.data.expiryDate = Math.round(Date.now() + 60 * 60 * 1000); |
| return res.status(403).json({ message: "Invalid webhook secret" }); | ||
| } | ||
|
|
||
| const reqBody = appCredentialWebhookRequestBodySchema.parse(req.body); |
There was a problem hiding this comment.
🟠 Unhandled ZodError from .parse() returns 500 instead of 400 for invalid request body (bug)
In apps/web/pages/api/webhook/app-credential.ts line 31, appCredentialWebhookRequestBodySchema.parse(req.body) is called without a try/catch. If the request body is missing required fields or has wrong types, Zod throws a ZodError. Since the handler is async and there is no global error handler for this route, Next.js will respond with a 500 Internal Server Error rather than a meaningful 400 response. This makes it harder to diagnose client-side integration errors and may expose error stack traces.
💡 Suggestion: Use .safeParse() instead of .parse() and return a 400 response with validation error details if parsing fails:
const result = appCredentialWebhookRequestBodySchema.safeParse(req.body);
if (!result.success) {
return res.status(400).json({ message: "Invalid request body", error: result.error.issues });
}
const reqBody = result.data;| 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" }); | ||
| } |
There was a problem hiding this comment.
🟠 Webhook endpoint accepts all HTTP methods without restriction (auth)
The new apps/web/pages/api/webhook/app-credential.ts handler does not validate req.method. Any HTTP method (GET, DELETE, PUT, etc.) will reach the authentication and write logic. Although Zod's body-parse will fail on method types that lack a body, explicitly restricting to POST is a necessary defence-in-depth control for a privileged credential-write endpoint, and aligns with the standard Next.js API route pattern. A misconfigured reverse proxy or CORS preflight could potentially trigger partial handler execution.
💡 Suggestion: Add a method guard at the top of the handler, before any other logic, to reject non-POST requests with 405.
| export const APP_CREDENTIAL_SHARING_ENABLED = | ||
| process.env.CALCOM_WEBHOOK_SECRET && process.env.CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY; |
There was a problem hiding this comment.
🟠 APP_CREDENTIAL_SHARING_ENABLED has incorrect type — evaluates to string, not boolean (bug)
In packages/lib/constants.ts lines 103-104:
export const APP_CREDENTIAL_SHARING_ENABLED =
process.env.CALCOM_WEBHOOK_SECRET && process.env.CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY;When both env vars are set, JavaScript short-circuit evaluation makes this expression equal to the string value of CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY, not true. When only one is set, it equals the string of the first one. The consumer in refreshOAuthTokens.ts (line 5) checks if (APP_CREDENTIAL_SHARING_ENABLED && ...) — this works correctly in a boolean context today, but the exported constant has type string | undefined rather than boolean. Code that uses strict equality (=== true) or TypeScript type checks will behave incorrectly.
💡 Suggestion: Wrap the expression with !!() to produce a proper boolean: export const APP_CREDENTIAL_SHARING_ENABLED = !!(process.env.CALCOM_WEBHOOK_SECRET && process.env.CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY);
| export const APP_CREDENTIAL_SHARING_ENABLED = | |
| process.env.CALCOM_WEBHOOK_SECRET && process.env.CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY; | |
| export const APP_CREDENTIAL_SHARING_ENABLED = | |
| !!(process.env.CALCOM_WEBHOOK_SECRET && process.env.CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY); |
| const response = await fetch(process.env.CALCOM_CREDENTIAL_SYNC_ENDPOINT, { | ||
| method: "POST", | ||
| body: new URLSearchParams({ | ||
| calcomUserId: userId.toString(), | ||
| appSlug, | ||
| }), | ||
| }); |
There was a problem hiding this comment.
🟠 Outbound credential sync request carries no authentication — sync endpoint response is implicitly trusted (auth)
In packages/app-store/_utils/oauth/refreshOAuthTokens.ts (lines 8–14), when credential sharing is enabled, Cal.com makes an outbound POST to CALCOM_CREDENTIAL_SYNC_ENDPOINT sending only calcomUserId and appSlug:
const response = await fetch(process.env.CALCOM_CREDENTIAL_SYNC_ENDPOINT, {
method: "POST",
body: new URLSearchParams({
calcomUserId: userId.toString(),
appSlug,
}),
});This request includes no authentication header (no HMAC signature, no Authorization: Bearer <secret>, no shared token). The sync endpoint has no way to verify the request came from Cal.com rather than any other client. Conversely, Cal.com has no way to verify the response originated from the legitimate sync service.
If an attacker can intercept the outbound request (e.g., via DNS poisoning of the endpoint hostname, or via an MITM on a non-TLS endpoint), or if the endpoint URL is misconfigured to point at an attacker-controlled server, the attacker can return a crafted token response that gets written to the credential database for any targeted user. The parseRefreshTokenResponse function uses a loose schema (minimumTokenResponseSchema) when in sync mode, further reducing validation of the returned data.
💡 Suggestion: Include a request authentication mechanism in the outbound sync call. Options: (1) add an Authorization: Bearer <CALCOM_WEBHOOK_SECRET> header so the sync endpoint can verify the caller, (2) include an HMAC-SHA256 signature of the request body using CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY as a request header, or (3) use mutual TLS. At minimum, ensure CALCOM_CREDENTIAL_SYNC_ENDPOINT always uses HTTPS. Validate in documentation that the endpoint must perform authentication.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
🟡 Salesforce uses stale access token for jsforce.Connection after explicit token refresh
After explicitly refreshing the token (lines 75-99) and storing the new accessTokenParsed.data in the DB, the code still creates jsforce.Connection with accessToken: credentialKey.access_token (the original, potentially expired token from credential.key) instead of accessTokenParsed.data.access_token (the newly refreshed token). This makes the explicit token refresh pointless since the Connection is initialized with the old token.
(Refers to line 106)
Was this helpful? React with 👍 or 👎 to provide feedback.
| googleCredentials.access_token = token.access_token; | ||
| googleCredentials.expiry_date = token.expiry_date; | ||
| const key = googleCredentialSchema.parse(googleCredentials); | ||
| const key = parseRefreshTokenResponse(googleCredentials, googleCredentialSchema); |
There was a problem hiding this comment.
🔴 Google Calendar stores SafeParseResult wrapper instead of parsed credential data
parseRefreshTokenResponse returns the full safeParse result object ({ success: true, data: { scope, token_type, ... } }) rather than just the parsed data. The old code at this line was const key = googleCredentialSchema.parse(googleCredentials) which returns the data directly. Now the DB stores the wrapper object. On the next token check, googleCredentialSchema.parse(credential.key) at packages/app-store/googlecalendar/lib/CalendarService.ts:75 will fail because it finds success and data properties instead of the expected scope, token_type, expiry_date, etc. This corrupts the credential for every Google Calendar user whose token gets refreshed, breaking the integration until the credential is re-created.
| const key = parseRefreshTokenResponse(googleCredentials, googleCredentialSchema); | |
| const key = parseRefreshTokenResponse(googleCredentials, googleCredentialSchema).data; |
Was this helpful? React with 👍 or 👎 to provide feedback.
| 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(), | ||
| }); |
There was a problem hiding this comment.
🔴 minimumTokenResponseSchema uses non-functional computed property names that strip all token fields
z.string().toString() and z.string().optional().toString() both evaluate to "[object Object]" (Zod schemas don't override toString()), so the second computed property overwrites the first, and neither matches any real response property. As a result, minimumTokenResponseSchema only validates that access_token is a string and, crucially, z.object().safeParse() strips all unrecognized keys from .data. When credential sharing is enabled, the parsed data loses refresh_token, expiry_date, expires_in, etc. Then line 26 sets refresh_token to the literal string "refresh_token", corrupting credentials for every integration that uses this path.
Prompt for agents
The minimumTokenResponseSchema in parseRefreshTokenResponse.ts uses computed property names like [z.string().toString()] which evaluate to the literal string "[object Object]" rather than creating dynamic key validators. Zod's z.object() does not support pattern-based keys this way. The intent was to allow any property name with a number value (for expiry) and allow passthrough of other properties.
To fix this, you should use z.object({ access_token: z.string() }).passthrough() instead, which will validate access_token while preserving all other properties in the parsed output. The .passthrough() modifier prevents Zod from stripping unrecognized keys. Without this fix, when credential sharing is enabled, all token properties except access_token are stripped from the parsed data, and refresh_token is set to the literal string "refresh_token".
Was this helpful? React with 👍 or 👎 to provide feedback.
| 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 | ||
| ); |
There was a problem hiding this comment.
🔴 HubSpot receives raw fetch Response instead of HubspotToken when credential sharing is enabled
When credential sharing is enabled, refreshOAuthTokens returns a raw fetch Response object (from packages/app-store/_utils/oauth/refreshOAuthTokens.ts:15). The HubSpot code assigns this directly to hubspotRefreshToken: HubspotToken and then accesses .expiresIn (line 192) and .accessToken (line 202), which don't exist on a Response object. This results in expiryDate = NaN and hubspotClient.setAccessToken(undefined), completely breaking HubSpot when credential sharing is enabled.
Prompt for agents
In packages/app-store/hubspot/lib/CalendarService.ts, the refreshOAuthTokens function returns a raw fetch Response when credential sharing is enabled, but the code expects a HubspotToken object. The same issue affects Zoho Bigin (packages/app-store/zoho-bigin/lib/CalendarService.ts:85-94, expects axios response with .data) and Zoho CRM (packages/app-store/zohocrm/lib/CalendarService.ts:204-216, also expects axios response with .data).
For integrations that use fetch internally (Office365, Zoom, Webex, Lark), both code paths return a fetch Response so they work. But for integrations using other HTTP clients (HubSpot SDK, axios), the return types are incompatible.
The fix should either: (1) have refreshOAuthTokens return parsed JSON instead of a raw Response, or (2) have each caller handle both return types, or (3) normalize the response inside refreshOAuthTokens to match the expected shape.
Was this helpful? React with 👍 or 👎 to provide feedback.
| }, | ||
| }), | ||
| "zoho-bigin", | ||
| credentialId |
There was a problem hiding this comment.
🟡 Zoho Bigin passes credential ID instead of user ID to refreshOAuthTokens
At packages/app-store/zoho-bigin/lib/CalendarService.ts:93, credentialId is passed as the third argument to refreshOAuthTokens, but this value is credential.id (the database ID of the credential record, set at line 53). The function's third parameter is userId (packages/app-store/_utils/oauth/refreshOAuthTokens.ts:3). When credential sharing is enabled, the credential sync endpoint receives the credential's DB ID as calcomUserId, which is incorrect and would sync tokens to the wrong user.
Was this helpful? React with 👍 or 👎 to provide feedback.
2 participants
Uh oh!
There was an error while loading. Please reload this page.