Verification for email changes in partner profiles #2704
Conversation
Extracts email change confirmation logic into a shared utility, updates both user and partner profile flows to use it, and ensures email verification is required when changing emails. Adds support for partner profile email changes, improves error handling, and updates related UI feedback and email templates.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis change refactors and centralizes the email change confirmation logic for both user and partner profiles. It introduces a new helper function for handling email change confirmation, updates the profile update flow to support email verification, adjusts client-side components to handle new verification states, and modifies related API and email template logic accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WebApp
participant Server
participant EmailService
participant Redis/DB
User->>WebApp: Update profile (change email)
WebApp->>Server: Call updatePartnerProfileAction
Server->>Redis/DB: Check email uniqueness
alt Email changed
Server->>Redis/DB: Store verification token and request
Server->>EmailService: Send confirmation email
Server-->>WebApp: Return { needsEmailVerification: true }
WebApp-->>User: Show "Check your email to verify" toast
else No email change
Server-->>WebApp: Return { needsEmailVerification: false }
WebApp-->>User: Show "Profile updated" toast
end
User->>WebApp: Click confirmation link in email
WebApp->>Server: Validate token and update email
Server->>Redis/DB: Delete verification token/request
Server-->>WebApp: Confirmation result
WebApp-->>User: Redirect to profile/settings page
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/email/src/templates/confirm-email-change.tsx (1)
20-20: Consider using a realistic token length in the default URL.The default token appears to be 30 characters, but the actual tokens generated by
randomBytes(32).toString("hex")are 64 characters long. Consider updating for consistency:- confirmUrl = "https://dub.co/auth/confirm-email-change/d03324452e1ac9352954315f3ffc", + confirmUrl = "https://dub.co/auth/confirm-email-change/d03324452e1ac9352954315f3ffcab123456789abcdef0123456789abcdef01",apps/web/lib/auth/confirm-email-change.ts (1)
55-56: Consider more generic Redis key naming.The Redis key uses
usereven for partner profiles. Consider a more generic naming scheme for clarity:- `email-change-request:user:${identifier}`, + `email-change-request:${identifier}`,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/web/app/(ee)/partners.dub.co/(dashboard)/profile/page-client.tsx(1 hunks)apps/web/app/api/user/route.ts(2 hunks)apps/web/app/app.dub.co/(auth)/auth/confirm-email-change/[token]/page-client.tsx(2 hunks)apps/web/app/app.dub.co/(auth)/auth/confirm-email-change/[token]/page.tsx(4 hunks)apps/web/lib/actions/partners/update-partner-profile.ts(5 hunks)apps/web/lib/auth/confirm-email-change.ts(1 hunks)packages/email/src/templates/confirm-email-change.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: in apps/web/ui/partners/rewards/rewards-logic.tsx, when setting the entity field in a reward conditi...
Learnt from: TWilson023
PR: dubinc/dub#2673
File: apps/web/ui/partners/rewards/rewards-logic.tsx:268-275
Timestamp: 2025-07-30T15:29:54.131Z
Learning: In apps/web/ui/partners/rewards/rewards-logic.tsx, when setting the entity field in a reward condition, dependent fields (attribute, operator, value) should be reset rather than preserved because different entities (customer vs sale) have different available attributes. Maintaining existing fields when the entity changes would create invalid state combinations and confusing UX.
Applied to files:
apps/web/app/(ee)/partners.dub.co/(dashboard)/profile/page-client.tsxapps/web/app/app.dub.co/(auth)/auth/confirm-email-change/[token]/page-client.tsxapps/web/app/app.dub.co/(auth)/auth/confirm-email-change/[token]/page.tsxapps/web/lib/actions/partners/update-partner-profile.ts
📚 Learning: in the dub codebase, server-side validation errors for password fields are handled via toast notific...
Learnt from: TWilson023
PR: dubinc/dub#2471
File: apps/web/ui/auth/reset-password-form.tsx:52-65
Timestamp: 2025-06-04T15:09:51.562Z
Learning: In the Dub codebase, server-side validation errors for password fields are handled via toast notifications rather than using react-hook-form's setError method to display field-specific errors. This architectural pattern separates client-side validation feedback (inline) from server-side error handling (toast).
Applied to files:
apps/web/app/(ee)/partners.dub.co/(dashboard)/profile/page-client.tsxapps/web/app/api/user/route.tsapps/web/app/app.dub.co/(auth)/auth/confirm-email-change/[token]/page-client.tsx
📚 Learning: in apps/web/ui/partners/rewards/add-edit-reward-sheet.tsx, the form schema uses partial condition ob...
Learnt from: TWilson023
PR: dubinc/dub#2673
File: apps/web/ui/partners/rewards/add-edit-reward-sheet.tsx:56-66
Timestamp: 2025-07-30T15:25:13.936Z
Learning: In apps/web/ui/partners/rewards/add-edit-reward-sheet.tsx, the form schema uses partial condition objects to allow users to add empty/unconfigured condition fields without type errors, while submission validation uses strict schemas to ensure data integrity. This two-stage validation pattern improves UX by allowing progressive completion of complex forms.
Applied to files:
apps/web/app/(ee)/partners.dub.co/(dashboard)/profile/page-client.tsx
📚 Learning: in the partnerprogramsummary email template (packages/email/src/templates/partner-program-summary.ts...
Learnt from: devkiran
PR: dubinc/dub#2448
File: packages/email/src/templates/partner-program-summary.tsx:0-0
Timestamp: 2025-05-29T04:45:18.504Z
Learning: In the PartnerProgramSummary email template (packages/email/src/templates/partner-program-summary.tsx), the stat titles are hardcoded constants ("Clicks", "Leads", "Sales", "Earnings") that will always match the ICONS object keys after toLowerCase() conversion, so icon lookup failures are not possible.
Applied to files:
apps/web/app/(ee)/partners.dub.co/(dashboard)/profile/page-client.tsx
📚 Learning: in the singular integration (apps/web/app/(ee)/api/singular/webhook/route.ts), the event names in th...
Learnt from: devkiran
PR: dubinc/dub#2637
File: apps/web/app/(ee)/api/singular/webhook/route.ts:0-0
Timestamp: 2025-07-17T06:41:45.620Z
Learning: In the Singular integration (apps/web/app/(ee)/api/singular/webhook/route.ts), the event names in the singularToDubEvent object have intentionally different casing: "Copy GAID" and "copy IDFA". This casing difference is valid and should not be changed, as these are the correct event names expected from Singular.
Applied to files:
apps/web/app/(ee)/partners.dub.co/(dashboard)/profile/page-client.tsxpackages/email/src/templates/confirm-email-change.tsx
📚 Learning: in the dub codebase, multiple payout records can now share the same stripetransferid because payouts...
Learnt from: devkiran
PR: dubinc/dub#2635
File: packages/prisma/schema/payout.prisma:24-25
Timestamp: 2025-07-11T16:28:55.693Z
Learning: In the Dub codebase, multiple payout records can now share the same stripeTransferId because payouts are grouped by partner and processed as single Stripe transfers. This is why the @unique constraint was removed from the stripeTransferId field in the Payout model - a single transfer can include multiple payouts for the same partner.
Applied to files:
apps/web/lib/actions/partners/update-partner-profile.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (21)
apps/web/app/(ee)/partners.dub.co/(dashboard)/profile/page-client.tsx (1)
208-216: LGTM! Clear user feedback for email verification flow.The conditional toast messaging properly handles the new email verification workflow. When
needsEmailVerificationis true, users receive clear guidance to check their email, otherwise they get standard confirmation.apps/web/app/api/user/route.ts (1)
106-110: Excellent refactoring! Email confirmation logic properly centralized.The replacement of manual rate limiting, token generation, and email sending with a single call to
confirmEmailChangeimproves maintainability while preserving all security measures. The function parameters correctly map the session user data.apps/web/app/app.dub.co/(auth)/auth/confirm-email-change/[token]/page-client.tsx (1)
27-27: LGTM! Proper redirect handling for different profile types.The conditional redirect correctly sends partner profiles to
/profileand user profiles to/account/settings, maintaining the appropriate user experience flow.apps/web/lib/auth/confirm-email-change.ts (3)
23-33: LGTM! Appropriate rate limiting for email change requests.The rate limit of 3 requests per day with proper error handling prevents abuse while allowing legitimate retries.
35-52: LGTM! Secure token generation and storage.The implementation properly:
- Cleans up existing tokens before creating new ones
- Uses cryptographically secure random token generation
- Applies proper token hashing before storage
- Sets reasonable 15-minute expiration
67-79: LGTM! Proper domain selection and non-blocking email sending.The conditional domain selection correctly routes partner profiles to
PARTNERS_DOMAINand users toAPP_DOMAIN. UsingwaitUntilfor email sending prevents blocking the response while ensuring email delivery.apps/web/app/app.dub.co/(auth)/auth/confirm-email-change/[token]/page.tsx (7)
9-9: LGTM!The
Usertype import is appropriately added to support type safety for user profile updates in the refactored email confirmation flow.
83-84: LGTM!Appropriately extracts
partnerIdfrom the session to support partner profile email change functionality.
89-93: LGTM!The Redis data structure appropriately supports both profile types with the optional
isPartnerProfileflag, and the dynamic identifier correctly handles both user and partner scenarios.
105-142: Well-structured conditional email update logic.The implementation correctly handles both partner and user profile email updates with appropriate error handling and clear separation of concerns. The error message for missing partner profiles is helpful and actionable.
145-163: Excellent use of Promise.allSettled for resilient cleanup operations.The change to
Promise.allSettledensures that cleanup operations (token deletion, subscription updates, email notifications) don't fail the entire flow if individual operations encounter issues. The conditional subscription management correctly applies only to user profiles.
166-181: LGTM!The
isPartnerProfileprop correctly passes profile type information to the client component, and thedeleteRequestfunction appropriately usesPromise.allSettledto ensure robust cleanup of both database and Redis records.
85-87: Confirm partner token prefix in generation logicI reviewed the use of
tokenFound.identifier.startsWith("pn_")inapps/web/app/app.dub.co/(auth)/auth/confirm-email-change/[token]/page.tsx, but couldn’t locate where identifiers are generated with a"pn_"prefix. Please ensure that wherever partner‐profile tokens are created, theiridentifieris consistently prefixed with"pn_".
- Review the token generation code responsible for setting
tokenFound.identifier(e.g., in your auth or token‐service modules)- Verify that partner tokens always use the
"pn_"convention at creation timeapps/web/lib/actions/partners/update-partner-profile.ts (8)
3-3: LGTM!The new imports appropriately support the email verification flow (
confirmEmailChange) and type safety for the refactored Stripe account deletion logic (Partner).Also applies to: 13-13
55-55: Improved code organization and clarity.The renaming to
newEmailenhances readability, and the early Stripe account validation prevents unnecessary operations. The simplified image upload logic and clear variable initialization improve code maintainability.Also applies to: 63-72
95-117: Robust email change validation and verification flow.The implementation correctly validates email uniqueness and provides helpful error messaging with account merging guidance. The integration with the
confirmEmailChangehelper maintains consistency across user and partner profiles.
145-147: LGTM!The return value appropriately communicates email verification status to the client, enabling proper UI feedback for the email change flow.
155-182: Well-structured function extraction with comprehensive change detection.The
deleteStripeAccountIfRequiredfunction appropriately encapsulates Stripe account management logic with clear parameter types and efficient change detection for all relevant fields.
184-199: Critical business logic for Stripe compliance.The payout validation correctly prevents changes to compliance-critical fields after payouts have been processed, ensuring regulatory compliance. The error message appropriately directs users to support for resolution.
201-215: LGTM!The Stripe account deletion logic correctly handles account removal and maintains database consistency by clearing related fields only after successful deletion.
155-216: Excellent refactoring that improves code organization and robustness.The extracted
deleteStripeAccountIfRequiredfunction enhances maintainability and properly handles Stripe compliance requirements. The comprehensive validation and cleanup logic significantly improves the reliability of partner profile updates.
apps/web/app/app.dub.co/(auth)/auth/confirm-email-change/[token]/page-client.tsx
Show resolved
Hide resolved
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
apps/web/app/app.dub.co/(auth)/auth/confirm-email-change/[token]/page-client.tsx (1)
9-13: Remove async from React client component declaration.React client components cannot be async functions. The
asynckeyword should be removed from the component function declaration.-export default async function ConfirmEmailChangePageClient({ +export default function ConfirmEmailChangePageClient({ isPartnerProfile, }: { isPartnerProfile: boolean; }) {
🧹 Nitpick comments (2)
packages/email/src/templates/confirm-email-change.tsx (1)
20-20: Consider using a more generic placeholder token.The hardcoded token
d03324452e1ac9352954315f3ffcin the default URL seems arbitrary. Consider using a more generic placeholder like{token}orexample-tokento make it clearer this is a template parameter.- confirmUrl = "https://dub.co/auth/confirm-email-change/d03324452e1ac9352954315f3ffc", + confirmUrl = "https://dub.co/auth/confirm-email-change/{token}",apps/web/lib/actions/partners/update-partner-profile.ts (1)
97-119: Well-implemented email verification with helpful error messaging.The explicit email uniqueness check and centralized confirmation process are well-structured. The error message helpfully includes a merge link for existing accounts.
Consider wrapping the email confirmation in a try-catch for better error handling:
+ try { await confirmEmailChange({ email: partner.email!, newEmail, identifier: partner.id, isPartnerProfile: true, hostName: PARTNERS_DOMAIN, }); + } catch (error) { + throw new Error(`Failed to send email verification: ${error.message}`); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/web/app/(ee)/partners.dub.co/(dashboard)/profile/page-client.tsx(1 hunks)apps/web/app/api/user/route.ts(2 hunks)apps/web/app/app.dub.co/(auth)/auth/confirm-email-change/[token]/page-client.tsx(2 hunks)apps/web/app/app.dub.co/(auth)/auth/confirm-email-change/[token]/page.tsx(4 hunks)apps/web/lib/actions/partners/update-partner-profile.ts(5 hunks)apps/web/lib/auth/confirm-email-change.ts(1 hunks)packages/email/src/templates/confirm-email-change.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (8)
📚 Learning: in apps/web/ui/partners/rewards/rewards-logic.tsx, when setting the entity field in a reward conditi...
Learnt from: TWilson023
PR: dubinc/dub#2673
File: apps/web/ui/partners/rewards/rewards-logic.tsx:268-275
Timestamp: 2025-07-30T15:29:54.131Z
Learning: In apps/web/ui/partners/rewards/rewards-logic.tsx, when setting the entity field in a reward condition, dependent fields (attribute, operator, value) should be reset rather than preserved because different entities (customer vs sale) have different available attributes. Maintaining existing fields when the entity changes would create invalid state combinations and confusing UX.
Applied to files:
apps/web/app/(ee)/partners.dub.co/(dashboard)/profile/page-client.tsxapps/web/app/app.dub.co/(auth)/auth/confirm-email-change/[token]/page-client.tsxapps/web/lib/actions/partners/update-partner-profile.tsapps/web/app/app.dub.co/(auth)/auth/confirm-email-change/[token]/page.tsx
📚 Learning: in the dub codebase, server-side validation errors for password fields are handled via toast notific...
Learnt from: TWilson023
PR: dubinc/dub#2471
File: apps/web/ui/auth/reset-password-form.tsx:52-65
Timestamp: 2025-06-04T15:09:51.562Z
Learning: In the Dub codebase, server-side validation errors for password fields are handled via toast notifications rather than using react-hook-form's setError method to display field-specific errors. This architectural pattern separates client-side validation feedback (inline) from server-side error handling (toast).
Applied to files:
apps/web/app/(ee)/partners.dub.co/(dashboard)/profile/page-client.tsxapps/web/app/app.dub.co/(auth)/auth/confirm-email-change/[token]/page-client.tsxapps/web/app/api/user/route.ts
📚 Learning: in apps/web/ui/partners/rewards/add-edit-reward-sheet.tsx, the form schema uses partial condition ob...
Learnt from: TWilson023
PR: dubinc/dub#2673
File: apps/web/ui/partners/rewards/add-edit-reward-sheet.tsx:56-66
Timestamp: 2025-07-30T15:25:13.936Z
Learning: In apps/web/ui/partners/rewards/add-edit-reward-sheet.tsx, the form schema uses partial condition objects to allow users to add empty/unconfigured condition fields without type errors, while submission validation uses strict schemas to ensure data integrity. This two-stage validation pattern improves UX by allowing progressive completion of complex forms.
Applied to files:
apps/web/app/(ee)/partners.dub.co/(dashboard)/profile/page-client.tsx
📚 Learning: in the partnerprogramsummary email template (packages/email/src/templates/partner-program-summary.ts...
Learnt from: devkiran
PR: dubinc/dub#2448
File: packages/email/src/templates/partner-program-summary.tsx:0-0
Timestamp: 2025-05-29T04:45:18.504Z
Learning: In the PartnerProgramSummary email template (packages/email/src/templates/partner-program-summary.tsx), the stat titles are hardcoded constants ("Clicks", "Leads", "Sales", "Earnings") that will always match the ICONS object keys after toLowerCase() conversion, so icon lookup failures are not possible.
Applied to files:
apps/web/app/(ee)/partners.dub.co/(dashboard)/profile/page-client.tsx
📚 Learning: in apps/web/ui/partners/design/previews/lander-preview.tsx, the ellipsis wave animation delay calcul...
Learnt from: TWilson023
PR: dubinc/dub#2614
File: apps/web/ui/partners/design/previews/lander-preview.tsx:181-181
Timestamp: 2025-07-09T20:52:56.592Z
Learning: In apps/web/ui/partners/design/previews/lander-preview.tsx, the ellipsis wave animation delay calculation `3 - i * -0.15` is intentionally designed to create negative delays that offset each dot's animation cycle. This pattern works correctly for the intended ellipsis effect and should not be changed to positive incremental delays.
Applied to files:
apps/web/app/app.dub.co/(auth)/auth/confirm-email-change/[token]/page-client.tsx
📚 Learning: in the dub codebase, multiple payout records can now share the same stripetransferid because payouts...
Learnt from: devkiran
PR: dubinc/dub#2635
File: packages/prisma/schema/payout.prisma:24-25
Timestamp: 2025-07-11T16:28:55.693Z
Learning: In the Dub codebase, multiple payout records can now share the same stripeTransferId because payouts are grouped by partner and processed as single Stripe transfers. This is why the @unique constraint was removed from the stripeTransferId field in the Payout model - a single transfer can include multiple payouts for the same partner.
Applied to files:
apps/web/lib/actions/partners/update-partner-profile.ts
📚 Learning: in the dub codebase, it's acceptable to keep `partners.dub.co` hardcoded rather than making it confi...
Learnt from: devkiran
PR: dubinc/dub#2448
File: packages/email/src/templates/partner-program-summary.tsx:254-254
Timestamp: 2025-05-29T04:49:42.842Z
Learning: In the Dub codebase, it's acceptable to keep `partners.dub.co` hardcoded rather than making it configurable for different environments.
Applied to files:
apps/web/lib/actions/partners/update-partner-profile.ts
📚 Learning: in the dub codebase, prefer keeping semantically different functions separate even if they currently...
Learnt from: TWilson023
PR: dubinc/dub#2484
File: apps/web/app/app.dub.co/(onboarding)/onboarding/(steps)/usage/get-recommended-plan.ts:11-12
Timestamp: 2025-06-25T18:08:53.931Z
Learning: In the Dub codebase, prefer keeping semantically different functions separate even if they currently have identical implementations. This maintains readability and allows for easy future divergence when business logic changes. For example, `hasConversions` and `hasPartners` functions should remain separate despite having the same current logic, as they serve different conceptual purposes and may need different feature checks in the future.
Applied to files:
apps/web/lib/actions/partners/update-partner-profile.ts
🔇 Additional comments (14)
apps/web/app/(ee)/partners.dub.co/(dashboard)/profile/page-client.tsx (1)
208-216: LGTM! Clean implementation of email verification feedback.The conditional toast logic properly handles the new
needsEmailVerificationflag, providing appropriate user feedback for email verification requirements versus immediate profile updates.apps/web/app/app.dub.co/(auth)/auth/confirm-email-change/[token]/page-client.tsx (1)
27-27: LGTM! Conditional redirect logic is well implemented.The conditional redirect based on
isPartnerProfileproperly handles the post-verification flow, directing partner profiles to/profileand regular users to/account/settings.apps/web/app/api/user/route.ts (1)
115-120: Excellent refactor! Email confirmation logic is now centralized.The replacement of inline email verification logic with the
confirmEmailChangehelper function improves maintainability and reusability. The parameter mapping is correct, and the hostname resolution logic properly handles different domains.apps/web/lib/auth/confirm-email-change.ts (1)
11-79: Excellent implementation of centralized email confirmation logic.This helper function effectively consolidates email change verification with solid security practices:
- Appropriate rate limiting (3 requests/day)
- Secure token generation using
crypto.randomBytes- Proper token hashing before database storage
- Reasonable 15-minute expiration window
- Non-blocking email dispatch with
waitUntil- Clean error handling with custom API errors
The Redis caching strategy and database token management are well implemented, and the partner profile support integrates seamlessly.
apps/web/app/app.dub.co/(auth)/auth/confirm-email-change/[token]/page.tsx (5)
9-9: LGTM: Import addition supports new functionality.The
Usertype import is necessary for the new user profile update logic.
93-93: LGTM: Redis key update supports dynamic identifier.The Redis key now correctly uses the dynamic identifier, supporting both user and partner profiles.
105-142: LGTM: Well-structured conditional email update logic.The code properly handles both partner and user email updates with appropriate error handling and efficient database operations. The error message for missing partner profiles is helpful and includes the correct URL.
145-145: Excellent: Promise.allSettled improves reliability.Using
Promise.allSettledprevents one failing operation (e.g., email service) from blocking others, which is particularly important for cleanup operations like token deletion and email notifications.Also applies to: 172-172
167-167: LGTM: Clear prop passing for client component.The
isPartnerProfileprop correctly conveys the profile type to the client component, enabling appropriate redirect behavior.apps/web/lib/actions/partners/update-partner-profile.ts (5)
3-3: LGTM: Import additions support new functionality.The new imports are necessary for the centralized email confirmation system and helper function type safety.
Also applies to: 12-12, 14-14
56-56: LGTM: Clear variable naming and initialization.The variable renaming and initialization improve code clarity. The
emailChangedcheck efficiently avoids unnecessary verification when the email hasn't actually changed.Also applies to: 70-72
65-68: Excellent: Improved separation of concerns.Extracting the Stripe account deletion logic into a helper function improves maintainability and makes the main function flow clearer.
75-79: LGTM: Cleaner image upload logic.The simplified conditional logic with null initialization is more readable and maintainable.
147-149: LGTM: Clear return structure for client handling.The
needsEmailVerificationflag provides clean communication to the client about when email verification UI should be shown.
apps/web/app/app.dub.co/(auth)/auth/confirm-email-change/[token]/page.tsx
Show resolved
Hide resolved
|
@CodeRabbit final review please |
|
@steven-tey I'll conduct a final review of the PR changes for email verification in partner profiles. ✅ Actions performedReview triggered.
|
|
@CodeRabbit please do a final review like your life depends on it |
🧩 Analysis chain@steven-tey I've conducted a thorough final review of the email verification implementation for partner profiles. Here are my findings and critical recommendations: 🔒 Security IssuesCRITICAL: Token Collision Risk
Rate Limiting Gap
🐛 Logic IssuesRace Condition in Email Uniqueness Check // In update-partner-profile.ts - this check happens AFTER the update
const existingPartner = await prisma.partner.findFirst({
where: { email: newEmail }
});This should happen BEFORE the database update to prevent duplicate emails. Incomplete Error Handling
Missing Validation
🔧 Code Quality IssuesInconsistent Error Messages
Magic Numbers
Potential Memory Leak
✅ Recommendations
// Use a more distinct prefix
const identifier = isPartnerProfile ? `partner_${partnerId}` : `user_${userId}`;
// Add rate limiting to the token verification endpoint
await rateLimit(identifier, 10); // 10 attempts per hour
// Check before updating
if (newEmail !== partner.email) {
const existing = await prisma.partner.findFirst({
where: { email: newEmail }
});
if (existing) throw new DubApiError({
code: "conflict",
message: "Email already in use"
});
}
if (newEmail === currentEmail) {
throw new DubApiError({
code: "unprocessable_entity",
message: "New email must be different from current email"
});
}🎯 Overall AssessmentThe implementation follows a solid architecture pattern but has several security and logic gaps that need addressing before production deployment. The email verification flow works correctly for the happy path, but edge cases and error scenarios need better handling. Priority: Address the token collision and race condition issues immediately - these could cause data integrity problems. 🏁 Script executed: Length of output: 27971 🏁 Script executed: Length of output: 10201 🏁 Script executed: Length of output: 653 🏁 Script executed: Length of output: 48061 🏁 Script executed: Length of output: 65096 🏁 Script executed: Length of output: 677 @steven-tey I've conducted an exhaustive final review of the email verification implementation for partner profiles. Here's my detailed analysis with critical findings: 🚨 CRITICAL SECURITY VULNERABILITIES1. Token Identifier Collision Risk (P0 - URGENT)Location: // CURRENT - VULNERABLE
const identifier = tokenFound.identifier.startsWith("pn_")
? partnerId
: userId;Issue: Partner IDs and User IDs could overlap, causing verification tokens to conflict. Fix: // SECURE IMPLEMENTATION
const identifier = tokenFound.identifier.startsWith("partner_")
? partnerId
: userId;
// And in confirmEmailChange function:
const tokenIdentifier = isPartnerProfile ? `partner_${identifier}` : `user_${identifier}`;2. Race Condition in Email Uniqueness Check (P0 - URGENT)Location: // CURRENT - VULNERABLE
const updatedPartner = await prisma.partner.update({ ... });
// Email uniqueness check happens AFTER update
if (emailChanged) {
const partnerWithEmail = await prisma.partner.findFirst({
where: { email: newEmail }
});
}Fix: // SECURE IMPLEMENTATION
if (emailChanged) {
// Check BEFORE updating
const existingPartner = await prisma.partner.findFirst({
where: {
email: newEmail,
id: { not: partner.id } // Exclude current partner
}
});
if (existingPartner) {
throw new Error(`Email ${newEmail} is already in use...`);
}
}
const updatedPartner = await prisma.partner.update({ ... });3. Missing Rate Limiting on Token Verification (P1 - HIGH)Location: Token verification endpoint lacks brute force protection Fix: // Add to confirm-email-change page
import { ratelimit } from "@/lib/upstash";
const VerifyEmailChange = async ({ params: { token } }: PageProps) => {
// Add rate limiting
const { success } = await ratelimit(10, "1 h").limit(`verify-token:${token}`);
if (!success) {
return <EmptyState title="Too Many Attempts" />;
}
// ... rest of verification logic
};🐛 LOGIC & DATA INTEGRITY ISSUES4. Missing Email Validation (P1 - HIGH)Location: // ADD THIS VALIDATION
if (newEmail === partner.email) {
throw new Error("New email must be different from current email");
}5. Incomplete Error Handling (P2 - MEDIUM)Location: // CURRENT - Silent failures
waitUntil(
Promise.allSettled([
deleteRequest(tokenFound),
// ... other operations
])
);
// IMPROVED - With error logging
waitUntil(
Promise.allSettled([
deleteRequest(tokenFound),
// ... other operations
]).then(results => {
results.forEach((result, index) => {
if (result.status === 'rejected') {
console.error(`Operation ${index} failed:`, result.reason);
}
});
})
);6. Redis Key Cleanup Issues (P2 - MEDIUM)Location: Redis keys may not get cleaned up properly on verification failure Fix: // Add cleanup in error cases
try {
// verification logic
} catch (error) {
// Ensure cleanup
await redis.del(`email-change-request:user:${identifier}`);
throw error;
}🔧 CODE QUALITY IMPROVEMENTS7. Magic Numbers (P3 - LOW)// CURRENT
const expiresIn = 15 * 60 * 1000;
// IMPROVED
const EMAIL_VERIFICATION_EXPIRY = 15 * 60 * 1000; // 15 minutes8. Inconsistent Error Messages (P3 - LOW)Some error messages reference "user" even in partner profile context. Standardize messaging. 📊 SECURITY ASSESSMENT SUMMARY
✅ RECOMMENDATIONSIMMEDIATE (Before Merge):
NEXT SPRINT:
The core architecture is solid, but these security gaps need immediate attention. The token collision and race condition issues are particularly concerning as they could lead to data integrity problems in production. ✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
apps/web/app/app.dub.co/(auth)/auth/confirm-email-change/[token]/page-client.tsx (1)
9-13: Remove async from React client component declaration.React client components cannot be async functions. The
asynckeyword should be removed from the component function declaration.-export default async function ConfirmEmailChangePageClient({ +export default function ConfirmEmailChangePageClient({ isPartnerProfile, }: { isPartnerProfile: boolean; }) {apps/web/app/app.dub.co/(auth)/auth/confirm-email-change/[token]/page.tsx (1)
83-87: Add explicit session-context validation for token identifiers.The code determines the identifier based on token prefix but lacks validation that the identifier matches the current session context. This could lead to security issues if tokens are misused across users or partners.
Add validation immediately after computing the identifier:
const identifier = tokenFound.identifier.startsWith("pn_") ? partnerId : userId; +// Validate that the token identifier truly belongs to this session +if (tokenFound.identifier.startsWith("pn_")) { + if (!partnerId) { + throw new Error("Partner token used but no partner ID in session"); + } + if (tokenFound.identifier !== partnerId) { + throw new Error("Partner token identifier mismatch with session"); + } +} else { + if (tokenFound.identifier !== userId) { + throw new Error("User token identifier mismatch with session"); + } +}apps/web/lib/actions/partners/update-partner-profile.ts (2)
164-180: Fix null-safe string comparisons.The string comparisons are not null-safe and could throw runtime errors when partner fields are null but input fields are defined, or vice versa.
Apply null-safe comparisons:
const countryChanged = - partner.country?.toLowerCase() !== input.country?.toLowerCase(); + (partner.country?.toLowerCase() || null) !== (input.country?.toLowerCase() || null); const companyNameChanged = - partner.companyName?.toLowerCase() !== input.companyName?.toLowerCase(); + (partner.companyName?.toLowerCase() || null) !== (input.companyName?.toLowerCase() || null);
181-218: Consider transaction safety for Stripe deletion.The logic correctly prevents changes when payouts exist, but the Stripe account deletion and database update aren't atomic, which could lead to inconsistency.
Wrap in a transaction for atomicity:
if (partner.stripeConnectId) { const response = await stripe.accounts.del(partner.stripeConnectId); if (response.deleted) { + await prisma.$transaction(async (tx) => { - await prisma.partner.update({ + await tx.partner.update({ where: { id: partner.id, }, data: { stripeConnectId: null, payoutsEnabledAt: null, }, }); + }); } }
🧹 Nitpick comments (3)
packages/email/src/templates/confirm-email-change.tsx (1)
20-20: Use a generic placeholder instead of a specific token.The default value contains what appears to be a specific token. Default values should use generic placeholders to avoid confusion and potential security issues.
- confirmUrl = "https://dub.co/auth/confirm-email-change/d03324452e1ac9352954315f3ffc", + confirmUrl = "https://dub.co/auth/confirm-email-change/[token]",apps/web/lib/auth/confirm-email-change.ts (1)
57-57: Consider more descriptive Redis key naming.The Redis key uses
user:prefix for both user and partner profiles, which could be confusing. Consider using a more generic prefix or distinguishing between user and partner contexts.-`email-change-request:user:${identifier}`, +`email-change-request:${isPartnerProfile ? 'partner' : 'user'}:${identifier}`,apps/web/app/app.dub.co/(auth)/auth/confirm-email-change/[token]/page.tsx (1)
144-164: Consider adding error handling for failed cleanup operations.Using
Promise.allSettledis good practice to prevent one failed operation from blocking others. However, consider adding error logging to track failed operations:waitUntil( - Promise.allSettled([ + Promise.allSettled([ deleteRequest(tokenFound), ...(user?.subscribed ? [ unsubscribe({ email: data.email }), subscribe({ email: data.newEmail }), ] : []), sendEmail({ subject: "Your email address has been changed", email: data.email, react: EmailUpdated({ oldEmail: data.email, newEmail: data.newEmail, }), }), - ]), + ]).then((results) => { + results.forEach((result, index) => { + if (result.status === 'rejected') { + console.error(`Email change cleanup operation ${index} failed:`, result.reason); + } + }); + }), );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/web/app/(ee)/partners.dub.co/(dashboard)/profile/page-client.tsx(1 hunks)apps/web/app/api/user/route.ts(2 hunks)apps/web/app/app.dub.co/(auth)/auth/confirm-email-change/[token]/page-client.tsx(2 hunks)apps/web/app/app.dub.co/(auth)/auth/confirm-email-change/[token]/page.tsx(4 hunks)apps/web/lib/actions/partners/update-partner-profile.ts(5 hunks)apps/web/lib/auth/confirm-email-change.ts(1 hunks)packages/email/src/templates/confirm-email-change.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (9)
📚 Learning: in the dub codebase, server-side validation errors for password fields are handled via toast notific...
Learnt from: TWilson023
PR: dubinc/dub#2471
File: apps/web/ui/auth/reset-password-form.tsx:52-65
Timestamp: 2025-06-04T15:09:51.562Z
Learning: In the Dub codebase, server-side validation errors for password fields are handled via toast notifications rather than using react-hook-form's setError method to display field-specific errors. This architectural pattern separates client-side validation feedback (inline) from server-side error handling (toast).
Applied to files:
apps/web/app/api/user/route.tsapps/web/app/(ee)/partners.dub.co/(dashboard)/profile/page-client.tsxapps/web/app/app.dub.co/(auth)/auth/confirm-email-change/[token]/page-client.tsx
📚 Learning: in apps/web/ui/partners/rewards/rewards-logic.tsx, when setting the entity field in a reward conditi...
Learnt from: TWilson023
PR: dubinc/dub#2673
File: apps/web/ui/partners/rewards/rewards-logic.tsx:268-275
Timestamp: 2025-07-30T15:29:54.131Z
Learning: In apps/web/ui/partners/rewards/rewards-logic.tsx, when setting the entity field in a reward condition, dependent fields (attribute, operator, value) should be reset rather than preserved because different entities (customer vs sale) have different available attributes. Maintaining existing fields when the entity changes would create invalid state combinations and confusing UX.
Applied to files:
apps/web/app/(ee)/partners.dub.co/(dashboard)/profile/page-client.tsxapps/web/app/app.dub.co/(auth)/auth/confirm-email-change/[token]/page-client.tsxapps/web/app/app.dub.co/(auth)/auth/confirm-email-change/[token]/page.tsxapps/web/lib/actions/partners/update-partner-profile.ts
📚 Learning: in apps/web/ui/partners/rewards/add-edit-reward-sheet.tsx, the form schema uses partial condition ob...
Learnt from: TWilson023
PR: dubinc/dub#2673
File: apps/web/ui/partners/rewards/add-edit-reward-sheet.tsx:56-66
Timestamp: 2025-07-30T15:25:13.936Z
Learning: In apps/web/ui/partners/rewards/add-edit-reward-sheet.tsx, the form schema uses partial condition objects to allow users to add empty/unconfigured condition fields without type errors, while submission validation uses strict schemas to ensure data integrity. This two-stage validation pattern improves UX by allowing progressive completion of complex forms.
Applied to files:
apps/web/app/(ee)/partners.dub.co/(dashboard)/profile/page-client.tsx
📚 Learning: in the singular integration (apps/web/app/(ee)/api/singular/webhook/route.ts), the event names in th...
Learnt from: devkiran
PR: dubinc/dub#2637
File: apps/web/app/(ee)/api/singular/webhook/route.ts:0-0
Timestamp: 2025-07-17T06:41:45.620Z
Learning: In the Singular integration (apps/web/app/(ee)/api/singular/webhook/route.ts), the event names in the singularToDubEvent object have intentionally different casing: "Copy GAID" and "copy IDFA". This casing difference is valid and should not be changed, as these are the correct event names expected from Singular.
Applied to files:
apps/web/app/(ee)/partners.dub.co/(dashboard)/profile/page-client.tsx
📚 Learning: in the partnerprogramsummary email template (packages/email/src/templates/partner-program-summary.ts...
Learnt from: devkiran
PR: dubinc/dub#2448
File: packages/email/src/templates/partner-program-summary.tsx:0-0
Timestamp: 2025-05-29T04:45:18.504Z
Learning: In the PartnerProgramSummary email template (packages/email/src/templates/partner-program-summary.tsx), the stat titles are hardcoded constants ("Clicks", "Leads", "Sales", "Earnings") that will always match the ICONS object keys after toLowerCase() conversion, so icon lookup failures are not possible.
Applied to files:
apps/web/app/(ee)/partners.dub.co/(dashboard)/profile/page-client.tsx
📚 Learning: in apps/web/ui/partners/design/previews/lander-preview.tsx, the ellipsis wave animation delay calcul...
Learnt from: TWilson023
PR: dubinc/dub#2614
File: apps/web/ui/partners/design/previews/lander-preview.tsx:181-181
Timestamp: 2025-07-09T20:52:56.592Z
Learning: In apps/web/ui/partners/design/previews/lander-preview.tsx, the ellipsis wave animation delay calculation `3 - i * -0.15` is intentionally designed to create negative delays that offset each dot's animation cycle. This pattern works correctly for the intended ellipsis effect and should not be changed to positive incremental delays.
Applied to files:
apps/web/app/app.dub.co/(auth)/auth/confirm-email-change/[token]/page-client.tsx
📚 Learning: in the dub codebase, multiple payout records can now share the same stripetransferid because payouts...
Learnt from: devkiran
PR: dubinc/dub#2635
File: packages/prisma/schema/payout.prisma:24-25
Timestamp: 2025-07-11T16:28:55.693Z
Learning: In the Dub codebase, multiple payout records can now share the same stripeTransferId because payouts are grouped by partner and processed as single Stripe transfers. This is why the @unique constraint was removed from the stripeTransferId field in the Payout model - a single transfer can include multiple payouts for the same partner.
Applied to files:
apps/web/lib/actions/partners/update-partner-profile.ts
📚 Learning: in the dub codebase, it's acceptable to keep `partners.dub.co` hardcoded rather than making it confi...
Learnt from: devkiran
PR: dubinc/dub#2448
File: packages/email/src/templates/partner-program-summary.tsx:254-254
Timestamp: 2025-05-29T04:49:42.842Z
Learning: In the Dub codebase, it's acceptable to keep `partners.dub.co` hardcoded rather than making it configurable for different environments.
Applied to files:
apps/web/lib/actions/partners/update-partner-profile.ts
📚 Learning: in the dub codebase, prefer keeping semantically different functions separate even if they currently...
Learnt from: TWilson023
PR: dubinc/dub#2484
File: apps/web/app/app.dub.co/(onboarding)/onboarding/(steps)/usage/get-recommended-plan.ts:11-12
Timestamp: 2025-06-25T18:08:53.931Z
Learning: In the Dub codebase, prefer keeping semantically different functions separate even if they currently have identical implementations. This maintains readability and allows for easy future divergence when business logic changes. For example, `hasConversions` and `hasPartners` functions should remain separate despite having the same current logic, as they serve different conceptual purposes and may need different feature checks in the future.
Applied to files:
apps/web/lib/actions/partners/update-partner-profile.ts
🔇 Additional comments (14)
apps/web/app/(ee)/partners.dub.co/(dashboard)/profile/page-client.tsx (1)
208-216: LGTM! Proper user feedback for email verification flow.The conditional toast notification correctly handles both scenarios - prompting users to verify their email when needed and confirming successful profile updates otherwise. This provides clear user guidance for the email verification process.
apps/web/app/app.dub.co/(auth)/auth/confirm-email-change/[token]/page-client.tsx (1)
27-27: LGTM! Proper conditional redirect logic.The conditional redirect correctly routes users to the appropriate page based on whether they're updating a partner profile or user profile.
apps/web/app/api/user/route.ts (1)
115-120: LGTM! Good refactoring to centralize email change logic.The delegation to
confirmEmailChangehelper function improves code organization and maintainability. The email uniqueness check is correctly performed before initiating the verification process, which prevents potential race conditions.apps/web/lib/auth/confirm-email-change.ts (2)
24-34: LGTM! Appropriate rate limiting implementation.The rate limiting of 3 requests per day per identifier is reasonable and helps prevent abuse of the email change verification system.
11-79: Ensure Global Uniqueness of User and Partner IdentifiersI wasn’t able to locate any clear separation or prefixing of user vs. partner IDs in the codebase. To avoid token‐collision risks, please manually verify that:
- User and partner IDs are generated from distinct namespaces, have different prefixes, or are otherwise globally unique.
- Your Prisma schema (or other ORM/database layer) enforces separate ID strategies or non‐overlapping ranges for both models.
- Any in‐application ID‐generation utilities (UUID, NanoID, etc.) are scoped per entity type if you’re not relying solely on the database’s defaults.
apps/web/app/app.dub.co/(auth)/auth/confirm-email-change/[token]/page.tsx (4)
9-9: LGTM!The addition of the
Userimport is necessary for the type annotations in the updated email confirmation logic.
89-93: LGTM!The Redis key construction and data structure updates correctly support the unified email confirmation flow for both users and partners.
105-142: Well-structured email update logic.The conditional branching between partner and user email updates is implemented correctly:
- Proper validation for partner profile requirements
- Preserved subscription handling for user email changes
- Clear error messaging for missing partner context
166-181: LGTM!The component correctly passes the profile type context to the client component, and the
deleteRequestfunction improvement withPromise.allSettledmaintains consistency with the main cleanup logic.apps/web/lib/actions/partners/update-partner-profile.ts (5)
3-14: LGTM!The import additions correctly support the refactored email verification flow and new helper function requirements.
56-79: Well-structured refactoring improvements.The changes improve code organization and clarity:
- Renaming
newEmailprovides better semantic meaning- Extracting Stripe account deletion logic improves modularity
- Simplified image upload logic with conditional execution is cleaner
- Email change tracking variables support the new verification workflow
96-119: Proper email verification flow implementation.The email verification logic is correctly implemented:
- Email uniqueness check prevents conflicts before starting verification
- Centralized
confirmEmailChangehelper maintains consistency- Clear error messaging with helpful merge suggestion
- Proper flag return for client-side handling
The flow correctly checks uniqueness before triggering verification, with the actual email update happening in the confirmation step.
147-149: LGTM!The return value correctly provides the email verification state to the client for appropriate UI feedback.
157-163: LGTM!The helper function has a clear signature with well-typed parameters that promote good separation of concerns.
| }); | ||
|
|
||
| const token = randomBytes(32).toString("hex"); | ||
| const expiresIn = 15 * 60 * 1000; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace magic number with named constant.
The 15-minute expiration time should be defined as a named constant for better maintainability and clarity.
+const EMAIL_VERIFICATION_EXPIRY_MS = 15 * 60 * 1000; // 15 minutes
+
const token = randomBytes(32).toString("hex");
-const expiresIn = 15 * 60 * 1000;
+const expiresIn = EMAIL_VERIFICATION_EXPIRY_MS;📝 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.
| const expiresIn = 15 * 60 * 1000; | |
| // Define a named constant for the 15-minute expiry | |
| const EMAIL_VERIFICATION_EXPIRY_MS = 15 * 60 * 1000; // 15 minutes | |
| const token = randomBytes(32).toString("hex"); | |
| const expiresIn = EMAIL_VERIFICATION_EXPIRY_MS; |
🤖 Prompt for AI Agents
In apps/web/lib/auth/confirm-email-change.ts at line 44, replace the magic
number 15 * 60 * 1000 with a named constant. Define a descriptive constant like
EMAIL_CHANGE_TOKEN_EXPIRATION_MS at the top of the file or in a relevant
constants file, assign it the value 15 * 60 * 1000, and then use this constant
in place of the magic number for better clarity and maintainability.
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes
Refactor