Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
139 changes: 64 additions & 75 deletions app/api/profile/route.ts
Original file line number Diff line number Diff line change
@@ -1,114 +1,103 @@
import { auth, currentUser } from '@clerk/nextjs/server'
import { NextRequest, NextResponse } from 'next/server'
import { connectDB } from '@/lib/mongodb'
import { Teacher } from '@/models/Teacher'
import { auth, currentUser } from "@clerk/nextjs/server";
import { NextRequest, NextResponse } from "next/server";
import { connectDB } from "@/lib/mongodb";
import { Teacher } from "@/models/Teacher";
import { updateSchema } from "@/lib/schemas";

export async function GET(req: NextRequest) {
const { searchParams } = new URL(req.url)
const queryUserId = searchParams.get('userId')
const { searchParams } = new URL(req.url);
const queryUserId = searchParams.get("userId");

let userId: string | null = queryUserId
let userId: string | null = queryUserId;
if (!userId) {
const session = await auth()
userId = session.userId
const session = await auth();
userId = session.userId;
}
if (!userId) return NextResponse.json({ error: 'Unauthorized' }, { status: 401 })
if (!userId)
return NextResponse.json({ error: "Unauthorized" }, { status: 401 });
Comment on lines +11 to +17

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

GET accepts any userId from the query string with no authorization check.

queryUserId from ?userId= completely bypasses the session check — any authenticated (or even unauthenticated, since the fallback to auth() only runs when the query param is missing) caller can fetch another user's teacher profile by passing their Clerk id. Given this route returns the full Teacher document including email, that's a PII exposure / IDOR.

At minimum, require auth() first and then either (a) only allow the query override when the caller is an admin, or (b) verify queryUserId === session.userId.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/api/profile/route.ts` around lines 11 - 17, The handler currently trusts
queryUserId and only calls auth() if the query param is missing, allowing IDOR;
change the flow to call auth() first (invoke auth() and obtain session.userId
and role), then set userId = session.userId and only allow overriding with
queryUserId when the authenticated user is an admin (e.g., session.role or
session.isAdmin) or when queryUserId === session.userId; update the
authorization check around auth(), queryUserId, and the NextResponse.json
unauthorized return to enforce this invariant before loading the Teacher
document.


try {
await connectDB()
let teacher = await Teacher.findOne({ clerkId: userId }).lean()
await connectDB();
let teacher = await Teacher.findOne({ clerkId: userId }).lean();

if (!teacher) {
const clerkUser = await currentUser()
const clerkUser = await currentUser();
const created = await Teacher.create({
clerkId: userId,
name: clerkUser?.fullName ?? '',
email: clerkUser?.emailAddresses[0]?.emailAddress ?? '',
department: '',
name: clerkUser?.fullName ?? "",
email: clerkUser?.emailAddresses[0]?.emailAddress ?? "",
department: "",
subjects: [],
})
teacher = created.toObject()
});
teacher = created.toObject();
}
Comment on lines 23 to 33

Copy link
Copy Markdown

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

🌐 Web query:

Does Clerk's currentUser() from @clerk/nextjs/server return null when unauthenticated, and is emailAddresses always an array on a non-null User?

💡 Result:

Yes, Clerk's currentUser from @clerk/nextjs/server returns null when the user is unauthenticated. Yes, on a non-null User object returned by currentUser, emailAddresses is always an array.

Citations:


Check for unauthenticated user before creating Teacher record.

The code silently creates an invalid Teacher with empty name and email when currentUser() returns null (which happens when the user is unauthenticated). Instead of relying on optional chaining fallbacks, add an explicit null check and fail fast:

Proposed fix
     if (!teacher) {
       const clerkUser = await currentUser();
+      if (!clerkUser) {
+        return NextResponse.json(
+          { error: "Unable to load Clerk user" },
+          { status: 500 },
+        );
+      }
       const created = await Teacher.create({
         clerkId: userId,
-        name: clerkUser?.fullName ?? "",
-        email: clerkUser?.emailAddresses[0]?.emailAddress ?? "",
+        name: clerkUser.fullName ?? "",
+        email: clerkUser.emailAddresses[0]?.emailAddress ?? "",
         department: "",
         subjects: [],
       });
📝 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 (!teacher) {
const clerkUser = await currentUser()
const clerkUser = await currentUser();
const created = await Teacher.create({
clerkId: userId,
name: clerkUser?.fullName ?? '',
email: clerkUser?.emailAddresses[0]?.emailAddress ?? '',
department: '',
name: clerkUser?.fullName ?? "",
email: clerkUser?.emailAddresses[0]?.emailAddress ?? "",
department: "",
subjects: [],
})
teacher = created.toObject()
});
teacher = created.toObject();
}
if (!teacher) {
const clerkUser = await currentUser();
if (!clerkUser) {
return NextResponse.json(
{ error: "Unable to load Clerk user" },
{ status: 500 },
);
}
const created = await Teacher.create({
clerkId: userId,
name: clerkUser.fullName ?? "",
email: clerkUser.emailAddresses[0]?.emailAddress ?? "",
department: "",
subjects: [],
});
teacher = created.toObject();
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/api/profile/route.ts` around lines 23 - 33, The current code creates a
Teacher record even when currentUser() returns null, leading to empty
name/email; update the logic in the route handler around currentUser(),
Teacher.create and the teacher assignment to explicitly check for an
authenticated clerkUser (e.g., const clerkUser = await currentUser(); if
(!clerkUser) return/throw a 401 error) before calling Teacher.create, and only
use clerkUser.fullName and clerkUser.emailAddresses after that check so you fail
fast instead of creating an invalid Teacher document.


return NextResponse.json(teacher)
return NextResponse.json(teacher);
} catch (error) {
console.error('GET /api/profile error:', error instanceof Error ? error.message : error)
return NextResponse.json({ error: 'Internal server error' }, { status: 500 })
console.error(
"GET /api/profile error:",
error instanceof Error ? error.message : error,
);
return NextResponse.json(
{ error: "Internal server error" },
{ status: 500 },
);
}
}

