(SP: 3) [Backend][Frontend][UI] Implement secure authentication lifecycle with email verification and password recovery#137
Conversation
✅ Deploy Preview for develop-devlovers ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the 📝 WalkthroughWalkthroughThis PR implements email verification and password reset flows end-to-end, adding database tables for verification and reset tokens, creating email templates and transporter configuration, introducing API routes for signup, login, password reset, and email verification confirmation, and updating frontend authentication pages with proper UI states and error handling. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Frontend as Signup Page
participant SignupAPI as /api/auth/signup
participant Database
participant Mailer as Email Service
User->>Frontend: Enter email & password
User->>Frontend: Click Sign Up
Frontend->>SignupAPI: POST {email, password}
SignupAPI->>Database: Check if user exists
alt User already exists
SignupAPI-->>Frontend: Return error
Frontend->>User: Show error message
else New user
SignupAPI->>Database: Hash password
SignupAPI->>Database: Insert user (emailVerified: null)
Database-->>SignupAPI: User created
SignupAPI->>Database: Create verification token
Database-->>SignupAPI: Token generated
SignupAPI->>Mailer: Send verification email with token
Mailer-->>SignupAPI: Email sent
SignupAPI-->>Frontend: Return 201 {verificationRequired: true}
Frontend->>User: Show verification notice<br/>(Check email for link)
User->>Mailer: Click verification link
Mailer->>Frontend: Redirect to /verify-email?token=...
Frontend->>VerifyAPI: GET /api/auth/verify-email?token=...
VerifyAPI->>Database: Lookup & validate token
alt Token valid
Database->>Database: Set emailVerified = now()
Database->>Database: Delete token
VerifyAPI->>Frontend: Redirect to /login?verified=1
Frontend->>User: Account verified!
else Token expired/invalid
VerifyAPI->>Frontend: Redirect to /login?error=...
Frontend->>User: Show error
end
end
sequenceDiagram
participant User
participant Frontend as Forgot Password<br/>Page
participant ResetAPI as /api/auth/<br/>password-reset
participant Database
participant Mailer as Email Service
participant ResetPage as Reset Password<br/>Page
User->>Frontend: Enter email
User->>Frontend: Click Send Reset Link
Frontend->>ResetAPI: POST {email}
ResetAPI->>Database: Lookup user by email
alt User exists & uses credentials
ResetAPI->>Database: Clear old reset tokens
ResetAPI->>Database: Create reset token (1hr expiry)
Database-->>ResetAPI: Token created
ResetAPI->>Mailer: Send reset email with token link
Mailer-->>ResetAPI: Email sent
end
ResetAPI-->>Frontend: Return {success: true}
Frontend->>User: Show confirmation message
User->>Mailer: Click reset link in email
Mailer->>ResetPage: Redirect to /reset-password?token=...
ResetPage->>User: Show password form with token pre-filled
User->>ResetPage: Enter new password & submit
ResetPage->>ConfirmAPI: POST {token, password} to<br/>/api/auth/password-reset/confirm
ConfirmAPI->>Database: Validate token & expiry
alt Valid token
Database->>Database: Hash new password
Database->>Database: Update user passwordHash
Database->>Database: Delete used token
ConfirmAPI-->>ResetPage: Return {success: true}
ResetPage->>User: Show success, link to login
User->>Frontend: Click login link
Frontend->>User: Back to login form
else Invalid/expired token
ConfirmAPI-->>ResetPage: Return error
ResetPage->>User: Show error message
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/drizzle/meta/_journal.json (1)
99-128: Unresolved merge conflict markers break this JSON file.This file contains unresolved git merge conflict markers (
<<<<<<<,=======,>>>>>>>), making it invalid JSON. This will break Drizzle migration tooling.Additionally, after resolving the conflict, note that:
- There are two entries claiming
idx: 13(lines 97 and 103)- There are two entries claiming
idx: 14(lines 108 and 123)The journal entries need to be reconciled to have sequential, non-duplicate indices.
To fix:
- Resolve the merge conflict by choosing the correct migration sequence
- Ensure indices are sequential and unique
- Verify the chosen migrations exist in
frontend/drizzle/directory
🤖 Fix all issues with AI agents
In @frontend/app/[locale]/forgot-password/page.tsx:
- Around line 16-35: Wrap the fetch in onSubmit with a try-catch-finally:
perform the POST inside try, set setSubmitted(true) when res.ok, and on non-ok
set setError as before; in catch set setError to a user-friendly network error
message and ensure setLoading(false) is always called in finally so loading
doesn't remain true; reference onSubmit, fetch("/api/auth/password-reset"), and
the state setters setLoading, setError, setSubmitted (and the email variable)
when making the changes.
In @frontend/app/[locale]/reset-password/page.tsx:
- Around line 26-42: The fetch to "/api/auth/password-reset/confirm" in the
reset handler can throw on network failures and currently has no try/catch,
which can leave loading state stuck; wrap the fetch and response handling in a
try/catch and use finally to ensure setLoading(false) always runs, set a
user-friendly error via setError on catch (e.g., "Network error, please try
again"), and keep the existing res.ok check to set the "Invalid or expired reset
link." message when the request completes but returns a non-OK response; update
the code surrounding the token/password POST (the fetch call and the calls to
setLoading and setError) accordingly.
- Around line 8-11: The ResetPasswordPage component uses useSearchParams() which
requires a Suspense boundary in Next.js 15+ when the route is statically
prerendered; to fix, either wrap the parent/server component that renders
ResetPasswordPage in a React.Suspense boundary, or convert this route to dynamic
by adding export const dynamic = 'force-dynamic' in the module, or stop calling
useSearchParams in ResetPasswordPage and instead accept searchParams/token as a
prop passed from the parent Server Component; update the code accordingly by
modifying the parent or the ResetPasswordPage signature (referencing
ResetPasswordPage and useSearchParams) to implement one of these fixes.
In @frontend/app/[locale]/signup/page.tsx:
- Around line 23-57: The onSubmit handler can throw if fetch fails, leaving
loading true and later code referencing res; wrap the fetch and subsequent
res.json() calls in a try/catch/finally inside onSubmit so network errors are
caught, call setError with a user-friendly message in the catch, and ensure
setLoading(false) runs in finally; also guard the res.ok check to only run if
res is defined (or handle absence by setting an error) and
setVerificationRequired when appropriate.
- Around line 55-56: The assignment to window.location.href uses the unvalidated
returnTo parameter (returnTo) which can enable open redirect attacks; before
setting window.location.href in the signup flow, validate returnTo by ensuring
it is a safe relative path (e.g., starts with '/' but not '//' and optionally
begins with `/${locale}/`) or by parsing it and confirming its origin matches
the current origin, and only use returnTo when it passes validation otherwise
fall back to `/${locale}/dashboard`; update the code around the
window.location.href assignment where returnTo is read to perform this check.
In @frontend/app/api/auth/password-reset/route.ts:
- Around line 51-56: The code uses origin = (await headers()).get("origin") to
build resetUrl passed to sendPasswordResetEmail, but origin may be null; update
the route to fall back to a safe base URL (e.g., an environment variable like
process.env.NEXT_PUBLIC_APP_URL or NEXT_PUBLIC_BASE_URL) when origin is falsy
before constructing `${origin}/reset-password?token=${token}` so the reset link
never becomes "null/..."; ensure the fallback is used for resetUrl and keep the
rest of the sendPasswordResetEmail call unchanged.
In @frontend/app/api/auth/resend-verification/route.ts:
- Around line 57-62: The origin header can be null causing malformed verifyUrl
("null/..."); update the URL construction in resend-verification route (and the
same pattern in signup/route.ts and password-reset/route.ts) to use a safe
fallback: compute const origin = (await headers()).get("origin") ||
process.env.NEXT_PUBLIC_APP_URL ||
`${req.protocol}://${req.headers.get('host')}` (or equivalent request-derived
base) and normalize trailing slashes before building verifyUrl, then call
sendVerificationEmail({ to: email, verifyUrl:
`${origin}/api/auth/verify-email?token=${token}` }); ensure the same fallback
logic is applied where verifyUrl (or reset URL) is built so token links are
never prefixed with "null".
In @frontend/app/api/auth/signup/route.ts:
- Line 22: Remove the leftover debug statement console.log('signup handler hit')
from the signup handler in route.ts; delete that console.log call (and any other
console.* debugging statements in the same handler) so no debug logs are left in
production code, leaving the handler logic unchanged.
- Around line 66-71: The origin header fetched via headers().get("origin") can
be null, so update the signup route to fall back to a canonical app URL when
building the verifyUrl: compute a baseUrl = origin ??
process.env.NEXT_PUBLIC_APP_URL ?? process.env.APP_URL (and normalize to remove
any trailing slash), then call sendVerificationEmail({ to: normalizedEmail,
verifyUrl: `${baseUrl}/api/auth/verify-email?token=${token}` }) so verifyUrl is
never built with "null" and always uses a valid absolute URL.
In @frontend/db/schema/emailVerificationTokens.ts:
- Around line 3-17: Add a foreign key constraint from
emailVerificationTokens.userId to the users table: update the
emailVerificationTokens pgTable definition to call .references(() => users.id, {
onDelete: "cascade" }) (or equivalent reference to your users primary key) on
the userId column so it enforces referential integrity and cascades deletes;
mirror the pattern used in other schemas (e.g., points, shop, quiz) and ensure
the symbol names match your users table export and primary key field.
In @frontend/drizzle/0015_glamorous_eternity.sql:
- Around line 1-16: The migration email_verification_tokens in
0015_glamorous_eternity.sql conflicts with 0013_low_roughhouse.sql (0013 uses
token as PK and user_id, 0015 uses id uuid PK and email); consolidate into a
single consistent schema for the email_verification_tokens table by choosing the
canonical shape (either keep token as PRIMARY KEY with user_id or keep id uuid
PRIMARY KEY with email), update the CREATE TABLE statement in the migration to
match that chosen schema, remove or replace the unsafe IF NOT EXISTS behavior
(use explicit create/drop/alter so the schema is deterministic), ensure the
index email_verification_tokens_token_idx matches the token column existence and
uniqueness, and adjust any dependent code/schema files to the chosen column
names (token, id, user_id/email) so all migrations and runtime code agree.
In @frontend/lib/auth/email-verification.ts:
- Around line 5-9: The addHours function currently ignores its date parameter
and always uses new Date(); modify addHours(date: Date, hours: number) to
operate on a clone of the passed-in date (e.g., new Date(date.getTime()) or new
Date(date)) so you don't mutate the original, then add the hours via
setHours/getHours and return that cloned date; keep the function name addHours
and ensure it uses the input date rather than creating a fresh Date().
In @frontend/lib/auth/password-reset.ts:
- Around line 5-9: The addHours function ignores its date parameter by creating
a new Date() instead of using the passed-in date; update addHours (in
frontend/lib/auth/password-reset.ts) to use the provided date argument and
return a new Date based on it (or better, replace the implementation to use
date-fns addHours for consistency with email-verification.ts), ensuring all call
sites (e.g., addHours(new Date(), 1)) behave correctly and import addHours from
date-fns if you switch to that approach.
🧹 Nitpick comments (15)
frontend/app/api/auth/github/callback/route.ts (1)
175-175: LGTM! Redirect to dashboard after OAuth login.The change correctly redirects authenticated users to
/dashboardinstead of the root path, aligning with the PR objectives.Optional enhancement: Consider preserving the user's original destination URL (e.g., via
stateparameter or a stored redirect path) to improve UX when users are redirected to login from a protected route. This would allow them to land on their intended page after authentication rather than always going to/dashboard.frontend/lib/email/transporter.ts (1)
6-8: Consider lazy initialization to avoid startup crashes.Throwing at module load time will crash the entire application if these environment variables are missing, even for code paths that don't require email functionality. This can be problematic in development environments or services that don't send emails.
Consider deferring validation to when
maileris first used, or exporting a factory function.♻️ Example: Lazy initialization approach
-if (!user || !pass) { - throw new Error("Missing Gmail SMTP credentials"); -} - -export const mailer = nodemailer.createTransport({ +let _mailer: nodemailer.Transporter | null = null; + +export function getMailer(): nodemailer.Transporter { + if (!_mailer) { + if (!user || !pass) { + throw new Error("Missing Gmail SMTP credentials"); + } + _mailer = nodemailer.createTransport({ service: "gmail", auth: { user, pass, }, -}); + }); + } + return _mailer; +}frontend/app/[locale]/forgot-password/page.tsx (1)
74-81: Add accessible label for the email input.Using only
placeholderas the label is not accessible for screen reader users. Consider adding a visually hidden label or using an explicit<label>element.♿ Suggested improvement
+ <label htmlFor="email" className="sr-only"> + Email address + </label> <input + id="email" type="email" required placeholder="Email" value={email} onChange={e => setEmail(e.target.value)} className="w-full rounded border px-3 py-2" />frontend/app/[locale]/reset-password/page.tsx (1)
84-92: Consider adding password visibility toggle for consistency.The PR summary mentions that LoginPage and SignupPage have show/hide password toggles. For consistency, consider adding the same functionality here.
frontend/app/api/auth/login/route.ts (3)
52-52: Missing semicolon.Add a semicolon after the assignment for consistency with the rest of the codebase.
- const user = result[0] + const user = result[0];
54-56: Consider graceful handling instead of throwing.Throwing an unhandled
Errorreturns a 500 status to the client, potentially leaking internal implementation details. Since theusers.providercolumn has a default of"credentials"and isNOT NULL, this condition should only occur with data corruption. Consider returning a generic 401 error or logging this as a critical issue while returning a user-friendly response.💡 Suggested alternative
if (!user.provider) { - throw new Error("User record missing provider"); + console.error("Data integrity issue: user record missing provider", { userId: user.id }); + return NextResponse.json( + { error: "Invalid email or password" }, + { status: 401 } + ); }
75-83: Useuservariable consistently.The
uservariable was introduced at line 52 but lines 75-83 still referenceresult[0]. Useuserthroughout for consistency and clarity.♻️ Suggested fix
const token = signAuthToken({ - userId: result[0].id, - role: result[0].role as "user" | "admin", + userId: user.id, + role: user.role as "user" | "admin", email: normalizedEmail, }); await setAuthCookie(token); revalidatePath('/[locale]', 'layout'); - return NextResponse.json({ success: true, userId: result[0].id }); + return NextResponse.json({ success: true, userId: user.id }); }frontend/db/schema/index.ts (1)
7-8: Minor: Inconsistent quote style.Line 7 uses single quotes while line 8 uses double quotes. For consistency with the rest of the file, use single quotes throughout.
export * from './emailVerificationTokens'; -export * from "./passwordResetTokens"; +export * from './passwordResetTokens';frontend/lib/email/sendPasswordResetEmail.ts (1)
12-17: Consider: Extract sharedEMAIL_FROMvalidation.Both
sendVerificationEmailandsendPasswordResetEmailduplicate theEMAIL_FROMenvironment check. You could optionally extract this to a shared helper or constant in the transporter module for DRYer code.// In transporter.ts export const emailFrom = (() => { const from = process.env.EMAIL_FROM; if (!from) throw new Error("EMAIL_FROM is not configured"); return from; })();This is a minor optional improvement—the current implementation is clear and functional.
frontend/app/api/auth/password-reset/confirm/route.ts (1)
10-13: Consider adding password complexity validation.The current schema only enforces a minimum length of 8 characters. Depending on security requirements, you may want to add additional password complexity rules (e.g., requiring mixed case, numbers, or special characters) or document that this is intentionally delegated to frontend validation.
frontend/drizzle/0014_dapper_kang.sql (1)
1-8: Consider adding a foreign key constraint onuser_id.The
user_idcolumn references theuserstable but lacks a foreign key constraint. While tokens naturally expire, orphaned tokens could accumulate if users are deleted. AddingREFERENCES users(id) ON DELETE CASCADEwould automatically clean up tokens when users are removed.This is a minor concern since:
- Tokens have short expiration times
- User deletion may be rare
- It matches the pattern used for
email_verification_tokensfrontend/app/api/auth/verify-email/route.ts (1)
2-2: Remove unused importlt.The
ltoperator is imported but never used in this file.Proposed fix
-import { eq, and, lt } from "drizzle-orm"; +import { eq } from "drizzle-orm";frontend/app/[locale]/login/page.tsx (1)
107-119: Consider adding error handling and loading state toresendVerification.The function silently ignores fetch failures and doesn't indicate to the user that a request is in progress. This could lead to confusion if the request fails or if users click multiple times.
Proposed improvement
+ const [resending, setResending] = useState(false); + async function resendVerification() { if (!email) return; - await fetch("/api/auth/resend-verification", { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ email }), - }); + setResending(true); + try { + await fetch("/api/auth/resend-verification", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ email }), + }); - setVerificationSent(true); - setErrorCode(null); - setErrorMessage(null); + setVerificationSent(true); + setErrorCode(null); + setErrorMessage(null); + } catch { + setErrorMessage("Failed to resend verification email. Please try again."); + } finally { + setResending(false); + } }Then update the button to show loading state:
<button type="button" onClick={resendVerification} disabled={resending} className="mt-2 underline" > {resending ? "Sending..." : "Resend verification email"} </button>frontend/app/api/auth/signup/route.ts (2)
52-76: Consider wrapping user creation and token generation in a transaction.If
createEmailVerificationTokenorsendVerificationEmailfails after the user is inserted, the user exists but may not receive a verification email. While the user can request a resend, wrapping in a transaction ensures atomicity.♻️ Optional transaction wrapper
+ await db.transaction(async (tx) => { - const [user] = await db + const [user] = await tx .insert(users) .values({ name, email: normalizedEmail, passwordHash, provider: "credentials", emailVerified: null, role: "user", }) .returning(); - const token = await createEmailVerificationToken(user.id) + // Note: createEmailVerificationToken would need to accept tx as parameter + const token = await createEmailVerificationToken(user.id, tx); - const origin = (await headers()).get("origin") + const origin = (await headers()).get("origin") || process.env.NEXT_PUBLIC_APP_URL; - await sendVerificationEmail({ + await sendVerificationEmail({ to: normalizedEmail, verifyUrl: `${origin}/api/auth/verify-email?token=${token}` - }) + }); + });This requires modifying
createEmailVerificationTokento accept a transaction parameter.
10-11: Minor: Remove extra blank line.import { sendVerificationEmail } from "@/lib/email/sendVerificationEmail"; - export const runtime = "nodejs";
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (33)
frontend/app/[locale]/forgot-password/page.tsxfrontend/app/[locale]/login/page.tsxfrontend/app/[locale]/reset-password/page.tsxfrontend/app/[locale]/signup/page.tsxfrontend/app/api/auth/github/callback/route.tsfrontend/app/api/auth/login/route.tsfrontend/app/api/auth/password-reset/confirm/route.tsfrontend/app/api/auth/password-reset/route.tsfrontend/app/api/auth/resend-verification/route.tsfrontend/app/api/auth/signup/route.tsfrontend/app/api/auth/verify-email/route.tsfrontend/db/index.tsfrontend/db/schema/emailVerificationTokens.tsfrontend/db/schema/index.tsfrontend/db/schema/passwordResetTokens.tsfrontend/db/schema/users.tsfrontend/drizzle/0012_inventory_moves_product_fk_restrict.sqlfrontend/drizzle/0013_low_roughhouse.sqlfrontend/drizzle/0014_dapper_kang.sqlfrontend/drizzle/0015_glamorous_eternity.sqlfrontend/drizzle/meta/0013_snapshot.jsonfrontend/drizzle/meta/0015_snapshot.jsonfrontend/drizzle/meta/_journal.jsonfrontend/lib/auth/email-verification.tsfrontend/lib/auth/password-reset.tsfrontend/lib/email/sendPasswordResetEmail.tsfrontend/lib/email/sendVerificationEmail.tsfrontend/lib/email/templates/base-layout.tsfrontend/lib/email/templates/reset-password.tsfrontend/lib/email/templates/verify-email.tsfrontend/lib/email/transporter.tsfrontend/package.jsonfrontend/scripts/guard-non-preview.ts
💤 Files with no reviewable changes (2)
- frontend/db/schema/users.ts
- frontend/scripts/guard-non-preview.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-30T16:01:02.523Z
Learnt from: kryvosheyin
Repo: DevLoversTeam/devlovers.net PR: 88
File: frontend/drizzle/schema.ts:153-153
Timestamp: 2025-12-30T16:01:02.523Z
Learning: The file frontend/drizzle/schema.ts is a legacy file not used by Drizzle or the runtime. The canonical schemas live in frontend/db/schema/* as configured in drizzle-config.ts, and that's what migrations and runtime use.
Applied to files:
frontend/db/schema/index.tsfrontend/db/schema/passwordResetTokens.tsfrontend/drizzle/meta/0015_snapshot.jsonfrontend/db/schema/emailVerificationTokens.tsfrontend/db/index.ts
🧬 Code graph analysis (14)
frontend/lib/email/sendPasswordResetEmail.ts (2)
frontend/lib/email/transporter.ts (1)
mailer(10-16)frontend/lib/email/templates/reset-password.ts (1)
resetPasswordTemplate(3-47)
frontend/app/api/auth/resend-verification/route.ts (6)
frontend/app/api/auth/password-reset/route.ts (1)
POST(16-59)frontend/app/api/auth/signup/route.ts (1)
POST(21-87)frontend/db/schema/users.ts (1)
users(10-41)frontend/db/schema/emailVerificationTokens.ts (1)
emailVerificationTokens(3-18)frontend/lib/auth/email-verification.ts (1)
createEmailVerificationToken(11-21)frontend/lib/email/sendVerificationEmail.ts (1)
sendVerificationEmail(9-25)
frontend/lib/email/templates/reset-password.ts (1)
frontend/lib/email/templates/base-layout.ts (1)
baseEmailLayout(6-103)
frontend/lib/auth/email-verification.ts (2)
frontend/db/index.ts (1)
db(54-54)frontend/db/schema/emailVerificationTokens.ts (1)
emailVerificationTokens(3-18)
frontend/lib/email/sendVerificationEmail.ts (2)
frontend/lib/email/transporter.ts (1)
mailer(10-16)frontend/lib/email/templates/verify-email.ts (1)
verifyEmailTemplate(3-47)
frontend/app/[locale]/signup/page.tsx (1)
frontend/components/ui/button.tsx (1)
Button(42-42)
frontend/app/api/auth/verify-email/route.ts (3)
frontend/db/index.ts (1)
db(54-54)frontend/db/schema/emailVerificationTokens.ts (1)
emailVerificationTokens(3-18)frontend/db/schema/users.ts (1)
users(10-41)
frontend/app/api/auth/login/route.ts (1)
frontend/db/schema/users.ts (1)
users(10-41)
frontend/app/[locale]/login/page.tsx (1)
frontend/lib/quiz/guest-quiz.ts (2)
getPendingQuizResult(27-49)clearPendingQuizResult(51-54)
frontend/lib/auth/password-reset.ts (2)
frontend/db/index.ts (1)
db(54-54)frontend/db/schema/passwordResetTokens.ts (1)
passwordResetTokens(3-18)
frontend/lib/email/templates/verify-email.ts (1)
frontend/lib/email/templates/base-layout.ts (1)
baseEmailLayout(6-103)
frontend/app/api/auth/signup/route.ts (3)
frontend/db/schema/users.ts (1)
users(10-41)frontend/lib/auth/email-verification.ts (1)
createEmailVerificationToken(11-21)frontend/lib/email/sendVerificationEmail.ts (1)
sendVerificationEmail(9-25)
frontend/app/api/auth/password-reset/route.ts (4)
frontend/db/schema/users.ts (1)
users(10-41)frontend/db/schema/passwordResetTokens.ts (1)
passwordResetTokens(3-18)frontend/lib/auth/password-reset.ts (1)
createPasswordResetToken(11-23)frontend/lib/email/sendPasswordResetEmail.ts (1)
sendPasswordResetEmail(9-26)
frontend/app/api/auth/password-reset/confirm/route.ts (3)
frontend/app/api/auth/password-reset/route.ts (1)
POST(16-59)frontend/db/schema/passwordResetTokens.ts (1)
passwordResetTokens(3-18)frontend/db/schema/users.ts (1)
users(10-41)
🪛 Biome (2.1.2)
frontend/drizzle/meta/_journal.json
[error] 99-99: unexpected character <
(parse)
[error] 99-99: unexpected character <
(parse)
[error] 99-99: unexpected character <
(parse)
[error] 99-99: unexpected character <
(parse)
[error] 99-99: unexpected character <
(parse)
[error] 99-99: unexpected character <
(parse)
[error] 99-99: expected , but instead found HEAD
Remove HEAD
(parse)
[error] 100-100: expected , but instead found "when"
Remove "when"
(parse)
[error] 100-100: expected , but instead found :
Remove :
(parse)
[error] 100-100: expected , but instead found 1768162667947
Remove 1768162667947
(parse)
[error] 101-101: expected , but instead found :
Remove :
(parse)
[error] 101-101: expected , but instead found "0013_brown_gamora"
Remove "0013_brown_gamora"
(parse)
[error] 101-102: unexpected character =
(parse)
[error] 102-102: unexpected character =
(parse)
[error] 102-102: unexpected character =
(parse)
[error] 102-102: unexpected character =
(parse)
[error] 102-102: unexpected character =
(parse)
[error] 102-102: unexpected character =
(parse)
[error] 102-102: unexpected character =
(parse)
[error] 103-103: expected , but instead found "when"
Remove "when"
(parse)
[error] 103-103: expected , but instead found :
Remove :
(parse)
[error] 103-103: expected , but instead found 1767697281389
Remove 1767697281389
(parse)
[error] 104-104: expected , but instead found :
Remove :
(parse)
[error] 104-104: expected , but instead found "0013_low_roughhouse"
Remove "0013_low_roughhouse"
(parse)
[error] 105-105: expected , but instead found :
Remove :
(parse)
[error] 105-105: expected , but instead found true
Remove true
(parse)
[error] 106-106: expected , but instead found }
Remove }
(parse)
[error] 106-106: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 107-113: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 113-113: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 114-118: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 118-119: unexpected character >
(parse)
[error] 119-119: unexpected character >
(parse)
[error] 119-119: unexpected character >
(parse)
[error] 119-119: unexpected character >
(parse)
[error] 119-119: unexpected character >
(parse)
[error] 119-119: unexpected character >
(parse)
[error] 119-119: unexpected character >
(parse)
[error] 119-119: String values must be double quoted.
(parse)
[error] 119-119: unexpected character (
(parse)
[error] 119-119: String values must be double quoted.
(parse)
[error] 119-119: unexpected character (
(parse)
[error] 119-119: String values must be double quoted.
(parse)
[error] 119-119: unexpected character )
(parse)
[error] 119-119: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 119-119: unexpected character )
(parse)
🔇 Additional comments (27)
frontend/package.json (3)
63-63: LGTM!TypeScript types for nodemailer are correctly added as a dev dependency with matching major version.
10-15: This review comment appears to be based on incorrect assumptions about the codebase.The
guard-non-preview.tsscript does not exist in the repository, and there is noscripts/directory. The current scripts inpackage.json(lines 10-15) do not have any guard prefix, and no evidence suggests a guard was removed as part of this change.If you intended to flag that these seed scripts lack environment safeguards, that concern is separate. The scripts currently only check for
DATABASE_URLexistence but do not prevent execution in production environments.Likely an incorrect or invalid review comment.
45-45: Appropriate dependency for email functionality.Adding
nodemailer(version 7.0.12) is the correct choice for implementing email verification and password reset features. The version specified is the latest stable release on npm, and the caret range allows for compatible patch and minor updates.frontend/drizzle/0012_inventory_moves_product_fk_restrict.sql (1)
1-18: LGTM! Safe conditional migration.The guarded
DO $$ ... END $$block correctly checks for table existence before modifying constraints, preventing errors in environments whereinventory_movesmay not exist. TheON DELETE RESTRICTconstraint appropriately protects referential integrity.frontend/app/[locale]/forgot-password/page.tsx (1)
37-99: Good privacy practice in the success message.The confirmation message appropriately uses "If an account for ... exists" rather than confirming account existence, which prevents email enumeration attacks.
frontend/drizzle/0013_low_roughhouse.sql (1)
1-8: This migration has conflicting table definitions with 0015_glamorous_eternity.sql.Migration 0013 creates
email_verification_tokenswithtokenas PRIMARY KEY anduser_idcolumn, while 0015 defines the same table withidas PRIMARY KEY andIF NOT EXISTSto prevent a runtime error (0013 will execute first and 0015 will skip), this schema mismatch indicates a design issue.Verify whether 0013 or 0015 represents the intended schema and remove or consolidate the conflicting migration definition.
frontend/app/api/auth/login/route.ts (1)
58-63: LGTM!Good implementation of email verification check that correctly distinguishes between credential-based and OAuth users. The
EMAIL_NOT_VERIFIEDcode enables the frontend to show appropriate contextual feedback.frontend/db/schema/passwordResetTokens.ts (1)
3-17: LGTM!The schema structure is consistent with
emailVerificationTokens, with appropriate columns for token-based flows. The same consideration about foreign key constraints applies here as noted in the email verification tokens review.frontend/db/index.ts (1)
1-54: LGTM! Clean environment-based database configuration.The conditional setup correctly branches between local PostgreSQL (via
pgPool) and production Neon HTTP. Error handling for missing environment variables is appropriate, and test-mode logging suppression is a nice touch.frontend/lib/email/sendVerificationEmail.ts (1)
1-25: LGTM! Clean email verification utility.The function correctly validates the required
EMAIL_FROMenvironment variable, provides both plain text and HTML content for email clients, and uses consistent branding in the subject line.frontend/lib/email/templates/reset-password.ts (1)
1-47: LGTM! Well-structured password reset email template.The template follows the established pattern from
verifyEmailTemplate, provides clear user instructions, and includes appropriate expiry and ignore notices. The direct URL interpolation is safe assumingresetUrlis always server-generated with controlled tokens.frontend/drizzle/meta/0013_snapshot.json (1)
2261-2314: Verify: Missing foreign key constraint onuser_id.The
email_verification_tokenstable hasuser_idwithout a foreign key constraint tousers.id. Other user-related tables (e.g.,quiz_attempts,point_transactions) define foreign keys withON DELETE CASCADE. Without an FK, orphaned tokens may persist if a user is deleted.If this is intentional (e.g., to avoid cascade complexity during signup flow before user is fully committed), please confirm. Otherwise, consider adding:
FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADEfrontend/lib/email/sendPasswordResetEmail.ts (1)
1-26: LGTM! Consistent implementation withsendVerificationEmail.The function correctly follows the same pattern as
sendVerificationEmailwith proper environment validation, plain text fallback, and branded subject line.frontend/lib/email/templates/verify-email.ts (1)
1-47: LGTM!The email template is well-structured with clear messaging, proper use of the base layout, and consistent styling. The 24-hour expiration notice correctly matches the token validity period defined in the verification token creation logic.
frontend/app/api/auth/password-reset/route.ts (1)
16-24: Good security practice: Silent success on validation failure.Returning
{ success: true }regardless of validation outcome prevents attackers from discovering valid email addresses through error responses. This is a solid security pattern for password reset flows.frontend/app/api/auth/password-reset/confirm/route.ts (1)
57-70: LGTM! Proper use of transaction for atomic password update.The transaction correctly ensures that both the password update and token deletion succeed together or fail together, preventing scenarios where a token could be reused or a password update is lost.
frontend/lib/auth/email-verification.ts (1)
11-21: No action needed. Token cleanup is already properly handled in theresend-verificationroute (lines 51–53) which deletes existing tokens before callingcreateEmailVerificationToken. Thesignuproute creates a new user with no pre-existing tokens, so deletion is unnecessary there. The current architecture—with cleanup at the call site rather than inside the utility function—is cleaner and avoids redundant operations.Likely an incorrect or invalid review comment.
frontend/app/api/auth/resend-verification/route.ts (1)
16-55: LGTM on the security-conscious implementation.The handler correctly:
- Returns
{ success: true }for all non-happy-path cases to prevent user enumeration- Validates provider type before sending verification
- Skips already-verified accounts
- Cleans up existing tokens before creating new ones
frontend/app/api/auth/verify-email/route.ts (1)
8-59: LGTM on the verification flow.The handler correctly:
- Validates token presence and existence
- Checks expiration and cleans up expired tokens
- Uses a transaction to atomically update the user and delete the token
- Provides appropriate redirect URLs with query parameters for UI feedback
frontend/lib/auth/password-reset.ts (1)
11-23: LGTM on the token creation logic.The function correctly generates a cryptographically random UUID and persists it with the user ID and expiration time.
frontend/app/[locale]/login/page.tsx (3)
143-160: Good implementation of the password visibility toggle.The toggle correctly uses
aria-labelfor accessibility, and the button type is properly set to"button"to prevent form submission.
162-173: Good preservation ofreturnToparameter across navigation links.Both the "Forgot password?" and "Sign up" links correctly preserve the
returnToquery parameter, ensuring a consistent user flow.Also applies to: 204-213
83-92: Thequiz_just_savedkey is consistently used across the codebase.The key is set in
frontend/app/[locale]/login/page.tsxand properly consumed and cleaned up infrontend/components/dashboard/QuizSavedBanner.tsxwith appropriate error handling. No inconsistencies detected.frontend/drizzle/meta/0015_snapshot.json (2)
2218-2271: LGTM - Email verification token table structure is appropriate.The table design with
tokenas primary key and index onuser_idsupports efficient token lookup and user-based queries. Note that there's no foreign key to theuserstable, which means tokens won't be automatically cleaned up if a user is deleted - this may be intentional for audit purposes but consider implementing a cleanup mechanism.
2272-2325: Password reset tokens table mirrors verification tokens structure.Consistent design with the email verification tokens table. Same consideration applies regarding orphaned tokens if users are deleted.
frontend/lib/email/templates/base-layout.ts (1)
1-103: Well-structured email template with good cross-client compatibility.The table-based layout is the correct approach for email clients, and the dark mode support via
prefers-color-schemeis a nice enhancement. The inline styles ensure consistent rendering across email clients that strip<style>tags.frontend/app/[locale]/signup/page.tsx (1)
97-147: Form implementation looks good with proper accessibility.The password visibility toggle includes proper
aria-labelfor screen readers, and the form fields have appropriate validation attributes (required,minLength).
| <Link | ||
| href={ | ||
| returnTo | ||
| ? `/login?returnTo=${encodeURIComponent(returnTo)}` | ||
| : "/login" | ||
| } | ||
| className="mt-4 inline-block underline" | ||
| > | ||
| Back to login | ||
| </Link> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find the login page and related files
find frontend -type f -name "*login*" | head -20Repository: DevLoversTeam/devlovers.net
Length of output: 53
🏁 Script executed:
# Search for returnTo usage across the codebase
rg -n "returnTo" --type ts --type tsx frontend/Repository: DevLoversTeam/devlovers.net
Length of output: 98
🏁 Script executed:
# Also check if there's any validation or redirect logic in api routes or components
rg -n "redirect\|encodeURI\|pathname" --type ts --type tsx frontend/app/ -A 2 -B 2 | head -100Repository: DevLoversTeam/devlovers.net
Length of output: 98
🏁 Script executed:
# Search for returnTo without file type restriction
rg -n "returnTo" frontend/Repository: DevLoversTeam/devlovers.net
Length of output: 1924
🏁 Script executed:
# Find and read the login page
find frontend -type f -name "*login*" -path "*/page.*"Repository: DevLoversTeam/devlovers.net
Length of output: 53
🏁 Script executed:
# Search for validation or redirect patterns
rg -n "redirect\|validate\|allowedDomains" frontend/ -A 3 | head -150Repository: DevLoversTeam/devlovers.net
Length of output: 53
🏁 Script executed:
# Read the login page to see the returnTo handling
cat -n frontend/app/[locale]/login/page.tsx | sed -n '1,110p'Repository: DevLoversTeam/devlovers.net
Length of output: 4197
🏁 Script executed:
# Check if there's any validation utility or helper
rg -n "validate\|isValidUrl\|isSafeUrl\|allowed" frontend/ -B 2 -A 2 | head -100Repository: DevLoversTeam/devlovers.net
Length of output: 53
Fix open redirect vulnerability in login redirect logic.
The login page directly redirects to the returnTo parameter without validation at line 104: window.location.href = returnTo || /${locale}/dashboard;. An attacker can craft a link like /login?returnTo=https://evil.com to redirect authenticated users to a malicious site for phishing attacks.
Implement validation to ensure returnTo is a relative path or belongs to an allowed domain before redirecting. Consider using a helper function to validate URLs:
function isSafeRedirectUrl(url: string, locale: string): boolean {
if (!url) return false;
// Allow relative paths starting with /
if (url.startsWith('/')) {
// Ensure it doesn't contain protocol indicators for external redirects
return !url.includes('://');
}
return false;
}Then use: window.location.href = isSafeRedirectUrl(returnTo, locale) ? returnTo : /${locale}/dashboard;
| export default function ResetPasswordPage() { | ||
| const searchParams = useSearchParams(); | ||
| const token = searchParams.get("token"); | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Next.js 15 useSearchParams Suspense boundary requirement client component
💡 Result:
Short answer
- useSearchParams must be called from a Client Component (add 'use client'). [1]
- If the route is statically prerendered, any Client Component that calls useSearchParams must be wrapped in a React Suspense boundary (closest Suspense catches it) or the build will bail with "Missing Suspense boundary" / the page will be deopted to CSR. Dev mode may hide this because rendering is on‑demand. [1][2]
Why
- Next.js needs Suspense so it can render the static shell and let the small dynamic subtree that reads query params hydrate / suspend separately; otherwise the whole page is forced to client render at build time. [1][2]
How to fix
- Move the useSearchParams code into a child Client Component and wrap that child in in the parent Server Component (page/layout). Or make the route dynamic (opt out of prerendering) or pass searchParams from the Server Component instead. [1][2][9]
References
- Next.js docs — useSearchParams behavior and recommendation. [1]
- Next.js error docs — "Missing Suspense boundary with useSearchParams" / CSR bailout guidance. [2]
- Active GitHub discussion / issue about build errors and Suspense with useSearchParams. [7]
Would you like a minimal code example showing the recommended pattern?
Wrap with Suspense boundary for useSearchParams.
In Next.js 15+, useSearchParams() in a Client Component requires the component to be wrapped in a <Suspense> boundary when the route is statically prerendered. Without it, the build will fail with a "Missing Suspense boundary" error or the entire page will be forced to client-side render. Either wrap this component in <Suspense> at the parent level, make the route dynamic with export const dynamic = 'force-dynamic', or pass searchParams from the parent Server Component instead.
🤖 Prompt for AI Agents
In @frontend/app/[locale]/reset-password/page.tsx around lines 8 - 11, The
ResetPasswordPage component uses useSearchParams() which requires a Suspense
boundary in Next.js 15+ when the route is statically prerendered; to fix, either
wrap the parent/server component that renders ResetPasswordPage in a
React.Suspense boundary, or convert this route to dynamic by adding export const
dynamic = 'force-dynamic' in the module, or stop calling useSearchParams in
ResetPasswordPage and instead accept searchParams/token as a prop passed from
the parent Server Component; update the code accordingly by modifying the parent
or the ResetPasswordPage signature (referencing ResetPasswordPage and
useSearchParams) to implement one of these fixes.
| setLoading(true); | ||
|
|
||
| const res = await fetch("/api/auth/password-reset/confirm", { | ||
| method: "POST", | ||
| headers: { "Content-Type": "application/json" }, | ||
| body: JSON.stringify({ | ||
| token, | ||
| password, | ||
| }), | ||
| }); | ||
|
|
||
| setLoading(false); | ||
|
|
||
| if (!res.ok) { | ||
| setError("Invalid or expired reset link."); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Add error handling for network failures.
The fetch call lacks a try-catch block. If the network request fails (e.g., offline, server unreachable), the promise rejection will be unhandled, setLoading(false) won't execute, and the UI will remain stuck in the loading state.
🛠️ Suggested fix
setLoading(true);
+ try {
const res = await fetch("/api/auth/password-reset/confirm", {
method: "POST",
headers: { "Content-Type": "application/json" },
body: JSON.stringify({
token,
password,
}),
});
- setLoading(false);
-
if (!res.ok) {
+ setLoading(false);
setError("Invalid or expired reset link.");
return;
}
+ setLoading(false);
setConfirmed(true);
+ } catch {
+ setLoading(false);
+ setError("Network error. Please try again.");
+ }
}📝 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.
| setLoading(true); | |
| const res = await fetch("/api/auth/password-reset/confirm", { | |
| method: "POST", | |
| headers: { "Content-Type": "application/json" }, | |
| body: JSON.stringify({ | |
| token, | |
| password, | |
| }), | |
| }); | |
| setLoading(false); | |
| if (!res.ok) { | |
| setError("Invalid or expired reset link."); | |
| return; | |
| } | |
| setLoading(true); | |
| try { | |
| const res = await fetch("/api/auth/password-reset/confirm", { | |
| method: "POST", | |
| headers: { "Content-Type": "application/json" }, | |
| body: JSON.stringify({ | |
| token, | |
| password, | |
| }), | |
| }); | |
| if (!res.ok) { | |
| setLoading(false); | |
| setError("Invalid or expired reset link."); | |
| return; | |
| } | |
| setLoading(false); | |
| setConfirmed(true); | |
| } catch { | |
| setLoading(false); | |
| setError("Network error. Please try again."); | |
| } |
🤖 Prompt for AI Agents
In @frontend/app/[locale]/reset-password/page.tsx around lines 26 - 42, The
fetch to "/api/auth/password-reset/confirm" in the reset handler can throw on
network failures and currently has no try/catch, which can leave loading state
stuck; wrap the fetch and response handling in a try/catch and use finally to
ensure setLoading(false) always runs, set a user-friendly error via setError on
catch (e.g., "Network error, please try again"), and keep the existing res.ok
check to set the "Invalid or expired reset link." message when the request
completes but returns a non-OK response; update the code surrounding the
token/password POST (the fetch call and the calls to setLoading and setError)
accordingly.
| async function onSubmit(e: React.FormEvent<HTMLFormElement>) { | ||
| e.preventDefault(); | ||
| setLoading(true); | ||
| setError(null); | ||
|
|
||
| try { | ||
| const formData = new FormData(e.currentTarget); | ||
|
|
||
| const res = await fetch("/api/auth/signup", { | ||
| method: "POST", | ||
| headers: { "Content-Type": "application/json" }, | ||
| body: JSON.stringify({ | ||
| name: formData.get("name"), | ||
| email: formData.get("email"), | ||
| password: formData.get("password"), | ||
| }), | ||
| }); | ||
|
|
||
| if (!res.ok) { | ||
| const data = await res.json(); | ||
| setError(data.error ?? "Signup failed"); | ||
| return; | ||
| } | ||
|
|
||
| const data = await res.json(); | ||
| const pendingResult = getPendingQuizResult(); | ||
|
|
||
| if (pendingResult && data.userId) { | ||
| try { | ||
| const quizRes = await fetch("/api/quiz/guest-result", { | ||
| method: "POST", | ||
| headers: { "Content-Type": "application/json" }, | ||
| body: JSON.stringify({ | ||
| userId: data.userId, | ||
| quizId: pendingResult.quizId, | ||
| answers: pendingResult.answers, | ||
| violations: pendingResult.violations, | ||
| timeSpentSeconds: pendingResult.timeSpentSeconds, | ||
| }), | ||
| }); | ||
| if (!quizRes.ok) { | ||
| throw new Error(`Failed to save quiz result: ${quizRes.status}`); | ||
| } | ||
| const result = await quizRes.json(); | ||
|
|
||
| if (result.success) { | ||
| sessionStorage.setItem('quiz_just_saved', JSON.stringify({ | ||
| score: result.score, | ||
| total: result.totalQuestions, | ||
| percentage: result.percentage, | ||
| pointsAwarded: result.pointsAwarded, | ||
| quizSlug: pendingResult.quizSlug, | ||
| })); | ||
| } | ||
| } catch (err) { | ||
| console.error('Failed to save quiz result:', err); | ||
| } finally { | ||
| clearPendingQuizResult(); | ||
| } | ||
|
|
||
| window.location.href = `/${locale}/dashboard`; | ||
| return; | ||
| } | ||
|
|
||
| window.location.href = returnTo || `/${locale}/dashboard`; | ||
| } catch (err) { | ||
| console.error('Signup submit error', err); | ||
| setError('Signup failed'); | ||
| } finally { | ||
| setLoading(false); | ||
| const formData = new FormData(e.currentTarget); | ||
| const emailValue = String(formData.get("email") || ""); | ||
| setEmail(emailValue); | ||
|
|
||
| const res = await fetch("/api/auth/signup", { | ||
| method: "POST", | ||
| headers: { "Content-Type": "application/json" }, | ||
| body: JSON.stringify({ | ||
| name: formData.get("name"), | ||
| email: emailValue, | ||
| password: formData.get("password"), | ||
| }), | ||
| }); | ||
|
|
||
| const data = await res.json().catch(() => null); | ||
| setLoading(false); | ||
|
|
||
| if (!res.ok) { | ||
| setError(data?.error ?? "Failed to sign up"); | ||
| return; | ||
| } | ||
|
|
||
| if (data?.verificationRequired) { | ||
| setVerificationRequired(true); | ||
| return; | ||
| } | ||
|
|
||
| window.location.href = | ||
| returnTo || `/${locale}/dashboard`; | ||
| } |
There was a problem hiding this comment.
Missing error handling for network failures.
If the fetch call throws (network error, timeout), the exception is unhandled and setLoading(false) is never reached, leaving the button in a perpetual loading state.
🔧 Proposed fix to wrap fetch in try/catch
async function onSubmit(e: React.FormEvent<HTMLFormElement>) {
e.preventDefault();
setLoading(true);
setError(null);
const formData = new FormData(e.currentTarget);
const emailValue = String(formData.get("email") || "");
setEmail(emailValue);
+ try {
const res = await fetch("/api/auth/signup", {
method: "POST",
headers: { "Content-Type": "application/json" },
body: JSON.stringify({
name: formData.get("name"),
email: emailValue,
password: formData.get("password"),
}),
});
const data = await res.json().catch(() => null);
- setLoading(false);
if (!res.ok) {
setError(data?.error ?? "Failed to sign up");
return;
}
if (data?.verificationRequired) {
setVerificationRequired(true);
return;
}
window.location.href =
returnTo || `/${locale}/dashboard`;
+ } catch {
+ setError("Network error. Please try again.");
+ } finally {
+ setLoading(false);
+ }
}🤖 Prompt for AI Agents
In @frontend/app/[locale]/signup/page.tsx around lines 23 - 57, The onSubmit
handler can throw if fetch fails, leaving loading true and later code
referencing res; wrap the fetch and subsequent res.json() calls in a
try/catch/finally inside onSubmit so network errors are caught, call setError
with a user-friendly message in the catch, and ensure setLoading(false) runs in
finally; also guard the res.ok check to only run if res is defined (or handle
absence by setting an error) and setVerificationRequired when appropriate.
| const origin = (await headers()).get("origin") | ||
|
|
||
| await sendVerificationEmail({ | ||
| to: normalizedEmail, | ||
| verifyUrl: `${origin}/api/auth/verify-email?token=${token}` | ||
| }) |
There was a problem hiding this comment.
Origin header may be null, causing broken verification links.
The origin header is not guaranteed to be present (e.g., for server-to-server requests, curl, or certain browser contexts). If origin is null, the verifyUrl will be malformed (null/api/auth/verify-email?token=...).
🔧 Proposed fix with fallback
- const origin = (await headers()).get("origin")
+ const headersList = await headers();
+ const origin = headersList.get("origin")
+ || headersList.get("x-forwarded-host") && `https://${headersList.get("x-forwarded-host")}`
+ || process.env.NEXT_PUBLIC_APP_URL;
+
+ if (!origin) {
+ throw new Error("Could not determine application origin for verification email");
+ }
await sendVerificationEmail({
to: normalizedEmail,
verifyUrl: `${origin}/api/auth/verify-email?token=${token}`
})Alternatively, use an environment variable like NEXT_PUBLIC_APP_URL or APP_URL as the canonical base URL.
🤖 Prompt for AI Agents
In @frontend/app/api/auth/signup/route.ts around lines 66 - 71, The origin
header fetched via headers().get("origin") can be null, so update the signup
route to fall back to a canonical app URL when building the verifyUrl: compute a
baseUrl = origin ?? process.env.NEXT_PUBLIC_APP_URL ?? process.env.APP_URL (and
normalize to remove any trailing slash), then call sendVerificationEmail({ to:
normalizedEmail, verifyUrl: `${baseUrl}/api/auth/verify-email?token=${token}` })
so verifyUrl is never built with "null" and always uses a valid absolute URL.
| export const emailVerificationTokens = pgTable( | ||
| "email_verification_tokens", | ||
| { | ||
| token: text("token").primaryKey(), | ||
| userId: text("user_id").notNull(), | ||
| expiresAt: timestamp("expires_at", { mode: "date" }).notNull(), | ||
| createdAt: timestamp("created_at", { mode: "date" }) | ||
| .notNull() | ||
| .defaultNow(), | ||
| }, | ||
| table => ({ | ||
| userIdIdx: index("email_verification_tokens_user_id_idx").on( | ||
| table.userId | ||
| ), | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for foreign key usage in schema files
rg -n "references|foreignKey" --type ts frontend/db/schema/Repository: DevLoversTeam/devlovers.net
Length of output: 2919
🏁 Script executed:
#!/bin/bash
# Also check what other tables exist and their patterns
find frontend/db/schema -type f -name "*.ts" | head -20Repository: DevLoversTeam/devlovers.net
Length of output: 362
🏁 Script executed:
#!/bin/bash
# Check if users table exists and see its structure
rg -A 10 "pgTable.*users" --type ts frontend/db/schema/Repository: DevLoversTeam/devlovers.net
Length of output: 53
🏁 Script executed:
#!/bin/bash
# Check the password reset tokens schema for comparison
cat -n frontend/db/schema/passwordResetTokens.tsRepository: DevLoversTeam/devlovers.net
Length of output: 751
🏁 Script executed:
#!/bin/bash
# Check the users table schema
cat -n frontend/db/schema/users.tsRepository: DevLoversTeam/devlovers.net
Length of output: 1382
🏁 Script executed:
#!/bin/bash
# Check if there are any comments in emailVerificationTokens about FK or lifecycle
cat -n frontend/db/schema/emailVerificationTokens.tsRepository: DevLoversTeam/devlovers.net
Length of output: 763
Add foreign key constraint to users table on userId.
Both emailVerificationTokens and passwordResetTokens lack foreign key constraints on userId, despite being referenced columns. The codebase consistently uses .references() with appropriate onDelete rules elsewhere (e.g., points, shop, quiz schemas). Add a constraint with onDelete: 'cascade' to maintain referential integrity, or document that token cleanup is handled at the application level.
🤖 Prompt for AI Agents
In @frontend/db/schema/emailVerificationTokens.ts around lines 3 - 17, Add a
foreign key constraint from emailVerificationTokens.userId to the users table:
update the emailVerificationTokens pgTable definition to call .references(() =>
users.id, { onDelete: "cascade" }) (or equivalent reference to your users
primary key) on the userId column so it enforces referential integrity and
cascades deletes; mirror the pattern used in other schemas (e.g., points, shop,
quiz) and ensure the symbol names match your users table export and primary key
field.
| CREATE TABLE | ||
| IF NOT EXISTS email_verification_tokens | ||
| ( | ||
| id uuid PRIMARY KEY DEFAULT gen_random_uuid | ||
| (), | ||
| email text NOT NULL, | ||
| token text NOT NULL, | ||
| expires_at timestamp NOT NULL, | ||
| created_at timestamp DEFAULT now | ||
| () NOT NULL | ||
| ); | ||
|
|
||
| CREATE UNIQUE INDEX | ||
| IF NOT EXISTS email_verification_tokens_token_idx | ||
| ON email_verification_tokens | ||
| (token); No newline at end of file |
There was a problem hiding this comment.
Schema mismatch: this migration conflicts with 0013.
This migration has a different schema than 0013_low_roughhouse.sql:
| Column | 0013 | 0015 |
|---|---|---|
| Primary Key | token |
id (uuid) |
| User reference | user_id |
email |
| Token column | PK | separate column |
With IF NOT EXISTS, this table won't be created if 0013 already ran, leaving an inconsistent schema. Dependent code (API routes, schema files) may expect one structure but get another.
Please consolidate these migrations into a single consistent schema definition.
🤖 Prompt for AI Agents
In @frontend/drizzle/0015_glamorous_eternity.sql around lines 1 - 16, The
migration email_verification_tokens in 0015_glamorous_eternity.sql conflicts
with 0013_low_roughhouse.sql (0013 uses token as PK and user_id, 0015 uses id
uuid PK and email); consolidate into a single consistent schema for the
email_verification_tokens table by choosing the canonical shape (either keep
token as PRIMARY KEY with user_id or keep id uuid PRIMARY KEY with email),
update the CREATE TABLE statement in the migration to match that chosen schema,
remove or replace the unsafe IF NOT EXISTS behavior (use explicit
create/drop/alter so the schema is deterministic), ensure the index
email_verification_tokens_token_idx matches the token column existence and
uniqueness, and adjust any dependent code/schema files to the chosen column
names (token, id, user_id/email) so all migrations and runtime code agree.
| function addHours(date: Date, hours: number) { | ||
| const d = new Date(); | ||
| d.setHours(d.getHours() + hours); | ||
| return d; | ||
| } |
There was a problem hiding this comment.
Bug: The date parameter is ignored.
The addHours function ignores its date parameter and always creates a new Date() internally. While this doesn't cause issues in the current usage (since new Date() is passed), the function is misleading and will produce incorrect results if reused with a different date.
Proposed fix
function addHours(date: Date, hours: number) {
- const d = new Date();
+ const d = new Date(date);
d.setHours(d.getHours() + hours);
return d;
}📝 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.
| function addHours(date: Date, hours: number) { | |
| const d = new Date(); | |
| d.setHours(d.getHours() + hours); | |
| return d; | |
| } | |
| function addHours(date: Date, hours: number) { | |
| const d = new Date(date); | |
| d.setHours(d.getHours() + hours); | |
| return d; | |
| } |
🤖 Prompt for AI Agents
In @frontend/lib/auth/email-verification.ts around lines 5 - 9, The addHours
function currently ignores its date parameter and always uses new Date(); modify
addHours(date: Date, hours: number) to operate on a clone of the passed-in date
(e.g., new Date(date.getTime()) or new Date(date)) so you don't mutate the
original, then add the hours via setHours/getHours and return that cloned date;
keep the function name addHours and ensure it uses the input date rather than
creating a fresh Date().
| function addHours(date: Date, hours: number) { | ||
| const d = new Date(); | ||
| d.setHours(d.getHours() + hours); | ||
| return d; | ||
| } |
There was a problem hiding this comment.
Bug: addHours ignores the date parameter.
The function creates a new Date() on line 6 instead of using the passed date argument. While the current call site (addHours(new Date(), 1)) happens to work, this is a latent bug.
Additionally, frontend/lib/auth/email-verification.ts uses date-fns for the same purpose. Consider using date-fns here for consistency and correctness.
Proposed fix using date-fns (consistent with email-verification.ts)
import crypto from "crypto";
+import { addHours } from "date-fns";
import { db } from "@/db";
import { passwordResetTokens } from "@/db/schema/passwordResetTokens";
-function addHours(date: Date, hours: number) {
- const d = new Date();
- d.setHours(d.getHours() + hours);
- return d;
-}
-
export async function createPasswordResetToken(
userId: string
) {🤖 Prompt for AI Agents
In @frontend/lib/auth/password-reset.ts around lines 5 - 9, The addHours
function ignores its date parameter by creating a new Date() instead of using
the passed-in date; update addHours (in frontend/lib/auth/password-reset.ts) to
use the provided date argument and return a new Date based on it (or better,
replace the implementation to use date-fns addHours for consistency with
email-verification.ts), ensuring all call sites (e.g., addHours(new Date(), 1))
behave correctly and import addHours from date-fns if you switch to that
approach.
2 participants
This pull request introduces a comprehensive overhaul of the authentication-related UI flows, focusing on improved user experience for login, signup, password reset, and error handling. The most important changes include the addition of dedicated pages for password reset and forgot password, enhanced feedback and error messaging in login and signup forms, and improved navigation and accessibility features.
New authentication flows and pages:
ForgotPasswordPage(frontend/app/[locale]/forgot-password/page.tsx) allowing users to request a password reset link by email, with clear feedback and navigation to login. (frontend/app/[locale]/forgot-password/page.tsxR1-R118)ResetPasswordPage(frontend/app/[locale]/reset-password/page.tsx) for users to set a new password using a token from their email, including error handling for invalid or missing tokens. (frontend/app/[locale]/reset-password/page.tsxR1-R113)Login flow improvements:
LoginPage(frontend/app/[locale]/login/page.tsx): now distinguishes between invalid credentials and unverified emails, provides a button to resend verification emails, and displays contextual feedback messages. (frontend/app/[locale]/login/page.tsxL3-R62, frontend/app/[locale]/login/page.tsxR140-R195)Signup flow improvements:
SignupPage(frontend/app/[locale]/signup/page.tsx): now prompts users to verify their email if required, provides clearer error messages, and adds a "Show/Hide password" toggle. (frontend/app/[locale]/signup/page.tsxL3-R63, frontend/app/[locale]/signup/page.tsxR73-R101, frontend/app/[locale]/signup/page.tsxR114-R164)Navigation and redirect updates:
/dashboardinstead of the homepage for a more consistent post-login experience.These changes collectively streamline the authentication experience, providing users with clearer guidance and feedback throughout the login, signup, and password reset processes.## Description
Database Changes (if applicable)
How Has This Been Tested?
Screenshots (if applicable)
Checklist
Before submitting
Reviewers
Summary by CodeRabbit
Release Notes
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.