-
Notifications
You must be signed in to change notification settings - Fork 112
fix(validation): correct Zod email schema and centralize authentication validation #370
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 3 commits
c8ed647
c55d20e
16596ea
f9fd5a1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -4,10 +4,13 @@ import crypto from "crypto"; | |||||||||||||||||||||||||||||||||
| import prisma from "@/lib/prisma"; | ||||||||||||||||||||||||||||||||||
| import { checkRateLimit } from "@/lib/rateLimit"; | ||||||||||||||||||||||||||||||||||
| import { sendVerificationEmail } from "@/lib/email"; | ||||||||||||||||||||||||||||||||||
| import { signupSchema } from "@/lib/validations/auth"; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| const REGISTER_LIMIT = 5; | ||||||||||||||||||||||||||||||||||
| const REGISTER_WINDOW_MS = 60 * 60 * 1000; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| const registerSchema = signupSchema.pick({ email: true, password: true }); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| export async function POST(req: Request) { | ||||||||||||||||||||||||||||||||||
| const ip = | ||||||||||||||||||||||||||||||||||
| req.headers.get("x-forwarded-for")?.split(",")[0]?.trim() ?? "unknown"; | ||||||||||||||||||||||||||||||||||
|
|
@@ -29,10 +32,15 @@ export async function POST(req: Request) { | |||||||||||||||||||||||||||||||||
| return NextResponse.json({ error: "Missing fields" }, { status: 400 }); | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| const normalizedEmail = email.toLowerCase().trim(); | ||||||||||||||||||||||||||||||||||
| const passwordRegex = /^(?=.*[a-z])(?=.*[A-Z])(?=.*\d)(?=.*[@$!%*?&]).{8,}$/; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| if (!passwordRegex.test(password)) { | ||||||||||||||||||||||||||||||||||
| const result = registerSchema.safeParse({ email, password }); | ||||||||||||||||||||||||||||||||||
| if (!result.success) { | ||||||||||||||||||||||||||||||||||
| const fieldErrors = result.error.flatten().fieldErrors; | ||||||||||||||||||||||||||||||||||
| if (fieldErrors.email) { | ||||||||||||||||||||||||||||||||||
| return NextResponse.json( | ||||||||||||||||||||||||||||||||||
| { error: "Invalid email address" }, | ||||||||||||||||||||||||||||||||||
| { status: 400 }, | ||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| return NextResponse.json( | ||||||||||||||||||||||||||||||||||
| { error: "Password does not meet requirements" }, | ||||||||||||||||||||||||||||||||||
| { status: 400 }, | ||||||||||||||||||||||||||||||||||
|
|
@@ -46,7 +54,7 @@ export async function POST(req: Request) { | |||||||||||||||||||||||||||||||||
| user = await prisma.user.create({ | ||||||||||||||||||||||||||||||||||
| data: { | ||||||||||||||||||||||||||||||||||
| name, | ||||||||||||||||||||||||||||||||||
| email: normalizedEmail, | ||||||||||||||||||||||||||||||||||
| email: result.data.email, | ||||||||||||||||||||||||||||||||||
| password: hashedPassword, | ||||||||||||||||||||||||||||||||||
| // emailVerified intentionally left null — set only after verification | ||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||
|
|
@@ -69,18 +77,18 @@ export async function POST(req: Request) { | |||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| await prisma.verificationToken.create({ | ||||||||||||||||||||||||||||||||||
| data: { | ||||||||||||||||||||||||||||||||||
| identifier: normalizedEmail, | ||||||||||||||||||||||||||||||||||
| identifier: result.data.email, | ||||||||||||||||||||||||||||||||||
| token, | ||||||||||||||||||||||||||||||||||
| expires, | ||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| // Send verification email — non-blocking in dev if SMTP not configured | ||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||
| await sendVerificationEmail(normalizedEmail, token); | ||||||||||||||||||||||||||||||||||
| await sendVerificationEmail(result.data.email, token); | ||||||||||||||||||||||||||||||||||
| } catch { | ||||||||||||||||||||||||||||||||||
| // Email sending failure should not block registration | ||||||||||||||||||||||||||||||||||
| console.error("Failed to send verification email to", normalizedEmail); | ||||||||||||||||||||||||||||||||||
| console.error("Failed to send verification email to", result.data.email); | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
Comment on lines
86
to
92
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win Avoid logging sensitive user data (PII). Logging the user's email address in plain text creates a privacy risk and violates best practices for log management. Consider masking the email or removing it entirely from the log message, as the 🛡️ Proposed fix // Send verification email — non-blocking in dev if SMTP not configured
try {
await sendVerificationEmail(result.data.email, token);
} catch {
// Email sending failure should not block registration
- console.error("Failed to send verification email to", result.data.email);
+ console.error("Failed to send verification email");
}📝 Committable suggestion
Suggested change
🧰 Tools🪛 ast-grep (0.44.1)[warning] 90-90: Avoid logging sensitive data (log-sensitive-data-typescript) 🤖 Prompt for AI AgentsSource: Linters/SAST tools |
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| return NextResponse.json( | ||||||||||||||||||||||||||||||||||
|
|
@@ -95,4 +103,4 @@ export async function POST(req: Request) { | |||||||||||||||||||||||||||||||||
| { status: 500 }, | ||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,103 @@ | ||
| import assert from "node:assert/strict"; | ||
| import test from "node:test"; | ||
|
|
||
| import { signupSchema, getPasswordError } from "@/lib/validations/auth"; | ||
|
|
||
| test("signupSchema accepts valid email and password", () => { | ||
| const result = signupSchema.safeParse({ | ||
| name: "John Doe", | ||
| email: "john@example.com", | ||
| password: "Secure@123", | ||
| }); | ||
|
|
||
| assert.equal(result.success, true); | ||
| }); | ||
|
|
||
| test("signupSchema rejects invalid email", () => { | ||
| const result = signupSchema.safeParse({ | ||
| name: "John Doe", | ||
| email: "not-an-email", | ||
| password: "Secure@123", | ||
| }); | ||
|
|
||
| assert.equal(result.success, false); | ||
| if (!result.success) { | ||
| const emailErrors = result.error.flatten().fieldErrors.email; | ||
| assert.ok(emailErrors?.some((e) => e.includes("Invalid email"))); | ||
| } | ||
| }); | ||
|
|
||
| test("signupSchema rejects empty email", () => { | ||
| const result = signupSchema.safeParse({ | ||
| name: "John Doe", | ||
| email: "", | ||
| password: "Secure@123", | ||
| }); | ||
|
|
||
| assert.equal(result.success, false); | ||
| }); | ||
|
|
||
| test("signupSchema accepts valid password", () => { | ||
| const result = signupSchema.safeParse({ | ||
| name: "John Doe", | ||
| email: "john@example.com", | ||
| password: "Abcdef1@", | ||
| }); | ||
|
|
||
| assert.equal(result.success, true); | ||
| }); | ||
|
|
||
| test("signupSchema rejects password below minimum length", () => { | ||
| const result = signupSchema.safeParse({ | ||
| name: "John Doe", | ||
| email: "john@example.com", | ||
| password: "Ab1@", | ||
| }); | ||
|
|
||
| assert.equal(result.success, false); | ||
| if (!result.success) { | ||
| const passwordErrors = result.error.flatten().fieldErrors.password; | ||
| assert.ok(passwordErrors?.some((e) => e.includes("at least 8"))); | ||
| } | ||
| }); | ||
|
|
||
| test("signupSchema rejects missing required fields", () => { | ||
| const result = signupSchema.safeParse({}); | ||
|
|
||
| assert.equal(result.success, false); | ||
| if (!result.success) { | ||
| const errors = result.error.flatten().fieldErrors; | ||
| assert.ok(errors.name); | ||
| assert.ok(errors.email); | ||
| assert.ok(errors.password); | ||
| } | ||
| }); | ||
|
|
||
| test("getPasswordError returns null for valid password", () => { | ||
| assert.equal(getPasswordError("Secure@123"), null); | ||
| }); | ||
|
|
||
| test("getPasswordError returns error for password below minimum length", () => { | ||
| const error = getPasswordError("Ab1@"); | ||
| assert.ok(error?.includes("at least 8 characters")); | ||
| }); | ||
|
|
||
| test("getPasswordError returns error for missing uppercase", () => { | ||
| const error = getPasswordError("secure@123"); | ||
| assert.ok(error?.includes("uppercase")); | ||
| }); | ||
|
|
||
| test("getPasswordError returns error for missing lowercase", () => { | ||
| const error = getPasswordError("SECURE@123"); | ||
| assert.ok(error?.includes("lowercase")); | ||
| }); | ||
|
|
||
| test("getPasswordError returns error for missing number", () => { | ||
| const error = getPasswordError("Secure@abc"); | ||
| assert.ok(error?.includes("number")); | ||
| }); | ||
|
|
||
| test("getPasswordError returns error for missing special character", () => { | ||
| const error = getPasswordError("Secure123"); | ||
| assert.ok(error?.includes("special character")); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Validate the
namefield to maintain consistency with the shared schema.By picking only
emailandpassword, the API bypasses thenamevalidation defined insignupSchema(which requires at least 2 characters). This means the API could accept registrations with an empty name even though the frontend enforces it.Consider using
signupSchemadirectly to validate all fields, ensuring the API and frontend rules remain perfectly aligned.♻️ Proposed fix
📝 Committable suggestion
🤖 Prompt for AI Agents