Skip to content

Fix: improve backend validation, security and data integrity across APIs #18

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.

Sign up for GitHub

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

Open
BhathiyaVicum wants to merge 11 commits into
base: main
Choose a base branch
from
Open

Fix: improve backend validation, security and data integrity across APIs #18

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
098f990
fix(profile): prevent empty name causing Teacher validation error
BhathiyaVicum Apr 20, 2026
b5f7a1e
fix(grades): resolve circular JSON error in grades POST
BhathiyaVicum Apr 20, 2026
a3d811f
fix(students): add missing validation check for request body
BhathiyaVicum Apr 20, 2026
b899a9e
fix(students): add missing validation check for request body
BhathiyaVicum Apr 20, 2026
dc2a08a
fix(announcements): add default audience field
BhathiyaVicum Apr 20, 2026
e1ddfe2
fix(grades): restrict grade updates to owner only
BhathiyaVicum Apr 20, 2026
1b2e0be
fix(grades): restrict grade deletion to owner only
BhathiyaVicum Apr 20, 2026
4fe3ca5
fix(students): improve authorization and input validation in update/d…
BhathiyaVicum Apr 20, 2026
4034a32
fix(grades): ensure studentId is cast to ObjectId in GET query
BhathiyaVicum Apr 20, 2026
6909cbb
fix(attendance): validate class field to prevent empty values
BhathiyaVicum Apr 20, 2026
563beff
fix(grade): prevent invalid marks in findOneAndUpdate
BhathiyaVicum Apr 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/api/announcements/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { z } from 'zod'
const AnnouncementSchema = z.object({
title: z.string().min(1),
content: z.string().min(1),
audience: z.string().optional(),
audience: z.string().optional().default('all'),
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -nP --type=ts -C2 "audience" -g '!**/node_modules/**'

Repository: JavaScript-Mastery-Pro/discord-challenge

Length of output: 4396

Fix audience default casing inconsistency ('all' → 'All').

The Mongoose schema in models/Announcement.ts defaults audience to 'All' (capital A), and the client UI in AnnouncementsClient.tsx defaults to 'All' and explicitly checks a.audience !== "All" to determine whether to display an audience badge. The API route defaults to lowercase 'all', creating a casing mismatch that causes records created via this endpoint to have 'all' while the client expects 'All', breaking the badge display logic and data consistency.

Fix 
-  audience: z.string().optional().default('all'),
+  audience: z.string().optional().default('All'),
📝 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.

Suggested change
audience: z.string().optional().default('all'),
audience: z.string().optional().default('All'),
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@app/api/announcements/route.ts` at line 10, The API route's zod schema sets
audience: z.string().optional().default('all') which conflicts with the Mongoose
model (models/Announcement.ts) and client (AnnouncementsClient.tsx) that expect
"All" (capital A); update the default in the API validation in route.ts from
'all' to 'All' (and optionally ensure any created/normalized value in the
handler uses the audience value from the parsed input so it persists "All") so
stored records match the model and the client's a.audience !== "All" checks.

category: z.enum(['academic', 'events', 'admin', 'general']).optional(),
pinned: z.boolean().optional(),
})
Expand Down
11 changes: 7 additions & 4 deletions app/api/grades/[id]/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export async function PUT(req: NextRequest, ctx: { params: Promise<{ id: string

await connectDB()
const grade = await Grade.findOneAndUpdate(
{ _id: id },
{ _id: id, teacherId: userId },
sanitizedBody,
{ new: true }
)
Expand All @@ -56,12 +56,15 @@ export async function DELETE(_req: NextRequest, ctx: { params: Promise<{ id: str
try {
const { id } = await ctx.params
await connectDB()
const deleted = await Grade.findOneAndDelete({ _id: id })

const deleted = await Grade.findOneAndDelete({
_id: id,
teacherId: userId
})

if (!deleted) {
return NextResponse.json({ error: 'Grade not found' }, { status: 404 })
}

return NextResponse.json({ success: true })
} catch (error) {
if (error instanceof Error) {
Expand Down
15 changes: 10 additions & 5 deletions app/api/grades/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { NextRequest, NextResponse } from 'next/server'
import { connectDB } from '@/lib/mongodb'
import { Grade } from '@/models/Grade'
import { z } from 'zod'
import mongoose from 'mongoose'

const GradeSchema = z.object({
studentId: z.string().min(1),
Expand Down Expand Up @@ -41,7 +42,11 @@ export async function GET(req: NextRequest) {
const subject = searchParams.get('subject')

const query: Record<string, unknown> = { teacherId: userId }
if (studentId) query.studentId = studentId

if (studentId && mongoose.Types.ObjectId.isValid(studentId)) {
query.studentId = new mongoose.Types.ObjectId(studentId)
}
Comment on lines +46 to +48
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Invalid studentId is silently ignored — returns all teacher grades.

When studentId is provided but not a valid ObjectId, the filter is silently dropped and the response returns every grade for the teacher. That's likely unexpected for callers and can mask client bugs. Prefer returning 400 (or an empty result) on invalid input.

🔧 Proposed fix 
-    if (studentId && mongoose.Types.ObjectId.isValid(studentId)) {
-      query.studentId = new mongoose.Types.ObjectId(studentId)
+    if (studentId) {
+      if (!mongoose.Types.ObjectId.isValid(studentId)) {
+        return NextResponse.json({ error: 'Invalid studentId' }, { status: 400 })
+      }
+      query.studentId = new mongoose.Types.ObjectId(studentId)
     }
📝 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.

Suggested change
if (studentId && mongoose.Types.ObjectId.isValid(studentId)) {
query.studentId = new mongoose.Types.ObjectId(studentId)
}
if (studentId) {
if (!mongoose.Types.ObjectId.isValid(studentId)) {
return NextResponse.json({ error: 'Invalid studentId' }, { status: 400 })
}
query.studentId = new mongoose.Types.ObjectId(studentId)
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@app/api/grades/route.ts` around lines 46 - 48, The current logic silently
ignores an invalid studentId (the mongoose.Types.ObjectId.isValid check) and
thus returns all grades; instead, in the handler where query and studentId are
processed (references: studentId, mongoose.Types.ObjectId.isValid, and
query.studentId = new mongoose.Types.ObjectId(studentId)), validate studentId
and if it is provided but invalid respond with a 400 Bad Request (or return an
empty result) with a clear error message; do this before mutating query so
invalid input does not fall through to returning all teacher grades.


if (subject) query.subject = subject

const grades = await Grade.find(query).sort({ createdAt: -1 }).lean()
Expand All @@ -58,22 +63,22 @@ export async function POST(req: NextRequest) {

try {
await connectDB()

let body
try {
body = await req.json()
} catch {
return NextResponse.json({ error: 'Invalid JSON in request body' }, { status: 400 })
}

const parsed = GradeSchema.safeParse(body)
if (!parsed.success) return NextResponse.json({ error: parsed.error.flatten() }, { status: 400 })

const data = parsed.data
const max = data.maxMarks!
const term = data.term ?? 'Term 1'
const grade = Grade.findOneAndUpdate(

const grade = await Grade.findOneAndUpdate(
{ teacherId: userId, studentId: data.studentId, subject: data.subject, term },
{ $set: { ...data, term, teacherId: userId, grade: calcGrade(data.marks, max) } },
{ upsert: true, new: true }
Expand Down
2 changes: 1 addition & 1 deletion app/api/profile/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export async function GET(req: NextRequest) {
const clerkUser = await currentUser()
const created = await Teacher.create({
clerkId: userId,
name: clerkUser?.fullName ?? '',
name: clerkUser?.fullName?.trim() || clerkUser?.firstName?.trim() || "Unknown User",
email: clerkUser?.emailAddresses[0]?.emailAddress ?? '',
department: '',
subjects: [],
Expand Down
11 changes: 7 additions & 4 deletions app/api/students/[id]/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export async function PUT(req: NextRequest, ctx: { params: Promise<{ id: string

await connectDB()
const student = await Student.findOneAndUpdate(
{ _id: id },
{ _id: id, teacherId: userId },
sanitizedBody,
{ new: true }
)
Expand Down Expand Up @@ -65,12 +65,15 @@ export async function DELETE(_req: NextRequest, ctx: { params: Promise<{ id: str
}

await connectDB()
const deleted = await Student.findOneAndDelete({ _id: id })

const deleted = await Student.findOneAndDelete({
_id: id,
teacherId: userId
})

if (!deleted) {
return NextResponse.json({ error: 'Student not found' }, { status: 404 })
}

return NextResponse.json({ success: true })
} catch (error) {
if (error instanceof Error) {
Expand Down
19 changes: 15 additions & 4 deletions app/api/students/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,17 +84,28 @@ export async function POST(req: NextRequest) {

try {
await connectDB()

let body
try {
body = await req.json()
} catch {
return NextResponse.json({ error: 'Malformed JSON' }, { status: 400 })
}

StudentSchema.safeParse(body)

const student = await Student.create({ ...(body as Record<string, unknown>), teacherId: userId })
const parsed = StudentSchema.safeParse(body)

if (!parsed.success) {
return NextResponse.json(
{ error: parsed.error.flatten() },
{ status: 400 }
)
}

const student = await Student.create({
...parsed.data,
teacherId: userId
})

return NextResponse.json(student, { status: 201 })
} catch (error) {
if (error instanceof Error) {
Expand Down
10 changes: 9 additions & 1 deletion models/Attendance.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,15 @@ const AttendanceSchema = new Schema<IAttendance>(
teacherId: { type: String, required: true, index: true },
studentId: { type: Schema.Types.ObjectId, ref: 'Student', required: true },
studentName: { type: String, required: true },
class: { type: String, required: true },
class: {
type: String,
required: true,
trim: true,
validate: {
validator: (v: string) => v.trim().length > 0,
message: 'Class cannot be empty',
},
},
Comment on lines +20 to +28
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Confirm the Zod schema for `class` in the attendance API route still uses min(1) without trim.
rg -nP -C2 '\bclass\s*:\s*z\.string\(\)' --type=ts

Repository: JavaScript-Mastery-Pro/discord-challenge

Length of output: 1057

Validator is redundant after trim: true; also ensure upstream Zod schema matches.

Since trim: true runs before validators, the input is already trimmed by the time the validator executes, so v.trim().length > 0 is equivalent to v.length > 0. More importantly, the Zod schema in app/api/attendance/route.ts (line 10) uses z.string().min(1) for class, which accepts whitespace-only strings like " ". Those will pass API validation but be trimmed to "" by Mongoose and fail here with a validation error instead of being rejected cleanly at the API layer. Align the Zod schema (e.g., .trim().min(1)) so the API rejects whitespace-only input before it reaches the model.

Suggested fixes 
     class: {
       type: String,
       required: true,
       trim: true,
       validate: {
-        validator: (v: string) => v.trim().length > 0,
+        validator: (v: string) => v.length > 0,
         message: 'Class cannot be empty',
       },
     },

And in app/api/attendance/route.ts:

-  class: z.string().min(1),
+  class: z.string().trim().min(1),
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@models/Attendance.ts` around lines 20 - 28, The Mongoose 'class' field
validator is redundant because trim: true runs first and the validator uses
v.trim(), and the upstream Zod schema (the z.string().min(1) used for the
attendance input) currently allows whitespace-only strings which will be trimmed
to empty and then fail at the model layer; update the Zod validation to
z.string().trim().min(1) so whitespace-only input is rejected at the API layer,
and simplify the Attendance model 'class' field by removing the redundant
validator (or change its check to v.length > 0 if you prefer an explicit guard)
so both layers are consistent.

date: { type: String, required: true },
status: { type: String, enum: ['present', 'absent', 'late'], required: true },
},
Expand Down
6 changes: 3 additions & 3 deletions models/Grade.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,10 @@ GradeSchema.pre("save", function () {
});

GradeSchema.pre("findOneAndUpdate", function () {
const update = this.getUpdate() as Record<string, unknown>;
const update = this.getUpdate() as any;
if (update && typeof update === "object") {
const marks = update.marks;
const maxMarks = update.maxMarks;
const marks = update?.$set?.marks ?? update?.marks;
const maxMarks = update?.$set?.maxMarks ?? update?.maxMarks;
if (
marks !== undefined && typeof marks === "number" &&
maxMarks !== undefined && typeof maxMarks === "number" &&
Expand Down