Skip to content

Require auth for order status and message read routes#367

Open
ayushshrivastv wants to merge 3 commits intoshopstr-eng:mainfrom
ayushshrivastv:fix-db-message-mutation-auth
Open

Require auth for order status and message read routes#367
ayushshrivastv wants to merge 3 commits intoshopstr-eng:mainfrom
ayushshrivastv:fix-db-message-mutation-auth

Conversation

@ayushshrivastv
Copy link
Copy Markdown
Contributor

@ayushshrivastv ayushshrivastv commented Apr 7, 2026

Unauthenticated order status update API enables tampering

import type { NextApiRequest, NextApiResponse } from "next";
import { updateOrderStatus } from "@/utils/db/db-service";
export default async function handler(
req: NextApiRequest,
res: NextApiResponse
) {
if (req.method !== "POST") {
return res.status(405).json({ error: "Method not allowed" });
}
const { orderId, status, messageId } = req.body;
if (!orderId || !status) {
return res
.status(400)
.json({ error: "Missing required fields: orderId, status" });
}
const validStatuses = [
"pending",
"confirmed",
"shipped",
"completed",
"canceled",
];
if (!validStatuses.includes(status)) {
return res.status(400).json({
error: `Invalid status. Must be one of: ${validStatuses.join(", ")}`,
});
}
try {
await updateOrderStatus(orderId, status, messageId);
return res.status(200).json({ success: true, orderId, status });

still accepts orderId, status, and messageId with no auth check, while

import type { NextApiRequest, NextApiResponse } from "next";
import { markMessagesAsRead } from "@/utils/db/db-service";
export default async function handler(
req: NextApiRequest,
res: NextApiResponse
) {
if (req.method !== "POST") {
return res.status(405).json({ error: "Method not allowed" });
}
try {
const { messageIds } = req.body;
if (!Array.isArray(messageIds)) {
return res.status(400).json({ error: "messageIds must be an array" });
}
await markMessagesAsRead(messageIds);
res.status(200).json({ success: true });

still accepts arbitrary messageIds with no auth check. the underlying db mutations in https://github.com/shopstr-eng/shopstr/blob/main/utils/db/db-service.ts#L856-L918 also still update message_events by id or order_id only, with no ownership validation

@vercel
Copy link
Copy Markdown

vercel bot commented Apr 7, 2026

@ayushshrivastv is attempting to deploy a commit to the shopstr-eng Team on Vercel.

A member of the Team first needs to authorize it.

@Aryan0699
Copy link
Copy Markdown
Contributor

Hi @ayushshrivastv
One small thing in db-service.ts , right now the check only verifies whether the user is part of the order (sender or recipient), but not whether they are the buyer or seller. Because of this, a buyer could also update the order status to something like "shipped", which should normally be seller-only.
I think we can fix this by adding a role check before calling the DB, in the route handler in update-order-status.ts.

@ayushshrivastv
Copy link
Copy Markdown
Contributor Author

@Aryan0699, thanks for pointing out the role based checks! I’ve included the fix here.

@Aryan0699
Copy link
Copy Markdown
Contributor

Aryan0699 commented Apr 9, 2026

@Aryan0699, thanks for pointing out the role based checks! I’ve included the fix here.

The fix looks perfect🙂 . A buyer can cancel but not ship. A seller manages the confirmation and pending is shared which makes sense as any party might need to set the intial state.
Just a few small changes are suggested !!

@Aryan0699
Copy link
Copy Markdown
Contributor

Aryan0699 commented Apr 10, 2026

@ayushshrivastv would be great if you address the review comments when you get a chance.

@ayushshrivastv
Copy link
Copy Markdown
Contributor Author

A buyer can cancel but not ship. A seller manages the confirmation and pending is shared which makes sense as any party might need to set the intial state. Just a few small changes are suggested !!

that is the intended behavior here, and this pr already enforces that same role split. buyer can cancel but not ship, seller manages confirmed, shipped, and completed, and pending is shared since either side may need to set the initial state.

https://github.com/ayushshrivastv/shopstr/blob/66ee961713e405a8e94ad2cb1afc934123446449/pages/api/db/update-order-status.ts#L8-L33

@Aryan0699
Copy link
Copy Markdown
Contributor

Yes, that part looks correct 👍 I was just referencing it. The pending review comments I mentioned are about the other suggestions I added above.

@ayushshrivastv
Copy link
Copy Markdown
Contributor Author

Yes, that part looks correct 👍 I was just referencing it. The pending review comments I mentioned are about the other suggestions I added above.

Can you elaborate?

@Aryan0699
Copy link
Copy Markdown
Contributor

Aryan0699 commented Apr 10, 2026

image

These reviews present just above

@Aryan0699
Copy link
Copy Markdown
Contributor

Aren't they visible ?

@ayushshrivastv
Copy link
Copy Markdown
Contributor Author

Aren't they visible ?

Actually, it’s not visible on my side!

Aryan0699

This comment was marked as resolved.

@ayushshrivastv
Copy link
Copy Markdown
Contributor Author

OK, now it's visible.

@Aryan0699
Copy link
Copy Markdown
Contributor

Perfect thanks 👍 Might be some glitch !!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants