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>
There was a problem hiding this comment.
Pull request overview
This PR introduces OAuth credential synchronization infrastructure to enable sharing and syncing of app credentials between Cal.com and external applications. The changes implement a webhook-based system for credential management and reorganize OAuth utilities into a dedicated directory structure.
Changes:
- Added new environment variables and configuration for app credential encryption, webhook authentication, and sync endpoints
- Refactored OAuth utilities into a dedicated
oauth/subdirectory with new helper functions for token refresh and response parsing - Integrated
refreshOAuthTokenswrapper across multiple app integrations (Zoom, Webex, Office 365, Google Calendar, Salesforce, HubSpot, Zoho, Lark) to support centralized credential sync
Reviewed changes
Copilot reviewed 40 out of 40 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| turbo.json | Added environment variables for credential encryption, webhook authentication, and sync endpoint configuration |
| packages/lib/constants.ts | Introduced APP_CREDENTIAL_SHARING_ENABLED flag based on required environment variables |
| packages/app-store/_utils/oauth/refreshOAuthTokens.ts | New utility to conditionally route OAuth token refresh through external sync endpoint or local refresh |
| packages/app-store/_utils/oauth/parseRefreshTokenResponse.ts | New utility to validate refresh token responses with flexible schema validation |
| packages/app-store/_utils/oauth/encodeOAuthState.ts | Moved from parent directory with updated import path |
| packages/app-store/_utils/oauth/decodeOAuthState.ts | Moved from parent directory with updated import path |
| packages/app-store/_utils/oauth/createOAuthAppCredential.ts | Moved from parent directory with updated import paths |
| apps/web/pages/api/webhook/app-credential.ts | New webhook endpoint for receiving and storing encrypted app credentials from external systems |
| .env.example | Added documentation and examples for new credential sync environment variables |
| Multiple app integrations | Updated to use refreshOAuthTokens wrapper and parseRefreshTokenResponse for consistent credential handling |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| 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.
The .toString() calls on z.string() and z.string().optional() are invalid. Zod schemas don't have a toString() method that produces meaningful keys. These lines appear to be attempting dynamic schema properties but are using incorrect syntax. For dynamic keys in Zod, use z.record() instead.
| const minimumTokenResponseSchema = z.object({ | |
| access_token: z.string(), | |
| // Assume that any property with a number is the expiry | |
| [z.string().toString()]: z.number(), | |
| // Allow other properties in the token response | |
| [z.string().optional().toString()]: z.unknown().optional(), | |
| }); | |
| const minimumTokenResponseSchema = z | |
| .object({ | |
| access_token: z.string(), | |
| }) | |
| // Allow any other properties in the token response | |
| .catchall(z.unknown().optional()); |
| } | ||
|
|
||
| if (!refreshTokenResponse.data.refresh_token) { | ||
| refreshTokenResponse.data.refresh_token = "refresh_token"; |
There was a problem hiding this comment.
Setting a hardcoded string 'refresh_token' as the refresh token value when it's missing is problematic. This will cause authentication to fail when the refresh token is actually used. If the refresh token is missing, this should either throw an error or use the original refresh token from the request.
| refreshTokenResponse.data.refresh_token = "refresh_token"; | |
| throw new Error("Missing refresh_token in token refresh response"); |
| const keys = JSON.parse( | ||
| symmetricDecrypt(reqBody.keys, process.env.CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY || "") | ||
| ); | ||
|
|
There was a problem hiding this comment.
Using an empty string as a fallback for the encryption key is dangerous. If CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY is undefined, decryption will fail silently or produce incorrect results. This should throw an error early if the encryption key is not configured.
| const keys = JSON.parse( | |
| symmetricDecrypt(reqBody.keys, process.env.CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY || "") | |
| ); | |
| const encryptionKey = process.env.CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY; | |
| if (!encryptionKey) { | |
| return res.status(500).json({ message: "Encryption key not configured" }); | |
| } | |
| const keys = JSON.parse(symmetricDecrypt(reqBody.keys, encryptionKey)); |
| const keys = JSON.parse( | ||
| symmetricDecrypt(reqBody.keys, process.env.CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY || "") | ||
| ); | ||
|
|
There was a problem hiding this comment.
If JSON.parse or symmetricDecrypt fails, the error messages won't be helpful for debugging. Wrap this in a try-catch block and provide a clear error message indicating whether decryption or JSON parsing failed.
| const keys = JSON.parse( | |
| symmetricDecrypt(reqBody.keys, process.env.CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY || "") | |
| ); | |
| let decryptedKeys: string; | |
| try { | |
| decryptedKeys = symmetricDecrypt( | |
| reqBody.keys, | |
| process.env.CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY || "" | |
| ); | |
| } catch (error) { | |
| console.error("Error decrypting app credential keys", error); | |
| return res.status(400).json({ message: "Failed to decrypt app credential keys" }); | |
| } | |
| let keys: any; | |
| try { | |
| keys = JSON.parse(decryptedKeys); | |
| } catch (error) { | |
| console.error("Error parsing decrypted app credential keys JSON", error); | |
| return res.status(400).json({ message: "Failed to parse decrypted app credential keys JSON" }); | |
| } |
| CALCOM_CREDENTIAL_SYNC_ENDPOINT="" | ||
| # Key should match on Cal.com and your application | ||
| # must be 32 bytes for AES256 encryption algorithm | ||
| # You can use: `openssl rand -base64 24` to generate one |
There was a problem hiding this comment.
The comment states the key must be 32 bytes for AES256, but the command generates 24 bytes of random data which results in a 32-character base64 string (not 32 bytes). For 32 bytes, use openssl rand -base64 32 which generates 44 base64 characters representing 32 bytes.
| # You can use: `openssl rand -base64 24` to generate one | |
| # You can use: `openssl rand -base64 32` to generate one |
| if (!clientId) return res.status(400).json({ message: "Zoho Bigin client_id missing." }); | ||
|
|
||
| const redirectUri = WEBAPP_URL + `/api/integrations/${appConfig.slug}/callback`; | ||
| const redirectUri = WEBAPP_URL + `/api/integrations/zoho-bigin/callback`; |
There was a problem hiding this comment.
The hardcoded string 'zoho-bigin' replaces appConfig.slug. While this makes the path explicit, it reduces maintainability if the app slug changes. Consider using appConfig.slug to keep the code consistent with the configuration.
| const redirectUri = WEBAPP_URL + `/api/integrations/zoho-bigin/callback`; | |
| const redirectUri = `${WEBAPP_URL}/api/integrations/${appConfig.slug}/callback`; |
| const responseJson = await handleErrorsJson(response); | ||
| const tokenResponse = refreshTokenResponseSchema.safeParse(responseJson); | ||
| const tokenResponse = parseRefreshTokenResponse(responseJson, refreshTokenResponseSchema); | ||
| o365AuthCredentials = { ...o365AuthCredentials, ...(tokenResponse.success && tokenResponse.data) }; |
There was a problem hiding this comment.
Line 264 uses a conditional spread operator that will add false to the credentials object when tokenResponse.success is false. This should check for success and handle the failure case explicitly, or throw an error since parseRefreshTokenResponse should already throw on invalid tokens.
| o365AuthCredentials = { ...o365AuthCredentials, ...(tokenResponse.success && tokenResponse.data) }; | |
| if (!tokenResponse.success) { | |
| throw new Error("Failed to refresh Office 365 access token"); | |
| } | |
| o365AuthCredentials = { ...o365AuthCredentials, ...tokenResponse.data }; |
| }), | ||
| }); | ||
|
|
||
| if (response.statusText !== "OK") throw new HttpError({ statusCode: 400, message: response.statusText }); |
There was a problem hiding this comment.
Using statusText for comparison is unreliable as it's not standardized across all HTTP implementations and may vary. Use response.ok or check response.status against numeric codes instead (e.g., response.status === 200).
| if (response.statusText !== "OK") throw new HttpError({ statusCode: 400, message: response.statusText }); | |
| if (!response.ok) throw new HttpError({ statusCode: 400, message: response.statusText }); |
| const key = parseRefreshTokenResponse(googleCredentials, googleCredentialSchema); | ||
| await prisma.credential.update({ | ||
| where: { id: credential.id }, | ||
| data: { key }, |
There was a problem hiding this comment.
The key variable receives the full return value from parseRefreshTokenResponse which is a SafeParseReturnType object with .success and .data properties. Only key.data should be passed to the database update. The current code will store the entire parse result object instead of just the credentials.
| const key = parseRefreshTokenResponse(googleCredentials, googleCredentialSchema); | |
| await prisma.credential.update({ | |
| where: { id: credential.id }, | |
| data: { key }, | |
| const keyParseResult = parseRefreshTokenResponse(googleCredentials, googleCredentialSchema); | |
| await prisma.credential.update({ | |
| where: { id: credential.id }, | |
| data: { key: keyParseResult.data }, |
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)*