OAuth credential sync and app integration enhancements#5
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>
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 6 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| 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 safeParse wrapper as credential key
High Severity
parseRefreshTokenResponse returns the full safeParse result object (with success and data properties), but the Google Calendar code assigns it directly to key and stores it in the database. Previously, googleCredentialSchema.parse() returned the parsed data directly. Now the credential is saved as { success: true, data: { access_token, ... } } instead of just { access_token, ... }. On the next load, googleCredentialSchema.parse(credential.key) will throw a ZodError, breaking Google Calendar for that user permanently after the first token refresh.
Additional Locations (1)
| }, | ||
| }), | ||
| "zoho-bigin", | ||
| credentialId |
There was a problem hiding this comment.
Zoho Bigin passes credential ID instead of user ID
Medium Severity
refreshOAuthTokens expects a userId as its third parameter, but credentialId (which is credential.id, the credential's database primary key) is passed instead. Every other integration correctly passes credential.userId. When credential sharing is enabled, this sends the wrong identifier (calcomUserId) to the sync endpoint, causing it to look up the wrong user or fail entirely.
| where: { id: credential.id }, | ||
| data: { key: { ...accessTokenParsed.data, refresh_token: credentialKey.refresh_token } }, | ||
| }); | ||
|
|
There was a problem hiding this comment.
Salesforce connection uses stale token after refresh
Medium Severity
The new code at lines 75–99 explicitly refreshes the Salesforce OAuth token and stores the updated access_token and instance_url in the database. However, the jsforce.Connection created immediately after still uses the original stale values from credentialKey (the pre-refresh credential). The refreshed accessTokenParsed.data.access_token and accessTokenParsed.data.instance_url are never used in the connection, making the refresh ineffective for the current request.
|
|
||
| if (!refreshTokenResponse.data.refresh_token) { | ||
| refreshTokenResponse.data.refresh_token = "refresh_token"; | ||
| } |
There was a problem hiding this comment.
Dummy refresh token overwrites real tokens on missing field
Medium Severity
When refresh_token is absent or falsy in the parsed response, parseRefreshTokenResponse sets it to the literal string "refresh_token". For callers that spread the result into stored credentials (e.g., Office 365 at o365AuthCredentials = { ...o365AuthCredentials, ...tokenResponse.data }), this dummy value overwrites the real refresh token. The next token refresh attempt would send "refresh_token" as the actual token, resulting in an invalid_grant error and broken authentication.
| appSlug, | ||
| }), | ||
| }); | ||
| return response; |
There was a problem hiding this comment.
Token refresh returns incompatible types when sharing enabled
Medium Severity
When credential sharing is enabled, refreshOAuthTokens returns a raw fetch Response, but several callers expect integration-specific types. Hubspot expects a HubspotToken and accesses .expiresIn (gets undefined, producing NaN). Zoho Bigin and Zoho CRM expect an Axios response and access .data.error (undefined.error throws TypeError). Google Calendar expects a GaxiosResponse and accesses res?.data for token fields (also crashes). These integrations all break at runtime when credential sharing is toggled on.
Additional Locations (2)
| [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.
Schema computed keys produce broken literal property names
Medium Severity
The minimumTokenResponseSchema uses [z.string().toString()] as computed property keys, which evaluate to the literal string "[object Object]" (from Object.prototype.toString()). Both computed keys resolve to the same literal, so the second overwrites the first. Since Zod's z.object() strips unknown keys by default, safeParse output retains only access_token, discarding critical fields like expires_in. When credential sharing is enabled and this schema is used, integrations like Office 365 and Zoom lose expiry data, storing NaN or stale values.
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)*Note
High Risk
Touches authentication/credential storage and token refresh logic across many OAuth integrations, and introduces a new webhook that can modify stored credentials, increasing blast radius if misconfigured or incorrect parsing/validation occurs.
Overview
Adds an app credential sharing/sync path for self-hosters: new env vars (
CALCOM_WEBHOOK_SECRET,CALCOM_WEBHOOK_HEADER_NAME,CALCOM_CREDENTIAL_SYNC_ENDPOINT,CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY) andAPP_CREDENTIAL_SHARING_ENABLED, plus a new webhook endpoint (/api/webhook/app-credential) that authenticates via header secret, AES-decrypts payload keys, and creates/updates the user’scredentialrecord.Refactors OAuth helpers into
packages/app-store/_utils/oauth/and updates integrations (Google, HubSpot, Lark, Office365 calendar/video, Salesforce, Stripe, Tandem, Webex, Zoho, Zoom) to use a sharedrefreshOAuthTokenswrapper andparseRefreshTokenResponseto support fetching refreshed access tokens from an external sync endpoint and relaxing refresh-token parsing when sync is enabled.Written by Cursor Bugbot for commit 824145b. Configure here.