-
-
Notifications
You must be signed in to change notification settings - Fork 366
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
✨ Enable changing user email address #1493
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
WalkthroughThis pull request introduces a comprehensive email change functionality for users. The implementation spans multiple files across the application, including configuration updates, localization files, API routes, TRPC routers, and email templates. The changes enable users to request an email change, receive a verification email, and complete the process by clicking a verification link, with robust error handling and user feedback mechanisms. Changes
Assessment against linked issues
Possibly related PRs
Poem
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 4
🧹 Nitpick comments (11)
packages/emails/src/previews/change-email-request.tsx (1)
7-10
: Consider using more realistic example data.While the current example data is functional, using more realistic examples like
[email protected]
and a proper UUID for the token would make the preview more representative of production data.- fromEmail="[email protected]" - toEmail="[email protected]" - verificationUrl="https://rallly.co/verify-email-change?token=1234567890" + fromEmail="[email protected]" + toEmail="[email protected]" + verificationUrl="https://rallly.co/verify-email-change?token=123e4567-e89b-12d3-a456-426614174000"packages/emails/src/templates.ts (1)
1-1
: LGTM! Consider alphabetizing template entries.The ChangeEmailRequest template is correctly imported and registered. For better maintainability, consider alphabetizing the entries in the templates object.
const templates = { + ChangeEmailRequest, FinalizeHostEmail, FinalizeParticipantEmail, LoginEmail, NewCommentEmail, NewParticipantEmail, NewParticipantConfirmationEmail, NewPollEmail, RegisterEmail, - ChangeEmailRequest, };Also applies to: 21-21
apps/web/src/app/api/trpc/[trpc]/route.ts (1)
32-32
: Consider explicit null handling for email.While setting
undefined
as a fallback works, consider being more explicit about null handling sincesession.user.email
can be null. This would make the type system more accurate and the code's intent clearer.- email: session.user.email ?? undefined, + email: session.user.email === null ? undefined : session.user.email,packages/emails/src/templates/change-email-request.tsx (2)
61-67
: Make expiration time configurable and consistent.The hardcoded "10 minutes" expiration message should be configurable and consistent with the actual token expiration time.
+ interface ChangeEmailRequestProps { + ctx: EmailContext; + verificationUrl: string; + fromEmail: string; + toEmail: string; + expirationMinutes: number; + } <Text light> {ctx.t("changeEmailRequest_text3", { ns: "emails", + expirationMinutes, defaultValue: - "This link will expire in 10 minutes. If you did not request this change, please contact support.", + "This link will expire in {{expirationMinutes}} minutes. If you did not request this change, please contact support.", })} </Text>
15-70
: Add plain text fallback for email clients.Consider adding a plain text version of the email for better compatibility with email clients that don't support HTML.
apps/web/src/trpc/trpc.ts (1)
67-82
: Enhance type safety and error messaging.The current implementation could be improved for better type safety and user experience.
const email = ctx.user.email; if (!email) { throw new TRPCError({ code: "UNAUTHORIZED", - message: "Login is required", + message: "Email verification is required to perform this action", }); } +// Narrow the type to non-null email +const user: typeof ctx.user & { email: string } = { + ...ctx.user, + email, +}; return next({ ctx: { - user: { - ...ctx.user, - email, - }, + user, }, });apps/web/src/app/[locale]/(admin)/settings/profile/profile-settings.tsx (3)
44-80
: Add type validation for error messages.The error handling is comprehensive, but it would be safer to validate the error type against known values.
Consider this improvement:
if (error) { + const isKnownError = error === "invalidToken"; posthog.capture("email change failed", { error }); toast({ variant: "destructive", title: t("emailChangeFailed", { defaultValue: "Email change failed", }), description: - error === "invalidToken" + isKnownError ? t("emailChangeInvalidToken", { defaultValue: "The verification link is invalid or has expired. Please try again.", }) : t("emailChangeError", { defaultValue: "An error occurred while changing your email", }), }); Cookies.remove("email-change-error"); }
88-92
: Add client-side email validation.While the server validates the email, adding client-side validation would provide immediate feedback to users.
Consider adding email validation:
if (data.email !== user.email) { + const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; + if (!emailRegex.test(data.email)) { + toast({ + variant: "destructive", + title: t("invalidEmail", { + defaultValue: "Invalid email address", + }), + }); + return; + } posthog.capture("email change requested"); await requestEmailChange.mutateAsync({ email: data.email }); setDidRequestEmailChange(true); }
128-143
: Add loading state during email verification.The alert provides good feedback, but users should also see a loading state while the verification is being processed.
Consider adding a loading state:
{didRequestEmailChange ? ( - <Alert icon={InfoIcon}> + <Alert icon={requestEmailChange.isLoading ? Loader : InfoIcon}> <AlertTitle> <Trans i18nKey="emailChangeRequestSent" defaults="Verify your new email address" /> </AlertTitle> <AlertDescription> <Trans i18nKey="emailChangeRequestSentDescription" defaults="To complete the change, please check your email for a verification link." /> + {requestEmailChange.isLoading && ( + <p className="mt-2 text-sm text-muted-foreground"> + {t("sendingVerification", "Sending verification email...")} + </p> + )} </AlertDescription> </Alert> ) : null}packages/emails/locales/en/emails.json (1)
50-56
: Add security-related information to email templates.Consider adding translations for additional security notices in the verification email.
Add these translations:
"changeEmailRequest_preview": "Please verify your email address", "changeEmailRequest_heading": "Verify Your New Email Address", "changeEmailRequest_text1": "We've received a request to change the email address for your account.", "changeEmailRequest_text2": "To complete this change, please click the button below:", "changeEmailRequest_button": "Verify Email Address", "changeEmailRequest_subject": "Verify your new email address", - "changeEmailRequest_text3": "This link will expire in 10 minutes. If you did not request this change, please contact support." + "changeEmailRequest_text3": "This link will expire in 10 minutes. If you did not request this change, please contact support immediately as your account may be compromised.", + "changeEmailRequest_security": "For security, this request was made from: IP: {ipAddress}, Browser: {userAgent}", + "changeEmailRequest_warning": "Never share this verification link with anyone"apps/web/public/locales/en/app.json (1)
284-290
: Add more specific error message translations.Consider adding translations for specific error cases to provide clearer feedback to users.
Add these translations:
"emailChangeSuccess": "Email changed successfully", "emailChangeSuccessDescription": "Your email has been updated", "emailChangeFailed": "Email change failed", "emailChangeInvalidToken": "The verification link is invalid or has expired. Please try again.", "emailChangeError": "An error occurred while changing your email", + "emailChangeAlreadyInUse": "This email address is already in use", + "emailChangeTooManyRequests": "Too many requests. Please wait before trying again", + "emailChangeNoMatch": "New email must be different from current email", "emailChangeRequestSent": "Verify your new email address", "emailChangeRequestSentDescription": "To complete the change, please check your email for a verification link."
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
apps/web/i18next-scanner.config.js
(1 hunks)apps/web/public/locales/en/app.json
(1 hunks)apps/web/src/app/[locale]/(admin)/settings/profile/profile-settings.tsx
(5 hunks)apps/web/src/app/api/trpc/[trpc]/route.ts
(1 hunks)apps/web/src/app/api/user/verify-email-change/route.ts
(1 hunks)apps/web/src/trpc/context.ts
(1 hunks)apps/web/src/trpc/routers/user.ts
(2 hunks)apps/web/src/trpc/trpc.ts
(1 hunks)packages/emails/locales/en/emails.json
(1 hunks)packages/emails/src/previews/change-email-request.tsx
(1 hunks)packages/emails/src/templates.ts
(2 hunks)packages/emails/src/templates/change-email-request.tsx
(1 hunks)
🔇 Additional comments (3)
apps/web/src/trpc/context.ts (1)
10-10
: LGTM! Verify email property usage in authentication flow.The optional email property is correctly typed and aligns with the authentication requirements.
Let's verify the email property usage in the authentication flow:
✅ Verification successful
✓ Email property usage verified and correctly implemented
The optional email property in the context is properly used throughout the authentication flow:
- Used for guest vs authenticated user determination
- Validated in protected TRPC routes
- Properly handled in auth state management
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for places where the email property is used in authentication ast-grep --pattern 'user.$email'Length of output: 34
Script:
#!/bin/bash # Search for email-related patterns in authentication context echo "=== Direct email property access ===" rg -i "\.email" --type ts --type js -A 2 echo -e "\n=== Destructured email usage ===" ast-grep --pattern 'const { email } = $user' echo -e "\n=== Email in type definitions ===" ast-grep --pattern 'email: string'Length of output: 19732
apps/web/i18next-scanner.config.js (1)
4-4
: Verify the auth file path change and translation key impacts.The exclusion path has been updated, and the keySeparator option has been removed. These changes could affect existing translations.
Let's verify the changes:
✅ Verification successful
Auth file move and translation changes are safe ✅
- The auth.ts file has been cleanly moved from utils/ to src/
- Translation configuration changes are safe as all translations use a flat structure in app.json files
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if auth.ts exists in both old and new locations echo "Checking auth file locations:" fd auth.ts src/ # Check for any translation keys using dots that might be affected by keySeparator removal echo "Checking for potentially affected translation keys:" fd -e json . public/locales/ -x jq 'paths | select(.[]) | join(".")' {} \; | grep "\."Length of output: 486
Script:
#!/bin/bash # First, let's understand the repository structure echo "Repository structure:" ls -la # Find all auth-related files echo -e "\nAuth-related files:" fd -t f auth # Find translation config and locale files echo -e "\nTranslation-related files:" fd -t f -e json locale fd -t f i18nLength of output: 2511
Script:
#!/bin/bash # Check if old auth location still exists echo "Checking old auth location:" fd -t f . apps/web/src/utils/ # Find and check translation files in web app echo -e "\nChecking translation files and patterns:" fd -t f -e json . apps/web/public/locales/ || true fd -t f -e json . apps/web/src/locales/ || true # Check i18next scanner config content echo -e "\nChecking i18next scanner config:" cat apps/web/i18next-scanner.config.jsLength of output: 2919
apps/web/src/app/[locale]/(admin)/settings/profile/profile-settings.tsx (1)
1-1
: LGTM! Appropriate hooks and imports for email change functionality.The added imports and hooks provide the necessary functionality for:
- Analytics tracking with PostHog
- Toast notifications for user feedback
- Cookie management for handling verification states
- Email change mutation via TRPC
Also applies to: 11-11, 13-14, 27-28, 38-42
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.
Actionable comments posted: 2
🧹 Nitpick comments (2)
apps/web/src/app/[locale]/(admin)/settings/profile/profile-settings.tsx (2)
Line range hint
1-42
: LGTM! Consider adding type safety for form data.The component setup and imports look good. For better type safety, consider defining an interface for the form data.
interface ProfileFormData { name: string; email: string; } const form = useForm<ProfileFormData>({ defaultValues: { name: user.isGuest ? "" : user.name, email: user.email ?? "", }, });
128-143
: Enhance alert accessibility.The alert implementation looks good, but consider adding ARIA attributes for better screen reader support.
- <Alert icon={InfoIcon}> + <Alert + icon={InfoIcon} + role="alert" + aria-live="polite" + >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/src/app/[locale]/(admin)/settings/profile/profile-settings.tsx
(5 hunks)apps/web/src/app/api/user/verify-email-change/route.ts
(1 hunks)packages/emails/locales/en/emails.json
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/src/app/api/user/verify-email-change/route.ts
- packages/emails/locales/en/emails.json
🔇 Additional comments (1)
apps/web/src/app/[locale]/(admin)/settings/profile/profile-settings.tsx (1)
123-123
: Verify email validation implementation.Ensure proper email validation is implemented to prevent invalid email submissions.
✅ Verification successful
Email validation is properly implemented
The implementation includes:
- Client-side validation using Zod schema
- Server-side validation in the tRPC router
- Proper error handling and user feedback
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for email validation in the codebase echo "Searching for email validation..." rg -A 5 'requestEmailChange' --type typescript # Check for validation schema definitions echo "Checking validation schemas..." rg -A 10 'z.string\(\).email\(\)' --type typescriptLength of output: 318
Script:
#!/bin/bash # Search for email validation patterns echo "Searching for email validation..." rg -A 5 'email.*validation|validation.*email|z\.string\(\)\.email|validateEmail' # Search for form handling related to email echo "Searching for email-related form handling..." rg -A 10 'handleSubmit.*email|onSubmit.*email|requestEmailChange' # Search for email input validation echo "Looking for email input validation..." rg -A 5 'type=["'\'']email["'\'']|<Input.*email'Length of output: 8521
apps/web/src/app/[locale]/(admin)/settings/profile/profile-settings.tsx
Outdated
Show resolved
Hide resolved
apps/web/src/app/[locale]/(admin)/settings/profile/profile-settings.tsx
Outdated
Show resolved
Hide resolved
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.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/web/src/app/api/user/verify-email-change/route.ts (1)
60-70
:⚠️ Potential issueAdd rate limiting to prevent abuse.
The email verification endpoint should be rate limited to prevent abuse.
Consider implementing rate limiting middleware similar to the TRPC endpoints.
🧹 Nitpick comments (1)
apps/web/src/app/[locale]/(admin)/settings/profile/profile-email-address.tsx (1)
84-105
: Optimize form reset timing.The form is reset before the async operation completes, which could lead to a confusing UX if the operation fails.
Move the reset after successful completion:
onSubmit={handleSubmit(async (data) => { - reset(data); if (data.email !== user.email) { posthog.capture("email change requested"); const res = await requestEmailChange.mutateAsync({ email: data.email, }); if (res.success === false) { if (res.reason === "emailAlreadyInUse") { form.setError("email", { message: t("emailAlreadyInUse"), }); } } else { + reset(data); setDidRequestEmailChange(true); } } await refresh(); })}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/web/public/locales/en/app.json
(1 hunks)apps/web/src/app/[locale]/(admin)/settings/profile/profile-email-address.tsx
(1 hunks)apps/web/src/app/[locale]/(admin)/settings/profile/profile-page.tsx
(2 hunks)apps/web/src/app/[locale]/(admin)/settings/profile/profile-settings.tsx
(3 hunks)apps/web/src/app/api/user/verify-email-change/route.ts
(1 hunks)apps/web/src/trpc/routers/user.ts
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/src/app/[locale]/(admin)/settings/profile/profile-settings.tsx
- apps/web/public/locales/en/app.json
🔇 Additional comments (4)
apps/web/src/app/[locale]/(admin)/settings/profile/profile-page.tsx (1)
83-95
: LGTM! Well-structured settings section for email management.The new email settings section is properly integrated with clear i18n support and logical placement in the settings hierarchy.
apps/web/src/app/api/user/verify-email-change/route.ts (2)
13-18
:⚠️ Potential issueStrengthen cookie security configuration.
The current cookie configuration has security vulnerabilities:
httpOnly: false
allows JavaScript accesssecure: false
allows transmission over HTTP- Missing SameSite attribute
Apply this security enhancement:
const COOKIE_CONFIG = { path: "/", - httpOnly: false, - secure: false, + httpOnly: true, + secure: process.env.NODE_ENV === "production", + sameSite: "lax", expires: new Date(Date.now() + 5 * 1000), // 5 seconds } as const;Likely invalid or redundant comment.
27-58
:⚠️ Potential issueAdd token expiration check.
The function doesn't verify if the token has expired before processing it.
Add expiration check:
const handleEmailChange = async (token: string) => { const verificationToken = await prisma.verificationToken.findUnique({ where: { token }, }); if (!verificationToken) { setEmailChangeCookie("error", "invalidToken"); return false; } + if (new Date() > verificationToken.expires) { + setEmailChangeCookie("error", "tokenExpired"); + return false; + } const payload = await decryptToken<EmailChangePayload>(token);Likely invalid or redundant comment.
apps/web/src/trpc/routers/user.ts (1)
99-140
: LGTM! Robust implementation with proper security measures.The implementation includes essential security features:
- Rate limiting middleware
- Email existence check
- Proper token generation and expiration
- Secure email delivery
apps/web/src/app/[locale]/(admin)/settings/profile/profile-email-address.tsx
Show resolved
Hide resolved
3b2edc5
to
7ccd6d2
Compare
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.
Actionable comments posted: 0
♻️ Duplicate comments (2)
apps/web/src/app/[locale]/(admin)/settings/profile/profile-email-address.tsx (1)
43-77
: 🛠️ Refactor suggestionAdd cookie cleanup after processing.
The cookies should be removed after processing to prevent stale states:
React.useEffect(() => { const success = Cookies.get("email-change-success"); const error = Cookies.get("email-change-error"); if (success) { posthog.capture("email change completed"); toast({ title: t("emailChangeSuccess"), description: t("emailChangeSuccessDescription"), }); + Cookies.remove("email-change-success", { path: "/" }); } if (error) { posthog.capture("email change failed", { error }); toast({ variant: "destructive", title: t("emailChangeFailed"), description: error === "invalidToken" ? t("emailChangeInvalidToken") : t("emailChangeError"), }); + Cookies.remove("email-change-error", { path: "/" }); } }, [posthog, refresh, t, toast]);apps/web/src/trpc/routers/user.ts (1)
125-164
: 🛠️ Refactor suggestionEnhance security measures for email change.
While the implementation is solid with rate limiting and email uniqueness check, consider these additional security measures:
requestEmailChange: privateProcedure .use(rateLimitMiddleware) .input(z.object({ email: z.string().email() })) .mutation(async ({ input, ctx }) => { + // Check for recent email change requests + const recentToken = await prisma.verificationToken.findFirst({ + where: { + identifier: ctx.user.email, + createdAt: { gt: new Date(Date.now() - 5 * 60 * 1000) }, + }, + }); + + if (recentToken) { + throw new TRPCError({ + code: "TOO_MANY_REQUESTS", + message: "Please wait 5 minutes before requesting another email change", + }); + } // check if the email is already in use const existingUser = await prisma.user.count({ where: { email: input.email }, }); if (existingUser) { return { success: false as const, reason: "emailAlreadyInUse" as const, }; } // create a verification token const token = await createToken( { fromEmail: ctx.user.email, toEmail: input.email, }, { ttl: 60 * 10, }, ); + // Store token in database for validation + await prisma.verificationToken.create({ + data: { + identifier: ctx.user.email, + token, + expires: new Date(Date.now() + 1000 * 60 * 10), + createdAt: new Date(), + }, + }); ctx.user.getEmailClient().sendTemplate("ChangeEmailRequest", { to: input.email, props: { verificationUrl: absoluteUrl( `/api/user/verify-email-change?token=${token}`, ), fromEmail: ctx.user.email, toEmail: input.email, }, }); return { success: true as const }; }),
🧹 Nitpick comments (2)
apps/web/src/trpc/trpc.ts (1)
67-82
: Consider standardizing authentication checks across procedures.The authentication logic now differs between procedures:
privateProcedure
: Uses email presenceproProcedure
andpossiblyPublicProcedure
: UseisGuest
This inconsistency might lead to confusion. Consider standardizing the authentication approach across all procedures or documenting the rationale for the different checks.
apps/web/src/app/[locale]/(admin)/settings/profile/profile-email-address.tsx (1)
84-105
: Add debouncing to form submission.Consider adding debouncing to prevent rapid-fire form submissions:
+import { useDebouncedCallback } from "use-debounce"; + // ... in component +const debouncedSubmit = useDebouncedCallback( + async (data: { email: string }) => { reset(data); if (data.email !== user.email) { posthog.capture("email change requested"); const res = await requestEmailChange.mutateAsync({ email: data.email, }); // ... rest of the logic } await refresh(); + }, + 1000 +);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
apps/web/i18next-scanner.config.js
(1 hunks)apps/web/public/locales/en/app.json
(1 hunks)apps/web/src/app/[locale]/(admin)/settings/profile/profile-email-address.tsx
(1 hunks)apps/web/src/app/[locale]/(admin)/settings/profile/profile-page.tsx
(2 hunks)apps/web/src/app/[locale]/(admin)/settings/profile/profile-settings.tsx
(3 hunks)apps/web/src/app/api/trpc/[trpc]/route.ts
(1 hunks)apps/web/src/app/api/user/verify-email-change/route.ts
(1 hunks)apps/web/src/trpc/context.ts
(1 hunks)apps/web/src/trpc/routers/user.ts
(2 hunks)apps/web/src/trpc/trpc.ts
(1 hunks)packages/emails/locales/en/emails.json
(1 hunks)packages/emails/src/previews/change-email-request.tsx
(1 hunks)packages/emails/src/templates.ts
(2 hunks)packages/emails/src/templates/change-email-request.tsx
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (9)
- apps/web/src/trpc/context.ts
- packages/emails/src/templates.ts
- apps/web/i18next-scanner.config.js
- apps/web/src/app/api/trpc/[trpc]/route.ts
- apps/web/src/app/[locale]/(admin)/settings/profile/profile-page.tsx
- apps/web/src/app/[locale]/(admin)/settings/profile/profile-settings.tsx
- packages/emails/src/previews/change-email-request.tsx
- apps/web/public/locales/en/app.json
- packages/emails/locales/en/emails.json
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Type check
- GitHub Check: Integration tests
- GitHub Check: Linting
- GitHub Check: Unit tests
🔇 Additional comments (6)
apps/web/src/trpc/trpc.ts (2)
75-82
: Ensure type safety for the enriched context.The middleware adds the email property to the context. Verify that:
- The
TRPCContext
type properly includes this email property- Downstream handlers expecting this property are type-safe
Let's verify the type definitions and usage:
✅ Verification successful
Type safety verification successful
The
TRPCContext
, and its usage is protected by both compile-time type checking and runtime validation in the middleware.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for TRPCContext type definition and its usage ast-grep --pattern 'type TRPCContext = { $$$ }' # Search for downstream handlers using the email property rg -A 5 "ctx\.user\.email" --type tsLength of output: 1634
67-74
: Verify authentication requirements for email-based checks.The change from
isGuest
to email presence validation might allow guest users with an email set. Consider if this aligns with the authentication requirements.Let's verify the authentication flow:
✅ Verification successful
The email-based check is consistent with the authentication model.
The codebase consistently defines guest status based on email presence (
isGuest = !email
). The change fromisGuest
to email validation in privateProcedure maintains this invariant and is safe.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for guest user handling and email validation patterns rg -A 5 "isGuest|email.*validation" --type tsLength of output: 17567
apps/web/src/app/api/user/verify-email-change/route.ts (3)
46-64
:⚠️ Potential issueAdd rate limiting and improve error handling.
The endpoint needs protection against abuse and should provide less verbose error messages:
+import { rateLimit } from "@/utils/rate-limit"; + +const limiter = rateLimit({ + interval: 60 * 1000, // 1 minute + uniqueTokenPerInterval: 500, +}); + export const GET = async (request: NextRequest) => { + try { + await limiter.check(request, 3); // 3 requests per minute + } catch { + return NextResponse.json( + { error: "Too many requests" }, + { status: 429 } + ); + } + const token = request.nextUrl.searchParams.get("token"); if (!token) { - return NextResponse.json({ error: "No token provided" }, { status: 400 }); + return NextResponse.json({ error: "Invalid request" }, { status: 400 }); }Likely invalid or redundant comment.
14-19
:⚠️ Potential issueStrengthen cookie security configuration.
The current cookie configuration needs security improvements:
const COOKIE_CONFIG = { path: "/", - httpOnly: false, - secure: false, + httpOnly: true, + secure: process.env.NODE_ENV === "production", expires: new Date(Date.now() + 5 * 1000), // 5 seconds + sameSite: "lax", } as const;Likely invalid or redundant comment.
28-44
:⚠️ Potential issueEnhance token verification and database operations.
The current implementation needs improvements in token handling and database operations:
const handleEmailChange = async (token: string) => { + const verificationToken = await prisma.verificationToken.findUnique({ + where: { token }, + }); + + if (!verificationToken) { + setEmailChangeCookie("error", "invalidToken"); + return false; + } + + if (new Date() > verificationToken.expires) { + setEmailChangeCookie("error", "tokenExpired"); + return false; + } + const payload = await decryptToken<EmailChangePayload>(token); if (!payload) { setEmailChangeCookie("error", "invalidToken"); return false; } - await prisma.user.update({ - where: { email: payload.fromEmail }, - data: { email: payload.toEmail }, - }); + await prisma.$transaction(async (tx) => { + await tx.user.update({ + where: { + email: payload.fromEmail, + AND: { NOT: { email: payload.toEmail } } + }, + data: { email: payload.toEmail }, + }); + + await tx.verificationToken.delete({ + where: { token }, + }); + }); setEmailChangeCookie("success"); return true; };Likely invalid or redundant comment.
packages/emails/src/templates/change-email-request.tsx (1)
1-82
: LGTM! Well-structured email template with proper i18n support.The implementation demonstrates good practices:
- Clear separation of concerns
- Proper internationalization
- Informative user communication
Close #626
Summary by CodeRabbit
Release Notes
New Features
Localization Updates
User Experience Improvements
Security Enhancements