-
Notifications
You must be signed in to change notification settings - Fork 0
Automated Test: oauth-security-enhanced #379
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,93 @@ | ||||||||||||||||||||||||
| import type { NextApiRequest, NextApiResponse } from "next"; | ||||||||||||||||||||||||
| import z from "zod"; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| import { appStoreMetadata } from "@calcom/app-store/appStoreMetaData"; | ||||||||||||||||||||||||
| import { APP_CREDENTIAL_SHARING_ENABLED } from "@calcom/lib/constants"; | ||||||||||||||||||||||||
| import { symmetricDecrypt } from "@calcom/lib/crypto"; | ||||||||||||||||||||||||
| import prisma from "@calcom/prisma"; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| const appCredentialWebhookRequestBodySchema = z.object({ | ||||||||||||||||||||||||
| // UserId of the cal.com user | ||||||||||||||||||||||||
| userId: z.number().int(), | ||||||||||||||||||||||||
| appSlug: z.string(), | ||||||||||||||||||||||||
| // Keys should be AES256 encrypted with the CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY | ||||||||||||||||||||||||
| keys: z.string(), | ||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||
| /** */ | ||||||||||||||||||||||||
| 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unhandled
🛡️ Proposed fix- const reqBody = appCredentialWebhookRequestBodySchema.parse(req.body);
+ const parsed = appCredentialWebhookRequestBodySchema.safeParse(req.body);
+ if (!parsed.success) {
+ return res.status(400).json({ message: "Invalid request body" });
+ }
+ const reqBody = parsed.data;📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // Check that the user exists | ||||||||||||||||||||||||
| const user = await prisma.user.findUnique({ where: { id: reqBody.userId } }); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| if (!user) { | ||||||||||||||||||||||||
| return res.status(404).json({ message: "User not found" }); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| const app = await prisma.app.findUnique({ | ||||||||||||||||||||||||
| where: { slug: reqBody.appSlug }, | ||||||||||||||||||||||||
| select: { slug: true }, | ||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| if (!app) { | ||||||||||||||||||||||||
| return res.status(404).json({ message: "App not found" }); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // Search for the app's slug and type | ||||||||||||||||||||||||
| const appMetadata = appStoreMetadata[app.slug as keyof typeof appStoreMetadata]; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| if (!appMetadata) { | ||||||||||||||||||||||||
| return res.status(404).json({ message: "App not found. Ensure that you have the correct app slug" }); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // Decrypt the keys | ||||||||||||||||||||||||
| const keys = JSON.parse( | ||||||||||||||||||||||||
| symmetricDecrypt(reqBody.keys, process.env.CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY || "") | ||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||
|
Comment on lines
+57
to
+59
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unhandled errors from decryption and JSON parsing. Both 🛡️ Proposed fix- const keys = JSON.parse(
- symmetricDecrypt(reqBody.keys, process.env.CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY || "")
- );
+ let keys;
+ try {
+ keys = JSON.parse(
+ symmetricDecrypt(reqBody.keys, process.env.CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY || "")
+ );
+ } catch (error) {
+ return res.status(400).json({ message: "Failed to decrypt or parse credential keys" });
+ }📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // Can't use prisma upsert as we don't know the id of the credential | ||||||||||||||||||||||||
| const appCredential = await prisma.credential.findFirst({ | ||||||||||||||||||||||||
| where: { | ||||||||||||||||||||||||
| userId: reqBody.userId, | ||||||||||||||||||||||||
| appId: appMetadata.slug, | ||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||
| select: { | ||||||||||||||||||||||||
| id: true, | ||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| if (appCredential) { | ||||||||||||||||||||||||
| await prisma.credential.update({ | ||||||||||||||||||||||||
| where: { | ||||||||||||||||||||||||
| id: appCredential.id, | ||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||
| data: { | ||||||||||||||||||||||||
| key: keys, | ||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||
| return res.status(200).json({ message: `Credentials updated for userId: ${reqBody.userId}` }); | ||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||
| await prisma.credential.create({ | ||||||||||||||||||||||||
| data: { | ||||||||||||||||||||||||
| key: keys, | ||||||||||||||||||||||||
| userId: reqBody.userId, | ||||||||||||||||||||||||
| appId: appMetadata.slug, | ||||||||||||||||||||||||
| type: appMetadata.type, | ||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||
| return res.status(200).json({ message: `Credentials created for userId: ${reqBody.userId}` }); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,32 @@ | ||||||||||||||||||||||||||
| import { z } from "zod"; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| import { APP_CREDENTIAL_SHARING_ENABLED } from "@calcom/lib/constants"; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| 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(), | ||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||
|
Comment on lines
+5
to
+11
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# First, verify the file exists and check its content
find . -name "parseRefreshTokenResponse.ts" -type fRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror Length of output: 152 🏁 Script executed: #!/bin/bash
# Test the Zod schema behavior directly
cd /tmp
cat > test_schema.mjs << 'EOF'
import { z } from "zod";
console.log("=== Testing z.string().toString() ===");
console.log("z.string().toString() =>", z.string().toString());
console.log("z.string().optional().toString() =>", z.string().optional().toString());
console.log("\n=== Testing current schema ===");
const currentSchema = z.object({
access_token: z.string(),
[z.string().toString()]: z.number(),
[z.string().optional().toString()]: z.unknown().optional(),
});
const testResponse = {
access_token: "test_token",
refresh_token: "test_refresh",
expires_in: 3600,
expiry_date: 1700000000,
};
const result = currentSchema.safeParse(testResponse);
console.log("Parse success:", result.success);
console.log("Input keys:", Object.keys(testResponse));
console.log("Output keys:", result.success ? Object.keys(result.data) : "N/A");
console.log("Full output:", JSON.stringify(result.data || result.error.issues, null, 2));
console.log("\n=== Testing with passthrough ===");
const passthroughSchema = z.object({
access_token: z.string(),
}).passthrough();
const result2 = passthroughSchema.safeParse(testResponse);
console.log("Parse success:", result2.success);
console.log("Output keys:", result2.success ? Object.keys(result2.data) : "N/A");
console.log("Full output:", JSON.stringify(result2.data, null, 2));
EOF
# Try to run with node
node test_schema.mjs 2>&1 || echo "Node execution failed or not available"Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror Length of output: 1039 🏁 Script executed: #!/bin/bash
# Read the file to verify the schema definition
cat -n ./packages/app-store/_utils/oauth/parseRefreshTokenResponse.tsRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror Length of output: 1344
To accept Proposed fix-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(),
+ })
+ .passthrough();📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| const parseRefreshTokenResponse = (response: any, schema: z.ZodTypeAny) => { | ||||||||||||||||||||||||||
| let refreshTokenResponse; | ||||||||||||||||||||||||||
| if (APP_CREDENTIAL_SHARING_ENABLED && process.env.CALCOM_CREDENTIAL_SYNC_ENDPOINT) { | ||||||||||||||||||||||||||
| refreshTokenResponse = minimumTokenResponseSchema.safeParse(response); | ||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||
| refreshTokenResponse = schema.safeParse(response); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| if (!refreshTokenResponse.success) { | ||||||||||||||||||||||||||
| throw new Error("Invalid refreshed tokens were returned"); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| if (!refreshTokenResponse.data.refresh_token) { | ||||||||||||||||||||||||||
| refreshTokenResponse.data.refresh_token = "refresh_token"; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
Comment on lines
+25
to
+27
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Setting If a provider legitimately omits 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| return refreshTokenResponse; | ||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| export default parseRefreshTokenResponse; | ||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,22 @@ | ||||||||||||||||||||||||||||||||||||||||||||||
| import { APP_CREDENTIAL_SHARING_ENABLED } from "@calcom/lib/constants"; | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| 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; | ||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+8
to
+15
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No error handling on credential sync endpoint request. The Proposed improvement const response = await fetch(process.env.CALCOM_CREDENTIAL_SYNC_ENDPOINT, {
method: "POST",
body: new URLSearchParams({
calcomUserId: userId.toString(),
appSlug,
}),
});
- return response;
+ if (!response.ok) {
+ throw new Error(
+ `Credential sync endpoint returned ${response.status}: ${response.statusText}`
+ );
+ }
+ const data = await response.json();
+ return data;📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||||||
| const response = await refreshFunction(); | ||||||||||||||||||||||||||||||||||||||||||||||
| return response; | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+3
to
+20
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Return type mismatch between credential-sharing and direct-refresh branches will cause runtime failures. When Callers downstream access provider-specific shapes (e.g., The To fix this, the credential-sharing branch should either:
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| export default refreshOAuthTokens; | ||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,6 +18,8 @@ import type { | |
| } from "@calcom/types/Calendar"; | ||
| import type { CredentialPayload } from "@calcom/types/Credential"; | ||
|
|
||
| import parseRefreshTokenResponse from "../../_utils/oauth/parseRefreshTokenResponse"; | ||
| import refreshOAuthTokens from "../../_utils/oauth/refreshOAuthTokens"; | ||
| import { getGoogleAppKeys } from "./getGoogleAppKeys"; | ||
| import { googleCredentialSchema } from "./googleCredentialSchema"; | ||
|
|
||
|
|
@@ -81,11 +83,18 @@ export default class GoogleCalendarService implements Calendar { | |
|
|
||
| const refreshAccessToken = async (myGoogleAuth: Awaited<ReturnType<typeof getGoogleAuth>>) => { | ||
| try { | ||
| const { res } = await myGoogleAuth.refreshToken(googleCredentials.refresh_token); | ||
| const res = await refreshOAuthTokens( | ||
| async () => { | ||
| const fetchTokens = await myGoogleAuth.refreshToken(googleCredentials.refresh_token); | ||
| return fetchTokens.res; | ||
| }, | ||
| "google-calendar", | ||
| credential.userId | ||
| ); | ||
|
Comment on lines
+86
to
+93
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: # Find refreshOAuthTokens function
fd -type f -name "*.ts" | xargs rg -l "refreshOAuthTokens" | head -20Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror Length of output: 813 🏁 Script executed: # Look for the refreshOAuthTokens implementation
rg -A 20 "function refreshOAuthTokens|export.*refreshOAuthTokens" --type tsRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror Length of output: 178 🏁 Script executed: # Check the Google Calendar implementation context
cat -n packages/app-store/googlecalendar/lib/CalendarService.ts | sed -n '80,100p'Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror Length of output: 1070 🏁 Script executed: # Find Zoom and Office365 integrations to compare token refresh
fd -type f -path "*zoom*" -name "*.ts" | xargs rg -l "refreshOAuthTokens"
fd -type f -path "*office365*" -name "*.ts" | xargs rg -l "refreshOAuthTokens"Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror Length of output: 1520 🏁 Script executed: # Check imports in the Google Calendar file
cat -n packages/app-store/googlecalendar/lib/CalendarService.ts | sed -n '1,30p'Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror Length of output: 1426 🏁 Script executed: # Read the refreshOAuthTokens implementation
cat -n packages/app-store/_utils/oauth/refreshOAuthTokens.tsRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror Length of output: 1094 🏁 Script executed: # Check how Office365 uses refreshOAuthTokens
grep -A 10 -B 2 "refreshOAuthTokens" packages/app-store/office365calendar/lib/CalendarService.ts | head -30Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror Length of output: 1163 🏁 Script executed: # Check how Zoom uses refreshOAuthTokens
grep -A 10 -B 2 "refreshOAuthTokens" packages/app-store/zoomvideo/lib/VideoApiAdapter.ts | head -30Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror Length of output: 1101 🏁 Script executed: # Check how Office365 extracts token after refreshOAuthTokens call
grep -A 15 "const response = await refreshOAuthTokens" packages/app-store/office365calendar/lib/CalendarService.ts | head -25Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror Length of output: 715 🏁 Script executed: # Check how Zoom extracts token after refreshOAuthTokens call
grep -A 15 "const response = await refreshOAuthTokens" packages/app-store/zoomvideo/lib/VideoApiAdapter.ts | head -25Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror Length of output: 567 🏁 Script executed: # Check what GaxiosResponse structure looks like - search for myGoogleAuth.refreshToken usage
grep -B 5 -A 10 "refreshToken" packages/app-store/googlecalendar/lib/CalendarService.ts | head -40Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror Length of output: 939 🏁 Script executed: # Get more context after refreshOAuthTokens in Office365
grep -A 25 "const response = await refreshOAuthTokens" packages/app-store/office365calendar/lib/CalendarService.ts | grep -A 25 "office365-calendar"Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror Length of output: 577 🏁 Script executed: # Get more context after refreshOAuthTokens in Zoom
grep -A 25 "const response = await refreshOAuthTokens" packages/app-store/zoomvideo/lib/VideoApiAdapter.ts | grep -A 25 "zoomvideo"Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror Length of output: 567 🏁 Script executed: # Check parseRefreshTokenResponse
cat -n packages/app-store/_utils/oauth/parseRefreshTokenResponse.tsRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror Length of output: 1344 🏁 Script executed: # Check what handleErrorsJson does in Office365
grep -B 5 -A 10 "handleErrorsJson" packages/app-store/office365calendar/lib/CalendarService.tsRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror Length of output: 4653 🏁 Script executed: # Check what handleZoomResponse does
grep -B 5 -A 10 "handleZoomResponse" packages/app-store/zoomvideo/lib/VideoApiAdapter.tsRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror Length of output: 1798 🏁 Script executed: # Search for GaxiosResponse type or google-auth-library usage
rg "GaxiosResponse|googleapis" packages/app-store/googlecalendar/ -A 3Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror Length of output: 2330 🏁 Script executed: # Check the full Office365 refresh token handling to see complete flow
sed -n '80,140p' packages/app-store/office365calendar/lib/CalendarService.tsRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror Length of output: 1806 🏁 Script executed: # Check if Google code has any handling for Response type
grep -B 5 -A 15 "const token = res" packages/app-store/googlecalendar/lib/CalendarService.tsRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror Length of output: 1001 🏁 Script executed: # Check if there's a type definition or handling for the response in Google Calendar
grep -B 10 "res?.data" packages/app-store/googlecalendar/lib/CalendarService.tsRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror Length of output: 519 Google Calendar's response handling is incompatible with the credential sync endpoint. The code assumes 🤖 Prompt for AI Agents |
||
| const token = res?.data; | ||
| googleCredentials.access_token = token.access_token; | ||
| googleCredentials.expiry_date = token.expiry_date; | ||
| const key = googleCredentialSchema.parse(googleCredentials); | ||
| const key = parseRefreshTokenResponse(googleCredentials, googleCredentialSchema); | ||
| await prisma.credential.update({ | ||
| where: { id: credential.id }, | ||
| data: { key }, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,6 +23,7 @@ import type { | |
| import type { CredentialPayload } from "@calcom/types/Credential"; | ||
|
|
||
| import getAppKeysFromSlug from "../../_utils/getAppKeysFromSlug"; | ||
| import refreshOAuthTokens from "../../_utils/oauth/refreshOAuthTokens"; | ||
| import type { HubspotToken } from "../api/callback"; | ||
|
|
||
| const hubspotClient = new hubspot.Client(); | ||
|
|
@@ -173,13 +174,18 @@ export default class HubspotCalendarService implements Calendar { | |
|
|
||
| const refreshAccessToken = async (refreshToken: string) => { | ||
| try { | ||
| const hubspotRefreshToken: HubspotToken = await hubspotClient.oauth.tokensApi.createToken( | ||
| "refresh_token", | ||
| undefined, | ||
| WEBAPP_URL + "/api/integrations/hubspot/callback", | ||
| this.client_id, | ||
| this.client_secret, | ||
| refreshToken | ||
| 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 | ||
| ); | ||
|
Comment on lines
+177
to
189
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Integration is correct for the default (non-sharing) path, but the type mismatch is particularly severe here. When credential sharing is enabled, This is the most fragile integration point among all the callers. The fix should be coordinated with the core module's return type contract (raised in 🤖 Prompt for AI Agents |
||
|
|
||
| // set expiry date as offset from current time. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing HTTP method restriction and brittle webhook secret validation.
No method check: This handler accepts any HTTP method. It should restrict to
POSTonly.Webhook secret bypass when unset: If
CALCOM_WEBHOOK_SECRETis undefined and the request omits the header, both sides areundefined, soundefined !== undefinedevaluates tofalseand the check passes. TheAPP_CREDENTIAL_SHARING_ENABLEDguard on line 19 mitigates this (since it requires the env var to be set), but defense-in-depth is warranted.🛡️ Proposed fix
export default async function handler(req: NextApiRequest, res: NextApiResponse) { + 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 + const webhookSecret = process.env.CALCOM_WEBHOOK_SECRET; + if (!webhookSecret) { + return res.status(500).json({ message: "Webhook secret is not configured" }); + } if ( req.headers[process.env.CALCOM_WEBHOOK_HEADER_NAME || "calcom-webhook-secret"] !== - process.env.CALCOM_WEBHOOK_SECRET + webhookSecret ) { return res.status(403).json({ message: "Invalid webhook secret" }); }🤖 Prompt for AI Agents