export async function PUT(req: NextRequest) {
const { userId } = await auth()
if (!userId) return NextResponse.json({ error: 'Unauthorized' }, { status: 401 })
const { userId } = await auth();
if (!userId)
return NextResponse.json({ error: "Unauthorized" }, { status: 401 });

try {
await connectDB()
let body
await connectDB();

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

const { name, department, subjects, phone, bio, academicHistory } = body

// Validate input
if (typeof name !== 'string' || !name.trim()) {
return NextResponse.json({ error: 'name must be a non-empty string' }, { status: 400 })
}
if (department !== undefined && typeof department !== 'string') {
return NextResponse.json({ error: 'department must be a string' }, { status: 400 })
}
if (!Array.isArray(subjects) || !subjects.every((s) => typeof s === 'string')) {
return NextResponse.json({ error: 'subjects must be an array of strings' }, { status: 400 })
}
if (phone !== undefined && typeof phone !== 'string') {
return NextResponse.json({ error: 'phone must be a string' }, { status: 400 })
}
if (bio !== undefined && typeof bio !== 'string') {
return NextResponse.json({ error: 'bio must be a string' }, { status: 400 })
}
if (academicHistory !== undefined) {
if (
!Array.isArray(academicHistory) ||
academicHistory.length > 20 ||
!academicHistory.every(
(entry: unknown) =>
entry !== null &&
typeof entry === 'object' &&
typeof (entry as Record<string, unknown>).year === 'string' &&
typeof (entry as Record<string, unknown>).title === 'string',
)
) {
return NextResponse.json(
{ error: 'academicHistory must be an array of objects with string year and title (max 20 items)' },
{ status: 400 },
)
}
const { name, department, subjects, phone, bio, academicHistory } = body;

const parsed = updateSchema.safeParse(body);

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

const updatePayload: Record<string, unknown> = { name, subjects }
if (department !== undefined) updatePayload.department = department
if (phone !== undefined) updatePayload.phone = phone
if (bio !== undefined) updatePayload.bio = bio
if (academicHistory !== undefined) updatePayload.academicHistory = academicHistory
const data = parsed.data;

const updatePayload = Object.fromEntries(
Object.entries(data).filter(([_, v]) => v !== undefined),
);
Comment on lines +66 to +81

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Dead destructure + only undefined is filtered (empty strings still overwrite).

Two things here:

  1. Line 66 destructures name, department, subjects, phone, bio, academicHistory from body, but none of those locals are used afterward — parsed.data is the source of truth. Leftover from the old manual-validation code; please remove.
  2. The Object.fromEntries(... v !== undefined) filter only drops undefined. Because Zod applies schema defaults/strip but preserves present keys, if the client sends e.g. phone: "" (which is valid per the schema), the $set will overwrite the stored phone with an empty string. That's arguably the point (clear a field), but combined with saveHistory spreading the entire profile into the body, it means a full-profile PUT can unintentionally re-write every field with whatever is currently in client state. Worth confirming this is the intended behavior; otherwise also filter empty strings, or have the client send only the fields it means to change.
Proposed cleanup
-    const { name, department, subjects, phone, bio, academicHistory } = body;
-
     const parsed = updateSchema.safeParse(body);
 
     if (!parsed.success) {
       return NextResponse.json(
         { error: parsed.error.flatten() },
         { status: 400 },
       );
     }
 
     const data = parsed.data;
-
     const updatePayload = Object.fromEntries(
-      Object.entries(data).filter(([_, v]) => v !== undefined),
+      Object.entries(data).filter(([, v]) => v !== undefined),
     );
📝 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
const { name, department, subjects, phone, bio, academicHistory } = body;
const parsed = updateSchema.safeParse(body);
if (!parsed.success) {
return NextResponse.json(
{ error: parsed.error.flatten() },
{ status: 400 },
);
}
const updatePayload: Record<string, unknown> = { name, subjects }
if (department !== undefined) updatePayload.department = department
if (phone !== undefined) updatePayload.phone = phone
if (bio !== undefined) updatePayload.bio = bio
if (academicHistory !== undefined) updatePayload.academicHistory = academicHistory
const data = parsed.data;
const updatePayload = Object.fromEntries(
Object.entries(data).filter(([_, v]) => v !== undefined),
);
const parsed = updateSchema.safeParse(body);
if (!parsed.success) {
return NextResponse.json(
{ error: parsed.error.flatten() },
{ status: 400 },
);
}
const data = parsed.data;
const updatePayload = Object.fromEntries(
Object.entries(data).filter(([, v]) => v !== undefined),
);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/api/profile/route.ts` around lines 66 - 81, Remove the dead destructure
of name, department, subjects, phone, bio, academicHistory (it’s unused;
parsed.data is the source) and update the updatePayload creation (where you
build Object.fromEntries from Object.entries(data).filter(...)) to also exclude
empty-string values so fields like phone: "" don’t unintentionally overwrite
stored values; e.g. replace the filter predicate with one that returns false for
v === undefined || v === "" (and adjust if you need to treat arrays/objects
differently), keeping updateSchema.safeParse and parsed.data as the single
source of truth and ensuring saveHistory (where the profile is later spread)
won’t cause unintended full-profile rewrites.


const teacher = await Teacher.findOneAndUpdate(
{ clerkId: userId },
{ $set: updatePayload },
{ new: true }
)
{ new: true },
);

if (!teacher) {
return NextResponse.json({ error: 'Teacher not found' }, { status: 404 })
return NextResponse.json({ error: "Teacher not found" }, { status: 404 });
}

return NextResponse.json(teacher)
return NextResponse.json(teacher);
} catch (error) {
if (error instanceof Error) {
console.error('PUT /api/profile error:', error.message)
console.error("PUT /api/profile error:", error.message);
}
return NextResponse.json({ error: 'Internal server error' }, { status: 500 })
return NextResponse.json(
{ error: "Internal server error" },
{ status: 500 },
);
}
}
Loading
Loading