-
Notifications
You must be signed in to change notification settings - Fork 23
Fix/bughunt anhtu081204 #4
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 all commits
ae8664f
eb9dfa3
4a53d0c
44136fa
f2834cf
ff631fd
4108ab6
2a3d23b
3670fc0
93e7a7c
139f0a6
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 | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -3,8 +3,9 @@ import { NextRequest, NextResponse } from 'next/server' | |||||||||||||||||||||||
| import mongoose from 'mongoose' | ||||||||||||||||||||||||
| import { connectDB } from '@/lib/mongodb' | ||||||||||||||||||||||||
| import { Grade } from '@/models/Grade' | ||||||||||||||||||||||||
| import { calculateLetterGrade } from '@/lib/grading' | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| const ALLOWED_UPDATE_FIELDS = ['marks', 'maxMarks', 'grade'] | ||||||||||||||||||||||||
| const ALLOWED_UPDATE_FIELDS = ['studentId', 'studentName', 'subject', 'term', 'marks', 'maxMarks'] | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| export async function PUT(req: NextRequest, ctx: { params: Promise<{ id: string }> }) { | ||||||||||||||||||||||||
| const { userId } = await auth() | ||||||||||||||||||||||||
|
|
@@ -25,26 +26,88 @@ export async function PUT(req: NextRequest, ctx: { params: Promise<{ id: string | |||||||||||||||||||||||
| return NextResponse.json({ error: 'Invalid JSON in request body' }, { status: 400 }) | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| if (!body || typeof body !== 'object' || Array.isArray(body)) { | ||||||||||||||||||||||||
| return NextResponse.json({ error: 'Invalid JSON in request body' }, { status: 400 }) | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| const payload = body as Record<string, unknown> | ||||||||||||||||||||||||
| const unknownFields = Object.keys(payload).filter((key) => !ALLOWED_UPDATE_FIELDS.includes(key)) | ||||||||||||||||||||||||
| if (unknownFields.length > 0) { | ||||||||||||||||||||||||
| return NextResponse.json({ error: `Unknown field(s): ${unknownFields.join(', ')}` }, { status: 400 }) | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // Sanitize: only allow whitelisted fields | ||||||||||||||||||||||||
| const sanitizedBody: Record<string, unknown> = {} | ||||||||||||||||||||||||
| for (const key of ALLOWED_UPDATE_FIELDS) { | ||||||||||||||||||||||||
| if (key in body) { | ||||||||||||||||||||||||
| sanitizedBody[key] = body[key] | ||||||||||||||||||||||||
| if (Object.prototype.hasOwnProperty.call(payload, key)) { | ||||||||||||||||||||||||
| sanitizedBody[key] = payload[key] | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| if (Object.keys(sanitizedBody).length === 0) { | ||||||||||||||||||||||||
| return NextResponse.json({ error: 'No updatable fields provided' }, { status: 400 }) | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| if ('studentId' in sanitizedBody) { | ||||||||||||||||||||||||
| const studentId = sanitizedBody.studentId | ||||||||||||||||||||||||
| if (typeof studentId !== 'string' || !mongoose.Types.ObjectId.isValid(studentId)) { | ||||||||||||||||||||||||
| return NextResponse.json({ error: 'studentId must be a valid id' }, { status: 400 }) | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| if ('studentName' in sanitizedBody && typeof sanitizedBody.studentName !== 'string') { | ||||||||||||||||||||||||
| return NextResponse.json({ error: 'studentName must be a string' }, { status: 400 }) | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| if ('subject' in sanitizedBody && typeof sanitizedBody.subject !== 'string') { | ||||||||||||||||||||||||
| return NextResponse.json({ error: 'subject must be a string' }, { status: 400 }) | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| if ('term' in sanitizedBody && typeof sanitizedBody.term !== 'string') { | ||||||||||||||||||||||||
| return NextResponse.json({ error: 'term must be a string' }, { status: 400 }) | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| if ('marks' in sanitizedBody) { | ||||||||||||||||||||||||
| const marks = sanitizedBody.marks | ||||||||||||||||||||||||
| if (typeof marks !== 'number' || Number.isNaN(marks) || marks < 0) { | ||||||||||||||||||||||||
| return NextResponse.json({ error: 'marks must be a number >= 0' }, { status: 400 }) | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| if ('maxMarks' in sanitizedBody) { | ||||||||||||||||||||||||
| const maxMarks = sanitizedBody.maxMarks | ||||||||||||||||||||||||
| if (typeof maxMarks !== 'number' || Number.isNaN(maxMarks) || maxMarks < 1) { | ||||||||||||||||||||||||
| return NextResponse.json({ error: 'maxMarks must be a number >= 1' }, { status: 400 }) | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| await connectDB() | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| const existingGrade = await Grade.findOne({ _id: id, teacherId: userId }) | ||||||||||||||||||||||||
| if (!existingGrade) return NextResponse.json({ error: 'Not found' }, { status: 404 }) | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| const nextMarks = typeof sanitizedBody.marks === 'number' ? sanitizedBody.marks : existingGrade.marks | ||||||||||||||||||||||||
| const nextMaxMarks = typeof sanitizedBody.maxMarks === 'number' ? sanitizedBody.maxMarks : (existingGrade.maxMarks ?? 100) | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| if (nextMarks > nextMaxMarks) { | ||||||||||||||||||||||||
| return NextResponse.json({ error: 'marks must be less than or equal to maxMarks' }, { status: 400 }) | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| const grade = await Grade.findOneAndUpdate( | ||||||||||||||||||||||||
| { _id: id }, | ||||||||||||||||||||||||
| sanitizedBody, | ||||||||||||||||||||||||
| { new: true } | ||||||||||||||||||||||||
| { _id: id, teacherId: userId }, | ||||||||||||||||||||||||
| { | ||||||||||||||||||||||||
| ...sanitizedBody, | ||||||||||||||||||||||||
| maxMarks: nextMaxMarks, | ||||||||||||||||||||||||
| grade: calculateLetterGrade(nextMarks, nextMaxMarks), | ||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||
| { new: true, runValidators: true, context: 'query' } | ||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||
| if (!grade) return NextResponse.json({ error: 'Not found' }, { status: 404 }) | ||||||||||||||||||||||||
| return NextResponse.json(grade) | ||||||||||||||||||||||||
| } catch (error) { | ||||||||||||||||||||||||
| if (error instanceof Error) { | ||||||||||||||||||||||||
| console.error('PUT /api/grades/[id] error:', error.message) | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| if ((error as { code?: number }).code === 11000) { | ||||||||||||||||||||||||
| return NextResponse.json({ error: 'A grade for this student, subject, and term already exists' }, { status: 409 }) | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| return NextResponse.json({ error: 'Internal server error' }, { status: 500 }) | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
@@ -56,7 +119,7 @@ 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 }) | ||||||||||||||||||||||||
|
Comment on lines
120
to
+122
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. 🧩 Analysis chain🏁 Script executed: # First, let's examine the DELETE route handler
cat -n app/api/grades/[id]/route.ts | head -150Repository: JavaScript-Mastery-Pro/discord-challenge Length of output: 6322 Add ObjectId validation to DELETE endpoint before querying Mongoose. The Proposed fix try {
const { id } = await ctx.params
+ if (!mongoose.Types.ObjectId.isValid(id)) {
+ return NextResponse.json({ error: 'Grade not found' }, { status: 404 })
+ }
+
await connectDB()
const deleted = await Grade.findOneAndDelete({ _id: id, teacherId: userId })📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| if (!deleted) { | ||||||||||||||||||||||||
| return NextResponse.json({ error: 'Grade not found' }, { status: 404 }) | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
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.
🧩 Analysis chain
🏁 Script executed:
Repository: JavaScript-Mastery-Pro/discord-challenge
Length of output: 6322
🏁 Script executed:
Repository: JavaScript-Mastery-Pro/discord-challenge
Length of output: 212
🏁 Script executed:
Repository: JavaScript-Mastery-Pro/discord-challenge
Length of output: 1302
🏁 Script executed:
Repository: JavaScript-Mastery-Pro/discord-challenge
Length of output: 3784
Verify student ownership before accepting
studentIdupdates.The
studentIdfield is shape-validated (lines 51–56) but lacks ownership verification. A teacher can update any grade to reference a student they don't own, breaking data isolation. Before accepting thestudentIdvalue, query the Student model to confirm it belongs to the authenticated teacher, and derivestudentNameserver-side to prevent spoofed associations.Suggested direction
import mongoose from 'mongoose' import { connectDB } from '@/lib/mongodb' import { Grade } from '@/models/Grade' +import { Student } from '@/models/Student' import { calculateLetterGrade } from '@/lib/grading'Also applies to: 93-101
🤖 Prompt for AI Agents