Skip to content

Comments

fix: RateLimit verify email and fix#23718

Merged
anikdhabal merged 14 commits intomainfrom
verify-email
Sep 12, 2025
Merged

fix: RateLimit verify email and fix#23718
anikdhabal merged 14 commits intomainfrom
verify-email

Conversation

@anikdhabal
Copy link
Contributor

@anikdhabal anikdhabal commented Sep 9, 2025

What does this PR do?

RateLimit verify email and fix

@anikdhabal anikdhabal requested a review from a team as a code owner September 9, 2025 15:49
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 9, 2025

Walkthrough

Adds email verification into the booking flow: frontend stores a verificationCode in the Booker store (persisted after successful verification) and includes it in booking mutations; backend handleNewBooking validates the code early when eventType.requiresBookerEmailVerification is true by calling a new verifyCodeUnAuthenticated utility (rate-limited TOTP check using an email-derived secret); API input types and services accept and forward emailVerificationCode/verificationCode; i18n keys for error messages added; unit tests cover required/missing/invalid/valid/rate-limit scenarios; minor formatting change in PiiHasher.

Possibly related PRs

Pre-merge checks (2 passed, 1 warning)

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title references rate-limiting and email verification which align with the primary changes in the PR (adding unauthenticated verification with rate limiting and integrating email verification into booking flows), so it is related to the changeset; however the phrase "and fix" is redundant and the wording is not as concise or clear as it could be. The title should be a single, concise sentence that clearly states the main change without extra words.
Description Check ✅ Passed The PR description "RateLimit verify email" is on-topic and related to the changeset (so it passes this lenient check), but it is very terse and lacks details, testing instructions, and context that would help reviewers.

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 96cd23f and 8de3ac4.

