Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -459,3 +459,7 @@ DATABASE_CHUNK_SIZE=
# Service Account Encryption Key for encrypting/decrypting service account keys
# You can use: `openssl rand -base64 24` to generate one
CALCOM_SERVICE_ACCOUNT_ENCRYPTION_KEY=

# Token to verify incoming webhooks from Microsoft Office 365
# Generate a random string for client state validation: openssl rand -base64 32
OFFICE365_WEBHOOK_CLIENT_STATE=
6 changes: 6 additions & 0 deletions apps/api/v1/test/lib/selected-calendars/_post.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,9 @@ describe("POST /api/selected-calendars", () => {
googleChannelResourceId: null,
googleChannelResourceUri: null,
googleChannelExpiration: null,
office365SubscriptionId: null,
office365SubscriptionExpiration: null,
office365SubscriptionClientState: null,
error: null,
lastErrorAt: null,
watchAttempts: 0,
Expand Down Expand Up @@ -130,6 +133,9 @@ describe("POST /api/selected-calendars", () => {
googleChannelResourceId: null,
googleChannelResourceUri: null,
googleChannelExpiration: null,
office365SubscriptionId: null,
office365SubscriptionExpiration: null,
office365SubscriptionClientState: null,
delegationCredentialId: null,
domainWideDelegationCredentialId: null,
eventTypeId: null,
Expand Down
1 change: 1 addition & 0 deletions packages/app-store/office365calendar/api/index.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
export { default as add } from "./add";
export { default as callback } from "./callback";
export { default as webhook } from "./webhook";
96 changes: 96 additions & 0 deletions packages/app-store/office365calendar/api/webhook.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
import type { NextApiRequest, NextApiResponse } from "next";

import { getCredentialForCalendarCache } from "@calcom/lib/delegationCredential/server";
import { HttpError } from "@calcom/lib/http-error";
import logger from "@calcom/lib/logger";
import { safeStringify } from "@calcom/lib/safeStringify";
import { defaultHandler } from "@calcom/lib/server/defaultHandler";
import { defaultResponder } from "@calcom/lib/server/defaultResponder";
import { SelectedCalendarRepository } from "@calcom/lib/server/repository/selectedCalendar";

import { getCalendar } from "../../_utils/getCalendar";

const log = logger.getSubLogger({ prefix: ["office365calendar", "webhook"] });

async function getHandler(req: NextApiRequest, res: NextApiResponse) {
const validationToken = req.query.validationToken as string;
if (!validationToken) {
throw new HttpError({ statusCode: 400, message: "Missing validation token" });
}

res.setHeader("Content-Type", "text/plain");
res.status(200).send(validationToken);
}

async function postHandler(req: NextApiRequest, res: NextApiResponse) {
const validationToken = req.query.validationToken;
if (validationToken && typeof validationToken === "string") {
res.setHeader("Content-Type", "text/plain");
res.status(200).send(validationToken);
return;
}

const { value: notifications } = req.body;

if (!notifications || !Array.isArray(notifications)) {
throw new HttpError({ statusCode: 400, message: "Invalid notification payload" });
}

const expectedClientState = process.env.OFFICE365_WEBHOOK_CLIENT_STATE;
if (!expectedClientState) {
log.error("OFFICE365_WEBHOOK_CLIENT_STATE not configured");
throw new HttpError({ statusCode: 500, message: "Webhook not configured" });
}

for (const notification of notifications) {
if (notification.clientState !== expectedClientState) {
throw new HttpError({ statusCode: 403, message: "Invalid client state" });
}
}
Comment on lines +45 to +49
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add protection against timing attacks in client state validation.

The client state comparison should use constant-time comparison to prevent timing attacks.

+import crypto from "crypto";
+
 for (const notification of notifications) {
-    if (notification.clientState !== expectedClientState) {
+    // Use constant-time comparison to prevent timing attacks
+    if (!crypto.timingSafeEqual(
+      Buffer.from(notification.clientState || ''),
+      Buffer.from(expectedClientState)
+    )) {
       throw new HttpError({ statusCode: 403, message: "Invalid client state" });
     }
 }
📝 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.

Suggested change
for (const notification of notifications) {
if (notification.clientState !== expectedClientState) {
throw new HttpError({ statusCode: 403, message: "Invalid client state" });
}
}
import crypto from "crypto";
for (const notification of notifications) {
// Use constant-time comparison to prevent timing attacks
if (!crypto.timingSafeEqual(
Buffer.from(notification.clientState || ''),
Buffer.from(expectedClientState)
)) {
throw new HttpError({ statusCode: 403, message: "Invalid client state" });
}
}
🤖 Prompt for AI Agents
In packages/app-store/office365calendar/api/webhook.ts around lines 45 to 49,
the clientState comparison uses a direct equality check which is vulnerable to
timing attacks. Replace the direct comparison with a constant-time comparison
function to securely compare notification.clientState and expectedClientState,
preventing timing attack vulnerabilities.


for (const notification of notifications) {
try {
const { subscriptionId, resource } = notification;

if (!subscriptionId || !resource) {
log.warn("Notification missing required fields");
continue;
}

log.debug("Processing notification", { subscriptionId });

const selectedCalendar = await SelectedCalendarRepository.findFirstByOffice365SubscriptionId(
subscriptionId
);

if (!selectedCalendar) {
log.debug("No selected calendar found for subscription", { subscriptionId });
continue;
}

const { credential } = selectedCalendar;
if (!credential) {
log.debug("No credential found for selected calendar", { subscriptionId });
continue;
}

const { selectedCalendars } = credential;
const credentialForCalendarCache = await getCredentialForCalendarCache({ credentialId: credential.id });
const calendarServiceForCalendarCache = await getCalendar(credentialForCalendarCache);
await calendarServiceForCalendarCache?.fetchAvailabilityAndSetCache?.(selectedCalendars);
log.debug("Successfully updated calendar cache", { subscriptionId });
} catch (error) {
log.error(
"Error processing notification",
safeStringify({ error, subscriptionId: notification.subscriptionId })
);
}
}

return { message: "ok" };
}
Comment on lines +25 to +91
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add request size and rate limiting protection.

The webhook endpoint should validate request size and implement rate limiting to prevent abuse.

Consider implementing:

  1. Request body size validation
  2. Rate limiting per subscription or IP
  3. Request timeout handling
  4. Dead letter queue for failed notifications
🤖 Prompt for AI Agents
In packages/app-store/office365calendar/api/webhook.ts between lines 25 and 91,
the webhook handler lacks protections against large request bodies and excessive
request rates, which can lead to abuse or service degradation. Add request body
size validation to reject overly large payloads early. Implement rate limiting
based on subscriptionId or client IP to control request frequency. Include
request timeout handling to avoid hanging requests. Consider adding a dead
letter queue mechanism to capture and retry or analyze failed notification
processing asynchronously.


export default defaultHandler({
GET: Promise.resolve({ default: defaultResponder(getHandler) }),
POST: Promise.resolve({ default: defaultResponder(postHandler) }),
});
Loading
Loading