OAuth credential sync and app integration enhancements#6
Conversation
…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>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
📝 WalkthroughWalkthroughThe PR implements Cal.com integration support for app credential synchronization. It adds environment variables for webhook handling and encryption, creates a new webhook endpoint for processing app credentials, refactors OAuth utilities into a dedicated directory structure, and introduces centralized token refresh and parsing utilities. Import paths are updated across multiple integrations to use the reorganized OAuth utilities. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as Caller Service
participant RefreshUtil as refreshOAuthTokens()
participant Endpoint as Sync Endpoint
participant LocalFunc as Local Refresh Func
participant Caller as Caller (persists token)
Caller->>RefreshUtil: Call with refreshFunc, appSlug, userId
alt APP_CREDENTIAL_SHARING_ENABLED && sync endpoint configured
RefreshUtil->>Endpoint: POST (userId, appSlug)
Endpoint-->>RefreshUtil: Token response
else
RefreshUtil->>LocalFunc: Call local refresh function
LocalFunc-->>RefreshUtil: Token response
end
RefreshUtil-->>Caller: Return response
Caller->>Caller: Parse & validate with parseRefreshTokenResponse()
Caller->>Caller: Persist updated token to database
sequenceDiagram
participant Client as External Client
participant Webhook as App Credential Webhook
participant Validator as Validator
participant Prisma as Prisma DB
participant Crypto as Crypto Service
participant Response as Response
Client->>Webhook: POST /api/webhook/app-credential
Webhook->>Validator: Check feature flag (APP_CREDENTIAL_SHARING_ENABLED)
Validator-->>Webhook: Feature enabled?
Webhook->>Validator: Verify webhook secret from header
Validator-->>Webhook: Secret valid?
Webhook->>Validator: Validate request body (userId, appSlug, keys)
Validator-->>Webhook: Body valid?
Webhook->>Prisma: Check user & app exist
Prisma-->>Webhook: User & app found?
Webhook->>Crypto: Decrypt keys with CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY
Crypto-->>Webhook: Decrypted JSON keys
Webhook->>Prisma: Find or create credential
Prisma-->>Webhook: Credential upserted
Webhook->>Response: Return 200 (success message)
Response-->>Client: Response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/app-store/webex/lib/VideoApiAdapter.ts (1)
151-152: Remove debug console.log statements.Multiple debug
console.logstatements are present in production code. These should be removed or replaced with a proper logging utility (e.g., using a logger from@calcom/lib).🧹 Suggested cleanup
Remove or replace the following console.log statements:
- Lines 151-152: In
fetchWebexApi- Lines 184-186: In
createMeeting- Line 194: In
createMeeting- Line 223: In
deleteMeetingIf logging is needed for debugging, consider using a structured logger with appropriate log levels.
Also applies to: 184-186, 194-194, 223-223
packages/app-store/salesforce/lib/CalendarService.ts (1)
101-108: Connection created with stale credentials instead of refreshed tokens.After refreshing the tokens (Lines 75-99), the
jsforce.Connectionis initialized with the OLDcredentialKeyproperties (accessToken,refreshToken,instanceUrlfrom Line 73). This means the connection will use expired credentials, defeating the purpose of the token refresh.🔧 Proposed fix to use refreshed tokens
+ const refreshedCredentials = { + ...accessTokenParsed.data, + refresh_token: credentialKey.refresh_token + }; + await prisma.credential.update({ where: { id: credential.id }, - data: { key: { ...accessTokenParsed.data, refresh_token: credentialKey.refresh_token } }, + data: { key: refreshedCredentials }, }); return new jsforce.Connection({ clientId: consumer_key, clientSecret: consumer_secret, redirectUri: WEBAPP_URL + "/api/integrations/salesforce/callback", - instanceUrl: credentialKey.instance_url, - accessToken: credentialKey.access_token, - refreshToken: credentialKey.refresh_token, + instanceUrl: refreshedCredentials.instance_url, + accessToken: refreshedCredentials.access_token, + refreshToken: refreshedCredentials.refresh_token, });
🤖 Fix all issues with AI agents
In @apps/web/pages/api/webhook/app-credential.ts:
- Around line 56-59: Wrap the decryption+parsing of reqBody.keys (the call to
symmetricDecrypt and JSON.parse that assigns keys) in a try-catch; on failure
catch the error, log only a sanitized message (no secret content) including the
operation context (e.g., "app credential decrypt/parse failed"), and return an
appropriate client error response (400 or 422) instead of letting the exception
bubble to a 500. Ensure you still reference
process.env.CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY when calling symmetricDecrypt,
and avoid returning or logging the decrypted payload or raw error stack.
- Around line 17-31: The handler currently allows any method and calls
appCredentialWebhookRequestBodySchema.parse directly which can throw and produce
a 500; update the exported handler to first reject non-POST methods (respond 405
with Allow: POST) and then wrap the call to
appCredentialWebhookRequestBodySchema.parse in a try/catch that returns a 400
with validation error details on parse failure; keep existing checks for
APP_CREDENTIAL_SHARING_ENABLED and the
CALCOM_WEBHOOK_HEADER_NAME/CALCOM_WEBHOOK_SECRET comparison, and reference the
same symbols (handler, APP_CREDENTIAL_SHARING_ENABLED,
CALCOM_WEBHOOK_HEADER_NAME, CALCOM_WEBHOOK_SECRET,
appCredentialWebhookRequestBodySchema.parse) when making the changes.
In @packages/app-store/_utils/oauth/createOAuthAppCredential.ts:
- Line 6: The import for decodeOAuthState in createOAuthAppCredential.ts uses
the wrong relative path; update the import statement that references
"../oauth/decodeOAuthState" to use "./decodeOAuthState" so it imports the
decodeOAuthState function from the same _utils/oauth directory (look for the
import line in createOAuthAppCredential.ts and change the module specifier
accordingly).
In @packages/app-store/_utils/oauth/parseRefreshTokenResponse.ts:
- Around line 5-11: The minimumTokenResponseSchema is invalid because using
z.string().toString() creates a literal property name; replace the object schema
with one that requires access_token and allows arbitrary additional
properties—e.g., define minimumTokenResponseSchema = z.object({ access_token:
z.string() }).passthrough() or use .catchall(z.unknown()) so dynamic fields like
expires_in are accepted; update any validation usage of
minimumTokenResponseSchema to work with the new passthrough/catchall behavior.
- Around line 25-27: The code unconditionally sets
refreshTokenResponse.data.refresh_token = "refresh_token" which writes a
hardcoded dummy into the DB; instead, update parseRefreshTokenResponse to stop
injecting that literal and handle missing refresh_token based on
APP_CREDENTIAL_SHARING_ENABLED and the minimumTokenResponseSchema: if credential
sharing is disabled and refresh_token is required, throw a clear error; if
credential sharing is enabled, treat refresh_token as optional (do not set a
dummy) and ensure downstream code can handle its absence; update any validation
logic around minimumTokenResponseSchema and the function refreshTokenResponse
handling to reflect the conditional behavior (refer to refreshTokenResponse,
minimumTokenResponseSchema, and APP_CREDENTIAL_SHARING_ENABLED).
In @packages/app-store/_utils/oauth/refreshOAuthTokens.ts:
- Around line 5-15: The fetch to process.env.CALCOM_CREDENTIAL_SYNC_ENDPOINT
inside the APP_CREDENTIAL_SHARING_ENABLED branch lacks error handling, timeout,
and response validation; wrap the request in a try/catch, use an AbortController
with a reasonable timeout to abort the fetch, await and check response.ok and
throw a descriptive error for non-2xx responses, parse the response JSON and
validate required fields (e.g., access token and expiry) before returning them
(instead of returning the raw Response), and ensure any thrown errors include
context (endpoint and userId/appSlug) so callers can handle failures from
refreshOAuthTokens safely.
In @packages/app-store/googlecalendar/lib/CalendarService.ts:
- Around line 86-96: The code assumes refreshOAuthTokens always returns an
object with a .data field but when the sync endpoint is enabled it returns a
fetch Response; update the handling after calling refreshOAuthTokens (the res
variable returned by refreshOAuthTokens in this block) to detect whether res is
a fetch Response or an OAuth provider response, and extract token accordingly:
if res has a .json method (or instanceof Response) call await res.json() to get
the payload, otherwise use res.data; then assign googleCredentials.access_token
and googleCredentials.expiry_date from the extracted token object (referencing
refreshOAuthTokens, myGoogleAuth.refreshToken, googleCredentials, and
credential.userId to locate the code).
In @packages/app-store/hubspot/lib/CalendarService.ts:
- Around line 177-189: The refreshOAuthTokens implementation returns
inconsistent types (fetch Response vs raw token) causing callers like the
HubSpot flow to crash when accessing fields like expiresIn; update
refreshOAuthTokens so both code paths parse the fetch Response (await
response.json()) and normalize/validate the resulting object into a consistent
token shape (include expiresIn, accessToken/refreshToken names matching callers)
before returning, and ensure any error responses are turned into thrown errors
or a consistent error object so callers like the hubspotRefreshToken usage can
safely read properties.
In @packages/app-store/office365calendar/lib/CalendarService.ts:
- Around line 20-21: The repository imports centralized OAuth helpers
parseRefreshTokenResponse and refreshOAuthTokens in CalendarService.ts but there
are no unit tests for them; add test files that exercise both functions: for
refreshOAuthTokens write tests that mock HTTP/token endpoint responses to assert
successful token retrieval, error handling, and retry/edge cases, and for
parseRefreshTokenResponse add tests that feed typical provider responses
(including expires_in, refresh_token rotation, missing fields) and assert the
parsed output shape and error paths; place tests targeting the functions'
exported names (parseRefreshTokenResponse, refreshOAuthTokens) and use the same
mocking utilities used elsewhere in integrations to ensure consistent behavior
and prevent regressions.
In @packages/app-store/salesforce/lib/CalendarService.ts:
- Around line 96-99: The file calls prisma.credential.update but never imports
prisma; add the missing import statement for the Prisma client (import prisma
from "@calcom/prisma") at the top of the file so references like
prisma.credential.update(...) in CalendarService (and any other uses in this
module) resolve correctly.
- Line 86: Replace the fragile statusText check in CalendarService (the if that
throws HttpError based on response.statusText) with a check on response.ok to
detect non-2xx responses; when response.ok is false, throw the HttpError
including the response.status and response.statusText (or response body) to
preserve diagnostic information. Ensure you modify the check that currently
reads "if (response.statusText !== 'OK')" to use "response.ok" and include
useful error details in the thrown HttpError.
- Line 4: The CalendarService is missing the import and use of the shared
refreshOAuthTokens wrapper; add an import for refreshOAuthTokens (from
../../_utils/oauth/refreshOAuthTokens) at the top of CalendarService and replace
the direct fetch-based token-refresh logic inside CalendarService (the block
currently doing a fetch to Salesforce's token endpoint, around the token refresh
section) with a call to refreshOAuthTokens so credential sync logic is
centralized and consistent with other providers (e.g., Google Calendar).
- Around line 75-84: Replace the direct fetch call in CalendarService with the
centralized refreshOAuthTokens wrapper: import and call refreshOAuthTokens from
its module, passing the existing credentialKey and options for Salesforce (token
URL "https://login.salesforce.com/services/oauth2/token", client_id:
consumer_key, client_secret: consumer_secret, and refresh_token from
credentialKey); then use the wrapper's returned tokens to update
credentialKey/access token as done elsewhere. Ensure you remove the manual
URLSearchParams fetch block and use refreshOAuthTokens consistently with how
Google Calendar code uses it.
In @packages/app-store/zoho-bigin/lib/CalendarService.ts:
- Around line 85-94: The call to refreshOAuthTokens is passing the credentialId
instead of the user ID; update the third argument in the refreshOAuthTokens call
(the one currently passing credentialId) to pass credential.userId so the
signature refreshOAuthTokens(..., "zoho-bigin", userId) receives the correct
user identifier and enables credential syncing when
APP_CREDENTIAL_SHARING_ENABLED is true; leave the axios.post wrapper and other
args unchanged.
In @packages/app-store/zohocrm/lib/CalendarService.ts:
- Around line 204-216: refreshOAuthTokens currently returns two incompatible
types: the axios call returns an object with a .data property while the fetch
branch returns a raw Fetch Response, breaking callers that expect .data (e.g.,
zohoCrmTokenInfo.data.*). Fix refreshOAuthTokens so both branches return the
same normalized shape: parse the Fetch Response with await response.json(), and
return an object matching axios-like structure (at minimum { data: parsedBody,
status: response.status, headers: response.headers } or equivalent), and
preserve error propagation; update the fetch branch inside refreshOAuthTokens to
await and parse the body and attach it to a .data property before returning.
- Line 19: The call site in CalendarService that uses refreshOAuthTokens must
handle both Fetch Response and axios response shapes: update the token refresh
logic (the code reading zohoCrmTokenInfo.data.error / .data.access_token around
the token refresh block) to detect if zohoCrmTokenInfo is a Fetch Response (has
.json and status) and, if so, await zohoCrmTokenInfo.json() and treat that
result as the `.data` payload; alternatively, change refreshOAuthTokens to
always return an object with a `.data` property (wrapping fetch.json() result
into { data: ... }) so existing code (accessing zohoCrmTokenInfo.data.*) keeps
working—implement one of these normalization approaches and ensure subsequent
checks (error, access_token, refresh_token) use the normalized `.data` shape.
🧹 Nitpick comments (3)
packages/lib/constants.ts (1)
103-104: Consider explicit boolean conversion for type clarity.The expression evaluates to either a string value or a falsy value. While this works correctly in boolean contexts, an explicit boolean cast would improve type safety and clarity.
♻️ Suggested improvement
export const APP_CREDENTIAL_SHARING_ENABLED = - process.env.CALCOM_WEBHOOK_SECRET && process.env.CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY; + !!(process.env.CALCOM_WEBHOOK_SECRET && process.env.CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY);apps/web/pages/api/webhook/app-credential.ts (1)
24-29: Consider timing-safe comparison for webhook secret validation.String comparison with
!==may be vulnerable to timing attacks. Using a constant-time comparison function is a security best practice for secret validation.🔒 Proposed improvement
+import { timingSafeEqual } from "crypto"; + +function safeCompare(a: string, b: string): boolean { + if (a.length !== b.length) return false; + return timingSafeEqual(Buffer.from(a), Buffer.from(b)); +} + // Check that the webhook secret matches -if ( - req.headers[process.env.CALCOM_WEBHOOK_HEADER_NAME || "calcom-webhook-secret"] !== - process.env.CALCOM_WEBHOOK_SECRET -) { +const headerValue = req.headers[process.env.CALCOM_WEBHOOK_HEADER_NAME || "calcom-webhook-secret"]; +const expectedSecret = process.env.CALCOM_WEBHOOK_SECRET || ""; +if (typeof headerValue !== "string" || !safeCompare(headerValue, expectedSecret)) { return res.status(403).json({ message: "Invalid webhook secret" }); }packages/app-store/_utils/oauth/refreshOAuthTokens.ts (1)
16-19: Remove unnecessary else branch.The else keyword is redundant since the if block returns.
♻️ Proposed fix
return response; - } else { - const response = await refreshFunction(); - return response; } + const response = await refreshFunction(); + return response; };
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (40)
.env.exampleapps/web/pages/api/webhook/app-credential.tspackages/app-store/_utils/oauth/createOAuthAppCredential.tspackages/app-store/_utils/oauth/decodeOAuthState.tspackages/app-store/_utils/oauth/encodeOAuthState.tspackages/app-store/_utils/oauth/parseRefreshTokenResponse.tspackages/app-store/_utils/oauth/refreshOAuthTokens.tspackages/app-store/googlecalendar/api/add.tspackages/app-store/googlecalendar/api/callback.tspackages/app-store/googlecalendar/lib/CalendarService.tspackages/app-store/hubspot/api/add.tspackages/app-store/hubspot/api/callback.tspackages/app-store/hubspot/lib/CalendarService.tspackages/app-store/larkcalendar/api/add.tspackages/app-store/larkcalendar/api/callback.tspackages/app-store/larkcalendar/lib/CalendarService.tspackages/app-store/office365calendar/api/add.tspackages/app-store/office365calendar/api/callback.tspackages/app-store/office365calendar/lib/CalendarService.tspackages/app-store/office365video/api/add.tspackages/app-store/office365video/api/callback.tspackages/app-store/office365video/lib/VideoApiAdapter.tspackages/app-store/salesforce/api/add.tspackages/app-store/salesforce/api/callback.tspackages/app-store/salesforce/lib/CalendarService.tspackages/app-store/stripepayment/api/callback.tspackages/app-store/tandemvideo/api/callback.tspackages/app-store/webex/api/callback.tspackages/app-store/webex/lib/VideoApiAdapter.tspackages/app-store/zoho-bigin/api/add.tspackages/app-store/zoho-bigin/api/callback.tspackages/app-store/zoho-bigin/lib/CalendarService.tspackages/app-store/zohocrm/api/_getAdd.tspackages/app-store/zohocrm/api/callback.tspackages/app-store/zohocrm/lib/CalendarService.tspackages/app-store/zoomvideo/api/add.tspackages/app-store/zoomvideo/api/callback.tspackages/app-store/zoomvideo/lib/VideoApiAdapter.tspackages/lib/constants.tsturbo.json
🧰 Additional context used
🧬 Code graph analysis (4)
packages/app-store/office365calendar/lib/CalendarService.ts (1)
packages/lib/errors.ts (1)
handleErrorsJson(14-30)
packages/app-store/zoho-bigin/api/add.ts (1)
packages/lib/constants.ts (1)
WEBAPP_URL(9-15)
apps/web/pages/api/webhook/app-credential.ts (2)
packages/lib/constants.ts (1)
APP_CREDENTIAL_SHARING_ENABLED(103-104)packages/prisma/index.ts (1)
prisma(56-56)
packages/app-store/hubspot/lib/CalendarService.ts (2)
packages/app-store/hubspot/api/callback.ts (1)
HubspotToken(17-19)packages/lib/constants.ts (1)
WEBAPP_URL(9-15)
🪛 dotenv-linter (4.0.0)
.env.example
[warning] 237-237: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 239-239: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 239-239: [UnorderedKey] The CALCOM_WEBHOOK_HEADER_NAME key should go before the CALCOM_WEBHOOK_SECRET key
(UnorderedKey)
[warning] 240-240: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 240-240: [UnorderedKey] The CALCOM_CREDENTIAL_SYNC_ENDPOINT key should go before the CALCOM_WEBHOOK_HEADER_NAME key
(UnorderedKey)
[warning] 244-244: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 244-244: [UnorderedKey] The CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY key should go before the CALCOM_CREDENTIAL_SYNC_ENDPOINT key
(UnorderedKey)
🔇 Additional comments (39)
packages/app-store/larkcalendar/api/callback.ts (1)
10-10: LGTM! Import path correctly updated.The import path for
decodeOAuthStatehas been correctly updated to reflect the new OAuth utilities location under_utils/oauth/. This aligns with the broader refactoring to centralize OAuth helpers.packages/app-store/_utils/oauth/createOAuthAppCredential.ts (1)
7-7: LGTM! Import path correctly updated.The import path for
throwIfNotHaveAdminAccessToTeamcorrectly references the parent_utilsdirectory.packages/app-store/tandemvideo/api/callback.ts (1)
7-7: LGTM! Import path correctly updated.The import path for
createOAuthAppCredentialhas been correctly updated to reference the new location under_utils/oauth/. This is consistent with the OAuth utilities refactoring across the codebase.packages/app-store/webex/api/callback.ts (1)
7-7: LGTM! Import path correctly updated.The import path for
createOAuthAppCredentialhas been correctly updated to reference the new location under_utils/oauth/. This change is consistent with the OAuth utilities reorganization.turbo.json (1)
200-201: Verify documentation and secure usage of new environment variables.Four new environment variables have been added for credential sync and webhook functionality. Ensure that:
- These variables are documented in
.env.examplewith appropriate descriptions- The encryption key and webhook secret are handled securely (not logged, properly validated)
- The credential sync endpoint and webhook header name are validated before use
#!/bin/bash # Description: Verify the new environment variables are documented and check their usage # Check if variables are documented in .env.example echo "=== Checking .env.example documentation ===" for var in "CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY" "CALCOM_CREDENTIAL_SYNC_ENDPOINT" "CALCOM_WEBHOOK_HEADER_NAME" "CALCOM_WEBHOOK_SECRET"; do if rg -q "$var" .env.example; then echo "✓ $var is documented" else echo "✗ $var is NOT documented" fi done # Check usage of these variables to ensure secure handling echo -e "\n=== Checking usage of CALCOM_WEBHOOK_SECRET ===" rg -n "CALCOM_WEBHOOK_SECRET" --type ts -C 2 echo -e "\n=== Checking usage of CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY ===" rg -n "CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY" --type ts -C 2Also applies to: 205-207
packages/app-store/stripepayment/api/callback.ts (1)
6-6: LGTM!The import path update aligns with the broader refactor consolidating OAuth utilities under
_utils/oauth/..env.example (1)
233-245: LGTM!The new environment variables are well-documented with clear comments explaining their purpose and how to generate secure values. The logical grouping by feature (credential sync) is appropriate despite static analysis warnings about alphabetical ordering.
packages/app-store/office365calendar/api/add.ts (1)
7-7: LGTM!The import path update aligns with the OAuth utilities consolidation.
packages/app-store/zoomvideo/api/callback.ts (1)
7-7: LGTM!The import path update aligns with the OAuth utilities consolidation under
_utils/oauth/.packages/app-store/googlecalendar/api/callback.ts (1)
10-10: LGTM: Import path correctly updated.The import path change aligns with the refactoring to organize OAuth utilities under the
oauth/subdirectory.packages/app-store/office365video/api/add.ts (1)
7-7: LGTM: Import path correctly updated.The import path change aligns with the refactoring to organize OAuth utilities under the
oauth/subdirectory.packages/app-store/googlecalendar/api/add.ts (1)
7-7: LGTM: Import path correctly updated.The import path change aligns with the refactoring to organize OAuth utilities under the
oauth/subdirectory.packages/app-store/office365calendar/api/callback.ts (1)
9-9: LGTM: Import path correctly updated.The import path change aligns with the refactoring to organize OAuth utilities under the
oauth/subdirectory.packages/app-store/_utils/oauth/encodeOAuthState.ts (2)
3-3: LGTM: Import path correctly adjusted for new location.The types import path correctly accounts for the file's new location in the
oauth/subdirectory.
1-12: All import paths have been successfully updated.All references to
encodeOAuthStateanddecodeOAuthStateare now correctly importing from the new_utils/oauth/paths. No old import paths remain in the codebase.packages/app-store/salesforce/api/add.ts (1)
7-7: LGTM! OAuth utility import path updated correctly.The import path refactoring to the nested
oauthdirectory is clean and consistent with the broader OAuth utilities reorganization mentioned in the PR.packages/app-store/zohocrm/api/_getAdd.ts (1)
7-7: LGTM! Import path refactoring applied correctly.The OAuth utility relocation is consistent with the repository-wide reorganization.
packages/app-store/hubspot/api/add.ts (1)
7-7: LGTM! OAuth import path updated correctly.The refactoring aligns with the OAuth utilities reorganization across the codebase.
packages/app-store/zoho-bigin/api/add.ts (2)
7-7: LGTM! OAuth import path updated correctly.The import refactoring is consistent with the OAuth utilities reorganization.
17-17: Hardcoded slug is correct and matches config.The hardcoded
"zoho-bigin"slug in the redirect URI matches theslugvalue in config.json, so OAuth callbacks will work correctly. No changes needed.packages/app-store/zoomvideo/api/add.ts (1)
8-8: LGTM! OAuth utility import relocated correctly.The import path update is consistent with the OAuth utilities reorganization across the app store integrations.
packages/app-store/hubspot/api/callback.ts (1)
10-11: LGTM!The import path updates are consistent with the OAuth utilities reorganization. The functions are used correctly at lines 50 and 52 without any behavioral changes.
packages/app-store/larkcalendar/api/add.ts (1)
9-9: LGTM!The
encodeOAuthStateimport path has been updated to use the new OAuth utilities directory structure. Usage at line 21 remains unchanged.packages/app-store/office365video/api/callback.ts (1)
9-10: LGTM!The OAuth utility imports have been updated to use the new directory structure. The functions are used correctly at lines 96 and 98, and no behavioral changes are introduced.
packages/app-store/zoho-bigin/api/callback.ts (1)
10-11: Import paths correctly resolve to the dedicated oauth utilities directory.Both OAuth utility files exist at the new import paths and are properly used throughout the file. The refactored imports maintain correct functionality.
packages/app-store/zohocrm/api/callback.ts (1)
10-11: LGTM - Consistent OAuth utilities import path updates.The import paths are updated consistently with the OAuth utilities reorganization across all integrations.
packages/app-store/_utils/oauth/decodeOAuthState.ts (1)
3-3: Type import path correctly adjusted for new directory structure.The import path
"../../types"properly reflects the file's new location in theoauthsubdirectory and correctly resolves topackages/app-store/types.d.tswhereIntegrationOAuthCallbackStateis exported.packages/app-store/salesforce/api/callback.ts (1)
9-10: Import paths are correct and files exist at new locations.The OAuth utilities have been successfully reorganized into the
oauthsubdirectory. Both import statements resolve to the correct files:
createOAuthAppCredential.tsdecodeOAuthState.tsThe refactoring is consistent with the OAuth utilities reorganization.
packages/app-store/webex/lib/VideoApiAdapter.ts (1)
11-11: Centralized token refresh pattern is correctly applied, but the utility is provider-agnostic.The
refreshOAuthTokensutility is a routing wrapper that decides between a centralized endpoint (if enabled) or the provided callback function. It does not handle provider-specific token endpoints or response formats—those remain in the adapter's inline fetch call (lines 62-78) and the subsequenthandleWebexResponse()handler (line 80). The pattern matches the Office 365 adapter consistently.packages/app-store/office365video/lib/VideoApiAdapter.ts (1)
12-12: Token refresh utility integration is correct.The refactor successfully uses
refreshOAuthTokensto centralize token refresh logic. Both concerns are verified:
refreshOAuthTokensreturns aResponseobject in all code paths (both the sync endpoint and direct refresh function branches), which is compatible withhandleErrorsJsonat line 77.- Credential synchronization is properly isolated within the utility and conditional on environment variables, so it doesn't disrupt the existing error handling or token update flow.
packages/app-store/office365calendar/lib/CalendarService.ts (2)
246-261: LGTM: Token refresh wrapped with centralized utility.The refactoring delegates token refresh orchestration to
refreshOAuthTokenswhile preserving the original OAuth request parameters. The pattern is consistent with other integrations in this PR.
263-263: LGTM: Centralized token response parsing.The use of
parseRefreshTokenResponsewith the existingrefreshTokenResponseSchemamaintains the validation logic while centralizing the parsing behavior across integrations.packages/app-store/larkcalendar/lib/CalendarService.ts (2)
14-14: LGTM: OAuth utility import added.The
refreshOAuthTokensimport centralizes token refresh logic for the Lark calendar integration.
67-82: LGTM: Token refresh wrapped while preserving Lark-specific flow.The refactoring wraps the token refresh request with
refreshOAuthTokenswhile maintaining Lark's unique dual-token authentication flow (app access token + user tokens).packages/app-store/zoho-bigin/lib/CalendarService.ts (1)
18-18: LGTM: OAuth utility import added.The
refreshOAuthTokensimport aligns with the centralized token refresh pattern used across other integrations.packages/app-store/zoomvideo/lib/VideoApiAdapter.ts (2)
12-13: LGTM: OAuth utility imports added.The imports for
parseRefreshTokenResponseandrefreshOAuthTokenscentralize token refresh logic for Zoom video integration.
79-104: LGTM: Token refresh centralized with proper error handling.The refactoring wraps the token refresh request with
refreshOAuthTokensand usesparseRefreshTokenResponsefor validation, while preserving Zoom-specific error handling forinvalid_granterrors.packages/app-store/salesforce/lib/CalendarService.ts (1)
39-47: LGTM: Salesforce token schema is well-defined.The schema appropriately validates all expected fields from Salesforce's OAuth token response.
packages/app-store/googlecalendar/lib/CalendarService.ts (1)
94-101: LGTM: Token refresh and persistence logic is correct.The implementation properly:
- Uses the centralized
refreshOAuthTokenswrapper- Updates the in-memory credentials with new tokens
- Validates and persists credentials using
parseRefreshTokenResponse- Updates the database with the refreshed credentials
The mutation of
googleCredentialsbefore passing it toparseRefreshTokenResponseis intentional and correct, as it ensures the validation and persistence includes the refreshed tokens.
| 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" }); | ||
| } | ||
|
|
||
| const reqBody = appCredentialWebhookRequestBodySchema.parse(req.body); |
There was a problem hiding this comment.
Add HTTP method restriction and wrap schema parsing in try-catch.
The endpoint should restrict to POST requests and handle parsing errors gracefully to return proper 400 responses instead of 500s.
🔒 Proposed fix
export default async function handler(req: NextApiRequest, res: NextApiResponse) {
+ // Only allow POST requests
+ if (req.method !== "POST") {
+ return res.status(405).json({ message: "Method not allowed" });
+ }
+
// Check that credential sharing is enabled
if (!APP_CREDENTIAL_SHARING_ENABLED) {
return res.status(403).json({ message: "Credential sharing is not enabled" });
}
// Check that the webhook secret matches
if (
req.headers[process.env.CALCOM_WEBHOOK_HEADER_NAME || "calcom-webhook-secret"] !==
process.env.CALCOM_WEBHOOK_SECRET
) {
return res.status(403).json({ message: "Invalid webhook secret" });
}
- const reqBody = appCredentialWebhookRequestBodySchema.parse(req.body);
+ const parseResult = appCredentialWebhookRequestBodySchema.safeParse(req.body);
+ if (!parseResult.success) {
+ return res.status(400).json({ message: "Invalid request body", errors: parseResult.error.flatten() });
+ }
+ const reqBody = parseResult.data;🤖 Prompt for AI Agents
In @apps/web/pages/api/webhook/app-credential.ts around lines 17 - 31, The
handler currently allows any method and calls
appCredentialWebhookRequestBodySchema.parse directly which can throw and produce
a 500; update the exported handler to first reject non-POST methods (respond 405
with Allow: POST) and then wrap the call to
appCredentialWebhookRequestBodySchema.parse in a try/catch that returns a 400
with validation error details on parse failure; keep existing checks for
APP_CREDENTIAL_SHARING_ENABLED and the
CALCOM_WEBHOOK_HEADER_NAME/CALCOM_WEBHOOK_SECRET comparison, and reference the
same symbols (handler, APP_CREDENTIAL_SHARING_ENABLED,
CALCOM_WEBHOOK_HEADER_NAME, CALCOM_WEBHOOK_SECRET,
appCredentialWebhookRequestBodySchema.parse) when making the changes.
| // Decrypt the keys | ||
| const keys = JSON.parse( | ||
| symmetricDecrypt(reqBody.keys, process.env.CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY || "") | ||
| ); |
There was a problem hiding this comment.
Add error handling for decryption and JSON parsing.
Both symmetricDecrypt and JSON.parse can throw exceptions. Unhandled failures would result in 500 errors and could potentially expose sensitive error details.
🔒 Proposed fix
// Decrypt the keys
- const keys = JSON.parse(
- symmetricDecrypt(reqBody.keys, process.env.CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY || "")
- );
+ let keys;
+ try {
+ const decrypted = symmetricDecrypt(reqBody.keys, process.env.CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY || "");
+ keys = JSON.parse(decrypted);
+ } catch (error) {
+ return res.status(400).json({ message: "Failed to decrypt or parse credentials" });
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Decrypt the keys | |
| const keys = JSON.parse( | |
| symmetricDecrypt(reqBody.keys, process.env.CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY || "") | |
| ); | |
| // Decrypt the keys | |
| let keys; | |
| try { | |
| const decrypted = symmetricDecrypt(reqBody.keys, process.env.CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY || ""); | |
| keys = JSON.parse(decrypted); | |
| } catch (error) { | |
| return res.status(400).json({ message: "Failed to decrypt or parse credentials" }); | |
| } |
🤖 Prompt for AI Agents
In @apps/web/pages/api/webhook/app-credential.ts around lines 56 - 59, Wrap the
decryption+parsing of reqBody.keys (the call to symmetricDecrypt and JSON.parse
that assigns keys) in a try-catch; on failure catch the error, log only a
sanitized message (no secret content) including the operation context (e.g.,
"app credential decrypt/parse failed"), and return an appropriate client error
response (400 or 422) instead of letting the exception bubble to a 500. Ensure
you still reference process.env.CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY when
calling symmetricDecrypt, and avoid returning or logging the decrypted payload
or raw error stack.
|
|
||
| import { decodeOAuthState } from "./decodeOAuthState"; | ||
| import { throwIfNotHaveAdminAccessToTeam } from "./throwIfNotHaveAdminAccessToTeam"; | ||
| import { decodeOAuthState } from "../oauth/decodeOAuthState"; |
There was a problem hiding this comment.
Fix incorrect import path for decodeOAuthState.
The import path ../oauth/decodeOAuthState is incorrect. Since both createOAuthAppCredential.ts and decodeOAuthState.ts are in the same _utils/oauth/ directory, the path should be ./decodeOAuthState.
The current path goes up to _utils/ and then back into oauth/, which is unnecessarily circuitous and may cause import resolution issues.
🔧 Proposed fix
-import { decodeOAuthState } from "../oauth/decodeOAuthState";
+import { decodeOAuthState } from "./decodeOAuthState";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { decodeOAuthState } from "../oauth/decodeOAuthState"; | |
| import { decodeOAuthState } from "./decodeOAuthState"; |
🤖 Prompt for AI Agents
In @packages/app-store/_utils/oauth/createOAuthAppCredential.ts at line 6, The
import for decodeOAuthState in createOAuthAppCredential.ts uses the wrong
relative path; update the import statement that references
"../oauth/decodeOAuthState" to use "./decodeOAuthState" so it imports the
decodeOAuthState function from the same _utils/oauth directory (look for the
import line in createOAuthAppCredential.ts and change the module specifier
accordingly).
| 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.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Step 1: Locate and examine the parseRefreshTokenResponse.ts file
echo "=== Searching for parseRefreshTokenResponse.ts ==="
fd -t f "parseRefreshTokenResponse" packages/app-store --type ts
# Step 2: Check file exists and read its content
echo -e "\n=== File content ==="
cat -n packages/app-store/_utils/oauth/parseRefreshTokenResponse.ts 2>/dev/null || echo "File not found at expected path"
# Step 3: Search for tests
echo -e "\n=== Searching for tests ==="
fd -t f "parseRefreshTokenResponse" packages/app-store --type ts --type tsx
# Step 4: Check for references to minimumTokenResponseSchema
echo -e "\n=== Searching for minimumTokenResponseSchema references ==="
rg "minimumTokenResponseSchema" packages/app-store --type ts -A 2 -B 2
# Step 5: Check overall file structure
echo -e "\n=== Full directory listing in _utils/oauth ==="
ls -la packages/app-store/_utils/oauth/ 2>/dev/null || echo "Directory not found"Repository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 3461
🏁 Script executed:
#!/bin/bash
# Check APP_CREDENTIAL_SHARING_ENABLED constant definition and default value
echo "=== APP_CREDENTIAL_SHARING_ENABLED definition ==="
rg "APP_CREDENTIAL_SHARING_ENABLED" packages/lib --type ts -A 2 -B 2
# Check if there are any tests in the app-store directory that use parseRefreshTokenResponse
echo -e "\n=== Test files in app-store ==="
find packages/app-store -type f \( -name "*.test.ts" -o -name "*.test.tsx" -o -name "*.spec.ts" -o -name "*.spec.tsx" \)
# Check what Zod version is used
echo -e "\n=== Check Zod version ==="
rg '"zod"' --type json -A 1 | head -20Repository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 1349
🏁 Script executed:
#!/bin/bash
# Let's check the Zod documentation pattern by examining how z.record() or other methods are used
echo "=== Searching for Zod usage patterns in codebase ==="
rg "z\.object\(" packages --type ts -A 5 | head -80
echo -e "\n=== Check for z.record or z.catchall usage ==="
rg "z\.record|z\.catchall|passthrough|discriminatedUnion" packages --type ts -B 2 -A 2 | head -100Repository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 12952
🏁 Script executed:
#!/bin/bash
# Create a quick test to see what z.string().toString() produces
cat > /tmp/test_zod.js << 'EOF'
// Test what z.string().toString() returns
const zod = require('zod');
const z = zod.z;
console.log("z.string().toString():", z.string().toString());
console.log("Type:", typeof z.string().toString());
// Try to create a schema with the computed property
try {
const schema = z.object({
access_token: z.string(),
[z.string().toString()]: z.number(),
});
console.log("\nSchema created successfully");
console.log("Schema keys:", Object.keys(schema.shape));
// Test parsing
const testData = {
access_token: "test",
"[object Object]": 123,
};
const result = schema.safeParse(testData);
console.log("Parse result:", result);
} catch (e) {
console.error("\nError creating schema:", e.message);
}
EOF
# Try to run it if Node and zod are available
cd /tmp && npm install zod@3.22.2 > /dev/null 2>&1 && node test_zod.js 2>&1 || echo "Cannot test directly - Node/npm not available in sandbox"Repository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 310
Critical: Invalid Zod schema logic prevents proper OAuth response validation.
Lines 8 and 10 use z.string().toString() which returns the literal string "[object Object]" in JavaScript. This creates a schema that expects properties literally named [object Object] instead of matching dynamic property names. Real OAuth token responses (e.g., {access_token: "xyz", expires_in: 3600}) will fail validation against this schema.
The intended behavior—accepting string access_token plus any additional properties—requires using .passthrough() or .catchall() instead. Replace the schema definition with:
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(),
-});
+}).passthrough();This allows the schema to accept access_token (required) plus any additional properties, matching the pattern used elsewhere in the codebase.
🤖 Prompt for AI Agents
In @packages/app-store/_utils/oauth/parseRefreshTokenResponse.ts around lines 5
- 11, The minimumTokenResponseSchema is invalid because using
z.string().toString() creates a literal property name; replace the object schema
with one that requires access_token and allows arbitrary additional
properties—e.g., define minimumTokenResponseSchema = z.object({ access_token:
z.string() }).passthrough() or use .catchall(z.unknown()) so dynamic fields like
expires_in are accepted; update any validation usage of
minimumTokenResponseSchema to work with the new passthrough/catchall behavior.
| if (!refreshTokenResponse.data.refresh_token) { | ||
| refreshTokenResponse.data.refresh_token = "refresh_token"; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and read the file
find . -path "*packages/app-store/_utils/oauth/parseRefreshTokenResponse.ts" -type fRepository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 149
🏁 Script executed:
# Search for how refresh_token is used after being returned
rg "parseRefreshTokenResponse" packages/app-store --type ts -A 5 -B 2
# Look for tests related to this function
fd "parseRefreshTokenResponse" packages --type ts | grep -i test
# Search for credential sharing patterns
rg "APP_CREDENTIAL_SHARING_ENABLED|CALCOM_CREDENTIAL_SYNC" packages --type ts -C 3Repository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 10852
🏁 Script executed:
cat -n packages/app-store/_utils/oauth/parseRefreshTokenResponse.tsRepository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 1341
🏁 Script executed:
# Look at refreshOAuthTokens.ts to understand usage
cat -n packages/app-store/_utils/oauth/refreshOAuthTokens.ts | head -80
# Search for how the returned refresh_token is used in callers
rg "parseRefreshTokenResponse.*data" packages/app-store --type ts -A 3Repository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 1091
🏁 Script executed:
# Check how the parsed result is used in actual integrations
cat -n packages/app-store/googlecalendar/lib/CalendarService.ts | sed -n '100,130p'
# Look for usage of refresh_token from parseRefreshTokenResponse
rg "parseRefreshTokenResponse\(" packages/app-store -A 10 -B 2 | head -100Repository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 6615
🏁 Script executed:
# Check how refresh_token is handled in each integration more carefully
cat -n packages/app-store/salesforce/lib/CalendarService.ts | sed -n '65,95p'
# Check Office365 more carefully
cat -n packages/app-store/office365calendar/lib/CalendarService.ts | sed -n '180,220p'
# Check Zoom integration
cat -n packages/app-store/zoomvideo/lib/VideoApiAdapter.ts | sed -n '140,180p'
# Look for tests
find packages/app-store -name "*.test.ts" -o -name "*.spec.ts" | head -20Repository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 4917
🏁 Script executed:
# Check what happens with the newTokens in Zoom
cat -n packages/app-store/zoomvideo/lib/VideoApiAdapter.ts | sed -n '252,300p'
# Check the Salesforce line more carefully - how is the parsed data used
cat -n packages/app-store/salesforce/lib/CalendarService.ts | sed -n '90,105p'
# Check if refresh_token is ever actually used from the returned parse result
rg "\.refresh_token" packages/app-store --type ts -B 2 -A 2 | head -80Repository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 9078
🏁 Script executed:
# Check the complete Zoom flow where refresh_token is used
cat -n packages/app-store/zoomvideo/lib/VideoApiAdapter.ts | sed -n '220,250p'
# See the full context of how newTokens is obtained and used
cat -n packages/app-store/zoomvideo/lib/VideoApiAdapter.ts | sed -n '200,240p'
# Check what zoomTokenSchema requires
rg "zoomTokenSchema\|zoomRefreshedTokenSchema" packages/app-store/zoomvideo --type ts -B 2 -A 5Repository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 2986
🏁 Script executed:
# Find where newTokens is used after parseRefreshTokenResponse
cat -n packages/app-store/zoomvideo/lib/VideoApiAdapter.ts | sed -n '305,330p'
# Check zoomTokenSchema definition
rg "const zoomTokenSchema\|const zoomRefreshedTokenSchema" packages/app-store/zoomvideo --type ts -A 8Repository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 1103
🏁 Script executed:
# Find the zoomAuth function and where tokens are updated
cat -n packages/app-store/zoomvideo/lib/VideoApiAdapter.ts | sed -n '325,365p'
# Look for zoomTokenSchema and zoomRefreshedTokenSchema more directly
cat packages/app-store/zoomvideo/lib/VideoApiAdapter.ts | grep -n "Schema\|schema" | head -30Repository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 2270
🏁 Script executed:
# Get the actual schema definitions
cat -n packages/app-store/zoomvideo/lib/VideoApiAdapter.ts | sed -n '52,75p'Repository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 1112
🏁 Script executed:
# Look at how refreshAccessToken uses the refresh_token parameter
cat -n packages/app-store/zoomvideo/lib/VideoApiAdapter.ts | sed -n '74,110p'
# Check if there are any comments or documentation about credential sharing
rg "credential.?sharing\|APP_CREDENTIAL_SHARING" packages/app-store --type ts -i -B 2 -A 2 | head -60Repository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 1849
🏁 Script executed:
# See the complete refreshAccessToken function and what happens after parsing
cat -n packages/app-store/zoomvideo/lib/VideoApiAdapter.ts | sed -n '74,140p'
# Check if minimumTokenResponseSchema requires refresh_token or makes it optional
cat -n packages/app-store/_utils/oauth/parseRefreshTokenResponse.tsRepository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 4493
🏁 Script executed:
# Verify there are no tests that might show intended behavior
find packages/app-store -path "*.test.ts" -o -path "*.spec.ts" | xargs grep -l "parseRefreshTokenResponse\|refresh_token" 2>/dev/null | head -10Repository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 148
🏁 Script executed:
# Check the Google Calendar test file for any tests related to parseRefreshTokenResponse
cat -n packages/app-store/googlecalendar/lib/CalendarService.test.ts | grep -A 10 -B 10 "parseRefreshTokenResponse\|refresh_token" | head -80Repository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 728
🏁 Script executed:
# Search for any comments or documentation explaining the dummy refresh_token assignment
rg "refresh_token.*=.*refresh_token\|dummy\|placeholder\|TODO.*refresh" packages/app-store --type ts -B 5 -A 2
# Check git history or any related issues/docs about credential sharing
rg "credential.*sharing\|CALCOM_CREDENTIAL_SYNC" packages --type ts -i -B 3 -A 3 | head -100Repository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 69
Critical: Hardcoded dummy refresh token will break OAuth flows.
Lines 25-27 set refresh_token to the literal string "refresh_token" when missing from the response. This value gets stored in the database and reused in subsequent token refresh attempts, causing OAuth providers to reject it as an invalid token and breaking the refresh flow.
When APP_CREDENTIAL_SHARING_ENABLED is true, the minimumTokenResponseSchema does not require refresh_token from the response, but the code still assigns this dummy value unconditionally. This is problematic for integrations like Zoom that store and reuse this token value.
Consider one of these approaches:
- Make
refresh_tokenoptional in the schema instead of defaulting to a dummy value - Throw an error if
refresh_tokenis required but missing (non-shared credential mode) - Conditionally handle it based on whether credential sharing is enabled, ensuring the dummy value isn't used downstream
🤖 Prompt for AI Agents
In @packages/app-store/_utils/oauth/parseRefreshTokenResponse.ts around lines 25
- 27, The code unconditionally sets refreshTokenResponse.data.refresh_token =
"refresh_token" which writes a hardcoded dummy into the DB; instead, update
parseRefreshTokenResponse to stop injecting that literal and handle missing
refresh_token based on APP_CREDENTIAL_SHARING_ENABLED and the
minimumTokenResponseSchema: if credential sharing is disabled and refresh_token
is required, throw a clear error; if credential sharing is enabled, treat
refresh_token as optional (do not set a dummy) and ensure downstream code can
handle its absence; update any validation logic around
minimumTokenResponseSchema and the function refreshTokenResponse handling to
reflect the conditional behavior (refer to refreshTokenResponse,
minimumTokenResponseSchema, and APP_CREDENTIAL_SHARING_ENABLED).
| }), | ||
| }); | ||
|
|
||
| if (response.statusText !== "OK") throw new HttpError({ statusCode: 400, message: response.statusText }); |
There was a problem hiding this comment.
Use response.ok instead of checking statusText.
Checking response.statusText !== "OK" is fragile because:
- Status text can vary by server implementation
- It doesn't catch all error status codes (e.g., 201 Created has statusText "Created" but is successful)
- The standard
response.okproperty reliably checks for 2xx status codes
🔧 Proposed fix
- 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.statusText}`
+ });
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 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.statusText}` | |
| }); | |
| } |
🤖 Prompt for AI Agents
In @packages/app-store/salesforce/lib/CalendarService.ts at line 86, Replace the
fragile statusText check in CalendarService (the if that throws HttpError based
on response.statusText) with a check on response.ok to detect non-2xx responses;
when response.ok is false, throw the HttpError including the response.status and
response.statusText (or response body) to preserve diagnostic information.
Ensure you modify the check that currently reads "if (response.statusText !==
'OK')" to use "response.ok" and include useful error details in the thrown
HttpError.
| 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 import for prisma.
The code uses prisma.credential.update but prisma is not imported in this file.
Add the missing import at the top of the file:
import prisma from "@calcom/prisma";🤖 Prompt for AI Agents
In @packages/app-store/salesforce/lib/CalendarService.ts around lines 96 - 99,
The file calls prisma.credential.update but never imports prisma; add the
missing import statement for the Prisma client (import prisma from
"@calcom/prisma") at the top of the file so references like
prisma.credential.update(...) in CalendarService (and any other uses in this
module) resolve correctly.
| const tokenInfo = await refreshOAuthTokens( | ||
| async () => | ||
| await axios.post(accountsUrl, qs.stringify(formData), { | ||
| headers: { | ||
| "Content-Type": "application/x-www-form-urlencoded;charset=utf-8", | ||
| }, | ||
| }), | ||
| "zoho-bigin", | ||
| credentialId | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the refreshOAuthTokens function implementation
find . -type f -name "*.ts" -o -name "*.js" | xargs grep -l "refreshOAuthTokens" | head -20Repository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 642
🏁 Script executed:
# Search for the export/definition of refreshOAuthTokens
rg -n "export.*refreshOAuthTokens|function refreshOAuthTokens" --type tsRepository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 178
🏁 Script executed:
# Get all calls to refreshOAuthTokens with context to see what parameters are passed
rg -n "refreshOAuthTokens\(" packages/app-store --type ts -B 2 -A 5Repository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 7170
🏁 Script executed:
# Get the full refreshOAuthTokens call in zoho-bigin with all parameters
sed -n '85,94p' packages/app-store/zoho-bigin/lib/CalendarService.tsRepository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 387
🏁 Script executed:
# Get the full refreshOAuthTokens call in googlecalendar with all parameters
sed -n '86,93p' packages/app-store/googlecalendar/lib/CalendarService.tsRepository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 371
🏁 Script executed:
# Get the full refreshOAuthTokens call in zohocrm with all parameters
sed -n '204,213p' packages/app-store/zohocrm/lib/CalendarService.tsRepository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 432
🏁 Script executed:
# Read the refreshOAuthTokens implementation to see its signature
cat packages/app-store/_utils/oauth/refreshOAuthTokens.tsRepository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 937
🏁 Script executed:
# Check the zoho-bigin CalendarService context to see what credentialId is
sed -n '40,100p' packages/app-store/zoho-bigin/lib/CalendarService.ts | cat -nRepository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 2625
🏁 Script executed:
# Get the full zohocrm refreshOAuthTokens call
sed -n '204,220p' packages/app-store/zohocrm/lib/CalendarService.tsRepository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 674
🏁 Script executed:
# Check what all integrations pass as third argument to refreshOAuthTokens
rg -n "refreshOAuthTokens\(" packages/app-store --type ts -A 8 | grep -E "(refreshOAuthTokens|^\s+[a-zA-Z0-9_\.]+\s*\))"Repository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 1062
Fix incorrect parameter: pass credential.userId instead of credentialId to refreshOAuthTokens.
The third parameter should be the user ID, not the credential ID. The refreshOAuthTokens function signature expects userId: number | null to enable credential syncing when APP_CREDENTIAL_SHARING_ENABLED is true. Other integrations (google-calendar, zohocrm) correctly pass credential.userId.
Change line 93 from credentialId to credential.userId.
🤖 Prompt for AI Agents
In @packages/app-store/zoho-bigin/lib/CalendarService.ts around lines 85 - 94,
The call to refreshOAuthTokens is passing the credentialId instead of the user
ID; update the third argument in the refreshOAuthTokens call (the one currently
passing credentialId) to pass credential.userId so the signature
refreshOAuthTokens(..., "zoho-bigin", userId) receives the correct user
identifier and enables credential syncing when APP_CREDENTIAL_SHARING_ENABLED is
true; leave the axios.post wrapper and other args unchanged.
| import type { CredentialPayload } from "@calcom/types/Credential"; | ||
|
|
||
| import getAppKeysFromSlug from "../../_utils/getAppKeysFromSlug"; | ||
| import refreshOAuthTokens from "../../_utils/oauth/refreshOAuthTokens"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for refreshOAuthTokens file
fd -t f "refreshOAuthTokens" packages/app-store/_utils/oauth/ || echo "Not found with fd"
# Also try alternate search
find packages/app-store -name "*refreshOAuthTokens*" -type f 2>/dev/null || echo "Not found with find"Repository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 194
🏁 Script executed:
# List the oauth utilities directory to see what's available
ls -la packages/app-store/_utils/oauth/ 2>/dev/null || echo "Directory not found"Repository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 562
🏁 Script executed:
# Search more broadly for oauth utils
find packages/app-store/_utils -type f -name "*.ts" -o -name "*.js" 2>/dev/null | head -20Repository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 881
🏁 Script executed:
# Read the refreshOAuthTokens implementation
cat -n packages/app-store/_utils/oauth/refreshOAuthTokens.tsRepository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 1091
🏁 Script executed:
# Check how the wrapper response is being used in CalendarService.ts
sed -n '200,250p' packages/app-store/zohocrm/lib/CalendarService.tsRepository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 1858
🏁 Script executed:
# Check if there's any response handling or normalization between the wrapper call and usage
sed -n '204,222p' packages/app-store/zohocrm/lib/CalendarService.tsRepository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 738
Critical bug: refreshOAuthTokens wrapper returns incompatible response formats.
The new refreshOAuthTokens wrapper returns different response types depending on the credential sharing configuration:
- When
APP_CREDENTIAL_SHARING_ENABLEDis true, it returns a Fetch APIResponseobject (which lacks a.dataproperty) - When disabled, it returns the axios response (which has a
.dataproperty)
The calling code at line 219 expects axios response format (zohoCrmTokenInfo.data.error, zohoCrmTokenInfo.data.access_token), so when credential sharing is enabled, accessing .data.error will fail with "Cannot read property 'error' of undefined" and cause runtime errors throughout the token refresh logic (lines 219–240).
The wrapper must either:
- Normalize the fetch Response to match axios response format (wrap
.json()in a.dataproperty), or - Handle both response types in the calling code
🤖 Prompt for AI Agents
In @packages/app-store/zohocrm/lib/CalendarService.ts at line 19, The call site
in CalendarService that uses refreshOAuthTokens must handle both Fetch Response
and axios response shapes: update the token refresh logic (the code reading
zohoCrmTokenInfo.data.error / .data.access_token around the token refresh block)
to detect if zohoCrmTokenInfo is a Fetch Response (has .json and status) and, if
so, await zohoCrmTokenInfo.json() and treat that result as the `.data` payload;
alternatively, change refreshOAuthTokens to always return an object with a
`.data` property (wrapping fetch.json() result into { data: ... }) so existing
code (accessing zohoCrmTokenInfo.data.*) keeps working—implement one of these
normalization approaches and ensure subsequent checks (error, access_token,
refresh_token) use the normalized `.data` shape.
| const zohoCrmTokenInfo = await refreshOAuthTokens( | ||
| async () => | ||
| await axios({ | ||
| method: "post", | ||
| url: url, | ||
| data: qs.stringify(formData), | ||
| headers: { | ||
| "Content-Type": "application/x-www-form-urlencoded;charset=utf-8", | ||
| }, | ||
| }), | ||
| "zohocrm", | ||
| credential.userId | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat packages/app-store/_utils/oauth/refreshOAuthTokens.tsRepository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 937
🏁 Script executed:
sed -n '217,240p' packages/app-store/zohocrm/lib/CalendarService.tsRepository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 910
Fix the response format inconsistency in refreshOAuthTokens.
The wrapper returns two incompatible response types:
- The
elsebranch returns an axios response (with.dataproperty) - The
ifbranch returns a FetchResponseobject (without.dataproperty)
The downstream code at lines 217-240 accesses zohoCrmTokenInfo.data.error, zohoCrmTokenInfo.data.access_token, etc. This will fail at runtime when the credential sync endpoint is enabled, since Fetch Response objects require .json() or .text() to extract the body, not a .data property.
Normalize the return format in refreshOAuthTokens to ensure both branches return a consistent object structure matching the axios response format (with .data containing the parsed response body).
🤖 Prompt for AI Agents
In @packages/app-store/zohocrm/lib/CalendarService.ts around lines 204 - 216,
refreshOAuthTokens currently returns two incompatible types: the axios call
returns an object with a .data property while the fetch branch returns a raw
Fetch Response, breaking callers that expect .data (e.g.,
zohoCrmTokenInfo.data.*). Fix refreshOAuthTokens so both branches return the
same normalized shape: parse the Fetch Response with await response.json(), and
return an object matching axios-like structure (at minimum { data: parsedBody,
status: response.status, headers: response.headers } or equivalent), and
preserve error propagation; update the fetch branch inside refreshOAuthTokens to
await and parse the body and attach it to a .data property before returning.
2 participants
Test 8nn
Summary by CodeRabbit
New Features
Chores
✏️ 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)*