📒 Files selected for processing (2)
  • apps/web/public/static/locales/en/common.json (1 hunks)
  • packages/features/bookings/lib/handleNewBooking.ts (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • apps/web/public/static/locales/en/common.json
  • packages/features/bookings/lib/handleNewBooking.ts
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Install dependencies / Yarn install & cache
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch verify-email

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@graphite-app graphite-app bot requested a review from a team September 9, 2025 15:49
@keithwillcode keithwillcode added the core area: core, team members only label Sep 9, 2025
@dosubot dosubot bot added the 🐛 bug Something isn't working label Sep 9, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 5

🧹 Nitpick comments (8)
packages/trpc/server/routers/viewer/auth/util.ts (5)

8-10: Validate code shape early

Consider validating the code format (e.g., 6 digits) to short-circuit obvious garbage and reduce load on TOTP/checkRateLimit.

Example:

   if (!email || !code) {
     throw new Error("Email and code are required");
   }
+  if (!/^\d{6}$/.test(code)) {
+    throw new Error("Invalid verification code");
+  }

12-15: Rate-limit identifier is email-only; consider adding IP to reduce lockout risk

Email-only keys let attackers lock out victims by hammering attempts for known addresses. If feasible in this layer or at the handler, key the limiter by both hashed email and requester IP.

Example concept: identifier: \emailVerifyCode.${hashEmail(email)}.${ip}``


17-21: Prefer HMAC-based derivation over MD5 for the TOTP secret

MD5 is fast and legacy. A keyed HMAC (e.g., SHA-256) is a stronger KDF for deriving secrets from email+server key. Changing this requires aligning the code generator path; plan a coordinated migration if you proceed.

Example (only if generation uses the same derivation):

-  const secret = createHash("md5")
-    .update(email + encryptionKey)
-    .digest("hex");
+  const secret = createHmac("sha256", encryptionKey).update(email).digest("hex");

Also import { createHash, createHmac } from "crypto";


21-21: Extract TOTP step to a named constant

Improves readability and reuse across modules.

-  const isValidToken = totpRawCheck(code, secret, { step: 900 });
+  const STEP_SECONDS = 15 * 60;
+  const isValidToken = totpRawCheck(code, secret, { step: STEP_SECONDS });

7-7: Naming nit: “Unauthenticated”

Project-wide, consider standardizing to Unauthenticated (single word) for readability.

packages/trpc/server/routers/viewer/auth/verifyCodeUnAuthenticated.handler.ts (3)

15-15: Align error message key with newly added locale

invalid_code vs invalid_verification_code—use the latter for consistency with UI and locales.

See diff above.


12-13: Simplify return handling

Given the util throws on invalid codes, there’s no need to propagate a boolean. Just await and return true.

See diff above.


9-17: Add structured logging for failed verifications
Import the existing logger (import logger from '@calcom/lib/logger'), create a sub-logger (e.g. const log = logger.getSubLogger({ prefix: ['[[Auth] '] })), and in the catch block log a sanitized event (hashed email, client IP from ctx, eventTypeId) before throwing the error.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 76bd90b and 00b07d6.

📒 Files selected for processing (5)
  • apps/web/public/static/locales/en/common.json (1 hunks)
  • packages/features/bookings/lib/handleNewBooking.ts (2 hunks)
  • packages/lib/server/PiiHasher.ts (2 hunks)
  • packages/trpc/server/routers/viewer/auth/util.ts (1 hunks)
  • packages/trpc/server/routers/viewer/auth/verifyCodeUnAuthenticated.handler.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

**/*.ts: For Prisma queries, only select data you need; never use include, always use select
Ensure the credential.key field is never returned from tRPC endpoints or APIs

Files:

  • packages/trpc/server/routers/viewer/auth/util.ts
  • packages/features/bookings/lib/handleNewBooking.ts
  • packages/lib/server/PiiHasher.ts
  • packages/trpc/server/routers/viewer/auth/verifyCodeUnAuthenticated.handler.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js .utc() in hot paths like loops

Files:

  • packages/trpc/server/routers/viewer/auth/util.ts
  • packages/features/bookings/lib/handleNewBooking.ts
  • packages/lib/server/PiiHasher.ts
  • packages/trpc/server/routers/viewer/auth/verifyCodeUnAuthenticated.handler.ts
**/*.{ts,tsx,js,jsx}

⚙️ CodeRabbit configuration file

Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.

Files:

  • packages/trpc/server/routers/viewer/auth/util.ts
  • packages/features/bookings/lib/handleNewBooking.ts
  • packages/lib/server/PiiHasher.ts
  • packages/trpc/server/routers/viewer/auth/verifyCodeUnAuthenticated.handler.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: anglerfishlyy
PR: calcom/cal.com#0
File: :0-0
Timestamp: 2025-08-27T16:39:38.192Z
Learning: anglerfishlyy successfully implemented CAL-3076 email invitation feature for Cal.com team event-types in PR #23312. The feature allows inviting people via email directly from assignment flow, with automatic team invitation if email doesn't belong to existing team member. Implementation includes Host type modifications (userId?: number, email?: string, isPending?: boolean), CheckedTeamSelect component updates with CreatableSelect, TRPC schema validation with zod email validation, and integration with existing teamInvite system.
🧬 Code graph analysis (2)
packages/features/bookings/lib/handleNewBooking.ts (2)
packages/lib/http-error.ts (1)
  • HttpError (1-45)
packages/trpc/server/routers/viewer/auth/util.ts (1)
  • verifyCodeUnAuthenticated (7-28)
packages/trpc/server/routers/viewer/auth/verifyCodeUnAuthenticated.handler.ts (2)
packages/trpc/server/routers/viewer/auth/util.ts (1)
  • verifyCodeUnAuthenticated (7-28)
packages/platform/libraries/index.ts (1)
  • verifyCodeUnAuthenticated (131-131)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Install dependencies / Yarn install & cache
🔇 Additional comments (4)
packages/lib/server/PiiHasher.ts (1)

10-12: LGTM — formatting-only change.

apps/web/public/static/locales/en/common.json (1)

3612-3614: LGTM — keys match server error messages.

Keys align with HttpError messages in booking flow. No typos.

packages/features/bookings/lib/handleNewBooking.ts (1)

84-84: LGTM — import location is correct and keeps concerns separated.

packages/trpc/server/routers/viewer/auth/util.ts (1)

12-15: Nice: PII-safe rate-limit key

Using hashEmail(email) avoids storing raw PII in limiter keys.

@vercel
Copy link

vercel bot commented Sep 9, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments
Project Deployment Preview Comments Updated (UTC)
cal Ignored Ignored Sep 12, 2025 1:56pm
cal-eu Ignored Ignored Sep 12, 2025 1:56pm

@anikdhabal anikdhabal marked this pull request as ready for review September 10, 2025 03:55
@anikdhabal anikdhabal requested a review from a team September 10, 2025 03:55
@dosubot dosubot bot added the authentication area: authentication, auth, google sign in, password, SAML, password reset, can't log in label Sep 10, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (1)
packages/trpc/server/routers/viewer/auth/verifyCodeUnAuthenticated.handler.ts (1)

13-18: Preserve upstream TRPC errors (e.g., 429 rate limit) and align message key

Don’t collapse rate‑limit and other TRPC errors into BAD_REQUEST; rethrow them and map only unknown errors to the i18n key.

   try {
     await verifyCodeUnAuthenticated(email, code);
     return true;
   } catch (error) {
-    throw new TRPCError({ code: "BAD_REQUEST", message: "invalid_code" });
+    if (error instanceof TRPCError) throw error; // e.g., TOO_MANY_REQUESTS
+    throw new TRPCError({ code: "BAD_REQUEST", message: "invalid_verification_code" });
   }
🧹 Nitpick comments (7)
packages/features/bookings/Booker/store.ts (3)

65-71: Reset verificationCode when context changes

The code is tied to a specific email/event. Clear it on re-init or when the verified email changes to prevent leaking a stale code into future bookings.

I’ve proposed diffs below for initialize() and setVerifiedEmail().


257-259: Clear code when verifiedEmail changes

Avoid carrying a code across emails.

-    setVerifiedEmail: (email: string | null) => {
-      set({ verifiedEmail: email });
-    },
+    setVerifiedEmail: (email: string | null) =>
+      set((state) => ({
+        verifiedEmail: email,
+        verificationCode: state.verifiedEmail !== email ? null : state.verificationCode,
+      })),

346-369: Clear code on store.initialize()

Ensure a fresh store has no residual verificationCode.

       set({
         username,
         eventSlug,
         eventId,
         org,
         rescheduleUid,
         rescheduledBy,
         bookingUid,
         bookingData,
+        verificationCode: null,
         layout: layout || BookerLayouts.MONTH_VIEW,
         isTeamEvent: isTeamEvent || false,
         durationConfig,
         timezone,
packages/features/bookings/components/VerifyCodeDialog.tsx (1)

40-41: Avoid verifying incomplete input and enable re‑verify on change

  • Guard verifyCode() to only run for 6 digits.
  • Reset hasVerified on input change so auto-verify can trigger again after a failed attempt.
   const digits = useDigitInput({
     acceptedCharacters: /^[0-9]$/,
     length: 6,
     value,
-    onChange: useCallback((value: string) => {
-      // whenever there's a change in the input, we reset the error value.
-      resetErrors();
-      setValue(value);
-    }, []),
+    onChange: useCallback(
+      (value: string) => {
+        resetErrors();
+        setValue(value);
+        setHasVerified(false);
+      },
+      [resetErrors]
+    ),
   });
@@
   const verifyCode = useCallback(() => {
     resetErrors();
     setIsPending(true);
-    setVerificationCode(value);
+    if (!/^\d{6}$/.test(value.trim())) {
+      setIsPending(false);
+      return;
+    }
+    setVerificationCode(value);

Also applies to: 53-57

packages/platform/atoms/hooks/bookings/useHandleBookEvent.ts (1)

66-67: Clear verificationCode after a successful booking

Prevent carrying a verified code into unrelated future bookings.

   const crmRecordId = useBookerStoreContext((state) => state.crmRecordId);
   const verificationCode = useBookerStoreContext((state) => state.verificationCode);
+  const setVerificationCode = useBookerStoreContext((state) => state.setVerificationCode);
@@
-    const callbacks = inputTimeSlot && !isPlatform ? { onError: handleError } : undefined;
+    const callbacks: Callbacks = {
+      onError: handleError,
+      onSuccess: () => setVerificationCode(null),
+    };

Also applies to: 73-81

packages/features/bookings/lib/book-event-form/booking-to-mutation-input-mapper.tsx (2)

34-34: Type signal for absence vs. emptiness

Consider allowing null to distinguish “not provided” from an empty string and to align with backend Zod/schema if it accepts nullables.

-  verificationCode?: string;
+  verificationCode?: string | null;

103-104: Optional: Omit empty/whitespace verificationCode to avoid spurious invalid-code errors

-    verificationCode,
+    ...(verificationCode?.trim() ? { verificationCode: verificationCode.trim() } : {}),
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 00b07d6 and 21d66de.

📒 Files selected for processing (10)
  • apps/web/playwright/event-types.e2e.ts (1 hunks)
  • apps/web/public/static/locales/en/common.json (1 hunks)
  • packages/features/bookings/Booker/store.ts (2 hunks)
  • packages/features/bookings/components/VerifyCodeDialog.tsx (3 hunks)
  • packages/features/bookings/lib/book-event-form/booking-to-mutation-input-mapper.tsx (3 hunks)
  • packages/features/bookings/lib/handleNewBooking.ts (2 hunks)
  • packages/platform/atoms/hooks/bookings/useHandleBookEvent.ts (2 hunks)
  • packages/prisma/zod/custom/booking.ts (1 hunks)
  • packages/trpc/server/routers/viewer/auth/util.ts (1 hunks)
  • packages/trpc/server/routers/viewer/auth/verifyCodeUnAuthenticated.handler.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • apps/web/public/static/locales/en/common.json
  • packages/features/bookings/lib/handleNewBooking.ts
  • packages/trpc/server/routers/viewer/auth/util.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

**/*.ts: For Prisma queries, only select data you need; never use include, always use select
Ensure the credential.key field is never returned from tRPC endpoints or APIs

Files:

  • packages/features/bookings/Booker/store.ts
  • packages/platform/atoms/hooks/bookings/useHandleBookEvent.ts
  • apps/web/playwright/event-types.e2e.ts
  • packages/prisma/zod/custom/booking.ts
  • packages/trpc/server/routers/viewer/auth/verifyCodeUnAuthenticated.handler.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js .utc() in hot paths like loops

Files:

  • packages/features/bookings/Booker/store.ts
  • packages/features/bookings/components/VerifyCodeDialog.tsx
  • packages/platform/atoms/hooks/bookings/useHandleBookEvent.ts
  • apps/web/playwright/event-types.e2e.ts
  • packages/prisma/zod/custom/booking.ts
  • packages/features/bookings/lib/book-event-form/booking-to-mutation-input-mapper.tsx
  • packages/trpc/server/routers/viewer/auth/verifyCodeUnAuthenticated.handler.ts
**/*.{ts,tsx,js,jsx}

⚙️ CodeRabbit configuration file

Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.

Files:

  • packages/features/bookings/Booker/store.ts
  • packages/features/bookings/components/VerifyCodeDialog.tsx
  • packages/platform/atoms/hooks/bookings/useHandleBookEvent.ts
  • apps/web/playwright/event-types.e2e.ts
  • packages/prisma/zod/custom/booking.ts
  • packages/features/bookings/lib/book-event-form/booking-to-mutation-input-mapper.tsx
  • packages/trpc/server/routers/viewer/auth/verifyCodeUnAuthenticated.handler.ts
**/*.tsx

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

Always use t() for text localization in frontend code; direct text embedding should trigger a warning

Files:

  • packages/features/bookings/components/VerifyCodeDialog.tsx
  • packages/features/bookings/lib/book-event-form/booking-to-mutation-input-mapper.tsx
🧠 Learnings (4)
📚 Learning: 2025-08-29T22:57:31.407Z
Learnt from: bandhan-majumder
PR: calcom/cal.com#23454
File: packages/features/bookings/Booker/components/EventMeta.tsx:16-16
Timestamp: 2025-08-29T22:57:31.407Z
Learning: In Cal.com's Booker architecture, components become client-side through BookerStoreProvider.tsx which has "use client". Any component using useBookerStoreContext automatically runs on the client and should use client-appropriate utilities like markdownToSafeHTMLClient, regardless of whether they have explicit "use client" directives.

Applied to files:

  • packages/features/bookings/components/VerifyCodeDialog.tsx
📚 Learning: 2025-08-27T13:32:46.887Z
Learnt from: supalarry
PR: calcom/cal.com#23364
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/transformers/internal-to-api/internal-to-api.spec.ts:295-296
Timestamp: 2025-08-27T13:32:46.887Z
Learning: In calcom/cal.com, when transforming booking fields from internal to API format, tests in organizations-event-types.e2e-spec.ts already expect name field label and placeholder to be empty strings ("") rather than undefined. PR changes that set these to explicit empty strings are typically fixing implementation to match existing test expectations rather than breaking changes.

Applied to files:

  • apps/web/playwright/event-types.e2e.ts
📚 Learning: 2025-08-21T13:44:06.805Z
Learnt from: supalarry
PR: calcom/cal.com#23217
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/services/output-event-types.service.ts:93-94
Timestamp: 2025-08-21T13:44:06.805Z
Learning: In apps/api/v2/src/ee/event-types/event-types_2024_06_14/event-types.repository.ts, repository functions that use explicit Prisma select clauses (like getEventTypeWithSeats) are used for specific purposes and don't need to include all EventType fields like bookingRequiresAuthentication. These methods don't feed into the general OutputEventTypesService_2024_06_14 flow.

Applied to files:

  • apps/web/playwright/event-types.e2e.ts
📚 Learning: 2025-07-15T12:59:34.389Z
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.389Z
Learning: In the FailedBookingsByField component (packages/features/insights/components/FailedBookingsByField.tsx), although routingFormId is typed as optional in useInsightsParameters, the system automatically enforces a routing form filter, so routingFormId is always present in practice. This means the data always contains only one entry, making the single-entry destructuring approach safe.

Applied to files:

  • packages/features/bookings/lib/book-event-form/booking-to-mutation-input-mapper.tsx
🧬 Code graph analysis (4)
packages/features/bookings/components/VerifyCodeDialog.tsx (1)
packages/features/bookings/Booker/BookerStoreProvider.tsx (1)
  • useBookerStoreContext (24-35)
packages/platform/atoms/hooks/bookings/useHandleBookEvent.ts (1)
packages/features/bookings/Booker/BookerStoreProvider.tsx (1)
  • useBookerStoreContext (24-35)
apps/web/playwright/event-types.e2e.ts (2)
packages/lib/constants.ts (1)
  • WEBAPP_URL (12-18)
apps/web/playwright/lib/testUtils.ts (2)
  • gotoFirstEventType (463-470)
  • submitAndWaitForResponse (488-497)
packages/trpc/server/routers/viewer/auth/verifyCodeUnAuthenticated.handler.ts (1)
packages/trpc/server/routers/viewer/auth/util.ts (1)
  • verifyCodeUnAuthenticated (7-28)
🔇 Additional comments (2)
apps/web/playwright/event-types.e2e.ts (1)

488-491: Stabilize test: cast eventTypeId, dry-run booking, assert message key

  • Wrap the eventTypeId string in Number(...) so it’s passed as a number.
  • Set _isDryRun: true to avoid real bookings.
  • Change the assertion to expect the backend key ("email_verification_required") instead of the translated message (or match both versions if the API still returns the full text).

Also apply these changes in the same block around lines 500–533.

packages/features/bookings/lib/book-event-form/booking-to-mutation-input-mapper.tsx (1)

58-59: LGTM: cleanly plumbs verificationCode into the mapper

Optional field, no breaking changes to callers.

@anikdhabal anikdhabal changed the title fix: RateLimit verify email fix: RateLimit verify email and fix Sep 10, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (2)
packages/features/bookings/lib/handleNewBooking/test/email-verification-booking.test.ts (2)

26-26: Increase CI timeout to reduce flakiness

DB-backed scenario setup + dynamic imports can exceed 5s in CI.

-const timeout = process.env.CI ? 5000 : 20000;
+const timeout = process.env.CI ? 20000 : 20000;

31-33: Reset mock implementations between tests, not just call history

clearAllMocks resets calls but keeps prior mock implementations, which can leak across tests.

-  beforeEach(() => {
-    vi.clearAllMocks();
-  });
+  beforeEach(() => {
+    vi.resetAllMocks();
+  });
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 7b70abe and 6355bc2.

📒 Files selected for processing (1)
  • packages/features/bookings/lib/handleNewBooking/test/email-verification-booking.test.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

**/*.ts: For Prisma queries, only select data you need; never use include, always use select
Ensure the credential.key field is never returned from tRPC endpoints or APIs

Files:

  • packages/features/bookings/lib/handleNewBooking/test/email-verification-booking.test.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js .utc() in hot paths like loops

Files:

  • packages/features/bookings/lib/handleNewBooking/test/email-verification-booking.test.ts
**/*.{ts,tsx,js,jsx}

⚙️ CodeRabbit configuration file

Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.

Files:

  • packages/features/bookings/lib/handleNewBooking/test/email-verification-booking.test.ts
🧬 Code graph analysis (1)
packages/features/bookings/lib/handleNewBooking/test/email-verification-booking.test.ts (1)
apps/web/test/utils/bookingScenario/bookingScenario.ts (5)
  • getBooker (2220-2234)
  • getOrganizer (1520-1576)
  • TestData (1239-1511)
  • getScenarioData (1578-1664)
  • createBookingScenario (978-1009)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
  • GitHub Check: Production builds / Build API v1
  • GitHub Check: Production builds / Build Web App
  • GitHub Check: Tests / Unit
  • GitHub Check: Production builds / Build API v2
  • GitHub Check: Type check / check-types
🔇 Additional comments (2)
packages/features/bookings/lib/handleNewBooking/test/email-verification-booking.test.ts (2)

226-229: Confirm product decision: rate-limit error maps to “invalid_verification_code”

If the intent is to prevent information leakage, OK. If UX should show a distinct “too_many_attempts”/“rate_limited” message or 429, adjust assertions and server mapping accordingly.


28-342: Solid coverage and isolation

Good use of module mock + dynamic imports; scenarios cover missing, invalid, success, rate-limited, and not-required paths.

Copy link
Contributor

@supalarry supalarry left a comment

Choose a reason for hiding this comment

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

Good stuff - have 2 comments below

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (4)
packages/platform/types/bookings/2024-08-13/inputs/create-booking.input.ts (1)

374-382: Tighten verification code validation (numeric + length).

Add a numeric regex guard (6–8 digits). This prevents accidental alphanumerics/whitespace and reduces garbage hitting the verifier.

   @IsOptional()
   @IsString()
+  @Matches(/^\d{6,8}$/, { message: "Verification code must be 6–8 digits." })
   emailVerificationCode?: string;

Also update imports:

-import {
+import {
   IsInt,
   IsDateString,
   IsTimeZone,
   IsEnum,
   ValidateNested,
   IsArray,
   IsString,
   isEmail,
   IsOptional,
   IsUrl,
   IsObject,
   IsBoolean,
   Min,
   registerDecorator,
   Validate,
   IsDefined,
+  Matches,
 } from "class-validator";
packages/platform/types/bookings/2024-08-13/inputs/reschedule-booking.input.ts (2)

31-38: Apply the same numeric/length guard to reschedule input.

Keep validation consistent with create flow to avoid divergent behavior.

   @IsString()
   @IsOptional()
   @ApiPropertyOptional({
     type: String,
     description: "Email verification code required when event type has email verification enabled.",
     example: "123456",
   })
+  @Matches(/^\d{6,8}$/, { message: "Verification code must be 6–8 digits." })
   emailVerificationCode?: string;

And ensure Matches is imported:

-import { IsDateString, IsOptional, IsString, Validate, isEmail } from "class-validator";
+import { IsDateString, IsOptional, IsString, Validate, isEmail, Matches } from "class-validator";

64-71: Mirror the validation for seated reschedules.

Same constraints as above for consistency.

   @IsString()
   @IsOptional()
   @ApiPropertyOptional({
     type: String,
     description: "Email verification code required when event type has email verification enabled.",
     example: "123456",
   })
+  @Matches(/^\d{6,8}$/, { message: "Verification code must be 6–8 digits." })
   emailVerificationCode?: string;
apps/api/v2/src/ee/bookings/2024-08-13/services/input.service.ts (1)

471-481: Make responses.email non-empty (align with create flow).

In create you default to ""; here it may be undefined, which can break downstream validators expecting a string.

-        email: attendeeEmail,
+        email: attendeeEmail ?? "",
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 6355bc2 and d8e0d47.

📒 Files selected for processing (3)
  • apps/api/v2/src/ee/bookings/2024-08-13/services/input.service.ts (4 hunks)
  • packages/platform/types/bookings/2024-08-13/inputs/create-booking.input.ts (1 hunks)
  • packages/platform/types/bookings/2024-08-13/inputs/reschedule-booking.input.ts (2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

**/*.ts: For Prisma queries, only select data you need; never use include, always use select
Ensure the credential.key field is never returned from tRPC endpoints or APIs

Files:

  • packages/platform/types/bookings/2024-08-13/inputs/reschedule-booking.input.ts
  • apps/api/v2/src/ee/bookings/2024-08-13/services/input.service.ts
  • packages/platform/types/bookings/2024-08-13/inputs/create-booking.input.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js .utc() in hot paths like loops

Files:

  • packages/platform/types/bookings/2024-08-13/inputs/reschedule-booking.input.ts
  • apps/api/v2/src/ee/bookings/2024-08-13/services/input.service.ts
  • packages/platform/types/bookings/2024-08-13/inputs/create-booking.input.ts
**/*.{ts,tsx,js,jsx}

⚙️ CodeRabbit configuration file

Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.

Files:

  • packages/platform/types/bookings/2024-08-13/inputs/reschedule-booking.input.ts
  • apps/api/v2/src/ee/bookings/2024-08-13/services/input.service.ts
  • packages/platform/types/bookings/2024-08-13/inputs/create-booking.input.ts
**/*.{service,repository}.ts

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

Avoid dot-suffixes like .service.ts or .repository.ts for new files; reserve .test.ts, .spec.ts, .types.ts for their specific purposes

Files:

  • apps/api/v2/src/ee/bookings/2024-08-13/services/input.service.ts
🧠 Learnings (1)
📚 Learning: 2025-08-27T13:32:46.887Z
Learnt from: supalarry
PR: calcom/cal.com#23364
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/transformers/internal-to-api/internal-to-api.spec.ts:295-296
Timestamp: 2025-08-27T13:32:46.887Z
Learning: In calcom/cal.com, when transforming booking fields from internal to API format, tests in organizations-event-types.e2e-spec.ts already expect name field label and placeholder to be empty strings ("") rather than undefined. PR changes that set these to explicit empty strings are typically fixing implementation to match existing test expectations rather than breaking changes.

Applied to files:

  • apps/api/v2/src/ee/bookings/2024-08-13/services/input.service.ts
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Install dependencies / Yarn install & cache
🔇 Additional comments (3)
apps/api/v2/src/ee/bookings/2024-08-13/services/input.service.ts (3)

605-606: LGTM: seated reschedule now carries a transient verifier input.

Propagation looks correct; same transient handling caveat as above applies.


667-668: LGTM: reschedule payload includes verificationCode.

Consistent with other transforms.


176-181: Ensure verificationCode is transient (never persisted or returned).

Local script produced no output; verification inconclusive — confirm this value is only consumed by verification logic and is not written to the DB, logged, or included in API responses.

File: apps/api/v2/src/ee/bookings/2024-08-13/services/input.service.ts (lines 176–181)

Run these checks locally and paste outputs:

# find any occurrences
rg -n --hidden -S '\bverificationCode\b' . -g '!**/test/**' || true

# detect Prisma writes where data includes verificationCode
rg -nP '\b(prisma|tx)\.\w+\.(create|update|upsert)\b[\s\S]*?\bdata\s*:\s*{[\s\S]*?\bverificationCode\b' --type=ts --type=js . -g '!**/test/**' || true

# check serializers, response builders, and logging
rg -nP '\bverificationCode\b' apps/ packages/ --type=ts --type=tsx -g '!**/test/**' || true
rg -nP 'console\.\w+|logger\.\w+|Logger\(' apps/ packages/ --type=ts --type=tsx -g '!**/test/**' | rg -n 'verificationCode' || true

If any matches are found: remove persistence/logging and exclude the field from API serializers/responses (strip it before saving or returning).

supalarry
supalarry previously approved these changes Sep 12, 2025
Copy link
Contributor

@volnei volnei left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

authentication area: authentication, auth, google sign in, password, SAML, password reset, can't log in 🐛 bug Something isn't working core area: core, team members only ready-for-e2e size/L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants