diff --git a/artifacts/api-server/src/routes/service-orders.ts b/artifacts/api-server/src/routes/service-orders.ts index 6fd20ba2..701567b2 100644 --- a/artifacts/api-server/src/routes/service-orders.ts +++ b/artifacts/api-server/src/routes/service-orders.ts @@ -493,23 +493,27 @@ async function authorizeAndDeleteOrder( const isOwner = existing.userId === actorId; const isTerminal = existing.status === "completed" || existing.status === "cancelled"; - // Receivers can only delete orders that appear in the incoming queue. - // Terminal orders (completed/cancelled) are not theirs to clean up. - const isIncomingQueue = - existing.status === "pending" || - existing.status === "received" || - existing.status === "preparing"; + // Receivers may only delete what they actually see on /orders/incoming + // (see GET /orders/incoming above): an unclaimed pending order, or one + // they have already claimed and is still active. Anything else — another + // receiver's claimed order, or any terminal order — is out of scope and + // must 403 even at the API layer. + const isUnclaimedPending = + existing.status === "pending" && existing.assignedTo === null; + const isMyActive = + existing.assignedTo === actorId && + (existing.status === "received" || existing.status === "preparing"); + const isInMyIncomingView = isUnclaimedPending || isMyActive; // Allowed when: // - admin (any order, any status) // - owner of a terminal order (completed/cancelled) - // - holder of orders.receive permission, but only for orders currently in - // the incoming queue (pending/received/preparing) — matches the page - // they can see and prevents privilege escalation against terminal orders. + // - holder of orders.receive permission, but only for orders that + // appear in their own incoming view (mirrors GET /orders/incoming). const allowed = flags.admin || (isOwner && isTerminal) || - (flags.receiver && isIncomingQueue); + (flags.receiver && isInMyIncomingView); if (!allowed) return { ok: false, reason: "forbidden" }; await db diff --git a/artifacts/api-server/tests/service-orders.test.mjs b/artifacts/api-server/tests/service-orders.test.mjs index 0469ef0b..00b6c07a 100644 --- a/artifacts/api-server/tests/service-orders.test.mjs +++ b/artifacts/api-server/tests/service-orders.test.mjs @@ -364,7 +364,7 @@ test("DELETE /orders/:id — owner can delete completed/cancelled, admin can del assert.equal(res.status, 401); }); -test("DELETE /orders/:id — receivers can delete any incoming-queue order (pending/received/preparing); non-receiver, non-owner still 403", async () => { +test("DELETE /orders/:id — receivers can delete orders visible in their incoming view (unclaimed pending or own active); non-receiver, non-owner still 403", async () => { const owner = await createUser("recvDelOwn", "user"); const recv = await createUser("recvDelRecv", "order_receiver"); const stranger = await createUser("recvDelStr", "user"); @@ -392,17 +392,32 @@ test("DELETE /orders/:id — receivers can delete any incoming-queue order (pend res = await call(cr, "DELETE", `/orders/${o2.id}`); assert.equal(res.status, 204); - // 3) Receiver can delete a preparing order assigned to a different receiver. - const otherRecv = await createUser("recvDelOther", "order_receiver"); - const cor = await login(otherRecv.username); + // 3) Receiver can delete an order they have claimed and moved to preparing + // (their own active order — visible in their incoming view). const o3 = await place(); - res = await call(cor, "PATCH", `/orders/${o3.id}/confirm-receipt`); + res = await call(cr, "PATCH", `/orders/${o3.id}/confirm-receipt`); assert.equal(res.status, 200); - res = await call(cor, "PATCH", `/orders/${o3.id}/status`, { status: "preparing" }); + res = await call(cr, "PATCH", `/orders/${o3.id}/status`, { status: "preparing" }); assert.equal(res.status, 200); res = await call(cr, "DELETE", `/orders/${o3.id}`); assert.equal(res.status, 204); + // 3b) Receiver may NOT delete an order claimed by a *different* receiver — + // it isn't in their incoming view, so the API must reject it (403). + const otherRecv = await createUser("recvDelOther", "order_receiver"); + const cor = await login(otherRecv.username); + const o3b = await place(); + res = await call(cor, "PATCH", `/orders/${o3b.id}/confirm-receipt`); + assert.equal(res.status, 200); + res = await call(cor, "PATCH", `/orders/${o3b.id}/status`, { status: "preparing" }); + assert.equal(res.status, 200); + res = await call(cr, "DELETE", `/orders/${o3b.id}`); + assert.equal( + res.status, + 403, + "receiver should NOT delete another receiver's claimed order", + ); + // 4) Non-receiver, non-owner still gets 403 on any state. const o4 = await place(); res = await call(cs, "DELETE", `/orders/${o4.id}`);