feat: Add email-password authentication #327
Conversation
- Add passwordHash field to User model and PasswordResetToken table - Configure NextAuth with JWT sessions and CredentialsProvider - Create password validation utilities with Zod schemas - Add signup API route with OAuth user password linking support - Update login page with password/magic link toggle - Create dedicated signup page with password registration - Implement complete password reset flow: - Forgot password API and page - Reset password API and page - Password reset email template - Add sendPasswordResetEmail to mailer This enables users to sign up and login with email/password alongside existing OAuth (Google, GitHub) authentication.
- Create user tRPC router with password management procedures: - hasPassword: check if user has password set - getLinkedAccounts: list OAuth providers linked - changePassword: change existing password - setPassword: add password for OAuth-only users - getProfile: get user profile info - Create account settings page at /settings/account: - Profile info display - Linked OAuth accounts section - Change/set password forms - Add Account tab to settings navigation
- Add rate limiting for credentials login (5 attempts/minute) - Add rate limiting for password reset (3 requests/hour) - Use existing Redis and IP detection patterns
|
@Divkix is attempting to deploy a commit to the kmkoushik's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds password-based authentication: DB changes (User.passwordHash, PasswordResetToken table and indexes); NextAuth credentials provider with JWT sessions; credential signin rate limiting; TRPC user router (hasPassword, getLinkedAccounts, changePassword, setPassword, getProfile); API routes for signup, forgot-password, reset-password; client pages for login, signup, forgot-password, reset-password, and account settings (password forms); password validation utilities; password-reset email template and mailer function. Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (6)
🧰 Additional context used📓 Path-based instructions (6)**/*.{tsx,ts,jsx,js}📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Files:
**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{tsx,jsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
apps/web/**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
apps/web/**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{ts,tsx,md}📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧬 Code graph analysis (4)apps/web/src/app/login/login-page.tsx (4)
apps/web/src/app/forgot-password/page.tsx (4)
apps/web/src/app/api/auth/reset-password/route.ts (5)
apps/web/src/server/auth.ts (3)
⏰ 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)
🔇 Additional comments (12)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
6 issues found across 20 files
Prompt for AI agents (all 6 issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/web/src/app/reset-password/page.tsx">
<violation number="1" location="apps/web/src/app/reset-password/page.tsx:21">
P2: Duplicate password validation schema. Import `passwordSchema` from `~/server/password-utils` instead of defining it locally. This ensures consistency across the codebase and reduces maintenance burden if password requirements change.</violation>
</file>
<file name="apps/web/src/app/api/auth/signup/route.ts">
<violation number="1" location="apps/web/src/app/api/auth/signup/route.ts:36">
P0: Critical account takeover vulnerability: Adding a password to an existing OAuth user's account without any ownership verification. An attacker who knows an OAuth user's email can add a password and gain full access to their account. The user should receive a verification email or be required to authenticate via their existing OAuth provider before a password can be added.</violation>
</file>
<file name="apps/web/src/app/login/login-page.tsx">
<violation number="1" location="apps/web/src/app/login/login-page.tsx:224">
P2: The `loginError` state is not cleared when switching between auth modes. If a user gets a password login error and then switches to "Magic Link" mode, the stale error message will still be displayed, which is confusing.</violation>
</file>
<file name="apps/web/src/app/api/auth/reset-password/route.ts">
<violation number="1" location="apps/web/src/app/api/auth/reset-password/route.ts:42">
P1: Password update and token deletion should be wrapped in a database transaction to prevent race conditions. Without atomicity, concurrent requests could reuse the same reset token, and partial failures could leave tokens valid after password updates.</violation>
</file>
<file name="apps/web/src/app/forgot-password/page.tsx">
<violation number="1" location="apps/web/src/app/forgot-password/page.tsx:28">
P2: Missing `defaultValues` in `useForm` hook. Without default values, the email input starts as `undefined`, causing React's controlled/uncontrolled component warning when the user types.</violation>
</file>
<file name="apps/web/src/server/auth.ts">
<violation number="1" location="apps/web/src/server/auth.ts:112">
P1: Timing attack vulnerability enables email enumeration. The early return when user doesn't exist (fast) vs. running `verifySecureHash` when user exists (slow scrypt computation) creates a measurable timing difference. Consider always running the hash verification against a dummy hash when user is not found to normalize response timing.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
| import Link from "next/link"; | ||
| import { useSearchParams } from "next/navigation"; | ||
|
|
||
| const passwordSchema = z |
There was a problem hiding this comment.
P2: Duplicate password validation schema. Import passwordSchema from ~/server/password-utils instead of defining it locally. This ensures consistency across the codebase and reduces maintenance burden if password requirements change.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/app/reset-password/page.tsx, line 21:
<comment>Duplicate password validation schema. Import `passwordSchema` from `~/server/password-utils` instead of defining it locally. This ensures consistency across the codebase and reduces maintenance burden if password requirements change.</comment>
<file context>
@@ -0,0 +1,244 @@
+import Link from "next/link";
+import { useSearchParams } from "next/navigation";
+
+const passwordSchema = z
+ .object({
+ password: z
</file context>
✅ Addressed in 48e480c
| ); | ||
| } | ||
|
|
||
| // User exists via OAuth, allow adding password |
There was a problem hiding this comment.
P0: Critical account takeover vulnerability: Adding a password to an existing OAuth user's account without any ownership verification. An attacker who knows an OAuth user's email can add a password and gain full access to their account. The user should receive a verification email or be required to authenticate via their existing OAuth provider before a password can be added.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/app/api/auth/signup/route.ts, line 36:
<comment>Critical account takeover vulnerability: Adding a password to an existing OAuth user's account without any ownership verification. An attacker who knows an OAuth user's email can add a password and gain full access to their account. The user should receive a verification email or be required to authenticate via their existing OAuth provider before a password can be added.</comment>
<file context>
@@ -0,0 +1,90 @@
+ );
+ }
+
+ // User exists via OAuth, allow adding password
+ const passwordHash = await createSecureHash(password);
+ await db.user.update({
</file context>
✅ Addressed in 48e480c
| type="button" | ||
| variant={authMode === "password" ? "default" : "outline"} | ||
| className="flex-1" | ||
| onClick={() => setAuthMode("password")} |
There was a problem hiding this comment.
P2: The loginError state is not cleared when switching between auth modes. If a user gets a password login error and then switches to "Magic Link" mode, the stale error message will still be displayed, which is confusing.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/app/login/login-page.tsx, line 224:
<comment>The `loginError` state is not cleared when switching between auth modes. If a user gets a password login error and then switches to "Magic Link" mode, the stale error message will still be displayed, which is confusing.</comment>
<file context>
@@ -178,7 +214,96 @@ export default function LoginPage({
+ type="button"
+ variant={authMode === "password" ? "default" : "outline"}
+ className="flex-1"
+ onClick={() => setAuthMode("password")}
+ >
+ Password
</file context>
✅ Addressed in 48e480c
|
|
||
| const passwordHash = await createSecureHash(password); | ||
|
|
||
| await db.user.update({ |
There was a problem hiding this comment.
P1: Password update and token deletion should be wrapped in a database transaction to prevent race conditions. Without atomicity, concurrent requests could reuse the same reset token, and partial failures could leave tokens valid after password updates.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/app/api/auth/reset-password/route.ts, line 42:
<comment>Password update and token deletion should be wrapped in a database transaction to prevent race conditions. Without atomicity, concurrent requests could reuse the same reset token, and partial failures could leave tokens valid after password updates.</comment>
<file context>
@@ -0,0 +1,61 @@
+
+ const passwordHash = await createSecureHash(password);
+
+ await db.user.update({
+ where: { email: resetToken.email },
+ data: {
</file context>
✅ Addressed in 48e480c
| export default function ForgotPasswordPage() { | ||
| const [status, setStatus] = useState<"idle" | "sending" | "success">("idle"); | ||
|
|
||
| const form = useForm<z.infer<typeof emailSchema>>({ |
There was a problem hiding this comment.
P2: Missing defaultValues in useForm hook. Without default values, the email input starts as undefined, causing React's controlled/uncontrolled component warning when the user types.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/app/forgot-password/page.tsx, line 28:
<comment>Missing `defaultValues` in `useForm` hook. Without default values, the email input starts as `undefined`, causing React's controlled/uncontrolled component warning when the user types.</comment>
<file context>
@@ -0,0 +1,139 @@
+export default function ForgotPasswordPage() {
+ const [status, setStatus] = useState<"idle" | "sending" | "success">("idle");
+
+ const form = useForm<z.infer<typeof emailSchema>>({
+ resolver: zodResolver(emailSchema),
+ });
</file context>
✅ Addressed in 48e480c
apps/web/src/server/auth.ts
Outdated
|
|
||
| const user = await db.user.findUnique({ where: { email } }); | ||
|
|
||
| if (!user || !user.passwordHash) { |
There was a problem hiding this comment.
P1: Timing attack vulnerability enables email enumeration. The early return when user doesn't exist (fast) vs. running verifySecureHash when user exists (slow scrypt computation) creates a measurable timing difference. Consider always running the hash verification against a dummy hash when user is not found to normalize response timing.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/server/auth.ts, line 112:
<comment>Timing attack vulnerability enables email enumeration. The early return when user doesn't exist (fast) vs. running `verifySecureHash` when user exists (slow scrypt computation) creates a measurable timing difference. Consider always running the hash verification against a dummy hash when user is not found to normalize response timing.</comment>
<file context>
@@ -88,6 +90,47 @@ function getProviders() {
+
+ const user = await db.user.findUnique({ where: { email } });
+
+ if (!user || !user.passwordHash) {
+ return null;
+ }
</file context>
✅ Addressed in 48e480c
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (17)
apps/web/prisma/schema.prisma (1)
96-105: Consider cleanup strategy for expired password reset tokens.The
PasswordResetTokenmodel is well-structured with appropriate indexes. However, consider these points:
- The composite unique constraint
@@unique([email, token])on Line 103 is redundant sincetokenalready has a@uniqueconstraint on Line 99.- No relation to the
Usermodel means orphaned tokens could accumulate if users are deleted.Recommended improvements
Option 1: Remove redundant composite unique constraint
@@unique([email, token])Since
tokenis already unique globally, this composite constraint provides no additional enforcement.Option 2: Add a background job to clean up expired tokens
Consider implementing a periodic cleanup task to delete expired tokens:// Example cleanup job await db.passwordResetToken.deleteMany({ where: { expires: { lt: new Date() } } });apps/web/src/app/api/auth/[...nextauth]/route.ts (1)
87-116: Rate limiting implementation is solid.The credentials rate limiting follows the established pattern from email signin rate limiting and uses appropriate limits (5 attempts/60 seconds).
Consider extracting the rate limiting logic into a reusable helper to reduce duplication:
Optional refactoring
The credentials rate limit uses a hardcoded value of
5(Line 98), while email rate limiting usesenv.AUTH_EMAIL_RATE_LIMIT. For consistency, consider:
- Adding an environment variable for credentials rate limiting
- Extracting common rate-limit logic into a helper function
async function checkRateLimit( req: Request, key: string, limit: number, ttl: number, errorMessage: string ): Promise<Response | null> { const ip = getClientIp(req); if (!ip) { logger.warn(`Rate limit skipped for ${key}: missing client IP`); return null; } const redis = getRedis(); const rateLimitKey = `${key}:${ip}`; const count = await redis.incr(rateLimitKey); if (count === 1) await redis.expire(rateLimitKey, ttl); if (count > limit) { logger.warn({ ip }, `Rate limit exceeded for ${key}`); return Response.json( { error: { code: "RATE_LIMITED", message: errorMessage } }, { status: 429 } ); } return null; }apps/web/src/app/api/auth/reset-password/route.ts (2)
54-55: Use logger instead of console.error for consistency.The codebase uses a structured logger (imported in other files). Use it here for consistent logging and proper log levels.
Recommended change
+ import { logger } from "~/server/logger/log"; } catch (error) { - console.error("Reset password error:", error); + logger.error({ err: error }, "Reset password error"); return NextResponse.json(
42-51: Consider wrapping database operations in a transaction.The user update (Lines 42-48) and token deletion (Line 51) are separate operations. If the user update succeeds but token deletion fails, the token could be reused.
Optional transaction wrapper
const passwordHash = await createSecureHash(password); - await db.user.update({ - where: { email: resetToken.email }, - data: { - passwordHash, - emailVerified: new Date(), - }, - }); - - // Delete used token - await db.passwordResetToken.delete({ where: { token } }); + await db.$transaction([ + db.user.update({ + where: { email: resetToken.email }, + data: { + passwordHash, + emailVerified: new Date(), + }, + }), + db.passwordResetToken.delete({ where: { token } }), + ]);This ensures both operations succeed or fail together.
apps/web/src/app/api/auth/forgot-password/route.ts (3)
62-63: Consider atomic rate limiting to avoid orphan keys.The
incrthen conditionalexpirepattern has a small race: if the process terminates afterincrbut beforeexpire(whencount === 1), the key persists indefinitely. While unlikely, this could leave orphan keys in Redis.A more robust approach uses a Lua script or the
SET NX EXpattern withINCR:🔎 Suggested fix using MULTI/EXEC
- const count = await redis.incr(key); - if (count === 1) await redis.expire(key, ttl); + const results = await redis + .multi() + .incr(key) + .expire(key, ttl) + .exec(); + const count = results?.[0]?.[1] as number;
115-117: Uselogger.errorfor consistent logging.The file imports and uses
loggerelsewhere (lines 65, 70, 73), but the catch block usesconsole.error. This inconsistency may affect log aggregation and structured logging.🔎 Suggested fix
} catch (error) { - console.error("Forgot password error:", error); + logger.error({ err: error }, "Forgot password error"); return NextResponse.json({ success: true }); }
13-51: Consider extractinggetClientIpto a shared utility.This IP extraction logic may be duplicated elsewhere (e.g., rate limiting in the auth route). Extracting it to a shared utility (e.g.,
~/server/utils/ip.ts) would improve maintainability and ensure consistent behavior across rate-limited endpoints.apps/web/src/app/api/auth/signup/route.ts (2)
7-89: Consider adding rate limiting to prevent mass account creation.The forgot-password route implements IP-based rate limiting, but signup does not. Without rate limiting, an attacker could create accounts in bulk, potentially exhausting database resources or facilitating spam/abuse.
Consider reusing the Redis-based rate limiting pattern from
forgot-password/route.ts.
83-85: Use a structured logger instead ofconsole.error.For consistency with other auth routes (e.g.,
forgot-password), use a structured logger for better observability.🔎 Suggested fix
+import { logger } from "~/server/logger/log"; + // ... in catch block: } catch (error) { - console.error("Signup error:", error); + logger.error({ err: error }, "Signup error"); return NextResponse.json(apps/web/prisma/migrations/20251219093000_add_password_auth/migration.sql (1)
21-22: Redundant composite unique index.Since
tokenalready has a unique constraint (line 16), the composite unique index on(email, token)provides no additional uniqueness guarantee—iftokenis globally unique, any(email, token)pair is inherently unique.If this index exists for query optimization (e.g., queries filtering by both
token), it's unnecessary since the application looks up tokens bytokenalone. Consider removing it to reduce storage and index maintenance overhead.🔎 Suggested fix
-- CreateIndex CREATE INDEX "PasswordResetToken_email_idx" ON "PasswordResetToken"("email"); - --- CreateIndex -CREATE UNIQUE INDEX "PasswordResetToken_email_token_key" ON "PasswordResetToken"("email", "token");Note: Also update the Prisma schema to remove
@@unique([email, token])if present.apps/web/src/app/forgot-password/page.tsx (1)
28-30: AdddefaultValuesto prevent controlled input warnings.Without
defaultValues,field.valuemay beundefinedon initial render, which can cause React warnings about switching between controlled and uncontrolled inputs.🔎 Suggested fix
const form = useForm<z.infer<typeof emailSchema>>({ resolver: zodResolver(emailSchema), + defaultValues: { + email: "", + }, });apps/web/src/app/reset-password/page.tsx (1)
21-35: Consider reusing the shared password schema.This schema duplicates the validation rules from
~/server/password-utils.ts. For consistency and maintainability, consider importing the basepasswordSchemaand extending it here:+import { passwordSchema as basePasswordSchema } from "~/server/password-utils"; -const passwordSchema = z +const resetPasswordSchema = z .object({ - password: z - .string() - .min(8, "Password must be at least 8 characters") - .max(128, "Password must be less than 128 characters") - .regex(/[a-z]/, "Password must contain at least one lowercase letter") - .regex(/[A-Z]/, "Password must contain at least one uppercase letter") - .regex(/[0-9]/, "Password must contain at least one number"), + password: basePasswordSchema, confirmPassword: z.string(), })This ensures password requirements stay synchronized across all forms.
apps/web/src/server/api/routers/user.ts (2)
40-43: Consider selecting only required fields.The query fetches the entire user record but only needs
passwordHashfor validation.🔎 Proposed fix
const user = await ctx.db.user.findUnique({ where: { id: ctx.session.user.id }, + select: { id: true, passwordHash: true }, });
86-89: Same optimization applies here.For consistency with
changePassword, consider selecting only required fields.🔎 Proposed fix
const user = await ctx.db.user.findUnique({ where: { id: ctx.session.user.id }, + select: { id: true, passwordHash: true }, });apps/web/src/server/auth.ts (2)
93-132: Credentials provider implementation looks solid, but consider timing attack mitigation.The authorize function correctly normalizes email and verifies the password hash. However, when a user doesn't exist (
!user), the function returns immediately without performing any hash verification, which could allow timing-based user enumeration attacks.Consider performing a dummy hash verification when the user doesn't exist to equalize response times:
🔎 Proposed timing-safe improvement
const user = await db.user.findUnique({ where: { email } }); if (!user || !user.passwordHash) { + // Perform dummy verification to prevent timing attacks + await verifySecureHash(password, "dummy$dummyhashvalue"); return null; } const isValid = await verifySecureHash(password, user.passwordHash); if (!isValid) { return null; }
161-170: Type assertions on token fields could be fragile.The
ascasts assume the token always contains these fields with correct types. If the token is corrupted or tampered with, this could lead to unexpected behavior. Consider adding runtime validation or using a more defensive approach.🔎 Proposed defensive typing
session: ({ session, token }) => ({ ...session, user: { ...session.user, - id: token.id as number, - isBetaUser: token.isBetaUser as boolean, - isAdmin: token.isAdmin as boolean, - isWaitlisted: token.isWaitlisted as boolean, + id: typeof token.id === "number" ? token.id : 0, + isBetaUser: Boolean(token.isBetaUser), + isAdmin: Boolean(token.isAdmin), + isWaitlisted: Boolean(token.isWaitlisted), }, }),apps/web/src/server/password-utils.ts (1)
31-35: Error message for optionalnamefield is misleading.The
namefield is marked as.optional(), but the.min(1)error says "Name is required". This is confusing since the field can be omitted. If provided, it must be non-empty, so consider a clearer message:🔎 Proposed fix
export const signupSchema = z.object({ email: z.string().email("Invalid email address"), password: passwordSchema, - name: z.string().min(1, "Name is required").optional(), + name: z.string().min(1, "Name cannot be empty").optional(), });
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
apps/web/prisma/migrations/20251219093000_add_password_auth/migration.sql(1 hunks)apps/web/prisma/schema.prisma(2 hunks)apps/web/src/app/(dashboard)/settings/account/page.tsx(1 hunks)apps/web/src/app/(dashboard)/settings/layout.tsx(1 hunks)apps/web/src/app/api/auth/[...nextauth]/route.ts(2 hunks)apps/web/src/app/api/auth/forgot-password/route.ts(1 hunks)apps/web/src/app/api/auth/reset-password/route.ts(1 hunks)apps/web/src/app/api/auth/signup/route.ts(1 hunks)apps/web/src/app/forgot-password/page.tsx(1 hunks)apps/web/src/app/login/login-page.tsx(6 hunks)apps/web/src/app/reset-password/page.tsx(1 hunks)apps/web/src/app/signup/page.tsx(2 hunks)apps/web/src/app/signup/signup-page.tsx(1 hunks)apps/web/src/server/api/root.ts(2 hunks)apps/web/src/server/api/routers/user.ts(1 hunks)apps/web/src/server/auth.ts(3 hunks)apps/web/src/server/email-templates/PasswordResetEmail.tsx(1 hunks)apps/web/src/server/email-templates/index.ts(1 hunks)apps/web/src/server/mailer.ts(3 hunks)apps/web/src/server/password-utils.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{tsx,ts,jsx,js}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Include all required imports and ensure proper naming of key components in React/NextJS code
Files:
apps/web/src/app/signup/page.tsxapps/web/src/app/api/auth/signup/route.tsapps/web/src/app/api/auth/reset-password/route.tsapps/web/src/app/(dashboard)/settings/layout.tsxapps/web/src/app/login/login-page.tsxapps/web/src/server/api/root.tsapps/web/src/server/email-templates/PasswordResetEmail.tsxapps/web/src/app/signup/signup-page.tsxapps/web/src/app/forgot-password/page.tsxapps/web/src/server/mailer.tsapps/web/src/server/api/routers/user.tsapps/web/src/app/api/auth/forgot-password/route.tsapps/web/src/server/email-templates/index.tsapps/web/src/app/(dashboard)/settings/account/page.tsxapps/web/src/app/reset-password/page.tsxapps/web/src/app/api/auth/[...nextauth]/route.tsapps/web/src/server/password-utils.tsapps/web/src/server/auth.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use TypeScript-first approach with 2-space indent and semicolons enabled by Prettier in apps/web (Next.js), apps/marketing, apps/smtp-server, and all packages
Never use dynamic imports; always import on the top level
Run ESLint via @usesend/eslint-config and ensure no warnings remain before submitting PRs
Files:
apps/web/src/app/signup/page.tsxapps/web/src/app/api/auth/signup/route.tsapps/web/src/app/api/auth/reset-password/route.tsapps/web/src/app/(dashboard)/settings/layout.tsxapps/web/src/app/login/login-page.tsxapps/web/src/server/api/root.tsapps/web/src/server/email-templates/PasswordResetEmail.tsxapps/web/src/app/signup/signup-page.tsxapps/web/src/app/forgot-password/page.tsxapps/web/src/server/mailer.tsapps/web/src/server/api/routers/user.tsapps/web/src/app/api/auth/forgot-password/route.tsapps/web/src/server/email-templates/index.tsapps/web/src/app/(dashboard)/settings/account/page.tsxapps/web/src/app/reset-password/page.tsxapps/web/src/app/api/auth/[...nextauth]/route.tsapps/web/src/server/password-utils.tsapps/web/src/server/auth.ts
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
React components must use PascalCase naming convention (e.g., AppSideBar.tsx)
Files:
apps/web/src/app/signup/page.tsxapps/web/src/app/(dashboard)/settings/layout.tsxapps/web/src/app/login/login-page.tsxapps/web/src/server/email-templates/PasswordResetEmail.tsxapps/web/src/app/signup/signup-page.tsxapps/web/src/app/forgot-password/page.tsxapps/web/src/app/(dashboard)/settings/account/page.tsxapps/web/src/app/reset-password/page.tsx
apps/web/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use alias
~/for src imports in apps/web (e.g.,import { x } from "~/utils/x")
Files:
apps/web/src/app/signup/page.tsxapps/web/src/app/api/auth/signup/route.tsapps/web/src/app/api/auth/reset-password/route.tsapps/web/src/app/(dashboard)/settings/layout.tsxapps/web/src/app/login/login-page.tsxapps/web/src/server/api/root.tsapps/web/src/server/email-templates/PasswordResetEmail.tsxapps/web/src/app/signup/signup-page.tsxapps/web/src/app/forgot-password/page.tsxapps/web/src/server/mailer.tsapps/web/src/server/api/routers/user.tsapps/web/src/app/api/auth/forgot-password/route.tsapps/web/src/server/email-templates/index.tsapps/web/src/app/(dashboard)/settings/account/page.tsxapps/web/src/app/reset-password/page.tsxapps/web/src/app/api/auth/[...nextauth]/route.tsapps/web/src/server/password-utils.tsapps/web/src/server/auth.ts
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/**/*.{ts,tsx}: Prefer to use TRPC for client-server communication unless explicitly asked otherwise in apps/web
Use Prisma for database access in apps/web
Files:
apps/web/src/app/signup/page.tsxapps/web/src/app/api/auth/signup/route.tsapps/web/src/app/api/auth/reset-password/route.tsapps/web/src/app/(dashboard)/settings/layout.tsxapps/web/src/app/login/login-page.tsxapps/web/src/server/api/root.tsapps/web/src/server/email-templates/PasswordResetEmail.tsxapps/web/src/app/signup/signup-page.tsxapps/web/src/app/forgot-password/page.tsxapps/web/src/server/mailer.tsapps/web/src/server/api/routers/user.tsapps/web/src/app/api/auth/forgot-password/route.tsapps/web/src/server/email-templates/index.tsapps/web/src/app/(dashboard)/settings/account/page.tsxapps/web/src/app/reset-password/page.tsxapps/web/src/app/api/auth/[...nextauth]/route.tsapps/web/src/server/password-utils.tsapps/web/src/server/auth.ts
**/*.{ts,tsx,md}
📄 CodeRabbit inference engine (AGENTS.md)
Run Prettier 3 for code formatting on TypeScript, TSX, and Markdown files
Files:
apps/web/src/app/signup/page.tsxapps/web/src/app/api/auth/signup/route.tsapps/web/src/app/api/auth/reset-password/route.tsapps/web/src/app/(dashboard)/settings/layout.tsxapps/web/src/app/login/login-page.tsxapps/web/src/server/api/root.tsapps/web/src/server/email-templates/PasswordResetEmail.tsxapps/web/src/app/signup/signup-page.tsxapps/web/src/app/forgot-password/page.tsxapps/web/src/server/mailer.tsapps/web/src/server/api/routers/user.tsapps/web/src/app/api/auth/forgot-password/route.tsapps/web/src/server/email-templates/index.tsapps/web/src/app/(dashboard)/settings/account/page.tsxapps/web/src/app/reset-password/page.tsxapps/web/src/app/api/auth/[...nextauth]/route.tsapps/web/src/server/password-utils.tsapps/web/src/server/auth.ts
🧬 Code graph analysis (11)
apps/web/src/app/signup/page.tsx (1)
apps/web/src/app/signup/signup-page.tsx (1)
SignupPage(57-320)
apps/web/src/app/api/auth/reset-password/route.ts (3)
apps/web/src/server/password-utils.ts (1)
resetPasswordSchema(63-66)apps/web/src/server/db.ts (1)
db(20-20)apps/web/src/server/crypto.ts (1)
createSecureHash(3-10)
apps/web/src/app/(dashboard)/settings/layout.tsx (1)
apps/web/src/app/(dashboard)/dev-settings/settings-nav-button.tsx (1)
SettingsNavButton(7-39)
apps/web/src/server/api/root.ts (1)
apps/web/src/server/api/routers/user.ts (1)
userRouter(7-133)
apps/web/src/app/signup/signup-page.tsx (5)
apps/web/src/server/password-utils.ts (1)
signupSchema(31-35)packages/ui/src/button.tsx (1)
Button(80-80)packages/ui/src/spinner.tsx (1)
Spinner(4-51)packages/ui/src/form.tsx (7)
Form(170-170)FormField(176-176)FormItem(171-171)FormLabel(172-172)FormControl(173-173)FormMessage(175-175)FormDescription(174-174)packages/ui/src/input.tsx (1)
Input(25-25)
apps/web/src/server/mailer.ts (2)
apps/web/src/server/email-templates/PasswordResetEmail.tsx (1)
renderPasswordResetEmail(83-87)apps/web/src/server/email-templates/index.ts (1)
renderPasswordResetEmail(13-13)
apps/web/src/app/api/auth/forgot-password/route.ts (4)
apps/web/src/app/api/auth/reset-password/route.ts (1)
POST(6-61)apps/web/src/server/redis.ts (1)
getRedis(6-13)apps/web/src/server/db.ts (1)
db(20-20)apps/web/src/server/mailer.ts (1)
sendPasswordResetEmail(76-95)
apps/web/src/app/(dashboard)/settings/account/page.tsx (1)
apps/web/src/server/password-utils.ts (1)
passwordSchema(11-17)
apps/web/src/app/reset-password/page.tsx (4)
apps/web/src/server/password-utils.ts (1)
passwordSchema(11-17)packages/ui/src/button.tsx (1)
Button(80-80)packages/ui/src/form.tsx (6)
Form(170-170)FormField(176-176)FormItem(171-171)FormLabel(172-172)FormControl(173-173)FormMessage(175-175)packages/ui/src/input.tsx (1)
Input(25-25)
apps/web/src/app/api/auth/[...nextauth]/route.ts (2)
apps/web/src/server/logger/log.ts (1)
logger(31-63)apps/web/src/server/redis.ts (1)
getRedis(6-13)
apps/web/src/server/auth.ts (3)
apps/web/src/server/db.ts (1)
db(20-20)apps/web/src/server/crypto.ts (1)
verifySecureHash(12-19)apps/web/src/env.js (2)
env(5-145)env(5-145)
⏰ 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). (2)
- GitHub Check: Agent
- GitHub Check: cubic · AI code reviewer
🔇 Additional comments (29)
apps/web/src/server/api/root.ts (1)
17-17: LGTM! User router properly integrated.The new user router follows the established pattern and is correctly imported and mounted at the top level.
Also applies to: 40-40
apps/web/src/app/signup/page.tsx (1)
3-3: LGTM! Clean refactor separating signup and login pages.The component properly uses the new dedicated
SignupPagecomponent and follows naming conventions.Also applies to: 6-6, 15-15
apps/web/src/app/(dashboard)/settings/layout.tsx (1)
29-29: LGTM! Account navigation properly added.The new Account settings link is unconditionally rendered for all users, which is appropriate for personal account management.
apps/web/src/server/email-templates/index.ts (1)
11-14: LGTM! Password reset email template properly exported.The export follows the established pattern for email templates in this module.
apps/web/prisma/schema.prisma (1)
87-87: LGTM! Nullable password hash field correctly supports OAuth-only users.The nullable
passwordHashfield appropriately accommodates users who authenticate via OAuth without setting a password.apps/web/src/app/api/auth/[...nextauth]/route.ts (1)
53-53: Good refactor moving URL construction to shared location.This avoids duplication between the rate limiting blocks.
apps/web/src/app/api/auth/reset-password/route.ts (1)
46-46: Verify email on password reset is a good security practice.Setting
emailVerifiedwhen resetting a password is correct because the user proved email ownership by receiving and using the reset token.apps/web/src/server/mailer.ts (1)
76-95: LGTM! Password reset email function follows established patterns.The implementation mirrors existing email functions (
sendSignUpEmail,sendTeamInviteEmail) and correctly handles dev/production environments. The token expiration time mentioned in the email text ("1 hour") matches the actual expiration set when creating the reset token (60 * 60 * 1000 milliseconds in forgot-password/route.ts).apps/web/src/server/email-templates/PasswordResetEmail.tsx (1)
1-87: LGTM!The email template is well-structured with clear messaging, consistent styling, and proper type definitions. The security guidance (1-hour expiry notice and "safe to ignore" message) follows email best practices.
apps/web/prisma/migrations/20251219093000_add_password_auth/migration.sql (1)
1-19: Schema additions look good.The
passwordHashas nullable TEXT appropriately supports optional password authentication alongside OAuth. ThePasswordResetTokentable structure with token uniqueness and email index aligns well with the password reset flow requirements.apps/web/src/app/forgot-password/page.tsx (1)
32-46: Good security practice: enumeration prevention.Always transitioning to
successstate regardless of the fetch outcome correctly prevents email enumeration attacks. The matching behavior on the server side (always returning success) completes this security pattern.apps/web/src/app/login/login-page.tsx (2)
121-142: LGTM!The password submit handler properly manages loading state, handles errors gracefully with a user-friendly message, and redirects correctly while respecting invite-based paths.
219-236: LGTM!The auth mode toggle provides a clean UX for switching between password and magic link authentication methods.
apps/web/src/app/reset-password/page.tsx (2)
54-84: LGTM!The form submission logic handles API errors gracefully with appropriate user feedback. The defensive token check at line 55 is redundant since the component already guards against missing tokens at line 86, but it's harmless.
223-244: LGTM!Properly wraps the form component in Suspense as required by Next.js App Router when using
useSearchParams.apps/web/src/app/signup/signup-page.tsx (2)
83-128: LGTM!The signup flow with auto-login after successful account creation provides excellent UX. The fallback redirect to the login page ensures users aren't stuck if auto-login fails.
206-315: LGTM!The form implementation is well-structured with proper validation feedback, password requirement hints, and loading states.
apps/web/src/app/(dashboard)/settings/account/page.tsx (5)
33-54: LGTM!The password schemas are well-defined with proper refinement for confirmation matching. Reusing
passwordSchemafrom password-utils ensures consistent validation rules.
84-184: LGTM!The ChangePasswordForm properly handles the mutation lifecycle with loading states, success toast, form reset, and error handling.
186-268: LGTM!Good use of query invalidation (
utils.user.hasPassword.invalidate()) to ensure the UI updates immediately after setting a password.
307-334: LGTM!The ProfileSection handles edge cases gracefully with proper fallbacks for missing name/email/image.
336-385: LGTM!The loading state aggregation and skeleton UI provide a polished loading experience. The page correctly waits for all queries before rendering content.
apps/web/src/server/api/routers/user.ts (2)
11-17: LGTM!The
hasPasswordquery efficiently checks for password existence without exposing the hash.
115-132: LGTM!The
getProfilequery properly selects only the necessary fields and handles the not-found case.apps/web/src/server/auth.ts (2)
146-172: Session and adapter configuration looks correct for mixed provider setup.Using JWT strategy with the PrismaAdapter is the right approach here. The adapter handles OAuth/magic-link users while credentials-based users are managed separately via the signup API route. Note that the
createUserevent (lines 176-203) won't trigger for email/password signups since they don't go through the adapter.
11-17: Imports are correctly structured.The new imports follow the project's coding guidelines using the
~/alias for src imports and are all top-level as required.apps/web/src/server/password-utils.ts (3)
11-17: Password validation schema is well-designed.The password requirements (8-128 chars, mixed case, number) strike a good balance between security and usability. The explicit error messages for each constraint will provide clear feedback to users.
40-66: Login and password management schemas are well-structured.Good design choices:
passwordLoginSchemacorrectly uses simple presence validation (not strength) since the password is verified against the stored hashchangePasswordSchemavalidates the new password but not the current one (correct, since current is verified server-side)resetPasswordSchemaincludes token validation alongside password requirements
24-26: Helper function provides clean API for password validation.The
validatePasswordwrapper is useful for getting detailed error information viasafeParsewhen you need to handle validation results programmatically.
| await db.user.create({ | ||
| data: { | ||
| email: normalizedEmail, | ||
| name: name ?? null, | ||
| passwordHash, | ||
| isBetaUser, | ||
| isWaitlisted, | ||
| emailVerified: new Date(), // Mark as verified since they're signing up directly | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Security: emailVerified is set without actual email verification.
Setting emailVerified: new Date() at signup allows users to claim any email address without proving ownership. This enables:
- Account squatting with emails they don't own
- The legitimate email owner being blocked from signing up later
- Potential bypassing of email-gated features
Either remove emailVerified from signup (leave it null) and implement an email verification flow, or acknowledge this as an intentional trade-off with appropriate downstream protections.
🔎 Suggested fix - remove premature verification
await db.user.create({
data: {
email: normalizedEmail,
name: name ?? null,
passwordHash,
isBetaUser,
isWaitlisted,
- emailVerified: new Date(), // Mark as verified since they're signing up directly
},
});📝 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.
| await db.user.create({ | |
| data: { | |
| email: normalizedEmail, | |
| name: name ?? null, | |
| passwordHash, | |
| isBetaUser, | |
| isWaitlisted, | |
| emailVerified: new Date(), // Mark as verified since they're signing up directly | |
| }, | |
| }); | |
| await db.user.create({ | |
| data: { | |
| email: normalizedEmail, | |
| name: name ?? null, | |
| passwordHash, | |
| isBetaUser, | |
| isWaitlisted, | |
| }, | |
| }); |
🤖 Prompt for AI Agents
In apps/web/src/app/api/auth/signup/route.ts around lines 68 to 77, the signup
flow is setting emailVerified: new Date() which marks emails as verified without
ownership proof; remove the emailVerified assignment so new users have a
null/unset emailVerified value and implement an email verification flow (send
verification token/email after account creation and set emailVerified only after
the token is validated) or, if intentionally allowing auto-verified emails, add
a clear code comment and downstream protections (feature gating, re-verification
for sensitive actions) and record an audit flag to indicate the trade-off.
| export default function SignupPage({ | ||
| providers, | ||
| }: { | ||
| providers?: ClientSafeProvider[]; | ||
| }) { | ||
| const router = useRouter(); | ||
| const searchParams = useNextSearchParams(); | ||
| const inviteId = searchParams.get("inviteId"); |
There was a problem hiding this comment.
Missing Suspense boundary for useSearchParams.
This component uses useSearchParams (line 63) but isn't wrapped in a Suspense boundary. In Next.js App Router, this can cause the entire page to opt into client-side rendering during static generation. Consider wrapping the component similar to how reset-password/page.tsx handles it:
🔎 Proposed fix
+import { Suspense } from "react";
// ... existing code ...
-export default function SignupPage({
+function SignupPageContent({
providers,
}: {
providers?: ClientSafeProvider[];
}) {
// ... component body ...
}
+
+export default function SignupPage({
+ providers,
+}: {
+ providers?: ClientSafeProvider[];
+}) {
+ return (
+ <Suspense fallback={<div className="min-h-screen flex justify-center items-center">Loading...</div>}>
+ <SignupPageContent providers={providers} />
+ </Suspense>
+ );
+}There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive email-password authentication to useSend, complementing the existing OAuth (Google, GitHub) and magic link providers. The implementation includes user signup, login, password reset flows, and account management capabilities. A significant architectural change migrates from database sessions to JWT sessions (required for NextAuth CredentialsProvider), which will require existing users to re-login after deployment.
Key Changes
- Complete email/password authentication flow with signup, login, and password reset capabilities
- JWT session strategy migration from database sessions to support CredentialsProvider
- Rate limiting for login (5 attempts/min) and password reset (3 requests/hour) endpoints
- New account settings page for password management and viewing linked OAuth providers
- Strong password validation requirements (8+ chars, uppercase, lowercase, number)
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/web/src/server/password-utils.ts | Centralized Zod schemas for password validation and form schemas |
| apps/web/src/server/mailer.ts | Added sendPasswordResetEmail function for password reset flow |
| apps/web/src/server/email-templates/index.ts | Exported PasswordResetEmail template |
| apps/web/src/server/email-templates/PasswordResetEmail.tsx | Password reset email template with 1-hour expiry notice |
| apps/web/src/server/auth.ts | Added CredentialsProvider and migrated to JWT session strategy |
| apps/web/src/server/api/routers/user.ts | New tRPC router for password management (change, set, check) and linked accounts |
| apps/web/src/server/api/root.ts | Registered user router in main app router |
| apps/web/src/app/signup/signup-page.tsx | New signup page with email/password form and OAuth options |
| apps/web/src/app/signup/page.tsx | Signup route using dedicated SignupPage component |
| apps/web/src/app/reset-password/page.tsx | Password reset page with token validation |
| apps/web/src/app/login/login-page.tsx | Added password/magic link toggle and password login form |
| apps/web/src/app/forgot-password/page.tsx | Forgot password page with email submission |
| apps/web/src/app/api/auth/signup/route.ts | Signup API with support for new users and OAuth users adding passwords |
| apps/web/src/app/api/auth/reset-password/route.ts | Password reset API with token validation and expiry handling |
| apps/web/src/app/api/auth/forgot-password/route.ts | Forgot password API with rate limiting and email enumeration prevention |
| apps/web/src/app/api/auth/[...nextauth]/route.ts | Added rate limiting for credentials authentication |
| apps/web/src/app/(dashboard)/settings/layout.tsx | Added Account settings navigation link |
| apps/web/src/app/(dashboard)/settings/account/page.tsx | Account settings page for password management and linked accounts display |
| apps/web/prisma/schema.prisma | Added passwordHash field to User and PasswordResetToken model |
| apps/web/prisma/migrations/20251219093000_add_password_auth/migration.sql | Database migration for password authentication schema changes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| await db.user.update({ | ||
| where: { id: existingUser.id }, | ||
| data: { passwordHash }, | ||
| }); | ||
|
|
There was a problem hiding this comment.
There is a potential race condition for OAuth users trying to add a password through the signup endpoint. Between checking if a user has a password (line 28) and updating the user (line 38-41), another concurrent request could also check and update, potentially causing one update to overwrite the other. While both would set a password, they would be different passwords, leading to user confusion.
Consider using a conditional update with a where clause that includes passwordHash: null to ensure only one request succeeds, or use optimistic locking.
| await db.user.update({ | |
| where: { id: existingUser.id }, | |
| data: { passwordHash }, | |
| }); | |
| const updateResult = await db.user.updateMany({ | |
| where: { id: existingUser.id, passwordHash: null }, | |
| data: { passwordHash }, | |
| }); | |
| if (updateResult.count === 0) { | |
| // Another request may have already set the password | |
| return NextResponse.json( | |
| { error: "An account with this email already exists" }, | |
| { status: 400 } | |
| ); | |
| } |
| <div className="flex gap-2 w-full"> | ||
| <Button | ||
| type="button" | ||
| variant={authMode === "password" ? "default" : "outline"} | ||
| className="flex-1" | ||
| onClick={() => setAuthMode("password")} | ||
| > | ||
| Password | ||
| </Button> | ||
| <Button | ||
| type="button" | ||
| variant={authMode === "otp" ? "default" : "outline"} | ||
| className="flex-1" | ||
| onClick={() => setAuthMode("otp")} | ||
| > | ||
| Magic Link | ||
| </Button> | ||
| </div> |
There was a problem hiding this comment.
When users switch between "Password" and "Magic Link" authentication modes, the loginError state is not cleared. This means a password login error will persist and display even after switching to magic link mode, which could confuse users.
Clear the loginError state when the auth mode changes by adding setLoginError(null) in the onClick handlers.
| const resetToken = await db.passwordResetToken.findUnique({ | ||
| where: { token }, | ||
| }); | ||
|
|
||
| if (!resetToken) { | ||
| return NextResponse.json( | ||
| { error: "Invalid or expired reset link" }, | ||
| { status: 400 } | ||
| ); | ||
| } | ||
|
|
||
| if (resetToken.expires < new Date()) { | ||
| // Clean up expired token | ||
| await db.passwordResetToken.delete({ where: { token } }); | ||
| return NextResponse.json( | ||
| { error: "This reset link has expired. Please request a new one." }, | ||
| { status: 400 } | ||
| ); | ||
| } | ||
|
|
||
| const passwordHash = await createSecureHash(password); | ||
|
|
||
| await db.user.update({ | ||
| where: { email: resetToken.email }, | ||
| data: { | ||
| passwordHash, | ||
| emailVerified: new Date(), // Verify email on password reset | ||
| }, | ||
| }); | ||
|
|
||
| // Delete used token | ||
| await db.passwordResetToken.delete({ where: { token } }); |
There was a problem hiding this comment.
There is a potential race condition where the same reset token could be used twice. Between checking the token validity (line 20-38) and deleting it (line 51), another concurrent request could also validate and use the same token. This could allow a token to reset the password twice.
Consider using a database transaction to ensure atomic token consumption, or delete the token before updating the password, or add a "used" flag to prevent reuse.
| export async function POST(request: Request) { | ||
| try { | ||
| const body = await request.json(); | ||
| const result = signupSchema.safeParse(body); | ||
|
|
||
| if (!result.success) { | ||
| return NextResponse.json( | ||
| { error: result.error.errors[0]?.message ?? "Invalid input" }, | ||
| { status: 400 } | ||
| ); | ||
| } | ||
|
|
||
| const { email, password, name } = result.data; | ||
| const normalizedEmail = email.toLowerCase().trim(); | ||
|
|
||
| // Check if user exists | ||
| const existingUser = await db.user.findUnique({ | ||
| where: { email: normalizedEmail }, | ||
| }); | ||
|
|
||
| if (existingUser) { | ||
| if (existingUser.passwordHash) { | ||
| // User already has password authentication | ||
| return NextResponse.json( | ||
| { error: "An account with this email already exists" }, | ||
| { status: 400 } | ||
| ); | ||
| } | ||
|
|
||
| // User exists via OAuth, allow adding password | ||
| const passwordHash = await createSecureHash(password); | ||
| await db.user.update({ | ||
| where: { id: existingUser.id }, | ||
| data: { passwordHash }, | ||
| }); | ||
|
|
||
| return NextResponse.json({ | ||
| success: true, | ||
| message: | ||
| "Password added to your existing account. You can now sign in with email and password.", | ||
| }); | ||
| } | ||
|
|
||
| // Create new user with password | ||
| const passwordHash = await createSecureHash(password); | ||
|
|
||
| // Check for pending team invites | ||
| const pendingInvites = await db.teamInvite.findMany({ | ||
| where: { email: normalizedEmail }, | ||
| }); | ||
|
|
||
| // Determine beta/waitlist status based on environment and invites | ||
| const isBetaUser = | ||
| !env.NEXT_PUBLIC_IS_CLOUD || | ||
| env.NODE_ENV === "development" || | ||
| pendingInvites.length > 0; | ||
| const isWaitlisted = | ||
| env.NEXT_PUBLIC_IS_CLOUD && | ||
| env.NODE_ENV !== "development" && | ||
| pendingInvites.length === 0; | ||
|
|
||
| await db.user.create({ | ||
| data: { | ||
| email: normalizedEmail, | ||
| name: name ?? null, | ||
| passwordHash, | ||
| isBetaUser, | ||
| isWaitlisted, | ||
| emailVerified: new Date(), // Mark as verified since they're signing up directly | ||
| }, | ||
| }); | ||
|
|
||
| return NextResponse.json({ | ||
| success: true, | ||
| message: "Account created successfully. You can now sign in.", | ||
| }); | ||
| } catch (error) { | ||
| console.error("Signup error:", error); | ||
| return NextResponse.json( | ||
| { error: "Something went wrong. Please try again." }, | ||
| { status: 500 } | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
The signup endpoint lacks rate limiting, which makes it vulnerable to abuse. An attacker could create many accounts rapidly, potentially for spam, resource exhaustion, or email bombing. This is especially concerning since the endpoint creates database records and may trigger email verifications.
Add rate limiting similar to the password reset endpoint (e.g., 5 signups per hour per IP) to prevent abuse while still allowing legitimate use.
| expires DateTime | ||
| createdAt DateTime @default(now()) | ||
| @@unique([email, token]) |
There was a problem hiding this comment.
The PasswordResetToken model is missing an index on the expires field. This field is frequently queried for validation (checking if tokens have expired) and would benefit from an index to optimize these lookups. Additionally, a periodic cleanup job that deletes expired tokens would also benefit from this index.
Consider adding @@index([expires]) to improve query performance for expiration checks and cleanup operations.
| @@unique([email, token]) | |
| @@unique([email, token]) | |
| @@index([expires]) |
| const passwordSchema = z | ||
| .object({ | ||
| password: z | ||
| .string() | ||
| .min(8, "Password must be at least 8 characters") | ||
| .max(128, "Password must be less than 128 characters") | ||
| .regex(/[a-z]/, "Password must contain at least one lowercase letter") | ||
| .regex(/[A-Z]/, "Password must contain at least one uppercase letter") | ||
| .regex(/[0-9]/, "Password must contain at least one number"), | ||
| confirmPassword: z.string(), | ||
| }) | ||
| .refine((data) => data.password === data.confirmPassword, { | ||
| message: "Passwords don't match", | ||
| path: ["confirmPassword"], | ||
| }); |
There was a problem hiding this comment.
The password validation schema is duplicated here instead of being imported from the centralized password-utils module. This violates DRY principles and creates a maintenance burden - if password requirements change, this schema would need to be updated separately.
Import and use the passwordSchema from ~/server/password-utils instead, similar to how it's done in the account settings page and login page.
apps/web/src/server/auth.ts
Outdated
| const isValid = await verifySecureHash(password, user.passwordHash); | ||
| if (!isValid) { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
The hash comparison uses simple string equality (===) which is vulnerable to timing attacks. An attacker could potentially use timing differences to determine the hash character-by-character. While scrypt provides some inherent protection, the final comparison should use a constant-time comparison to prevent timing side-channel attacks.
Consider using crypto.timingSafeEqual for the final hash comparison to ensure constant-time equality checking.
|
|
||
| const schema = z.object({ | ||
| email: z.string().email(), | ||
| }); | ||
|
|
||
| function getClientIp(req: Request): string | null { | ||
| const h = req.headers; | ||
| const direct = | ||
| h.get("x-forwarded-for") ?? | ||
| h.get("x-real-ip") ?? | ||
| h.get("cf-connecting-ip") ?? | ||
| h.get("x-client-ip") ?? | ||
| h.get("true-client-ip") ?? | ||
| h.get("fastly-client-ip") ?? | ||
| h.get("x-cluster-client-ip") ?? | ||
| null; | ||
|
|
||
| let ip = direct?.split(",")[0]?.trim() ?? ""; | ||
|
|
||
| if (!ip) { | ||
| const fwd = h.get("forwarded"); | ||
| if (fwd) { | ||
| const first = fwd.split(",")[0]; | ||
| const match = first?.match(/for=([^;]+)/i); | ||
| if (match && match[1]) { | ||
| const raw = match[1].trim().replace(/^"|"$/g, ""); | ||
| if (raw.startsWith("[")) { | ||
| const end = raw.indexOf("]"); | ||
| ip = end !== -1 ? raw.slice(1, end) : raw; | ||
| } else { | ||
| const parts = raw.split(":"); | ||
| if (parts.length > 0 && parts[0]) { | ||
| ip = | ||
| parts.length === 2 && /^\d+(?:\.\d+){3}$/.test(parts[0]) | ||
| ? parts[0] | ||
| : raw; | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return ip || null; | ||
| } | ||
|
|
There was a problem hiding this comment.
The getClientIp function is duplicated across two files (here and in [...nextauth]/route.ts). This identical 40-line function should be extracted into a shared utility module to maintain DRY principles and ensure consistency in IP extraction logic across the application.
Consider moving this to a shared utility file like ~/server/utils/ip.ts.
| const schema = z.object({ | |
| email: z.string().email(), | |
| }); | |
| function getClientIp(req: Request): string | null { | |
| const h = req.headers; | |
| const direct = | |
| h.get("x-forwarded-for") ?? | |
| h.get("x-real-ip") ?? | |
| h.get("cf-connecting-ip") ?? | |
| h.get("x-client-ip") ?? | |
| h.get("true-client-ip") ?? | |
| h.get("fastly-client-ip") ?? | |
| h.get("x-cluster-client-ip") ?? | |
| null; | |
| let ip = direct?.split(",")[0]?.trim() ?? ""; | |
| if (!ip) { | |
| const fwd = h.get("forwarded"); | |
| if (fwd) { | |
| const first = fwd.split(",")[0]; | |
| const match = first?.match(/for=([^;]+)/i); | |
| if (match && match[1]) { | |
| const raw = match[1].trim().replace(/^"|"$/g, ""); | |
| if (raw.startsWith("[")) { | |
| const end = raw.indexOf("]"); | |
| ip = end !== -1 ? raw.slice(1, end) : raw; | |
| } else { | |
| const parts = raw.split(":"); | |
| if (parts.length > 0 && parts[0]) { | |
| ip = | |
| parts.length === 2 && /^\d+(?:\.\d+){3}$/.test(parts[0]) | |
| ? parts[0] | |
| : raw; | |
| } | |
| } | |
| } | |
| } | |
| } | |
| return ip || null; | |
| } | |
| import { getClientIp } from "~/server/utils/ip"; | |
| const schema = z.object({ | |
| email: z.string().email(), | |
| }); |
| router.push(callbackUrl); | ||
| } else { | ||
| // If auto-login fails, redirect to login page | ||
| router.push("/login?message=Account created successfully. Please sign in."); |
There was a problem hiding this comment.
When the auto-login after signup fails (line 121-122), the user is redirected with a URL query parameter "message" that is never read or displayed to the user. This provides a suboptimal user experience - the message parameter is ignored by the login page implementation, so users won't know their account was created successfully.
Either ensure the login page reads and displays this message parameter, or change the redirect to use a more robust notification mechanism like session storage or a toast notification.
| router.push("/login?message=Account created successfully. Please sign in."); | |
| router.push("/login"); |
P0: Remove account takeover vulnerability in signup route - Removed OAuth password-linking from /api/auth/signup endpoint - Attackers could previously add passwords to OAuth accounts without auth - OAuth users must now use authenticated settings page to add passwords P1: Fix timing attack vulnerability in credentials provider - Always run hash verification regardless of user existence - Use dummy hash for non-existent users to normalize response time - Prevents email enumeration via response timing analysis P1: Fix race condition in password reset flow - Wrap password update and token deletion in db.$transaction() - Add user existence verification before password update - Prevents concurrent token reuse and partial update failures P2: Code quality improvements - Import passwordSchema from shared utils (reset-password/page.tsx) - Clear loginError when switching auth modes (login-page.tsx) - Remove debug console.log statement (login-page.tsx) - Add defaultValues to useForm (forgot-password/page.tsx)
1 participant
Summary
This PR adds complete email-password authentication alongside the existing OAuth (Google, GitHub) and magic link providers, making useSend a more complete Resend self-hosted clone.
Features
/settings/accountpage to change/set password and view linked OAuth providersTechnical Changes
Database
passwordHashfield toUsermodel (nullable for OAuth-only users)PasswordResetTokenmodel for secure password reset flow20251219093000_add_password_authAuthentication
CredentialsProviderfor email/password loginNew Files
src/server/password-utils.tssrc/app/api/auth/signup/route.tssrc/app/api/auth/forgot-password/route.tssrc/app/api/auth/reset-password/route.tssrc/app/signup/signup-page.tsxsrc/app/forgot-password/page.tsxsrc/app/reset-password/page.tsxsrc/server/email-templates/PasswordResetEmail.tsxsrc/server/api/routers/user.tssrc/app/(dashboard)/settings/account/page.tsxModified Files
prisma/schema.prismasrc/server/auth.tssrc/app/login/login-page.tsxsrc/server/mailer.tssrc/app/api/auth/[...nextauth]/route.tsSecurity Considerations
createSecureHashutility)Breaking Changes
Test plan
Summary by cubic
Adds full email/password authentication alongside OAuth and magic link, including signup, login, and password reset. Switches NextAuth to JWT sessions and hardens security with rate limits and timing-safe password checks.
New Features
Migration
Written for commit 48e480c. Summary will update automatically on new commits.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.