feat: add email-verification flag support to signup flow #24294
Conversation
- Check email-verification feature flag during signup - Set emailVerified to null when flag is enabled, current date when disabled - Only send verification emails when flag is enabled - Apply to both Cal.com and self-hosted signup handlers - Keep OAuth/SAML flows with verified emails (verified by IdP) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
WalkthroughThe changes add a global feature flag check for "email-verification" via FeaturesRepository. Calcom and self-hosted signup handlers instantiate the repository, read isEmailVerificationEnabled, log it, and c 8000 onditionally set user.emailVerified to null when enabled or to the current timestamp when disabled. Sending verification emails is now guarded by the flag. An import for FeaturesRepository was also added to packages/features/auth/lib/next-auth-options.ts but is unused. Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
📜 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.
📒 Files selected for processing (3)
packages/features/auth/lib/next-auth-options.ts(1 hunks)packages/features/auth/signup/handlers/calcomHandler.ts(6 hunks)packages/features/auth/signup/handlers/selfHostedHandler.ts(5 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 useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/features/auth/lib/next-auth-options.tspackages/features/auth/signup/handlers/selfHostedHandler.tspackages/features/auth/signup/handlers/calcomHandler.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/auth/lib/next-auth-options.tspackages/features/auth/signup/handlers/selfHostedHandler.tspackages/features/auth/signup/handlers/calcomHandler.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/auth/lib/next-auth-options.tspackages/features/auth/signup/handlers/selfHostedHandler.tspackages/features/auth/signup/handlers/calcomHandler.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/auth/signup/handlers/selfHostedHandler.ts (1)
packages/features/auth/lib/verifyEmail.ts (1)
sendEmailVerification(29-85)
packages/features/auth/signup/handlers/calcomHandler.ts (2)
packages/features/auth/lib/verifyEmail.ts (1)
sendEmailVerification(29-85)apps/web/lib/buildLegacyCtx.ts (1)
buildLegacyRequest(47-49)
⏰ 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 (6)
packages/features/auth/signup/handlers/calcomHandler.ts (3)
48-53: LGTM!The feature flag check is correctly implemented. The repository is instantiated and the global flag value is retrieved appropriately.
161-161: LGTM!The conditional
emailVerifiedlogic is correctly implemented across all user creation and update paths. When the feature flag is enabled, users are created withemailVerified: null(unverified), and when disabled, users are auto-verified withemailVerified: new Date(Date.now()).Also applies to: 173-173, 205-205
218-225: Verify email verification in token-based signup
The team-invite signup branch (foundToken && foundToken.teamId) doesn’t callsendEmailVerificationeven whenisEmailVerificationEnabledis true—only the non-token path does. Confirm if invite signups should trigger verification emails and, if so, add a guardedsendEmailVerificationcall in that branch (around line 191).packages/features/auth/signup/handlers/selfHostedHandler.ts (3)
35-38: LGTM!The feature flag check is correctly implemented, consistent with the pattern in
calcomHandler.ts.
107-107: LGTM!The conditional
emailVerifiedlogic is correctly and consistently implemented across all user creation and update paths (both token-based and non-token paths). The logic matches the implementation incalcomHandler.ts.Also applies to: 113-113, 163-163, 169-169
179-186: Verify email flow for team-invite signups
The token-based signup path (lines 69–138) setsemailVerifiedbut doesn’t invokesendEmailVerification, unlike the non-token path (lines 179–186). Confirm whether to trigger verification emails underisEmailVerificationEnabledfor team invites and, if so, add the guarded call afterjoinAnyChildTeamOnOrgInvite(around line 131).
| import ImpersonationProvider from "@calcom/features/ee/impersonation/lib/ImpersonationProvider"; | ||
| import { getOrgFullOrigin, subdomainSuffix } from "@calcom/features/ee/organizations/lib/orgDomains"; | ||
| import { clientSecretVerifier, hostedCal, isSAMLLoginEnabled } from "@calcom/features/ee/sso/lib/saml"; | ||
| import { FeaturesRepository } from "@calcom/features/flags/features.repository"; |
There was a problem hiding this comment.
Remove unused import.
The FeaturesRepository import is not used anywhere in this file. Unused imports add unnecessary dependencies and should be removed.
Apply this diff to remove the unused import:
-import { FeaturesRepository } from "@calcom/features/flags/features.repository";📝 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.
| import { FeaturesRepository } from "@calcom/features/flags/features.repository"; |
🤖 Prompt for AI Agents
In packages/features/auth/lib/next-auth-options.ts around line 19, remove the
unused import statement "import { FeaturesRepository } from
\"@calcom/features/flags/features.repository\";" since it is not referenced
anywhere in the file; update imports to eliminate this unused dependency and run
the linter/TypeScript build to confirm no remaining unused-import errors.
| // Only send verification email if the feature is enabled | ||
| if (isEmailVerificationEnabled) { | ||
| sendEmailVerification({ | ||
| email, | ||
| language: await getLocaleFromRequest(buildLegacyRequest(await headers(), await cookies())), | ||
| username: username || "", | ||
| }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Inconsistent async handling of sendEmailVerification.
The sendEmailVerification call is not awaited here (fire-and-forget), but in selfHostedHandler.ts (line 181) it is awaited. This inconsistency could lead to different error handling behavior between the two handlers.
Consider either:
- Making both handlers consistent by awaiting the call in both places, or
- Documenting why fire-and-forget is intentional here but not in the self-hosted handler
Run the following script to confirm the inconsistency:
🏁 Script executed:
#!/bin/bash
# Description: Check for inconsistent await usage of sendEmailVerification across handlers
# Search for sendEmailVerification calls in both handlers
rg -n -A 2 -B 2 'sendEmailVerification\(' packages/features/auth/signup/handlers/Length of output: 1066
Await sendEmailVerification in calcomHandler.ts
In calcomHandler.ts (line 220), add await before sendEmailVerification(...) to match selfHostedHandler.ts and ensure consistent error handling.
🤖 Prompt for AI Agents
In packages/features/auth/signup/handlers/calcomHandler.ts around lines 218 to
225, the call to sendEmailVerification is not awaited so errors or async
completion aren't handled consistently with selfHostedHandler.ts; update the
code to await sendEmailVerification(...) and propagate or catch errors as
appropriate (i.e., prefix the call with await and ensure the containing function
is async and any surrounding error handling/try-catch remains correct) so email
sending is awaited and failures are handled consistently.
- Add unit test scaffolds for calcomHandler and selfHostedHandler - Update existing E2E test to verify emailVerified field is null when flag enabled - Add new E2E test to verify emailVerified is set when flag disabled - Verify email verification emails are sent/not sent based on flag state 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
- Add assertions for emailVerified value based on flag state - Verify Date object when flag disabled, null when enabled - Add verification for sendEmailVerification call behavior - Include detailed comments explaining test strategy 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Implement full integration tests that actually call the handlers - Test emailVerified is null when flag enabled, Date when disabled - Test sendEmailVerification is called/not called based on flag - Add comprehensive tests for both regular and team invite flows - Verify both create and update paths in upsert calls 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Work in progress - unit tests need more mocking setup for NextResponse and other dependencies. E2E tests are complete and working.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 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.
📒 Files selected for processing (1)
apps/web/playwright/signup.e2e.ts(2 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 useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
apps/web/playwright/signup.e2e.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:
apps/web/playwright/signup.e2e.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:
apps/web/playwright/signup.e2e.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). (3)
- GitHub Check: Tests / Unit
- GitHub Check: Type check / check-types
- GitHub Check: Linters / lint
🔇 Additional comments (1)
apps/web/playwright/signup.e2e.ts (1)
293-298: LGTM!The assertion correctly verifies that
emailVerifiedisnullwhen the email-verification flag is enabled, and properly usesselectto fetch only the required field.
| test("Email auto-verified when flag disabled", async ({ page, prisma, users, features }) => { | ||
| // Ensure email verification is disabled for this test | ||
| await prisma.feature.update({ | ||
| where: { slug: "email-verification" }, | ||
| data: { enabled: false }, | ||
| }); | ||
|
|
||
| const userToCreate = users.buildForSignup({ | ||
| email: `auto-verify-${Date.now()}@example.com`, | ||
| username: `auto-verify-${Date.now()}`, | ||
| password: "Password99!", | ||
| }); | ||
|
|
||
| await page.goto("/signup"); | ||
| await preventFlakyTest(page); | ||
| const continueWithEmailButton = page.getByTestId("continue-with-email-button"); | ||
| await expect(continueWithEmailButton).toBeVisible(); | ||
| await continueWithEmailButton.click(); | ||
|
|
||
| // Fill form | ||
| await page.locator('input[name="username"]').fill(userToCreate.username); | ||
| await page.locator('input[name="email"]').fill(userToCreate.email); | ||
| await page.locator('input[name="password"]').fill(userToCreate.password); | ||
|
|
||
| // Submit form | ||
| const submitButton = page.getByTestId("signup-submit-button"); | ||
| await submitButton.click(); | ||
|
|
||
| // Should NOT redirect to verify-email when flag is disabled | ||
| await page.waitForURL((url) => !url.pathname.includes("/auth/verify-email")); | ||
|
|
||
| // Verify that emailVerified is set to a date when flag is disabled | ||
| const dbUser = await prisma.user.findUnique({ | ||
| where: { email: userToCreate.email }, | ||
| select: { emailVerified: true }, | ||
| }); | ||
| expect(dbUser?.emailVerified).not.toBeNull(); | ||
| expect(dbUser?.emailVerified).toBeInstanceOf(Date); | ||
| }); |
There was a problem hiding this comment.
Strengthen the URL assertion after signup.
Line 343 waits for any URL that doesn't include /auth/verify-email, which could pass even if the page navigates to an error page or doesn't navigate at all. This creates a potential false positive in the test.
Replace line 343 with an explicit check for the expected success URL:
- // Should NOT redirect to verify-email when flag is disabled
- await page.waitForURL((url) => !url.pathname.includes("/auth/verify-email"));
+ // Should redirect to onboarding/getting-started when flag is disabled
+ await page.waitForURL("/getting-started?from=signup");Optional: Verify no email is sent.
Consider adding a check that no verification email is sent when the flag is disabled, similar to the email verification check in the other test (lines 300-313). This would more thoroughly validate the PR objective: "Verification emails are only sent when the flag is enabled."
📝 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.
| test("Email auto-verified when flag disabled", async ({ page, prisma, users, features }) => { | |
| // Ensure email verification is disabled for this test | |
| await prisma.feature.update({ | |
| where: { slug: "email-verification" }, | |
| data: { enabled: false }, | |
| }); | |
| const userToCreate = users.buildForSignup({ | |
| email: `auto-verify-${Date.now()}@example.com`, | |
| username: `auto-verify-${Date.now()}`, | |
| password: "Password99!", | |
| }); | |
| await page.goto("/signup"); | |
| await preventFlakyTest(page); | |
| const continueWithEmailButton = page.getByTestId("continue-with-email-button"); | |
| await expect(continueWithEmailButton).toBeVisible(); | |
| await continueWithEmailButton.click(); | |
| // Fill form | |
| await page.locator('input[name="username"]').fill(userToCreate.username); | |
| await page.locator('input[name="email"]').fill(userToCreate.email); | |
| await page.locator('input[name="password"]').fill(userToCreate.password); | |
| // Submit form | |
| const submitButton = page.getByTestId("signup-submit-button"); | |
| await submitButton.click(); | |
| // Should NOT redirect to verify-email when flag is disabled | |
| await page.waitForURL((url) => !url.pathname.includes("/auth/verify-email")); | |
| // Verify that emailVerified is set to a date when flag is disabled | |
| const dbUser = await prisma.user.findUnique({ | |
| where: { email: userToCreate.email }, | |
| select: { emailVerified: true }, | |
| }); | |
| expect(dbUser?.emailVerified).not.toBeNull(); | |
| expect(dbUser?.emailVerified).toBeInstanceOf(Date); | |
| }); | |
| test("Email auto-verified when flag disabled", async ({ page, prisma, users, features }) => { | |
| // Ensure email verification is disabled for this test | |
| await prisma.feature.update({ | |
| where: { slug: "email-verification" }, | |
| data: { enabled: false }, | |
| }); | |
| const userToCreate = users.buildForSignup({ | |
| email: `auto-verify-${Date.now()}@example.com`, | |
| username: `auto-verify-${Date.now()}`, | |
| password: "Password99!", | |
| }); | |
| await page.goto("/signup"); | |
| await preventFlakyTest(page); | |
| const continueWithEmailButton = page.getByTestId("continue-with-email-button"); | |
| await expect(continueWithEmailButton).toBeVisible(); | |
| await continueWithEmailButton.click(); | |
| // Fill form | |
| await page.locator('input[name="username"]').fill(userToCreate.username); | |
| await page.locator('input[name="email"]').fill(userToCreate.email); | |
| await page.locator('input[name="password"]').fill(userToCreate.password); | |
| // Submit form | |
| const submitButton = page.getByTestId("signup-submit-button"); | |
| await submitButton.click(); | |
| // Should redirect to onboarding/getting-started when flag is disabled | |
| await page.waitForURL("/getting-started?from=signup"); | |
| // Verify that emailVerified is set to a date when flag is disabled | |
| const dbUser = await prisma.user.findUnique({ | |
| where: { email: userToCreate.email }, | |
| select: { emailVerified: true }, | |
| }); | |
| expect(dbUser?.emailVerified).not.toBeNull(); | |
| expect(dbUser?.emailVerified).toBeInstanceOf(Date); | |
| }); |
🤖 Prompt for AI Agents
In apps/web/playwright/signup.e2e.ts around lines 314 to 352, replace the broad
await page.waitForURL((url) => !url.pathname.includes("/auth/verify-email"))
with an explicit assertion that the page reached the known post-signup success
URL (e.g. await expect(page).toHaveURL("/" ) or the actual success route your
app uses) so the test fails if it lands on an error or a different page;
optionally also add the no-email-sent verification by asserting the mailer was
not called or checking the outgoing_emails table/fixture to ensure no
verification email was queued when the feature flag is disabled.
E2E results are ready! |
Summary
Implements proper support for the
email-verificationfeature flag in the signup flow. When enabled, new users will haveemailVerifiedset tonulland receive a verification email. When disabled, users are automatically verified.Changes
email-verificationfeature flag during signup in both handlersemailVerified:emailVerified = null(requires verification)emailVerified = new Date()(auto-verified)Test plan
email-verificationflag disabled - user should be auto-verifiedemail-verificationflag enabled - user should receive verification email andemailVerifiedshould be nullGenerated with Claude Code