2FA #2505
2FA #2505
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis update introduces two-factor authentication (2FA) throughout the web application. It extends the user model and authentication logic to support enabling, confirming, and disabling 2FA, integrates TOTP-based code verification, and adds new UI components and modals for 2FA setup and management. Email notifications and error handling for 2FA events are also implemented. Changes
Sequence Diagram(s)Two-Factor Authentication Login FlowsequenceDiagram
participant User
participant LoginForm
participant Server (Credentials Provider)
participant Cookie
participant 2FA Challenge Page
participant Server (2FA Challenge Provider)
User->>LoginForm: Enter email & password
LoginForm->>Server (Credentials Provider): Submit credentials
Server (Credentials Provider)->>Server (Credentials Provider): Validate credentials
alt 2FA enabled
Server (Credentials Provider)->>Cookie: Set 2FA cookie
Server (Credentials Provider)->>LoginForm: Return "two-factor-required" error
LoginForm->>2FA Challenge Page: Redirect
User->>2FA Challenge Page: Enter 2FA code
2FA Challenge Page->>Server (2FA Challenge Provider): Submit code + 2FA cookie
Server (2FA Challenge Provider)->>Server (2FA Challenge Provider): Validate code and cookie
alt Code valid
Server (2FA Challenge Provider)->>2FA Challenge Page: Success (user authenticated)
else Code invalid
Server (2FA Challenge Provider)->>2FA Challenge Page: Error
end
else 2FA not enabled
Server (Credentials Provider)->>LoginForm: Success (user authenticated)
end
Enabling Two-Factor AuthenticationsequenceDiagram
participant User
participant SecurityPage
participant Server (Enable 2FA Action)
participant Enable2FAModal
participant Server (Confirm 2FA Action)
participant Email
User->>SecurityPage: Click "Enable 2FA"
SecurityPage->>Server (Enable 2FA Action): Request secret & QR code
Server (Enable 2FA Action)->>SecurityPage: Return secret & QR code URL
SecurityPage->>Enable2FAModal: Open modal with QR code & secret
User->>Enable2FAModal: Enter 6-digit code
Enable2FAModal->>Server (Confirm 2FA Action): Submit code
Server (Confirm 2FA Action)->>Server (Confirm 2FA Action): Validate code
alt Code valid
Server (Confirm 2FA Action)->>Email: Send 2FA enabled notification
Server (Confirm 2FA Action)->>Enable2FAModal: Success
Enable2FAModal->>SecurityPage: Close modal, refresh user
else Code invalid
Server (Confirm 2FA Action)->>Enable2FAModal: Error
end
Disabling Two-Factor AuthenticationsequenceDiagram
participant User
participant SecurityPage
participant Disable2FAModal
participant Server (Disable 2FA Action)
participant Email
User->>SecurityPage: Click "Disable 2FA"
SecurityPage->>Disable2FAModal: Open modal
User->>Disable2FAModal: Confirm disable
Disable2FAModal->>Server (Disable 2FA Action): Request disable
Server (Disable 2FA Action)->>Server (Disable 2FA Action): Clear 2FA fields
Server (Disable 2FA Action)->>Email: Send 2FA disabled notification
Server (Disable 2FA Action)->>Disable2FAModal: Success
Disable2FAModal->>SecurityPage: Close modal, refresh user
Poem
🪧 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 (
|
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
🤖 Bug0 QA Agent Here are the results of the automated tests for PR #2505:
To re-run the tests, please comment |
|
🤖 Bug0 QA Agent Here are the results of the automated tests for PR #2505:
To re-run the tests, please comment |
|
🤖 Bug0 QA Agent Here are the results of the automated tests for PR #2505:
To re-run the tests, please comment |
|
🤖 Bug0 QA Agent Here are the results of the automated tests for PR #2505:
To re-run the tests, please comment |
- Updated user API to include twoFactorConfirmedAt and twoFactorRecoveryCodes. - Enhanced TwoFactorAuth component to display loading state and toggle button text based on user's 2FA status. - Introduced disableTwoFactorAuthAction to handle disabling of 2FA for users. - Updated UserProps interface to reflect new 2FA properties.
|
🤖 Bug0 QA Agent Here are the results of the automated tests for PR #2505:
To re-run the tests, please comment |
|
🤖 Bug0 QA Agent Here are the results of the automated tests for PR #2505:
To re-run the tests, please comment |
|
🤖 Bug0 QA Agent Here are the results of the automated tests for PR #2505:
To re-run the tests, please comment |
|
🤖 Bug0 QA Agent Here are the results of the automated tests for PR #2505:
To re-run the tests, please comment |
|
🤖 Bug0 QA Agent Here are the results of the automated tests for PR #2505:
To re-run the tests, please comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (7)
packages/prisma/schema/schema.prisma (1)
31-34: Consider adding database index for 2FA queries.The
twoFactorConfirmedAtfield will likely be queried frequently to check if users have 2FA enabled. Consider adding an index for better query performance.+ @@index(twoFactorConfirmedAt)Also, storing recovery codes as a comma-separated string in
twoFactorRecoveryCodesrequires parsing. Consider if this approach aligns with your data access patterns or if a separate table would be more appropriate for better normalization.packages/email/src/templates/two-factor-disabled.tsx (1)
18-18: Consider updating the default email placeholder.The default email "panic@thedis.co" appears to be a placeholder. Consider using a more generic placeholder like "user@example.com" for consistency.
- email = "panic@thedis.co", + email = "user@example.com",packages/email/src/templates/two-factor-enabled.tsx (1)
18-18: Consider updating the default email placeholder for consistency.Same as the disabled template, consider using "user@example.com" instead of "panic@thedis.co".
- email = "panic@thedis.co", + email = "user@example.com",apps/web/ui/modals/enable-two-factor-auth-modal.tsx (2)
84-91: Accessibility: OTP cells need an explicit label
divelements that visually represent input fields lackaria-label/role="textbox"which breaks screen-reader navigation.
Wrap theOTPInputin a single visually hidden<label>(or addaria-labelto each cell) so assistive tech can announce what the user is expected to enter.
128-139: Memoised modal callback still re-creates on everyshowModaltoggleBecause
showModalis part ofModalCallback’s dependency list, the component returned fromuseEnableTwoFactorAuthModalchanges on every open/close which defeats memoisation at the parent level.If only the visibility should change, exclude
showModalfrom the dependencies and pass it as prop instead:- const ModalCallback = useCallback( - () => ( + const ModalCallback = useCallback( + ({ visible }: { visible: boolean }) => ( <EnableTwoFactorAuthModal - showModal={showModal} + showModal={visible} … /> ), - [showModal, onSuccess, secret, qrCodeUrl], + [onSuccess, secret, qrCodeUrl], );Callers would render
<EnableTwoFactorAuthModal visible={showModal} …/>.
Keeps a stable component reference and prevents unnecessary remounts.apps/web/lib/auth/options.ts (2)
337-344: Cookie is deleted before challenge outcome is known
cookies().delete(TWO_FA_COOKIE_NAME)executes immediately, so any network failure or invalid code forces the user to start the login flow from scratch.
Delete the cookie only after successful verification, or re-issue a fresh token on failure to maintain UX while still preventing replay attacks.
395-400: Verbose logging leaks potentially sensitive identifiers
console.log/console.errorincludesubandReplace with structured, redacted logging or remove entirely.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (22)
apps/web/app/api/user/route.ts(1 hunks)apps/web/app/app.dub.co/(auth)/two-factor-challenge/form.tsx(1 hunks)apps/web/app/app.dub.co/(auth)/two-factor-challenge/page.tsx(1 hunks)apps/web/app/app.dub.co/(dashboard)/account/settings/security/page-client.tsx(1 hunks)apps/web/app/app.dub.co/(dashboard)/account/settings/security/two-factor-auth.tsx(1 hunks)apps/web/lib/actions/auth/confirm-two-factor-auth.ts(1 hunks)apps/web/lib/actions/auth/disable-two-factor-auth.ts(1 hunks)apps/web/lib/actions/auth/enable-two-factor-auth.ts(1 hunks)apps/web/lib/auth/constants.ts(1 hunks)apps/web/lib/auth/options.ts(3 hunks)apps/web/lib/auth/totp.ts(1 hunks)apps/web/lib/middleware/app.ts(1 hunks)apps/web/lib/swr/use-user.ts(1 hunks)apps/web/lib/types.ts(1 hunks)apps/web/package.json(1 hunks)apps/web/ui/auth/login/email-sign-in.tsx(1 hunks)apps/web/ui/auth/login/login-form.tsx(1 hunks)apps/web/ui/modals/disable-two-factor-auth-modal.tsx(1 hunks)apps/web/ui/modals/enable-two-factor-auth-modal.tsx(1 hunks)packages/email/src/templates/two-factor-disabled.tsx(1 hunks)packages/email/src/templates/two-factor-enabled.tsx(1 hunks)packages/prisma/schema/schema.prisma(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (7)
apps/web/app/app.dub.co/(dashboard)/account/settings/security/page-client.tsx (2)
apps/web/lib/swr/use-user.ts (1)
useUser(5-13)apps/web/app/app.dub.co/(dashboard)/account/settings/security/two-factor-auth.tsx (1)
TwoFactorAuth(12-101)
apps/web/lib/swr/use-user.ts (1)
apps/web/lib/types.ts (1)
UserProps(178-192)
apps/web/lib/actions/auth/disable-two-factor-auth.ts (3)
apps/web/lib/actions/safe-action.ts (1)
authUserActionClient(19-31)packages/email/src/index.ts (1)
sendEmail(6-29)packages/email/src/templates/two-factor-disabled.tsx (1)
TwoFactorDisabled(17-49)
apps/web/lib/actions/auth/confirm-two-factor-auth.ts (4)
apps/web/lib/actions/safe-action.ts (1)
authUserActionClient(19-31)apps/web/lib/auth/totp.ts (1)
getTOTPInstance(14-26)packages/email/src/index.ts (1)
sendEmail(6-29)packages/email/src/templates/two-factor-enabled.tsx (1)
TwoFactorEnabled(17-49)
packages/email/src/templates/two-factor-enabled.tsx (2)
packages/utils/src/constants/main.ts (1)
DUB_WORDMARK(68-68)packages/ui/src/footer.tsx (1)
Footer(85-310)
apps/web/app/app.dub.co/(dashboard)/account/settings/security/two-factor-auth.tsx (4)
apps/web/lib/swr/use-user.ts (1)
useUser(5-13)apps/web/ui/modals/enable-two-factor-auth-modal.tsx (1)
useEnableTwoFactorAuthModal(117-148)apps/web/ui/modals/disable-two-factor-auth-modal.tsx (1)
useDisableTwoFactorAuthModal(72-92)apps/web/lib/actions/auth/enable-two-factor-auth.ts (1)
enableTwoFactorAuthAction(8-55)
apps/web/ui/modals/disable-two-factor-auth-modal.tsx (2)
apps/web/lib/swr/use-user.ts (1)
useUser(5-13)apps/web/lib/actions/auth/disable-two-factor-auth.ts (1)
disableTwoFactorAuthAction(10-44)
🔇 Additional comments (26)
apps/web/lib/auth/constants.ts (1)
12-12: LGTM! Clean constant addition for 2FA cookie.The constant follows good naming conventions with a descriptive name and appropriate prefix.
apps/web/package.json (1)
99-99:✅ Verification successful
Verify the otpauth package version and security status.
The
otpauthlibrary is a good choice for TOTP implementation, but please ensure version 9.4.0 is current and free from vulnerabilities.
🌐 Web query:
What is the latest version of the otpauth npm package and are there any known security vulnerabilities in version 9.4.0?💡 Result:
The latest version of the otpauth npm package is 9.4.0, released on March 29, 2025, approximately 2.5 months ago from the current date[1][2]. This package is a One Time Password (HOTP/TOTP) library designed for Node.js, Deno, Bun, and browsers, supporting the generation and validation of time-based and HMAC-based one-time passwords commonly used in multi-factor authentication systems[1].
Regarding security vulnerabilities in version 9.4.0, according to the Snyk Vulnerability Database, there are no known direct vulnerabilities in this latest version[2]. When examining the vulnerability table from Snyk, version 9.4.0 shows no critical, high, medium, or low severity vulnerabilities[2].
It's worth noting that earlier versions of otpauth (specifically versions below 3.2.8) were vulnerable to an Authentication Bypass issue[5], but this vulnerability has been addressed in more recent versions, including 9.4.0.
The otpauth package is maintained by Héctor Molinero Fernández under the MIT License[7]. The package supports various algorithms for the HMAC function, including SHA1, SHA224, SHA256, SHA384, SHA512, and several SHA3 variants, making it a versatile option for implementing two-factor authentication in applications[1].
Citations:
- 1: https://www.npmjs.com/package/otpauth
- 2: https://security.snyk.io/package/npm/otpauth
- 3: https://www.jsdelivr.com/package/npm/url-otpauth
- 4: https://github.com/hectorm/otpauth/blob/master/package.json
- 5: https://security.snyk.io/package/npm/otpauth/2.2.3
- 6: https://www.jit.io/resources/app-security/npm-security-vulnerabilities-what-developers-need-to-know
- 7: https://npm.io/package/otpauth
- 8: https://codevoweb.com/two-factor-authentication-2fa-in-nodejs/
- 9: https://www.youtube.com/watch?v=VAHkHj3I6Wg
- 10: https://www.npmjs.com/package/otpauth/v/9.3.3?activeTab=readme
otpauth@9.4.0 version and security status verified
Version 9.4.0 is the latest release (Mar 29, 2025) and has no known vulnerabilities according to the Snyk database. No further action required.
apps/web/ui/auth/login/email-sign-in.tsx (1)
95-98: LGTM! Proper 2FA redirect handling.The logic correctly handles the "two-factor-required" error by redirecting to the challenge page and returning early to bypass normal error handling.
apps/web/app/api/user/route.ts (1)
42-43: LGTM! Appropriate addition of 2FA fields to user API.The inclusion of
twoFactorConfirmedAtandtwoFactorRecoveryCodesfields is necessary for the frontend to manage 2FA state and recovery codes.apps/web/lib/middleware/app.ts (1)
30-30: LGTM! Correct middleware exclusion for 2FA flow.The exclusion of
/two-factor-challengefrom authentication requirements is necessary to allow users in the partial authentication state to complete their 2FA verification.apps/web/lib/swr/use-user.ts (2)
2-2: Good change to enable data revalidation for 2FA operations.Switching from
useSWRImmutabletouseSWRis appropriate since user data can now change (e.g., 2FA status updates).
6-6: Adding mutate function enables proper state synchronization.The
mutatefunction allows components to trigger revalidation after 2FA operations, ensuring the UI stays in sync with the backend state.Also applies to: 11-11
apps/web/ui/auth/login/login-form.tsx (1)
47-53: Well-designed 2FA error messages.The new error codes provide clear, user-friendly feedback for 2FA scenarios:
- Appropriate prompting for 2FA code entry
- Clear messaging for expired authentication sessions
- Helpful feedback for incorrect codes
The messages guide users toward resolution without exposing sensitive implementation details.
apps/web/app/app.dub.co/(dashboard)/account/settings/security/page-client.tsx (2)
5-5: Clean integration of 2FA management.The addition of the
TwoFactorAuthcomponent properly integrates 2FA settings into the security page alongside password management.Also applies to: 37-37
32-39:✅ Verification successful
Verify child component loading states.
Since the parent loading state was removed, ensure that both
UpdatePassword/RequestSetPasswordandTwoFactorAuthcomponents handle their own loading states appropriately to prevent UI issues.
🏁 Script executed:
#!/bin/bash # Description: Check if child components handle loading states properly # Check TwoFactorAuth component for loading state handling ast-grep --pattern $'const { user, loading } = useUser()' # Check UpdatePassword component for loading state handling rg -A 10 "useUser|loading" apps/web/app/app.dub.co/\(dashboard\)/account/settings/security/update-password.tsx # Check RequestSetPassword component for loading state handling rg -A 10 "useUser|loading" apps/web/app/app.dub.co/\(dashboard\)/account/settings/security/request-set-password.tsxLength of output: 1162
🏁 Script executed:
#!/bin/bash # Description: Verify TwoFactorAuth component for its own loading state handling # Inspect useUser usage rg -A 10 "useUser" apps/web/app/app.dub.co/\(dashboard\)/account/settings/security/two-factor-auth.tsx # Inspect any loading or internal submission state rg -A 10 "loading" apps/web/app/app.dub.co/\(dashboard\)/account/settings/security/two-factor-auth.tsxLength of output: 2025
No action needed: child components each manage their own loading states
All three components independently handle loading:
- UpdatePassword uses
isSubmitting(via form) for itsloadingprop.- RequestSetPassword tracks a
sendingstate forloadinganddisabled.- TwoFactorAuth uses
loadingfromuseUser(disables the button and shows “Loading…”) plus its ownisEnablingfor theloadingprop when toggling 2FA.packages/email/src/templates/two-factor-disabled.tsx (1)
17-21: LGTM! Clean email template implementation.The template structure and security messaging are appropriate for notifying users about 2FA being disabled.
apps/web/lib/actions/auth/confirm-two-factor-auth.ts (4)
11-13: Excellent input validation with appropriate constraints.The 6-digit length validation ensures TOTP code format compliance and provides clear error messaging.
32-38: Good security checks for preventing misuse.The validation logic properly handles edge cases:
- Prevents confirming already-enabled 2FA
- Ensures secret exists before validation
- Provides informative error messages without exposing sensitive details
44-53: Secure TOTP validation with appropriate tolerance window.The window of 1 step (±30 seconds) provides good user experience while maintaining security. The null check for invalid tokens is correctly implemented.
65-72: Proper async email handling for better performance.Using
waitUntilfor email sending ensures the user action completes quickly while the notification is sent asynchronously.apps/web/lib/auth/totp.ts (2)
3-8: Standard TOTP configuration looks good.The options follow RFC 6238 recommendations with appropriate defaults for issuer, digits, and period.
14-26: Function implementation is correct for TOTP instance creation.The
getTOTPInstancefunction properly spreads options and accepts the necessary parameters.packages/email/src/templates/two-factor-enabled.tsx (1)
17-21: LGTM! Consistent with the disabled template.The template structure and security messaging appropriately notify users about 2FA being enabled.
apps/web/app/app.dub.co/(auth)/two-factor-challenge/page.tsx (3)
8-10: Good use of dynamic metadata with environment variables.The metadata construction properly includes the app name from environment variables for better branding consistency.
12-17: Proper authentication guard with cookie validation.The cookie check and redirect logic correctly prevents unauthorized access to the 2FA challenge page while allowing legitimate users to proceed.
19-31: Clean component structure with consistent layout.The use of
AuthLayoutand proper semantic structure (heading + form) creates a good user experience for the 2FA challenge flow.apps/web/lib/actions/auth/disable-two-factor-auth.ts (1)
1-44: Well-implemented 2FA disable action with proper security checks.The server action correctly:
- Ensures user authentication via
authUserActionClient- Validates that 2FA is currently enabled before disabling
- Clears all 2FA-related fields atomically
- Sends notification email asynchronously using
waitUntilThe implementation follows security best practices and maintains data consistency.
apps/web/lib/actions/auth/enable-two-factor-auth.ts (1)
1-55: Secure 2FA enablement flow with proper two-step verification.The implementation correctly:
- Validates that 2FA isn't already enabled
- Generates and stores the TOTP secret without immediately enabling 2FA
- Creates a QR code URL for authenticator app setup
- Requires separate confirmation step for security
This two-step approach (setup + confirmation) is a security best practice that prevents accidental lockouts.
apps/web/app/app.dub.co/(auth)/two-factor-challenge/form.tsx (1)
1-67: Well-designed 2FA challenge form with good UX considerations.The form implementation includes:
- Proper input validation (6-digit numeric pattern)
- Mobile-responsive autofocus behavior
- Button state management (disabled until valid input)
- Comprehensive error handling with user-friendly messages
- Correct integration with NextAuth's "two-factor-challenge" provider
The user experience is well thought out for both mobile and desktop users.
apps/web/app/app.dub.co/(dashboard)/account/settings/security/two-factor-auth.tsx (1)
1-101: Comprehensive 2FA management component with excellent state management.The component provides:
- Clean integration between enable/disable modals
- Proper state management for secrets and QR codes
- Automatic user data refresh after 2FA state changes
- Conditional UI that clearly shows current 2FA status
- Appropriate loading states and error feedback
- Good separation of concerns with dedicated modal hooks
The user experience flows smoothly between enabling and disabling 2FA.
apps/web/ui/modals/disable-two-factor-auth-modal.tsx (1)
1-92: Well-structured modal with excellent state management patterns.The modal implementation provides:
- Clear warning message about consequences of disabling 2FA
- Proper success/error handling with user feedback
- Clean custom hook pattern for state management
- Good separation between modal logic and presentation
- Appropriate button states and loading indicators
The
useDisableTwoFactorAuthModalhook follows React best practices with proper memoization.
apps/web/lib/types.ts
Outdated
| provider: string | null; | ||
| twoFactorConfirmedAt: Date | null; | ||
| twoFactorRecoveryCodes: string[] | null; | ||
| } |
There was a problem hiding this comment.
Type inconsistency between Prisma schema and TypeScript interface.
There's a mismatch in how twoFactorRecoveryCodes is typed:
- In the Prisma schema (
packages/prisma/schema/schema.prismaline 33):String?(single string for comma-separated values) - In this TypeScript interface:
string[] | null(array of strings)
This inconsistency could lead to runtime errors when the API returns a string but the frontend expects an array.
Ensure consistent typing by either:
- Changing the TypeScript type to
string | nullto match the schema - Or updating the API layer to parse the comma-separated string into an array
- twoFactorRecoveryCodes: string[] | null;
+ twoFactorRecoveryCodes: string | null;📝 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.
| provider: string | null; | |
| twoFactorConfirmedAt: Date | null; | |
| twoFactorRecoveryCodes: string[] | null; | |
| } | |
| provider: string | null; | |
| twoFactorConfirmedAt: Date | null; | |
| twoFactorRecoveryCodes: string | null; | |
| } |
🤖 Prompt for AI Agents
In apps/web/lib/types.ts around lines 189 to 192, the twoFactorRecoveryCodes
field is typed as string[] | null, but the Prisma schema defines it as a single
nullable string representing comma-separated values. To fix this inconsistency,
update the TypeScript type to string | null to match the Prisma schema, or
alternatively, modify the API layer to parse the comma-separated string into an
array so the frontend can correctly handle string[]. Choose one approach and
ensure both schema and TypeScript types align to prevent runtime errors.
| // if (loading) { | ||
| // return ( | ||
| // <div className="rounded-lg border border-neutral-200 bg-white"> | ||
| // <div className="flex flex-col gap-3 border-b border-neutral-200 p-5 sm:p-10"> | ||
| // <h2 className="text-xl font-medium">Password</h2> | ||
| // <div className="h-3 w-56 rounded-full bg-neutral-100"></div> | ||
| // </div> | ||
| // <div className="p-5 sm:p-10"> | ||
| // <div className="flex justify-between gap-2"> | ||
| // <div className="h-3 w-56 rounded-full bg-neutral-100"></div> | ||
| // <div className="h-3 w-56 rounded-full bg-neutral-100"></div> | ||
| // </div> | ||
| // <div className="mt-5 h-3 rounded-full bg-neutral-100"></div> | ||
| // </div> | ||
| // </div> | ||
| // ); | ||
| // } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider the implications of removing loading state.
Commenting out the loading state might cause the page to render before user data is available, potentially leading to:
- UI flicker when components transition from loading to loaded states
- Brief display of incorrect states (e.g., showing "Enable 2FA" before checking if it's already enabled)
Consider keeping a minimal loading state or ensure child components handle their own loading states properly.
🤖 Prompt for AI Agents
In apps/web/app/app.dub.co/(dashboard)/account/settings/security/page-client.tsx
around lines 13 to 29, the loading state rendering is commented out, which can
cause UI flicker and incorrect display before user data is ready. To fix this,
restore a minimal loading state UI that shows placeholders or skeletons while
data is loading, or ensure all child components independently handle their
loading states to prevent premature rendering of incomplete or incorrect
information.
apps/web/lib/auth/totp.ts
Outdated
| export const totpSecret = new OTPAuth.Secret({ | ||
| size: 20, // 160 bits = 32 characters | ||
| }); |
There was a problem hiding this comment.
CRITICAL: Shared secret creates major security vulnerability.
Exporting a single totpSecret instance means all users would share the same 2FA secret, completely undermining the security of the 2FA system. Each user must have a unique secret.
Replace the exported constant with a function that generates a new secret per user:
-export const totpSecret = new OTPAuth.Secret({
- size: 20, // 160 bits = 32 characters
-});
+export const generateTotpSecret = () => {
+ return new OTPAuth.Secret({
+ size: 20, // 160 bits = 32 characters
+ });
+};📝 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.
| export const totpSecret = new OTPAuth.Secret({ | |
| size: 20, // 160 bits = 32 characters | |
| }); | |
| export const generateTotpSecret = () => { | |
| return new OTPAuth.Secret({ | |
| size: 20, // 160 bits = 32 characters | |
| }); | |
| }; |
🤖 Prompt for AI Agents
In apps/web/lib/auth/totp.ts around lines 10 to 12, the current code exports a
single totpSecret instance, causing all users to share the same 2FA secret,
which is a major security flaw. Replace the exported constant with a function
that generates and returns a new OTPAuth.Secret instance each time it is called,
ensuring each user gets a unique secret for their 2FA.
| </div> | ||
|
|
||
| <div className="bg-neutral-50"> | ||
| <form => disable2FA()}> |
There was a problem hiding this comment.
Fix form submission to prevent default behavior.
The form's onSubmit handler should prevent the default form submission to avoid potential page refresh or navigation issues.
Apply this fix:
- <form => disable2FA()}>
+ <form => { e.preventDefault(); disable2FA(); }}>📝 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.
| <form onSubmit={() => disable2FA()}> | |
| <form onSubmit={(e) => { e.preventDefault(); disable2FA(); }}> |
🤖 Prompt for AI Agents
In apps/web/ui/modals/disable-two-factor-auth-modal.tsx at line 47, the form's
onSubmit handler currently calls disable2FA() directly without preventing the
default form submission behavior. Modify the onSubmit handler to accept the
event parameter and call event.preventDefault() before invoking disable2FA() to
prevent page refresh or navigation issues.
| const [token, setToken] = useState(""); | ||
| const [touched, setTouched] = useState(false); | ||
| const [error, setError] = useState<string | undefined>(undefined); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
State isn’t reset when the modal re-opens
token, touched, and error keep their previous values if the modal is closed and re-opened in the same session, which produces confusing UI (old errors, red borders, pre-filled digits).
Reset local state whenever the modal becomes visible.
+ useEffect(() => {
+ if (showModal) {
+ setToken("");
+ setTouched(false);
+ setError(undefined);
+ }
+ }, [showModal]);📝 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 [token, setToken] = useState(""); | |
| const [touched, setTouched] = useState(false); | |
| const [error, setError] = useState<string | undefined>(undefined); | |
| const [token, setToken] = useState(""); | |
| const [touched, setTouched] = useState(false); | |
| const [error, setError] = useState<string | undefined>(undefined); | |
| useEffect(() => { | |
| if (showModal) { | |
| setToken(""); | |
| setTouched(false); | |
| setError(undefined); | |
| } | |
| }, [showModal]); |
🤖 Prompt for AI Agents
In apps/web/ui/modals/enable-two-factor-auth-modal.tsx around lines 24 to 27,
the state variables token, touched, and error are not reset when the modal
re-opens, causing stale UI data to persist. Add an effect hook that listens for
the modal's visibility change and resets these state variables to their initial
values whenever the modal becomes visible.
apps/web/lib/auth/options.ts
Outdated
| // Two-factor challenge | ||
| CredentialsProvider({ | ||
| id: "two-factor-challenge", | ||
| name: "Two-factor challenge", | ||
| type: "credentials", | ||
| credentials: { | ||
| code: { type: "text" }, | ||
| }, | ||
| async authorize(credentials, req) { | ||
| if (!credentials) { | ||
| throw new Error("no-credentials"); | ||
| } | ||
|
|
||
| const { code } = credentials; | ||
|
|
||
| if (!code) { | ||
| throw new Error("no-credentials"); | ||
| } | ||
|
|
||
| // TODO: | ||
| // Rate limit the request |
There was a problem hiding this comment.
Brute-force window: rate-limiting is still a TODO
two-factor-challenge provider lacks rate limiting, making it trivial to brute-force 6-digit TOTP codes within the two-minute window, especially for high-value accounts.
Implement the same ratelimit logic used in the credentials provider, keyed by user id or the JWT sub. Do this before TOTP validation to avoid unnecessary DB hits.
🤖 Prompt for AI Agents
In apps/web/lib/auth/options.ts around lines 315 to 335, the
two-factor-challenge credentials provider lacks rate limiting, allowing
brute-force attacks on TOTP codes. Implement the existing ratelimit logic used
in the credentials provider by applying it at the start of the authorize
function, keyed by the user id or JWT sub from the request. This should be done
before any TOTP validation or database access to prevent unnecessary load and
improve security.
| if (user.twoFactorConfirmedAt) { | ||
| await setTwoFactorAuthCookie(user); | ||
| throw new Error("two-factor-required"); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Race-condition risk when throwing after async cookie write
await setTwoFactorAuthCookie(user) is followed by throw new Error("two-factor-required").
If setTwoFactorAuthCookie ever fails or is significantly delayed (e.g. edge runtime latency), the error is thrown without the cookie, forcing an impossible challenge flow.
Add try/catch and ensure the cookie write succeeds before signalling 2FA:
- if (user.twoFactorConfirmedAt) {
- await setTwoFactorAuthCookie(user);
- throw new Error("two-factor-required");
- }
+ if (user.twoFactorConfirmedAt) {
+ try {
+ await setTwoFactorAuthCookie(user);
+ } catch (e) {
+ console.error("Unable to set 2FA cookie", e);
+ throw new Error("internal-error");
+ }
+ throw new Error("two-factor-required");
+ }📝 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.
| if (user.twoFactorConfirmedAt) { | |
| await setTwoFactorAuthCookie(user); | |
| throw new Error("two-factor-required"); | |
| } | |
| if (user.twoFactorConfirmedAt) { | |
| try { | |
| await setTwoFactorAuthCookie(user); | |
| } catch (e) { | |
| console.error("Unable to set 2FA cookie", e); | |
| throw new Error("internal-error"); | |
| } | |
| throw new Error("two-factor-required"); | |
| } |
🤖 Prompt for AI Agents
In apps/web/lib/auth/options.ts around lines 301 to 304, the code throws an
error immediately after awaiting setTwoFactorAuthCookie(user), which risks
throwing before the cookie is successfully set if the async call fails or is
delayed. To fix this, wrap the await setTwoFactorAuthCookie(user) call in a
try/catch block to handle any errors during cookie setting, ensuring the cookie
write completes successfully before throwing the "two-factor-required" error.
| const setTwoFactorAuthCookie = async (user: Pick<User, "id" | "email">) => { | ||
| const token = await encode({ | ||
| secret: process.env.NEXTAUTH_SECRET as string, | ||
| maxAge: 2 * 60, | ||
| token: { | ||
| sub: user.id, | ||
| email: user.email, | ||
| purpose: "2fa", | ||
| iat: Math.floor(Date.now() / 1000), | ||
| }, | ||
| }); | ||
|
|
||
| cookies().set({ | ||
| name: TWO_FA_COOKIE_NAME, | ||
| value: token, | ||
| path: "/", | ||
| httpOnly: true, | ||
| secure: VERCEL_DEPLOYMENT, | ||
| expires: new Date(Date.now() + 2 * 60 * 1000), | ||
| sameSite: "lax", | ||
| }); | ||
| }; |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Cookie security: consider SameSite="strict"
The 2-minute 2FA cookie is sensitive – it should never be sent in a cross-site context. sameSite: "lax" allows it on top-level GET navigations, which an attacker could exploit if they can trick the user into following a link before the cookie expires.
- sameSite: "lax",
+ sameSite: "strict",Also consider adding priority: "high" for modern browsers.
Cookie security: enforce SameSite="strict" for the 2FA cookie
The 2-minute 2FA cookie carries highly sensitive data and must never be sent in a cross-site context. sameSite: "lax" still allows it on top-level GET navigations, which an attacker could exploit.
• File: apps/web/lib/auth/options.ts
Function: setTwoFactorAuthCookie
Update the cookie options as follows:
- sameSite: "lax",
+ sameSite: "strict",
+ priority: "high", // ensure this cookie is sent ahead of lower-priority cookiesThis change prevents the cookie from being sent on any cross-site navigation.
📝 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 setTwoFactorAuthCookie = async (user: Pick<User, "id" | "email">) => { | |
| const token = await encode({ | |
| secret: process.env.NEXTAUTH_SECRET as string, | |
| maxAge: 2 * 60, | |
| token: { | |
| sub: user.id, | |
| email: user.email, | |
| purpose: "2fa", | 47C5|
| iat: Math.floor(Date.now() / 1000), | |
| }, | |
| }); | |
| cookies().set({ | |
| name: TWO_FA_COOKIE_NAME, | |
| value: token, | |
| path: "/", | |
| httpOnly: true, | |
| secure: VERCEL_DEPLOYMENT, | |
| expires: new Date(Date.now() + 2 * 60 * 1000), | |
| sameSite: "lax", | |
| }); | |
| }; | |
| const setTwoFactorAuthCookie = async (user: Pick<User, "id" | "email">) => { | |
| const token = await encode({ | |
| secret: process.env.NEXTAUTH_SECRET as string, | |
| maxAge: 2 * 60, | |
| token: { | |
| sub: user.id, | |
| email: user.email, | |
| purpose: "2fa", | |
| iat: Math.floor(Date.now() / 1000), | |
| }, | |
| }); | |
| cookies().set({ | |
| name: TWO_FA_COOKIE_NAME, | |
| value: token, | |
| path: "/", | |
| httpOnly: true, | |
| secure: VERCEL_DEPLOYMENT, | |
| expires: new Date(Date.now() + 2 * 60 * 1000), | |
| sameSite: "strict", | |
| priority: "high", | |
| }); | |
| }; |
🤖 Prompt for AI Agents
In apps/web/lib/auth/options.ts around lines 35 to 56, update the
setTwoFactorAuthCookie function to set the cookie's sameSite attribute to
"strict" instead of "lax" to prevent the 2FA cookie from being sent in any
cross-site context. Additionally, add the priority: "high" option to the cookie
settings to ensure modern browsers treat this cookie with higher precedence.
This enhances the security of the sensitive 2FA cookie.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (8)
apps/web/lib/types.ts (1)
190-191: Type inconsistency between Prisma schema and TypeScript interface.There's a mismatch in how
twoFactorRecoveryCodesis typed:
- In the Prisma schema:
String?(single string for comma-separated values)- In this TypeScript interface:
string[] | null(array of strings)This inconsistency could lead to runtime errors when the API returns a string but the frontend expects an array.
Ensure consistent typing by either:
- Changing the TypeScript type to
string | nullto match the schema- Or updating the API layer to parse the comma-separated string into an array
- twoFactorRecoveryCodes: string[] | null; + twoFactorRecoveryCodes: string | null;apps/web/app/app.dub.co/(dashboard)/account/settings/security/page-client.tsx (1)
13-29: Consider the implications of removing loading state.The commented-out loading state could cause UI flicker and incorrect display before user data is available. While the
TwoFactorAuthcomponent handles its own loading state, the password section might still render prematurely.apps/web/lib/auth/totp.ts (1)
10-12:⚠️ Potential issueCRITICAL: Shared secret creates major security vulnerability.
This is the same critical security issue identified in the previous review. The exported
totpSecretconstant means all users would share the same 2FA secret, completely undermining the security of the 2FA system.apps/web/ui/modals/disable-two-factor-auth-modal.tsx (1)
47-47: Fix form submission to prevent default behavior.The form's
onSubmithandler should prevent the default form submission to avoid potential page refresh or navigation issues.Apply this fix:
- <form => disable2FA()}> + <form => { e.preventDefault(); disable2FA(); }}>apps/web/ui/modals/enable-two-factor-auth-modal.tsx (1)
24-27: State isn't reset when the modal re-opens
token,touched, anderrorkeep their previous values if the modal is closed and re-opened in the same session, which produces confusing UI (old errors, red borders, pre-filled digits).
Reset local state whenever the modal becomes visible.Add an effect hook to reset state when modal opens:
+import { useCallback, useEffect, useMemo, useState } from "react"; -import { useCallback, useMemo, useState } from "react";After line 27, add:
+ useEffect(() => { + if (showModal) { + setToken(""); + setTouched(false); + setError(undefined); + } + }, [showModal]);apps/web/lib/auth/options.ts (3)
35-56: Cookie security: enforceSameSite="strict"for the 2FA cookieThe 2-minute 2FA cookie carries highly sensitive data and must never be sent in a cross-site context.
sameSite: "lax"still allows it on top-level GET navigations, which an attacker could exploit.- sameSite: "lax", + sameSite: "strict", + priority: "high",
301-304: Race-condition risk when throwing after async cookie write
await setTwoFactorAuthCookie(user)is followed bythrow new Error("two-factor-required").
IfsetTwoFactorAuthCookieever fails or is significantly delayed (e.g. edge runtime latency), the error is thrown without the cookie, forcing an impossible challenge flow.if (user.twoFactorConfirmedAt) { - await setTwoFactorAuthCookie(user); - throw new Error("two-factor-required"); + try { + await setTwoFactorAuthCookie(user); + } catch (e) { + console.error("Unable to set 2FA cookie", e); + throw new Error("internal-error"); + } + throw new Error("two-factor-required"); }
334-335:⚠️ Potential issueBrute-force window: rate-limiting is still a TODO
two-factor-challengeprovider lacks rate limiting, making it trivial to brute-force 6-digit TOTP codes within the two-minute window, especially for high-value accounts.Replace the TODO with actual rate limiting implementation:
- // TODO: - // Rate limit the request + const { sub, email } = decoded as any; + + const { success } = await ratelimit(5, "1 m").limit( + `2fa-attempts:${sub || email}`, + ); + + if (!success) { + throw new Error("too-many-2fa-attempts"); + }Move this rate limiting check before the cookie operations to avoid unnecessary processing.
🧹 Nitpick comments (5)
apps/web/app/app.dub.co/(auth)/two-factor-challenge/form.tsx (1)
47-56: Enhance input validation for better UX and security.The current implementation allows any characters in the 6-digit code field. Consider these improvements:
<Input type="text" autoFocus={!isMobile} value={code} placeholder="012345" pattern="[0-9]*" + inputMode="numeric" => setCode(e.target.value)} maxLength={6} + => { + e.currentTarget.value = e.currentTarget.value.replace(/\D/g, ''); + setCode(e.currentTarget.value); + }} />This ensures only numeric input is accepted and provides better mobile keyboard experience.
apps/web/app/app.dub.co/(dashboard)/account/settings/security/two-factor-auth.tsx (1)
34-38: Improve error handling specificity.The current error handling is generic. Consider providing more specific error messages based on the type of failure:
onSuccess: async ({ data }) => { if (!data) { - toast.error("Failed to enable 2FA. Please try again."); + toast.error("Failed to generate 2FA setup data. Please try again."); return; } + if (!data.secret || !data.qrCodeUrl) { + toast.error("Incomplete 2FA setup data received. Please try again."); + return; + } setSecret(data.secret); setQrCodeUrl(data.qrCodeUrl); setShowEnableTwoFactorAuthModal(true); },apps/web/ui/modals/enable-two-factor-auth-modal.tsx (2)
41-49: Prevent duplicate form submissionsWhile the button is disabled when
isPending, the form could still be submitted via Enter key or if there's a race condition. Consider adding a guard to prevent multiple concurrent requests.const confirmTwoFactorAuth = async (e: React.FormEvent) => { e.preventDefault(); + if (isPending) return; setTouched(true); setError(undefined); await executeAsync({ token, }); };
84-84: Simplify complex conditional stylingThe error styling logic is complex and hard to read. Consider extracting it into a separate variable for better maintainability.
- className={`relative flex h-14 w-12 items-center justify-center rounded-lg border bg-white text-xl transition-all ${isActive ? "z-10 border-neutral-800 ring-2 ring-neutral-200" : "border-neutral-200"} ${(touched && token.length < 6) || error ? "border-red-500 ring-red-200" : ""}`} + className={`relative flex h-14 w-12 items-center justify-center rounded-lg border bg-white text-xl transition-all ${ + isActive + ? "z-10 border-neutral-800 ring-2 ring-neutral-200" + : "border-neutral-200" + } ${ + (touched && token.length < 6) || error + ? "border-red-500 ring-red-200" + : "" + }`}apps/web/lib/auth/options.ts (1)
372-398: Sensitive information in error logsMultiple console.error statements include user email and ID which could leak sensitive information in production logs. Consider using structured logging with appropriate log levels or removing these in production.
if (!user) { - console.error("User not found", { sub, email }); + if (process.env.NODE_ENV === "development") { + console.error("User not found", { sub, email }); + } throw new Error("invalid-credentials"); } if (!user.twoFactorConfirmedAt) { - console.error("Two-factor not confirmed", { sub, email }); + if (process.env.NODE_ENV === "development") { + console.error("Two-factor not confirmed", { sub, email }); + } throw new Error("invalid-credentials"); } if (!user.twoFactorSecret) { - console.error("Two-factor secret not found", { sub, email }); + if (process.env.NODE_ENV === "development") { + console.error("Two-factor secret not found", { sub, email }); + } throw new Error("invalid-credentials"); }And after line 398:
if (delta === null) { - console.error("Invalid 2FA code entered", { sub, email }); + if (process.env.NODE_ENV === "development") { + console.error("Invalid 2FA code entered", { sub, email }); + } throw new Error("invalid-2fa-code"); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (22)
apps/web/app/api/user/route.ts(1 hunks)apps/web/app/app.dub.co/(auth)/two-factor-challenge/form.tsx(1 hunks)apps/web/app/app.dub.co/(auth)/two-factor-challenge/page.tsx(1 hunks)apps/web/app/app.dub.co/(dashboard)/account/settings/security/page-client.tsx(1 hunks)apps/web/app/app.dub.co/(dashboard)/account/settings/security/two-factor-auth.tsx(1 hunks)apps/web/lib/actions/auth/confirm-two-factor-auth.ts(1 hunks)apps/web/lib/actions/auth/disable-two-factor-auth.ts(1 hunks)apps/web/lib/actions/auth/enable-two-factor-auth.ts(1 hunks)apps/web/lib/auth/constants.ts(1 hunks)apps/web/lib/auth/options.ts(3 hunks)apps/web/lib/auth/totp.ts(1 hunks)apps/web/lib/middleware/app.ts(1 hunks)apps/web/lib/swr/use-user.ts(1 hunks)apps/web/lib/types.ts(1 hunks)apps/web/package.json(1 hunks)apps/web/ui/auth/login/email-sign-in.tsx(1 hunks)apps/web/ui/auth/login/login-form.tsx(1 hunks)apps/web/ui/modals/disable-two-factor-auth-modal.tsx(1 hunks)apps/web/ui/modals/enable-two-factor-auth-modal.tsx(1 hunks)packages/email/src/templates/two-factor-disabled.tsx(1 hunks)packages/email/src/templates/two-factor-enabled.tsx(1 hunks)packages/prisma/schema/schema.prisma(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (8)
apps/web/lib/swr/use-user.ts (1)
apps/web/lib/types.ts (1)
UserProps(178-192)
apps/web/app/app.dub.co/(dashboard)/account/settings/security/page-client.tsx (2)
apps/web/lib/swr/use-user.ts (1)
useUser(5-13)apps/web/app/app.dub.co/(dashboard)/account/settings/security/two-factor-auth.tsx (1)
TwoFactorAuth(12-101)
apps/web/app/app.dub.co/(auth)/two-factor-challenge/page.tsx (3)
packages/utils/src/functions/construct-metadata.ts (1)
constructMetadata(3-89)apps/web/lib/auth/constants.ts (1)
TWO_FA_COOKIE_NAME(12-12)apps/web/app/app.dub.co/(auth)/two-factor-challenge/form.tsx (1)
TwoFactorChallengeForm(9-67)
apps/web/lib/actions/auth/disable-two-factor-auth.ts (3)
apps/web/lib/actions/safe-action.ts (1)
authUserActionClient(19-31)packages/email/src/index.ts (1)
sendEmail(6-29)packages/email/src/templates/two-factor-disabled.tsx (1)
TwoFactorDisabled(17-49)
apps/web/lib/actions/auth/confirm-two-factor-auth.ts (4)
apps/web/lib/actions/safe-action.ts (1)
authUserActionClient(19-31)apps/web/lib/auth/totp.ts (1)
getTOTPInstance(14-26)packages/email/src/index.ts (1)
sendEmail(6-29)packages/email/src/templates/two-factor-enabled.tsx (1)
TwoFactorEnabled(17-49)
packages/email/src/templates/two-factor-enabled.tsx (2)
packages/utils/src/constants/main.ts (1)
DUB_WORDMARK(68-68)packages/ui/src/footer.tsx (1)
Footer(85-310)
apps/web/ui/modals/disable-two-factor-auth-modal.tsx (2)
apps/web/lib/swr/use-user.ts (1)
useUser(5-13)apps/web/lib/actions/auth/disable-two-factor-auth.ts (1)
disableTwoFactorAuthAction(10-44)
apps/web/app/app.dub.co/(dashboard)/account/settings/security/two-factor-auth.tsx (4)
apps/web/lib/swr/use-user.ts (1)
useUser(5-13)apps/web/ui/modals/enable-two-factor-auth-modal.tsx (1)
useEnableTwoFactorAuthModal(117-148)apps/web/ui/modals/disable-two-factor-auth-modal.tsx (1)
useDisableTwoFactorAuthModal(72-92)apps/web/lib/actions/auth/enable-two-factor-auth.ts (1)
enableTwoFactorAuthAction(8-55)
🔇 Additional comments (27)
apps/web/package.json (1)
99-99:✅ Verification successful
Verify the otpauth package version for security and currency.
Ensure that version 9.4.0 is the latest stable release and check for any known security vulnerabilities in the otpauth package.
🌐 Web query:
What is the latest version of the otpauth npm package and are there any known security vulnerabilities in version 9.4.0?💡 Result:
The latest version of the
otpauthnpm package is 9.4.0, published approximately a month ago. (npmjs.com)Regarding security vulnerabilities, there are no known issues in version 9.4.0. The most recent version with a reported vulnerability is 9.3.6, which has a high-severity authentication bypass vulnerability affecting versions prior to 3.2.8. (security.snyk.io) Since version 9.4.0 is newer than 3.2.8, it does not contain this vulnerability.
For more details on the latest releases and their changes, you can refer to the GitHub repository's release notes. (github.com)
Citations:
- 1: https://www.npmjs.com/package/otpauth?utm_source=openai
- 2: https://security.snyk.io/package/npm/otpauth?utm_source=openai
- 3: https://github.com/hectorm/otpauth/releases?utm_source=openai
otpauth Dependency Update Verified
The
otpauthdependency added in apps/web/package.json (line 99) is at the latest stable release (v9.4.0) and has no reported security vulnerabilities. No further action is required.apps/web/lib/auth/constants.ts (1)
12-12: LGTM! Well-structured 2FA cookie constant.The constant follows good naming conventions and includes the app identifier to prevent cookie conflicts.
apps/web/app/api/user/route.ts (1)
42-43: LGTM! Proper extension of user data for 2FA support.The addition of
twoFactorConfirmedAtandtwoFactorRecoveryCodesfields appropriately extends the user API response to support 2FA functionality and UI state management.apps/web/ui/auth/login/email-sign-in.tsx (1)
95-98: LGTM! Proper 2FA flow handling in authentication.The conditional check correctly identifies when 2FA is required and redirects users to the challenge page, providing a smooth authentication experience.
apps/web/lib/middleware/app.ts (1)
30-30: LGTM: Correctly allows unauthenticated access to 2FA challenge page.This change properly integrates with the 2FA authentication flow by allowing users to access the two-factor challenge page without being fully authenticated, consistent with other authentication-related paths.
packages/prisma/schema/schema.prisma (1)
31-33: LGTM: Well-designed schema additions for 2FA support.The three new optional fields appropriately support the 2FA implementation:
twoFactorSecretfor storing TOTP secretstwoFactorConfirmedAtfor tracking 2FA enablement statustwoFactorRecoveryCodesfor backup recovery optionsThe optional nature ensures backward compatibility with existing users.
apps/web/lib/swr/use-user.ts (2)
2-2: LGTM: Appropriate change to support data revalidation.Switching from
useSWRImmutabletouseSWRis correct for the 2FA implementation, as user data can now change when 2FA is enabled/disabled.
6-11: LGTM: Exposing mutate function enables proper state management.Returning the
mutatefunction allows 2FA components to refresh user data after state changes, ensuring the UI stays synchronized with the backend state.apps/web/ui/auth/login/login-form.tsx (1)
47-52: LGTM: Comprehensive error handling for 2FA scenarios.The error codes appropriately cover the main 2FA authentication scenarios with clear, user-friendly messages. The duplicate messages for
"no-2fa-token"and"invalid-2fa-token"are likely intentional for security reasons.apps/web/app/app.dub.co/(dashboard)/account/settings/security/page-client.tsx (2)
5-5: LGTM: Clean integration of TwoFactorAuth component.The import and usage of the
TwoFactorAuthcomponent properly integrates 2FA management into the security settings page.Also applies to: 37-37
31-39: Well-structured layout for security settings.The new layout cleanly separates password management and 2FA settings, providing a logical organization for security-related features.
packages/email/src/templates/two-factor-disabled.tsx (1)
17-49: LGTM! Well-structured email template for security notifications.The email template follows best practices with clear messaging, proper branding, and includes a support contact link for users who didn't initiate the change. The component structure and styling are appropriate for email rendering.
apps/web/lib/actions/auth/confirm-two-factor-auth.ts (4)
11-13: Good input validation with appropriate constraints.The Zod schema correctly validates the 6-digit token format, ensuring proper input before processing.
32-38: Proper state validation and error handling.The checks for existing 2FA confirmation and missing secret prevent invalid state transitions and provide clear error messages.
44-53: Correct TOTP validation with appropriate time window.The
window: 1parameter provides a ±30-second tolerance for clock skew, which is a security best practice for TOTP validation. The delta check properly handles both valid and invalid tokens.
65-72: Proper async email notification with waitUntil.Using
waitUntilfor the email notification ensures the user response isn't delayed while still guaranteeing the email is sent. The email content and variant are appropriate for this security event.apps/web/lib/auth/totp.ts (1)
14-26: LGTM! Proper TOTP instance factory function.The
getTOTPInstancefunction correctly accepts individual secrets and optional labels, allowing each user to have their own unique TOTP configuration.packages/email/src/templates/two-factor-enabled.tsx (1)
17-49: LGTM! Consistent email template for 2FA enabled notifications.The template maintains consistency with the disabled notification template while providing appropriate messaging for the enabled state. The security advisory to contact support if the change wasn't initiated by the user is a good security practice.
apps/web/app/app.dub.co/(auth)/two-factor-challenge/page.tsx (3)
8-10: Good metadata configuration with dynamic app name.The metadata construction properly includes the application name from environment variables, providing proper SEO and browser tab context.
13-17: Correct 2FA flow enforcement with cookie validation.The cookie check ensures users can only access this page as part of the proper 2FA challenge flow, redirecting unauthorized access back to login.
19-31: Clean UI structure with proper layout and form integration.The component uses consistent auth layout patterns and provides clear context with the heading before rendering the challenge form.
apps/web/lib/actions/auth/disable-two-factor-auth.ts (1)
10-44: LGTM! Well-implemented 2FA disable action.The implementation correctly:
- Validates user authentication via
authUserActionClient- Checks if 2FA is currently enabled before attempting to disable
- Atomically clears all 2FA-related fields
- Sends notification email asynchronously without blocking the response
Minor suggestion: Consider making the error message on line 21 more user-friendly, e.g., "Two-factor authentication is not currently enabled for your account."
apps/web/lib/actions/auth/enable-two-factor-auth.ts (1)
28-37: LGTM! Proper storage of unconfirmed 2FA secret.The comment correctly explains that this step only stores the secret without enabling 2FA, requiring user confirmation via TOTP code. This is the correct security approach for 2FA setup.
apps/web/app/app.dub.co/(auth)/two-factor-challenge/form.tsx (1)
14-37: LGTM! Proper 2FA challenge submission handling.The form correctly:
- Prevents default form submission
- Uses the appropriate NextAuth provider
- Handles loading states
- Provides user-friendly error messages
- Disables redirect to handle errors manually
apps/web/app/app.dub.co/(dashboard)/account/settings/security/two-factor-auth.tsx (1)
75-95: LGTM! Well-implemented button logic and state management.The button correctly:
- Shows appropriate text based on loading and 2FA status
- Uses proper variants (danger for disable, primary for enable)
- Handles loading states from both user fetch and enable action
- Triggers the correct modal or action based on current 2FA status
apps/web/ui/modals/disable-two-factor-auth-modal.tsx (2)
19-31: LGTM! Proper server action integration with error handling.The modal correctly:
- Uses the authenticated server action
- Provides success feedback with toast
- Closes modal and refreshes user data on success
- Shows server error messages on failure
- Handles loading states appropriately
72-92: LGTM! Well-structured custom hook with proper memoization.The hook correctly:
- Manages modal state
- Provides memoized modal component to prevent unnecessary re-renders
- Returns both setter and component for easy integration
| const secret = totpSecret.base32; | ||
|
|
||
| if (!secret) { | ||
| throw new Error("Failed to generate 2FA secret."); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify that TOTP secret generation is unique per request.
The code accesses totpSecret.base32 directly, which raises a concern about whether a fresh secret is generated for each enable request. Reusing secrets across different users or requests would be a critical security vulnerability.
Please verify that totpSecret generates a new unique secret for each invocation:
🏁 Script executed:
#!/bin/bash
# Description: Check the implementation of totpSecret to ensure it generates unique secrets per request
# Expected: totpSecret should be a function call or generate fresh secrets, not a static value
ast-grep --pattern 'export const totpSecret = $_'
ast-grep --pattern 'export function totpSecret($_) { $$$ }'
ast-grep --pattern 'const totpSecret = $_'Length of output: 514
Ensure TOTP secret is generated per request
Our verification shows that in apps/web/lib/auth/totp.ts you export a single OTPAuth.Secret instance at module load time:
- apps/web/lib/auth/totp.ts:10
export const totpSecret = new OTPAuth.Secret({ size: 20, });
This means the same base32 secret is reused across all users/requests—a critical security risk. You must generate a fresh secret each time.
Locations to update:
- apps/web/lib/auth/totp.ts
- apps/web/lib/actions/auth/enable-two-factor-auth.ts
Suggested changes:
--- a/apps/web/lib/auth/totp.ts
- export const totpSecret = new OTPAuth.Secret({
- size: 20,
- });
+ export function createTotpSecret(): OTPAuth.Secret {
+ return new OTPAuth.Secret({
+ size: 20,
+ });
+ }--- a/apps/web/lib/actions/auth/enable-two-factor-auth.ts
- import { totpSecret } from 'apps/web/lib/auth/totp';
+ import { createTotpSecret } from 'apps/web/lib/auth/totp';
…
- const secret = totpSecret.base32;
+ const secretInstance = createTotpSecret();
+ const secret = secretInstance.base32;This ensures each enable-2FA action uses a unique TOTP secret.
🤖 Prompt for AI Agents
In apps/web/lib/actions/auth/enable-two-factor-auth.ts around lines 22 to 26,
the code uses a static totpSecret instance imported from
apps/web/lib/auth/totp.ts, causing the same TOTP secret to be reused for all
users, which is a security risk. To fix this, refactor apps/web/lib/auth/totp.ts
to export a function that generates and returns a new OTPAuth.Secret instance on
each call instead of a single static instance. Then, update
enable-two-factor-auth.ts to call this function to obtain a fresh secret for
every enable-2FA request, ensuring unique secrets per user.
apps/web/lib/auth/options.ts
Outdated
| window: 1, | ||
| }); | ||
|
|
||
| console.log("totp delta", delta); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove debug logging in production
Debug console.log statement should be removed as it may leak information in production logs.
- console.log("totp delta", delta);📝 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.
| console.log("totp delta", delta); |
🤖 Prompt for AI Agents
In apps/web/lib/auth/options.ts at line 395, remove the debug console.log
statement that outputs "totp delta" and its value to prevent sensitive
information from being logged in production environments.
apps/web/lib/auth/options.ts
Outdated
| throw new Error("no-2fa-token"); | ||
| } | ||
|
|
||
| cookies().delete(TWO_FA_COOKIE_NAME); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify cookie deletion behavior across different scenarios
The 2FA cookie is deleted immediately after reading, but if the subsequent validation fails, the user would need to login again from scratch. Consider deleting the cookie only after successful validation.
🏁 Script executed:
#!/bin/bash
# Search for how the two-factor-required error is handled in the UI
rg -A 5 "two-factor-required" --type tsx --type ts
# Check if there's any retry mechanism for 2FA
ast-grep --pattern 'error === "two-factor-required"'Length of output: 133
🏁 Script executed:
#!/bin/bash
# Search for any occurrences of the two-factor-required error code
rg -n "two-factor-required"
echo "-----"
# Show surrounding lines where the 2FA cookie is deleted to inspect subsequent validation/error handling
rg -n -A10 "cookies().delete(TWO_FA_COOKIE_NAME)" apps/web/lib/auth/options.tsLength of output: 418
🏁 Script executed:
#!/bin/bash
# Find the definition of the 2FA cookie constant
rg -n "TWO_FA_COOKIE_NAME" -A3 -B3 .
# Inspect all cookie-related calls in the 2FA handling file
rg -n "cookies" -A5 -B5 apps/web/lib/auth/options.ts
# Show context around the two-factor-required error throw
rg -n -A5 -B5 "two-factor-required" apps/web/lib/auth/options.tsLength of output: 5077
Delay deleting 2FA cookie until after successful decoding and validation
Ensure that TWO_FA_COOKIE_NAME is only removed once the token is successfully decoded and the one-time code has been verified. Deleting it before decode() (and any OTP checks) forces users back to login on any decode/validation failure.
• File: apps/web/lib/auth/options.ts
– Remove or move the call at line 343 (cookies().delete(TWO_FA_COOKIE_NAME))
– Place it after the decode() call and after your 2FA code has been confirmed
Suggested diff:
- const cookie = cookies().get(TWO_FA_COOKIE_NAME);
- if (!cookie) {
- throw new Error("no-2fa-token");
- }
- cookies().delete(TWO_FA_COOKIE_NAME);
- const decoded = await decode({ token: cookie.value, secret: process.env.NEXTAUTH_SECRET });
+ const cookie = cookies().get(TWO_FA_COOKIE_NAME);
+ if (!cookie) {
+ throw new Error("no-2fa-token");
+ }
+ const decoded = await decode({ token: cookie.value, secret: process.env.NEXTAUTH_SECRET });
+ // Only delete the cookie after decode + 2FA code validation succeeds
+ cookies().delete(TWO_FA_COOKIE_NAME);This change preserves the cookie on decode/validation errors, allowing users to retry entering their 2FA code instead of restarting the login flow from scratch.
📝 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.
| cookies().delete(TWO_FA_COOKIE_NAME); | |
| const cookie = cookies().get(TWO_FA_COOKIE_NAME); | |
| if (!cookie) { | |
| throw new Error("no-2fa-token"); | |
| } | |
| const decoded = await decode({ token: cookie.value, secret: process.env.NEXTAUTH_SECRET }); | |
| // Only delete the cookie after decode + 2FA code validation succeeds | |
| cookies().delete(TWO_FA_COOKIE_NAME); |
🤖 Prompt for AI Agents
In apps/web/lib/auth/options.ts at line 343, the TWO_FA_COOKIE_NAME cookie is
deleted immediately after reading, which causes users to restart login if
validation fails. To fix this, move the cookies().delete(TWO_FA_COOKIE_NAME)
call so it occurs only after the token is successfully decoded and the one-time
2FA code is verified. This ensures the cookie remains available for retries on
decode or validation errors, improving user experience.
- Updated UserProps interface to change twoFactorRecoveryCodes type from string[] to string. - Implemented rate limiting for 2FA attempts in authOptions. - Modified DisableTwoFactorAuthModal to prevent default form submission behavior. - Added useEffect in EnableTwoFactorAuthModal to reset state when modal is shown.
|
🤖 Bug0 QA Agent Here are the results of the automated tests for PR #2505:
To re-run the tests, please comment |
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes
Chores