From 177b8fde7f8587c10fb41019884ede22b293967a Mon Sep 17 00:00:00 2001 From: Arpit Gupta Date: Mon, 30 Mar 2026 21:29:19 +0530 Subject: [PATCH 1/9] feat: add delete chats endpoint with auth and cascade delete --- app/api/chats/route.ts | 18 +++ lib/chats/__tests__/deleteChatHandler.test.ts | 79 +++++++++ .../__tests__/validateDeleteChatBody.test.ts | 152 ++++++++++++++++++ lib/chats/deleteChatHandler.ts | 46 ++++++ lib/chats/validateDeleteChatBody.ts | 84 ++++++++++ lib/supabase/rooms/deleteRoomWithRelations.ts | 70 ++++++++ 6 files changed, 449 insertions(+) create mode 100644 lib/chats/__tests__/deleteChatHandler.test.ts create mode 100644 lib/chats/__tests__/validateDeleteChatBody.test.ts create mode 100644 lib/chats/deleteChatHandler.ts create mode 100644 lib/chats/validateDeleteChatBody.ts create mode 100644 lib/supabase/rooms/deleteRoomWithRelations.ts diff --git a/app/api/chats/route.ts b/app/api/chats/route.ts index 3dac0d5e..722acc3d 100644 --- a/app/api/chats/route.ts +++ b/app/api/chats/route.ts @@ -4,6 +4,7 @@ import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; import { createChatHandler } from "@/lib/chats/createChatHandler"; import { getChatsHandler } from "@/lib/chats/getChatsHandler"; import { updateChatHandler } from "@/lib/chats/updateChatHandler"; +import { deleteChatHandler } from "@/lib/chats/deleteChatHandler"; /** * OPTIONS handler for CORS preflight requests. @@ -72,3 +73,20 @@ export async function POST(request: NextRequest): Promise { export async function PATCH(request: NextRequest): Promise { return updateChatHandler(request); } + +/** + * DELETE /api/chats + * + * Delete a chat room and related records. + * + * Authentication: x-api-key header or Authorization Bearer token required. + * + * Body parameters: + * - chatId (required): UUID of the chat room to delete + * + * @param request - The request object + * @returns A NextResponse with deletion result or an error + */ +export async function DELETE(request: NextRequest): Promise { + return deleteChatHandler(request); +} diff --git a/lib/chats/__tests__/deleteChatHandler.test.ts b/lib/chats/__tests__/deleteChatHandler.test.ts new file mode 100644 index 00000000..50991860 --- /dev/null +++ b/lib/chats/__tests__/deleteChatHandler.test.ts @@ -0,0 +1,79 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { NextRequest, NextResponse } from "next/server"; +import { deleteChatHandler } from "../deleteChatHandler"; +import { validateDeleteChatBody } from "../validateDeleteChatBody"; +import { deleteRoomWithRelations } from "@/lib/supabase/rooms/deleteRoomWithRelations"; + +vi.mock("@/lib/networking/getCorsHeaders", () => ({ + getCorsHeaders: vi.fn(() => ({ "Access-Control-Allow-Origin": "*" })), +})); + +vi.mock("@/lib/chats/validateDeleteChatBody", () => ({ + validateDeleteChatBody: vi.fn(), +})); + +vi.mock("@/lib/supabase/rooms/deleteRoomWithRelations", () => ({ + deleteRoomWithRelations: vi.fn(), +})); + +describe("deleteChatHandler", () => { + const chatId = "123e4567-e89b-12d3-a456-426614174000"; + + const request = new NextRequest("http://localhost/api/chats", { + method: "DELETE", + headers: { "Content-Type": "application/json", "x-api-key": "test-key" }, + body: JSON.stringify({ chatId }), + }); + + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("deletes chat and returns success response", async () => { + vi.mocked(validateDeleteChatBody).mockResolvedValue({ chatId }); + vi.mocked(deleteRoomWithRelations).mockResolvedValue(true); + + const response = await deleteChatHandler(request); + + expect(response.status).toBe(200); + const body = await response.json(); + expect(body).toEqual({ + status: "success", + chatId, + message: "Chat deleted successfully", + }); + expect(deleteRoomWithRelations).toHaveBeenCalledWith(chatId); + }); + + it("returns validation response when request is invalid", async () => { + vi.mocked(validateDeleteChatBody).mockResolvedValue( + NextResponse.json({ status: "error", error: "chatId is required" }, { status: 400 }), + ); + + const response = await deleteChatHandler(request); + expect(response.status).toBe(400); + expect(deleteRoomWithRelations).not.toHaveBeenCalled(); + }); + + it("returns 500 when deletion fails", async () => { + vi.mocked(validateDeleteChatBody).mockResolvedValue({ chatId }); + vi.mocked(deleteRoomWithRelations).mockResolvedValue(false); + + const response = await deleteChatHandler(request); + expect(response.status).toBe(500); + const body = await response.json(); + expect(body.status).toBe("error"); + expect(body.error).toBe("Failed to delete chat"); + }); + + it("returns 500 when deletion throws", async () => { + vi.mocked(validateDeleteChatBody).mockResolvedValue({ chatId }); + vi.mocked(deleteRoomWithRelations).mockRejectedValue(new Error("Database down")); + + const response = await deleteChatHandler(request); + expect(response.status).toBe(500); + const body = await response.json(); + expect(body.status).toBe("error"); + expect(body.error).toBe("Database down"); + }); +}); diff --git a/lib/chats/__tests__/validateDeleteChatBody.test.ts b/lib/chats/__tests__/validateDeleteChatBody.test.ts new file mode 100644 index 00000000..4d9c2edf --- /dev/null +++ b/lib/chats/__tests__/validateDeleteChatBody.test.ts @@ -0,0 +1,152 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { NextRequest, NextResponse } from "next/server"; +import { validateDeleteChatBody } from "../validateDeleteChatBody"; +import { validateAuthContext } from "@/lib/auth/validateAuthContext"; +import selectRoom from "@/lib/supabase/rooms/selectRoom"; +import { buildGetChatsParams } from "@/lib/chats/buildGetChatsParams"; + +vi.mock("@/lib/networking/getCorsHeaders", () => ({ + getCorsHeaders: vi.fn(() => ({ "Access-Control-Allow-Origin": "*" })), +})); + +vi.mock("@/lib/auth/validateAuthContext", () => ({ + validateAuthContext: vi.fn(), +})); + +vi.mock("@/lib/supabase/rooms/selectRoom", () => ({ + default: vi.fn(), +})); + +vi.mock("@/lib/chats/buildGetChatsParams", () => ({ + buildGetChatsParams: vi.fn(), +})); + +describe("validateDeleteChatBody", () => { + const chatId = "123e4567-e89b-12d3-a456-426614174000"; + const accountId = "123e4567-e89b-12d3-a456-426614174001"; + + const createRequest = (body: object | string) => + new NextRequest("http://localhost/api/chats", { + method: "DELETE", + headers: { "Content-Type": "application/json", "x-api-key": "test-key" }, + body: typeof body === "string" ? body : JSON.stringify(body), + }); + + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("returns validated chatId when caller has access", async () => { + vi.mocked(validateAuthContext).mockResolvedValue({ + accountId, + orgId: null, + authToken: "test-key", + }); + vi.mocked(selectRoom).mockResolvedValue({ + id: chatId, + account_id: accountId, + artist_id: null, + topic: "Topic", + updated_at: "2026-03-30T00:00:00Z", + }); + vi.mocked(buildGetChatsParams).mockResolvedValue({ + params: { account_ids: [accountId] }, + error: null, + }); + + const result = await validateDeleteChatBody(createRequest({ chatId })); + expect(result).toEqual({ chatId }); + }); + + it("allows admin access when account_ids is undefined", async () => { + vi.mocked(validateAuthContext).mockResolvedValue({ + accountId, + orgId: "admin-org", + authToken: "admin-key", + }); + vi.mocked(selectRoom).mockResolvedValue({ + id: chatId, + account_id: "another-account", + artist_id: null, + topic: "Topic", + updated_at: "2026-03-30T00:00:00Z", + }); + vi.mocked(buildGetChatsParams).mockResolvedValue({ + params: {}, + error: null, + }); + + const result = await validateDeleteChatBody(createRequest({ chatId })); + expect(result).toEqual({ chatId }); + }); + + it("returns 400 when chatId is missing", async () => { + const result = await validateDeleteChatBody(createRequest({})); + expect(result).toBeInstanceOf(NextResponse); + const response = result as NextResponse; + expect(response.status).toBe(400); + }); + + it("returns 400 when chatId is invalid UUID", async () => { + const result = await validateDeleteChatBody(createRequest({ chatId: "invalid" })); + expect(result).toBeInstanceOf(NextResponse); + const response = result as NextResponse; + expect(response.status).toBe(400); + }); + + it("returns auth response when authentication fails", async () => { + vi.mocked(validateAuthContext).mockResolvedValue( + NextResponse.json({ status: "error", error: "Unauthorized" }, { status: 401 }), + ); + + const result = await validateDeleteChatBody(createRequest({ chatId })); + expect(result).toBeInstanceOf(NextResponse); + const response = result as NextResponse; + expect(response.status).toBe(401); + }); + + it("returns 404 when chat is not found", async () => { + vi.mocked(validateAuthContext).mockResolvedValue({ + accountId, + orgId: null, + authToken: "test-key", + }); + vi.mocked(selectRoom).mockResolvedValue(null); + + const result = await validateDeleteChatBody(createRequest({ chatId })); + expect(result).toBeInstanceOf(NextResponse); + const response = result as NextResponse; + expect(response.status).toBe(404); + }); + + it("returns 403 when account cannot access chat", async () => { + vi.mocked(validateAuthContext).mockResolvedValue({ + accountId, + orgId: null, + authToken: "test-key", + }); + vi.mocked(selectRoom).mockResolvedValue({ + id: chatId, + account_id: "another-account", + artist_id: null, + topic: "Topic", + updated_at: "2026-03-30T00:00:00Z", + }); + vi.mocked(buildGetChatsParams).mockResolvedValue({ + params: { account_ids: [accountId] }, + error: null, + }); + + const result = await validateDeleteChatBody(createRequest({ chatId })); + expect(result).toBeInstanceOf(NextResponse); + const response = result as NextResponse; + expect(response.status).toBe(403); + }); + + it("returns 400 on invalid JSON body", async () => { + const result = await validateDeleteChatBody(createRequest("{invalid-json")); + expect(result).toBeInstanceOf(NextResponse); + const response = result as NextResponse; + expect(response.status).toBe(400); + }); +}); diff --git a/lib/chats/deleteChatHandler.ts b/lib/chats/deleteChatHandler.ts new file mode 100644 index 00000000..8ff23da5 --- /dev/null +++ b/lib/chats/deleteChatHandler.ts @@ -0,0 +1,46 @@ +import type { NextRequest } from "next/server"; +import { NextResponse } from "next/server"; +import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; +import { validateDeleteChatBody } from "./validateDeleteChatBody"; +import { deleteRoomWithRelations } from "@/lib/supabase/rooms/deleteRoomWithRelations"; + +/** + * Handles DELETE /api/chats - Delete a chat room and related records. + * + * @param request - The NextRequest object + * @returns NextResponse with deletion result or error + */ +export async function deleteChatHandler(request: NextRequest): Promise { + const validated = await validateDeleteChatBody(request); + if (validated instanceof NextResponse) { + return validated; + } + + const { chatId } = validated; + + try { + const deleted = await deleteRoomWithRelations(chatId); + + if (!deleted) { + return NextResponse.json( + { status: "error", error: "Failed to delete chat" }, + { status: 500, headers: getCorsHeaders() }, + ); + } + + return NextResponse.json( + { + status: "success", + chatId, + message: "Chat deleted successfully", + }, + { status: 200, headers: getCorsHeaders() }, + ); + } catch (error) { + const message = error instanceof Error ? error.message : "Server error"; + return NextResponse.json( + { status: "error", error: message }, + { status: 500, headers: getCorsHeaders() }, + ); + } +} diff --git a/lib/chats/validateDeleteChatBody.ts b/lib/chats/validateDeleteChatBody.ts new file mode 100644 index 00000000..625c3236 --- /dev/null +++ b/lib/chats/validateDeleteChatBody.ts @@ -0,0 +1,84 @@ +import type { NextRequest } from "next/server"; +import { NextResponse } from "next/server"; +import { z } from "zod"; +import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; +import { validateAuthContext } from "@/lib/auth/validateAuthContext"; +import selectRoom from "@/lib/supabase/rooms/selectRoom"; +import { buildGetChatsParams } from "./buildGetChatsParams"; + +export const deleteChatBodySchema = z.object({ + chatId: z.string().uuid("chatId must be a valid UUID"), +}); + +export type DeleteChatBody = z.infer; + +export interface ValidatedDeleteChat { + chatId: string; +} + +/** + * Validates request for DELETE /api/chats. + * Parses JSON, validates schema, authenticates, verifies room exists, and checks access. + * + * @param request - The NextRequest object + * @returns A NextResponse with an error if validation fails, or validated data if it passes + */ +export async function validateDeleteChatBody( + request: NextRequest, +): Promise { + let body: unknown; + try { + body = await request.json(); + } catch { + body = {}; + } + + const result = deleteChatBodySchema.safeParse(body); + if (!result.success) { + const firstError = result.error.issues[0]; + return NextResponse.json( + { + status: "error", + missing_fields: firstError.path, + error: firstError.message, + }, + { + status: 400, + headers: getCorsHeaders(), + }, + ); + } + + const { chatId } = result.data; + + const authResult = await validateAuthContext(request); + if (authResult instanceof NextResponse) { + return authResult; + } + + const { accountId } = authResult; + + const room = await selectRoom(chatId); + if (!room) { + return NextResponse.json( + { status: "error", error: "Chat room not found" }, + { status: 404, headers: getCorsHeaders() }, + ); + } + + const { params } = await buildGetChatsParams({ + account_id: accountId, + }); + + // If params.account_ids is undefined, it means admin access (all records) + if (params.account_ids && room.account_id) { + if (!params.account_ids.includes(room.account_id)) { + return NextResponse.json( + { status: "error", error: "Access denied to this chat" }, + { status: 403, headers: getCorsHeaders() }, + ); + } + } + + return { chatId }; +} diff --git a/lib/supabase/rooms/deleteRoomWithRelations.ts b/lib/supabase/rooms/deleteRoomWithRelations.ts new file mode 100644 index 00000000..8460e8eb --- /dev/null +++ b/lib/supabase/rooms/deleteRoomWithRelations.ts @@ -0,0 +1,70 @@ +import supabase from "../serverClient"; + +/** + * Deletes a room and related room data. + * This removes memory_emails, memories, room_reports, segment_rooms, then the room. + * + * @param roomId - The room ID to delete + * @returns True when deletion succeeds, false when any step fails + */ +export async function deleteRoomWithRelations(roomId: string): Promise { + const { data: memories, error: selectMemoriesError } = await supabase + .from("memories") + .select("id") + .eq("room_id", roomId); + + if (selectMemoriesError) { + console.error("[ERROR] deleteRoomWithRelations select memories:", selectMemoriesError); + return false; + } + + const memoryIds = (memories || []).map(memory => memory.id); + + if (memoryIds.length > 0) { + const { error: deleteMemoryEmailsError } = await supabase + .from("memory_emails") + .delete() + .in("memory", memoryIds); + + if (deleteMemoryEmailsError) { + console.error("[ERROR] deleteRoomWithRelations memory_emails:", deleteMemoryEmailsError); + return false; + } + } + + const { error: deleteMemoriesError } = await supabase + .from("memories") + .delete() + .eq("room_id", roomId); + if (deleteMemoriesError) { + console.error("[ERROR] deleteRoomWithRelations memories:", deleteMemoriesError); + return false; + } + + const { error: deleteRoomReportsError } = await supabase + .from("room_reports") + .delete() + .eq("room_id", roomId); + if (deleteRoomReportsError) { + console.error("[ERROR] deleteRoomWithRelations room_reports:", deleteRoomReportsError); + return false; + } + + const { error: deleteSegmentRoomsError } = await supabase + .from("segment_rooms") + .delete() + .eq("room_id", roomId); + if (deleteSegmentRoomsError) { + console.error("[ERROR] deleteRoomWithRelations segment_rooms:", deleteSegmentRoomsError); + return false; + } + + const { error: deleteRoomError } = await supabase.from("rooms").delete().eq("id", roomId); + + if (deleteRoomError) { + console.error("[ERROR] deleteRoomWithRelations room:", deleteRoomError); + return false; + } + + return true; +} From 16eff7e5d73b8c26744e2385e34f2c58596442a2 Mon Sep 17 00:00:00 2001 From: Arpit Gupta Date: Mon, 30 Mar 2026 21:38:55 +0530 Subject: [PATCH 2/9] refactor: use id field for delete chat payload --- app/api/chats/route.ts | 2 +- lib/chats/__tests__/deleteChatHandler.test.ts | 14 ++++----- .../__tests__/validateDeleteChatBody.test.ts | 30 +++++++++---------- lib/chats/deleteChatHandler.ts | 6 ++-- lib/chats/validateDeleteChatBody.ts | 10 +++---- 5 files changed, 31 insertions(+), 31 deletions(-) diff --git a/app/api/chats/route.ts b/app/api/chats/route.ts index 722acc3d..3a307095 100644 --- a/app/api/chats/route.ts +++ b/app/api/chats/route.ts @@ -82,7 +82,7 @@ export async function PATCH(request: NextRequest): Promise { * Authentication: x-api-key header or Authorization Bearer token required. * * Body parameters: - * - chatId (required): UUID of the chat room to delete + * - id (required): UUID of the chat room to delete * * @param request - The request object * @returns A NextResponse with deletion result or an error diff --git a/lib/chats/__tests__/deleteChatHandler.test.ts b/lib/chats/__tests__/deleteChatHandler.test.ts index 50991860..1d0ccb76 100644 --- a/lib/chats/__tests__/deleteChatHandler.test.ts +++ b/lib/chats/__tests__/deleteChatHandler.test.ts @@ -17,12 +17,12 @@ vi.mock("@/lib/supabase/rooms/deleteRoomWithRelations", () => ({ })); describe("deleteChatHandler", () => { - const chatId = "123e4567-e89b-12d3-a456-426614174000"; + const id = "123e4567-e89b-12d3-a456-426614174000"; const request = new NextRequest("http://localhost/api/chats", { method: "DELETE", headers: { "Content-Type": "application/json", "x-api-key": "test-key" }, - body: JSON.stringify({ chatId }), + body: JSON.stringify({ id }), }); beforeEach(() => { @@ -30,7 +30,7 @@ describe("deleteChatHandler", () => { }); it("deletes chat and returns success response", async () => { - vi.mocked(validateDeleteChatBody).mockResolvedValue({ chatId }); + vi.mocked(validateDeleteChatBody).mockResolvedValue({ id }); vi.mocked(deleteRoomWithRelations).mockResolvedValue(true); const response = await deleteChatHandler(request); @@ -39,10 +39,10 @@ describe("deleteChatHandler", () => { const body = await response.json(); expect(body).toEqual({ status: "success", - chatId, + id, message: "Chat deleted successfully", }); - expect(deleteRoomWithRelations).toHaveBeenCalledWith(chatId); + expect(deleteRoomWithRelations).toHaveBeenCalledWith(id); }); it("returns validation response when request is invalid", async () => { @@ -56,7 +56,7 @@ describe("deleteChatHandler", () => { }); it("returns 500 when deletion fails", async () => { - vi.mocked(validateDeleteChatBody).mockResolvedValue({ chatId }); + vi.mocked(validateDeleteChatBody).mockResolvedValue({ id }); vi.mocked(deleteRoomWithRelations).mockResolvedValue(false); const response = await deleteChatHandler(request); @@ -67,7 +67,7 @@ describe("deleteChatHandler", () => { }); it("returns 500 when deletion throws", async () => { - vi.mocked(validateDeleteChatBody).mockResolvedValue({ chatId }); + vi.mocked(validateDeleteChatBody).mockResolvedValue({ id }); vi.mocked(deleteRoomWithRelations).mockRejectedValue(new Error("Database down")); const response = await deleteChatHandler(request); diff --git a/lib/chats/__tests__/validateDeleteChatBody.test.ts b/lib/chats/__tests__/validateDeleteChatBody.test.ts index 4d9c2edf..700fd104 100644 --- a/lib/chats/__tests__/validateDeleteChatBody.test.ts +++ b/lib/chats/__tests__/validateDeleteChatBody.test.ts @@ -22,7 +22,7 @@ vi.mock("@/lib/chats/buildGetChatsParams", () => ({ })); describe("validateDeleteChatBody", () => { - const chatId = "123e4567-e89b-12d3-a456-426614174000"; + const id = "123e4567-e89b-12d3-a456-426614174000"; const accountId = "123e4567-e89b-12d3-a456-426614174001"; const createRequest = (body: object | string) => @@ -36,14 +36,14 @@ describe("validateDeleteChatBody", () => { vi.clearAllMocks(); }); - it("returns validated chatId when caller has access", async () => { + it("returns validated id when caller has access", async () => { vi.mocked(validateAuthContext).mockResolvedValue({ accountId, orgId: null, authToken: "test-key", }); vi.mocked(selectRoom).mockResolvedValue({ - id: chatId, + id, account_id: accountId, artist_id: null, topic: "Topic", @@ -54,8 +54,8 @@ describe("validateDeleteChatBody", () => { error: null, }); - const result = await validateDeleteChatBody(createRequest({ chatId })); - expect(result).toEqual({ chatId }); + const result = await validateDeleteChatBody(createRequest({ id })); + expect(result).toEqual({ id }); }); it("allows admin access when account_ids is undefined", async () => { @@ -65,7 +65,7 @@ describe("validateDeleteChatBody", () => { authToken: "admin-key", }); vi.mocked(selectRoom).mockResolvedValue({ - id: chatId, + id, account_id: "another-account", artist_id: null, topic: "Topic", @@ -76,19 +76,19 @@ describe("validateDeleteChatBody", () => { error: null, }); - const result = await validateDeleteChatBody(createRequest({ chatId })); - expect(result).toEqual({ chatId }); + const result = await validateDeleteChatBody(createRequest({ id })); + expect(result).toEqual({ id }); }); - it("returns 400 when chatId is missing", async () => { + it("returns 400 when id is missing", async () => { const result = await validateDeleteChatBody(createRequest({})); expect(result).toBeInstanceOf(NextResponse); const response = result as NextResponse; expect(response.status).toBe(400); }); - it("returns 400 when chatId is invalid UUID", async () => { - const result = await validateDeleteChatBody(createRequest({ chatId: "invalid" })); + it("returns 400 when id is invalid UUID", async () => { + const result = await validateDeleteChatBody(createRequest({ id: "invalid" })); expect(result).toBeInstanceOf(NextResponse); const response = result as NextResponse; expect(response.status).toBe(400); @@ -99,7 +99,7 @@ describe("validateDeleteChatBody", () => { NextResponse.json({ status: "error", error: "Unauthorized" }, { status: 401 }), ); - const result = await validateDeleteChatBody(createRequest({ chatId })); + const result = await validateDeleteChatBody(createRequest({ id })); expect(result).toBeInstanceOf(NextResponse); const response = result as NextResponse; expect(response.status).toBe(401); @@ -113,7 +113,7 @@ describe("validateDeleteChatBody", () => { }); vi.mocked(selectRoom).mockResolvedValue(null); - const result = await validateDeleteChatBody(createRequest({ chatId })); + const result = await validateDeleteChatBody(createRequest({ id })); expect(result).toBeInstanceOf(NextResponse); const response = result as NextResponse; expect(response.status).toBe(404); @@ -126,7 +126,7 @@ describe("validateDeleteChatBody", () => { authToken: "test-key", }); vi.mocked(selectRoom).mockResolvedValue({ - id: chatId, + id, account_id: "another-account", artist_id: null, topic: "Topic", @@ -137,7 +137,7 @@ describe("validateDeleteChatBody", () => { error: null, }); - const result = await validateDeleteChatBody(createRequest({ chatId })); + const result = await validateDeleteChatBody(createRequest({ id })); expect(result).toBeInstanceOf(NextResponse); const response = result as NextResponse; expect(response.status).toBe(403); diff --git a/lib/chats/deleteChatHandler.ts b/lib/chats/deleteChatHandler.ts index 8ff23da5..42b21679 100644 --- a/lib/chats/deleteChatHandler.ts +++ b/lib/chats/deleteChatHandler.ts @@ -16,10 +16,10 @@ export async function deleteChatHandler(request: NextRequest): Promise; export interface ValidatedDeleteChat { - chatId: string; + id: string; } /** @@ -49,7 +49,7 @@ export async function validateDeleteChatBody( ); } - const { chatId } = result.data; + const { id } = result.data; const authResult = await validateAuthContext(request); if (authResult instanceof NextResponse) { @@ -58,7 +58,7 @@ export async function validateDeleteChatBody( const { accountId } = authResult; - const room = await selectRoom(chatId); + const room = await selectRoom(id); if (!room) { return NextResponse.json( { status: "error", error: "Chat room not found" }, @@ -80,5 +80,5 @@ export async function validateDeleteChatBody( } } - return { chatId }; + return { id }; } From 426cdc160306707bf73ba5ddeeb11218180b5216 Mon Sep 17 00:00:00 2001 From: Arpit Gupta Date: Mon, 30 Mar 2026 22:58:21 +0530 Subject: [PATCH 3/9] refactor: share chat access validation and align delete parity --- .../__tests__/validateChatAccess.test.ts | 127 +++++ .../__tests__/validateDeleteChatBody.test.ts | 101 +--- .../__tests__/validateUpdateChatBody.test.ts | 437 +++--------------- lib/chats/validateChatAccess.ts | 53 +++ lib/chats/validateDeleteChatBody.ts | 34 +- lib/chats/validateUpdateChatBody.ts | 37 +- lib/supabase/rooms/deleteRoomWithRelations.ts | 15 +- 7 files changed, 256 insertions(+), 548 deletions(-) create mode 100644 lib/chats/__tests__/validateChatAccess.test.ts create mode 100644 lib/chats/validateChatAccess.ts diff --git a/lib/chats/__tests__/validateChatAccess.test.ts b/lib/chats/__tests__/validateChatAccess.test.ts new file mode 100644 index 00000000..c369dac3 --- /dev/null +++ b/lib/chats/__tests__/validateChatAccess.test.ts @@ -0,0 +1,127 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { NextRequest, NextResponse } from "next/server"; +import { validateChatAccess } from "../validateChatAccess"; +import { validateAuthContext } from "@/lib/auth/validateAuthContext"; +import selectRoom from "@/lib/supabase/rooms/selectRoom"; +import { buildGetChatsParams } from "@/lib/chats/buildGetChatsParams"; + +vi.mock("@/lib/networking/getCorsHeaders", () => ({ + getCorsHeaders: vi.fn(() => ({ "Access-Control-Allow-Origin": "*" })), +})); + +vi.mock("@/lib/auth/validateAuthContext", () => ({ + validateAuthContext: vi.fn(), +})); + +vi.mock("@/lib/supabase/rooms/selectRoom", () => ({ + default: vi.fn(), +})); + +vi.mock("@/lib/chats/buildGetChatsParams", () => ({ + buildGetChatsParams: vi.fn(), +})); + +describe("validateChatAccess", () => { + const roomId = "123e4567-e89b-12d3-a456-426614174000"; + const accountId = "123e4567-e89b-12d3-a456-426614174001"; + const request = new NextRequest("http://localhost/api/chats", { + headers: { "x-api-key": "test-key" }, + }); + + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("returns roomId when account has access", async () => { + vi.mocked(validateAuthContext).mockResolvedValue({ + accountId, + orgId: null, + authToken: "test-key", + }); + vi.mocked(selectRoom).mockResolvedValue({ + id: roomId, + account_id: accountId, + artist_id: null, + topic: "Topic", + updated_at: "2026-03-30T00:00:00Z", + }); + vi.mocked(buildGetChatsParams).mockResolvedValue({ + params: { account_ids: [accountId] }, + error: null, + }); + + const result = await validateChatAccess(request, roomId); + expect(result).toEqual({ roomId }); + }); + + it("returns auth response when auth fails", async () => { + vi.mocked(validateAuthContext).mockResolvedValue( + NextResponse.json({ status: "error", error: "Unauthorized" }, { status: 401 }), + ); + + const result = await validateChatAccess(request, roomId); + expect(result).toBeInstanceOf(NextResponse); + const response = result as NextResponse; + expect(response.status).toBe(401); + }); + + it("returns 404 when room does not exist", async () => { + vi.mocked(validateAuthContext).mockResolvedValue({ + accountId, + orgId: null, + authToken: "test-key", + }); + vi.mocked(selectRoom).mockResolvedValue(null); + + const result = await validateChatAccess(request, roomId); + expect(result).toBeInstanceOf(NextResponse); + const response = result as NextResponse; + expect(response.status).toBe(404); + }); + + it("returns 403 when room belongs to a different account", async () => { + vi.mocked(validateAuthContext).mockResolvedValue({ + accountId, + orgId: null, + authToken: "test-key", + }); + vi.mocked(selectRoom).mockResolvedValue({ + id: roomId, + account_id: "another-account", + artist_id: null, + topic: "Topic", + updated_at: "2026-03-30T00:00:00Z", + }); + vi.mocked(buildGetChatsParams).mockResolvedValue({ + params: { account_ids: [accountId] }, + error: null, + }); + + const result = await validateChatAccess(request, roomId); + expect(result).toBeInstanceOf(NextResponse); + const response = result as NextResponse; + expect(response.status).toBe(403); + }); + + it("allows admin access when account_ids is undefined", async () => { + vi.mocked(validateAuthContext).mockResolvedValue({ + accountId, + orgId: "admin-org", + authToken: "admin-key", + }); + vi.mocked(selectRoom).mockResolvedValue({ + id: roomId, + account_id: "another-account", + artist_id: null, + topic: "Topic", + updated_at: "2026-03-30T00:00:00Z", + }); + vi.mocked(buildGetChatsParams).mockResolvedValue({ + params: {}, + error: null, + }); + + const result = await validateChatAccess(request, roomId); + expect(result).toEqual({ roomId }); + }); +}); diff --git a/lib/chats/__tests__/validateDeleteChatBody.test.ts b/lib/chats/__tests__/validateDeleteChatBody.test.ts index 700fd104..ed059fed 100644 --- a/lib/chats/__tests__/validateDeleteChatBody.test.ts +++ b/lib/chats/__tests__/validateDeleteChatBody.test.ts @@ -1,29 +1,18 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; import { NextRequest, NextResponse } from "next/server"; import { validateDeleteChatBody } from "../validateDeleteChatBody"; -import { validateAuthContext } from "@/lib/auth/validateAuthContext"; -import selectRoom from "@/lib/supabase/rooms/selectRoom"; -import { buildGetChatsParams } from "@/lib/chats/buildGetChatsParams"; +import { validateChatAccess } from "../validateChatAccess"; vi.mock("@/lib/networking/getCorsHeaders", () => ({ getCorsHeaders: vi.fn(() => ({ "Access-Control-Allow-Origin": "*" })), })); -vi.mock("@/lib/auth/validateAuthContext", () => ({ - validateAuthContext: vi.fn(), -})); - -vi.mock("@/lib/supabase/rooms/selectRoom", () => ({ - default: vi.fn(), -})); - -vi.mock("@/lib/chats/buildGetChatsParams", () => ({ - buildGetChatsParams: vi.fn(), +vi.mock("../validateChatAccess", () => ({ + validateChatAccess: vi.fn(), })); describe("validateDeleteChatBody", () => { const id = "123e4567-e89b-12d3-a456-426614174000"; - const accountId = "123e4567-e89b-12d3-a456-426614174001"; const createRequest = (body: object | string) => new NextRequest("http://localhost/api/chats", { @@ -36,48 +25,12 @@ describe("validateDeleteChatBody", () => { vi.clearAllMocks(); }); - it("returns validated id when caller has access", async () => { - vi.mocked(validateAuthContext).mockResolvedValue({ - accountId, - orgId: null, - authToken: "test-key", - }); - vi.mocked(selectRoom).mockResolvedValue({ - id, - account_id: accountId, - artist_id: null, - topic: "Topic", - updated_at: "2026-03-30T00:00:00Z", - }); - vi.mocked(buildGetChatsParams).mockResolvedValue({ - params: { account_ids: [accountId] }, - error: null, - }); - - const result = await validateDeleteChatBody(createRequest({ id })); - expect(result).toEqual({ id }); - }); - - it("allows admin access when account_ids is undefined", async () => { - vi.mocked(validateAuthContext).mockResolvedValue({ - accountId, - orgId: "admin-org", - authToken: "admin-key", - }); - vi.mocked(selectRoom).mockResolvedValue({ - id, - account_id: "another-account", - artist_id: null, - topic: "Topic", - updated_at: "2026-03-30T00:00:00Z", - }); - vi.mocked(buildGetChatsParams).mockResolvedValue({ - params: {}, - error: null, - }); + it("returns validated id when access check passes", async () => { + vi.mocked(validateChatAccess).mockResolvedValue({ roomId: id }); const result = await validateDeleteChatBody(createRequest({ id })); expect(result).toEqual({ id }); + expect(validateChatAccess).toHaveBeenCalledWith(expect.any(NextRequest), id); }); it("returns 400 when id is missing", async () => { @@ -94,8 +47,8 @@ describe("validateDeleteChatBody", () => { expect(response.status).toBe(400); }); - it("returns auth response when authentication fails", async () => { - vi.mocked(validateAuthContext).mockResolvedValue( + it("returns access response when access check fails", async () => { + vi.mocked(validateChatAccess).mockResolvedValue( NextResponse.json({ status: "error", error: "Unauthorized" }, { status: 401 }), ); @@ -105,44 +58,6 @@ describe("validateDeleteChatBody", () => { expect(response.status).toBe(401); }); - it("returns 404 when chat is not found", async () => { - vi.mocked(validateAuthContext).mockResolvedValue({ - accountId, - orgId: null, - authToken: "test-key", - }); - vi.mocked(selectRoom).mockResolvedValue(null); - - const result = await validateDeleteChatBody(createRequest({ id })); - expect(result).toBeInstanceOf(NextResponse); - const response = result as NextResponse; - expect(response.status).toBe(404); - }); - - it("returns 403 when account cannot access chat", async () => { - vi.mocked(validateAuthContext).mockResolvedValue({ - accountId, - orgId: null, - authToken: "test-key", - }); - vi.mocked(selectRoom).mockResolvedValue({ - id, - account_id: "another-account", - artist_id: null, - topic: "Topic", - updated_at: "2026-03-30T00:00:00Z", - }); - vi.mocked(buildGetChatsParams).mockResolvedValue({ - params: { account_ids: [accountId] }, - error: null, - }); - - const result = await validateDeleteChatBody(createRequest({ id })); - expect(result).toBeInstanceOf(NextResponse); - const response = result as NextResponse; - expect(response.status).toBe(403); - }); - it("returns 400 on invalid JSON body", async () => { const result = await validateDeleteChatBody(createRequest("{invalid-json")); expect(result).toBeInstanceOf(NextResponse); diff --git a/lib/chats/__tests__/validateUpdateChatBody.test.ts b/lib/chats/__tests__/validateUpdateChatBody.test.ts index e59e8c64..376a26c0 100644 --- a/lib/chats/__tests__/validateUpdateChatBody.test.ts +++ b/lib/chats/__tests__/validateUpdateChatBody.test.ts @@ -1,415 +1,88 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; import { NextRequest, NextResponse } from "next/server"; import { validateUpdateChatBody } from "../validateUpdateChatBody"; - -import { validateAuthContext } from "@/lib/auth/validateAuthContext"; -import selectRoom from "@/lib/supabase/rooms/selectRoom"; -import { buildGetChatsParams } from "@/lib/chats/buildGetChatsParams"; +import { validateChatAccess } from "../validateChatAccess"; vi.mock("@/lib/networking/getCorsHeaders", () => ({ getCorsHeaders: vi.fn(() => ({ "Access-Control-Allow-Origin": "*" })), })); -vi.mock("@/lib/auth/validateAuthContext", () => ({ - validateAuthContext: vi.fn(), -})); - -vi.mock("@/lib/supabase/rooms/selectRoom", () => ({ - default: vi.fn(), -})); - -vi.mock("@/lib/chats/buildGetChatsParams", () => ({ - buildGetChatsParams: vi.fn(), +vi.mock("../validateChatAccess", () => ({ + validateChatAccess: vi.fn(), })); describe("validateUpdateChatBody", () => { - beforeEach(() => { - vi.clearAllMocks(); - }); + const chatId = "123e4567-e89b-12d3-a456-426614174000"; - const createRequest = (body: object) => { - return new NextRequest("http://localhost/api/chats", { + const createRequest = (body: object | string) => + new NextRequest("http://localhost/api/chats", { method: "PATCH", headers: { "Content-Type": "application/json", "x-api-key": "test-key" }, - body: JSON.stringify(body), - }); - }; - - describe("successful validation", () => { - it("returns validated data when user owns the chat", async () => { - const chatId = "123e4567-e89b-12d3-a456-426614174000"; - const accountId = "123e4567-e89b-12d3-a456-426614174001"; - const topic = "Valid Topic"; - const room = { - id: chatId, - account_id: accountId, - artist_id: null, - topic: "Old Topic", - updated_at: "2024-01-01T00:00:00Z", - }; - - vi.mocked(validateAuthContext).mockResolvedValue({ - accountId, - orgId: null, - authToken: "test-key", - }); - - vi.mocked(selectRoom).mockResolvedValue(room); - - vi.mocked(buildGetChatsParams).mockResolvedValue({ - params: { account_ids: [accountId] }, - error: null, - }); - - const request = createRequest({ chatId, topic }); - const result = await validateUpdateChatBody(request); - - expect(result).not.toBeInstanceOf(NextResponse); - expect(result).toEqual({ chatId, topic }); - }); - - it("accepts topic at minimum length (3 chars)", async () => { - const chatId = "123e4567-e89b-12d3-a456-426614174000"; - const accountId = "123e4567-e89b-12d3-a456-426614174001"; - const topic = "abc"; - const room = { - id: chatId, - account_id: accountId, - artist_id: null, - topic: "Old", - updated_at: "2024-01-01T00:00:00Z", - }; - - vi.mocked(validateAuthContext).mockResolvedValue({ - accountId, - orgId: null, - authToken: "test-key", - }); - - vi.mocked(selectRoom).mockResolvedValue(room); - - vi.mocked(buildGetChatsParams).mockResolvedValue({ - params: { account_ids: [accountId] }, - error: null, - }); - - const request = createRequest({ chatId, topic }); - const result = await validateUpdateChatBody(request); - - expect(result).not.toBeInstanceOf(NextResponse); - expect(result).toEqual({ chatId, topic }); + body: typeof body === "string" ? body : JSON.stringify(body), }); - it("accepts topic at maximum length (50 chars)", async () => { - const chatId = "123e4567-e89b-12d3-a456-426614174000"; - const accountId = "123e4567-e89b-12d3-a456-426614174001"; - const topic = "a".repeat(50); - const room = { - id: chatId, - account_id: accountId, - artist_id: null, - topic: "Old", - updated_at: "2024-01-01T00:00:00Z", - }; - - vi.mocked(validateAuthContext).mockResolvedValue({ - accountId, - orgId: null, - authToken: "test-key", - }); - - vi.mocked(selectRoom).mockResolvedValue(room); - - vi.mocked(buildGetChatsParams).mockResolvedValue({ - params: { account_ids: [accountId] }, - error: null, - }); - - const request = createRequest({ chatId, topic }); - const result = await validateUpdateChatBody(request); - - expect(result).not.toBeInstanceOf(NextResponse); - expect(result).toEqual({ chatId, topic }); - }); - - it("allows org key to access member's chat", async () => { - const chatId = "123e4567-e89b-12d3-a456-426614174000"; - const memberAccountId = "123e4567-e89b-12d3-a456-426614174001"; - const orgId = "123e4567-e89b-12d3-a456-426614174002"; - const topic = "Valid Topic"; - const room = { - id: chatId, - account_id: memberAccountId, - artist_id: null, - topic: "Old", - updated_at: "2024-01-01T00:00:00Z", - }; - - vi.mocked(validateAuthContext).mockResolvedValue({ - accountId: orgId, - orgId, - authToken: "org-key", - }); - - vi.mocked(selectRoom).mockResolvedValue(room); - - vi.mocked(buildGetChatsParams).mockResolvedValue({ - params: { account_ids: [memberAccountId, "other-member"] }, - error: null, - }); - - const request = createRequest({ chatId, topic }); - const result = await validateUpdateChatBody(request); - - expect(result).not.toBeInstanceOf(NextResponse); - expect(result).toEqual({ chatId, topic }); - }); - - it("allows admin access when account_ids is undefined", async () => { - const chatId = "123e4567-e89b-12d3-a456-426614174000"; - const accountId = "123e4567-e89b-12d3-a456-426614174001"; - const topic = "Valid Topic"; - const room = { - id: chatId, - account_id: "any-account", - artist_id: null, - topic: "Old", - updated_at: "2024-01-01T00:00:00Z", - }; - - vi.mocked(validateAuthContext).mockResolvedValue({ - accountId, - orgId: "admin-org", - authToken: "admin-key", - }); - - vi.mocked(selectRoom).mockResolvedValue(room); - - vi.mocked(buildGetChatsParams).mockResolvedValue({ - params: {}, // No account_ids = admin access - error: null, - }); - - const request = createRequest({ chatId, topic }); - const result = await validateUpdateChatBody(request); - - expect(result).not.toBeInstanceOf(NextResponse); - expect(result).toEqual({ chatId, topic }); - }); + beforeEach(() => { + vi.clearAllMocks(); }); - describe("body validation errors", () => { - it("returns 400 when chatId is missing", async () => { - const request = createRequest({ topic: "Valid Topic" }); + it("returns validated data when access check passes", async () => { + const topic = "Valid Topic"; + vi.mocked(validateChatAccess).mockResolvedValue({ roomId: chatId }); - const result = await validateUpdateChatBody(request); - - expect(result).toBeInstanceOf(NextResponse); - const response = result as NextResponse; - expect(response.status).toBe(400); - const body = await response.json(); - expect(body.status).toBe("error"); - }); - - it("returns 400 when chatId is not a valid UUID", async () => { - const request = createRequest({ chatId: "not-a-uuid", topic: "Valid Topic" }); - - const result = await validateUpdateChatBody(request); - - expect(result).toBeInstanceOf(NextResponse); - const response = result as NextResponse; - expect(response.status).toBe(400); - const body = await response.json(); - expect(body.error).toContain("UUID"); - }); - - it("returns 400 when topic is missing", async () => { - const request = createRequest({ chatId: "123e4567-e89b-12d3-a456-426614174000" }); - - const result = await validateUpdateChatBody(request); - - expect(result).toBeInstanceOf(NextResponse); - const response = result as NextResponse; - expect(response.status).toBe(400); - }); - - it("returns 400 when topic is too short", async () => { - const request = createRequest({ - chatId: "123e4567-e89b-12d3-a456-426614174000", - topic: "ab", - }); - - const result = await validateUpdateChatBody(request); - - expect(result).toBeInstanceOf(NextResponse); - const response = result as NextResponse; - expect(response.status).toBe(400); - const body = await response.json(); - expect(body.error).toContain("3 and 50"); - }); - - it("returns 400 when topic is too long", async () => { - const request = createRequest({ - chatId: "123e4567-e89b-12d3-a456-426614174000", - topic: "a".repeat(51), - }); - - const result = await validateUpdateChatBody(request); - - expect(result).toBeInstanceOf(NextResponse); - const response = result as NextResponse; - expect(response.status).toBe(400); - const body = await response.json(); - expect(body.error).toContain("3 and 50"); - }); + const result = await validateUpdateChatBody(createRequest({ chatId, topic })); + expect(result).toEqual({ chatId, topic }); + expect(validateChatAccess).toHaveBeenCalledWith(expect.any(NextRequest), chatId); }); - describe("JSON parsing errors", () => { - it("handles invalid JSON gracefully", async () => { - const request = new NextRequest("http://localhost/api/chats", { - method: "PATCH", - headers: { "Content-Type": "application/json" }, - body: "not valid json", - }); - - const result = await validateUpdateChatBody(request); - - expect(result).toBeInstanceOf(NextResponse); - const response = result as NextResponse; - expect(response.status).toBe(400); - }); - - it("handles empty body gracefully", async () => { - const request = new NextRequest("http://localhost/api/chats", { - method: "PATCH", - headers: { "Content-Type": "application/json" }, - }); - - const result = await validateUpdateChatBody(request); - - expect(result).toBeInstanceOf(NextResponse); - const response = result as NextResponse; - expect(response.status).toBe(400); - }); + it("accepts topic at minimum length (3 chars)", async () => { + vi.mocked(validateChatAccess).mockResolvedValue({ roomId: chatId }); + const result = await validateUpdateChatBody(createRequest({ chatId, topic: "abc" })); + expect(result).toEqual({ chatId, topic: "abc" }); }); - describe("authentication errors", () => { - it("returns 401 when auth fails", async () => { - vi.mocked(validateAuthContext).mockResolvedValue( - NextResponse.json({ status: "error", error: "Unauthorized" }, { status: 401 }), - ); - - const request = createRequest({ - chatId: "123e4567-e89b-12d3-a456-426614174000", - topic: "Valid Topic", - }); - const result = await validateUpdateChatBody(request); - - expect(result).toBeInstanceOf(NextResponse); - const response = result as NextResponse; - expect(response.status).toBe(401); - }); + it("accepts topic at maximum length (50 chars)", async () => { + vi.mocked(validateChatAccess).mockResolvedValue({ roomId: chatId }); + const topic = "a".repeat(50); + const result = await validateUpdateChatBody(createRequest({ chatId, topic })); + expect(result).toEqual({ chatId, topic }); }); - describe("room not found errors", () => { - it("returns 404 when chat does not exist", async () => { - vi.mocked(validateAuthContext).mockResolvedValue({ - accountId: "123e4567-e89b-12d3-a456-426614174001", - orgId: null, - authToken: "test-key", - }); - - vi.mocked(selectRoom).mockResolvedValue(null); - - const request = createRequest({ - chatId: "123e4567-e89b-12d3-a456-426614174000", - topic: "Valid Topic", - }); - const result = await validateUpdateChatBody(request); - - expect(result).toBeInstanceOf(NextResponse); - const response = result as NextResponse; - expect(response.status).toBe(404); - const body = await response.json(); - expect(body.error).toContain("not found"); - }); + it("returns 400 when chatId is missing", async () => { + const result = await validateUpdateChatBody(createRequest({ topic: "Valid Topic" })); + expect(result).toBeInstanceOf(NextResponse); + const response = result as NextResponse; + expect(response.status).toBe(400); }); - describe("access denied errors", () => { - it("returns 403 when user tries to update another user's chat", async () => { - const userAccountId = "123e4567-e89b-12d3-a456-426614174001"; - const otherAccountId = "123e4567-e89b-12d3-a456-426614174002"; - const chatId = "123e4567-e89b-12d3-a456-426614174000"; - const room = { - id: chatId, - account_id: otherAccountId, - artist_id: null, - topic: "Old Topic", - updated_at: "2024-01-01T00:00:00Z", - }; - - vi.mocked(validateAuthContext).mockResolvedValue({ - accountId: userAccountId, - orgId: null, - authToken: "test-key", - }); - - vi.mocked(selectRoom).mockResolvedValue(room); - - vi.mocked(buildGetChatsParams).mockResolvedValue({ - params: { account_ids: [userAccountId] }, - error: null, - }); - - const request = createRequest({ - chatId, - topic: "Valid Topic", - }); - const result = await validateUpdateChatBody(request); - - expect(result).toBeInstanceOf(NextResponse); - const response = result as NextResponse; - expect(response.status).toBe(403); - const body = await response.json(); - expect(body.error).toContain("Access denied"); - }); - - it("returns 403 when org key cannot access non-member's chat", async () => { - const orgId = "123e4567-e89b-12d3-a456-426614174001"; - const nonMemberAccountId = "123e4567-e89b-12d3-a456-426614174002"; - const chatId = "123e4567-e89b-12d3-a456-426614174000"; - const room = { - id: chatId, - account_id: nonMemberAccountId, - artist_id: null, - topic: "Old Topic", - updated_at: "2024-01-01T00:00:00Z", - }; - - vi.mocked(validateAuthContext).mockResolvedValue({ - accountId: orgId, - orgId, - authToken: "org-key", - }); + it("returns 400 when topic is too short", async () => { + const result = await validateUpdateChatBody(createRequest({ chatId, topic: "ab" })); + expect(result).toBeInstanceOf(NextResponse); + const response = result as NextResponse; + expect(response.status).toBe(400); + }); - vi.mocked(selectRoom).mockResolvedValue(room); + it("returns 400 when topic is too long", async () => { + const result = await validateUpdateChatBody(createRequest({ chatId, topic: "a".repeat(51) })); + expect(result).toBeInstanceOf(NextResponse); + const response = result as NextResponse; + expect(response.status).toBe(400); + }); - vi.mocked(buildGetChatsParams).mockResolvedValue({ - params: { account_ids: ["member-1", "member-2"] }, // non-member not in list - error: null, - }); + it("returns access response when access check fails", async () => { + vi.mocked(validateChatAccess).mockResolvedValue( + NextResponse.json({ status: "error", error: "Access denied" }, { status: 403 }), + ); - const request = createRequest({ - chatId, - topic: "Valid Topic", - }); - const result = await validateUpdateChatBody(request); + const result = await validateUpdateChatBody(createRequest({ chatId, topic: "Valid Topic" })); + expect(result).toBeInstanceOf(NextResponse); + const response = result as NextResponse; + expect(response.status).toBe(403); + }); - expect(result).toBeInstanceOf(NextResponse); - const response = result as NextResponse; - expect(response.status).toBe(403); - const body = await response.json(); - expect(body.error).toContain("Access denied"); - }); + it("returns 400 on invalid JSON body", async () => { + const result = await validateUpdateChatBody(createRequest("{invalid-json")); + expect(result).toBeInstanceOf(NextResponse); + const response = result as NextResponse; + expect(response.status).toBe(400); }); }); diff --git a/lib/chats/validateChatAccess.ts b/lib/chats/validateChatAccess.ts new file mode 100644 index 00000000..2743b945 --- /dev/null +++ b/lib/chats/validateChatAccess.ts @@ -0,0 +1,53 @@ +import type { NextRequest } from "next/server"; +import { NextResponse } from "next/server"; +import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; +import { validateAuthContext } from "@/lib/auth/validateAuthContext"; +import selectRoom from "@/lib/supabase/rooms/selectRoom"; +import { buildGetChatsParams } from "./buildGetChatsParams"; + +export interface ValidatedChatAccess { + roomId: string; +} + +/** + * Validates that the authenticated caller can access a chat room. + * + * @param request - The incoming request (used for auth context) + * @param roomId - The room/chat UUID to validate access for + * @returns NextResponse on auth/access failure, or validated roomId + */ +export async function validateChatAccess( + request: NextRequest, + roomId: string, +): Promise { + const authResult = await validateAuthContext(request); + if (authResult instanceof NextResponse) { + return authResult; + } + + const { accountId } = authResult; + + const room = await selectRoom(roomId); + if (!room) { + return NextResponse.json( + { status: "error", error: "Chat room not found" }, + { status: 404, headers: getCorsHeaders() }, + ); + } + + const { params } = await buildGetChatsParams({ + account_id: accountId, + }); + + // If params.account_ids is undefined, it means admin access (all records) + if (params.account_ids && room.account_id) { + if (!params.account_ids.includes(room.account_id)) { + return NextResponse.json( + { status: "error", error: "Access denied to this chat" }, + { status: 403, headers: getCorsHeaders() }, + ); + } + } + + return { roomId }; +} diff --git a/lib/chats/validateDeleteChatBody.ts b/lib/chats/validateDeleteChatBody.ts index bd1e1fab..b8b6db85 100644 --- a/lib/chats/validateDeleteChatBody.ts +++ b/lib/chats/validateDeleteChatBody.ts @@ -2,9 +2,7 @@ import type { NextRequest } from "next/server"; import { NextResponse } from "next/server"; import { z } from "zod"; import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; -import { validateAuthContext } from "@/lib/auth/validateAuthContext"; -import selectRoom from "@/lib/supabase/rooms/selectRoom"; -import { buildGetChatsParams } from "./buildGetChatsParams"; +import { validateChatAccess } from "./validateChatAccess"; export const deleteChatBodySchema = z.object({ id: z.string().uuid("id must be a valid UUID"), @@ -51,33 +49,9 @@ export async function validateDeleteChatBody( const { id } = result.data; - const authResult = await validateAuthContext(request); - if (authResult instanceof NextResponse) { - return authResult; - } - - const { accountId } = authResult; - - const room = await selectRoom(id); - if (!room) { - return NextResponse.json( - { status: "error", error: "Chat room not found" }, - { status: 404, headers: getCorsHeaders() }, - ); - } - - const { params } = await buildGetChatsParams({ - account_id: accountId, - }); - - // If params.account_ids is undefined, it means admin access (all records) - if (params.account_ids && room.account_id) { - if (!params.account_ids.includes(room.account_id)) { - return NextResponse.json( - { status: "error", error: "Access denied to this chat" }, - { status: 403, headers: getCorsHeaders() }, - ); - } + const accessResult = await validateChatAccess(request, id); + if (accessResult instanceof NextResponse) { + return accessResult; } return { id }; diff --git a/lib/chats/validateUpdateChatBody.ts b/lib/chats/validateUpdateChatBody.ts index 95e3c61f..b2c9597a 100644 --- a/lib/chats/validateUpdateChatBody.ts +++ b/lib/chats/validateUpdateChatBody.ts @@ -1,10 +1,8 @@ import type { NextRequest } from "next/server"; import { NextResponse } from "next/server"; import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; -import { validateAuthContext } from "@/lib/auth/validateAuthContext"; -import selectRoom from "@/lib/supabase/rooms/selectRoom"; -import { buildGetChatsParams } from "./buildGetChatsParams"; import { z } from "zod"; +import { validateChatAccess } from "./validateChatAccess"; /** * Zod schema for PATCH /api/chats request body. @@ -64,36 +62,9 @@ export async function validateUpdateChatBody( const { chatId, topic } = result.data; - // Validate authentication - const authResult = await validateAuthContext(request); - if (authResult instanceof NextResponse) { - return authResult; - } - - const { accountId, orgId } = authResult; - - // Verify room exists - const room = await selectRoom(chatId); - if (!room) { - return NextResponse.json( - { status: "error", error: "Chat room not found" }, - { status: 404, headers: getCorsHeaders() }, - ); - } - - // Check access control - const { params } = await buildGetChatsParams({ - account_id: accountId, - }); - - // If params.account_ids is undefined, it means admin access (all records) - if (params.account_ids && room.account_id) { - if (!params.account_ids.includes(room.account_id)) { - return NextResponse.json( - { status: "error", error: "Access denied to this chat" }, - { status: 403, headers: getCorsHeaders() }, - ); - } + const accessResult = await validateChatAccess(request, chatId); + if (accessResult instanceof NextResponse) { + return accessResult; } return { diff --git a/lib/supabase/rooms/deleteRoomWithRelations.ts b/lib/supabase/rooms/deleteRoomWithRelations.ts index 8460e8eb..6c96ea71 100644 --- a/lib/supabase/rooms/deleteRoomWithRelations.ts +++ b/lib/supabase/rooms/deleteRoomWithRelations.ts @@ -14,8 +14,7 @@ export async function deleteRoomWithRelations(roomId: string): Promise .eq("room_id", roomId); if (selectMemoriesError) { - console.error("[ERROR] deleteRoomWithRelations select memories:", selectMemoriesError); - return false; + console.error("[WARN] deleteRoomWithRelations select memories:", selectMemoriesError); } const memoryIds = (memories || []).map(memory => memory.id); @@ -27,8 +26,7 @@ export async function deleteRoomWithRelations(roomId: string): Promise .in("memory", memoryIds); if (deleteMemoryEmailsError) { - console.error("[ERROR] deleteRoomWithRelations memory_emails:", deleteMemoryEmailsError); - return false; + console.error("[WARN] deleteRoomWithRelations memory_emails:", deleteMemoryEmailsError); } } @@ -37,8 +35,7 @@ export async function deleteRoomWithRelations(roomId: string): Promise .delete() .eq("room_id", roomId); if (deleteMemoriesError) { - console.error("[ERROR] deleteRoomWithRelations memories:", deleteMemoriesError); - return false; + console.error("[WARN] deleteRoomWithRelations memories:", deleteMemoriesError); } const { error: deleteRoomReportsError } = await supabase @@ -46,8 +43,7 @@ export async function deleteRoomWithRelations(roomId: string): Promise .delete() .eq("room_id", roomId); if (deleteRoomReportsError) { - console.error("[ERROR] deleteRoomWithRelations room_reports:", deleteRoomReportsError); - return false; + console.error("[WARN] deleteRoomWithRelations room_reports:", deleteRoomReportsError); } const { error: deleteSegmentRoomsError } = await supabase @@ -55,8 +51,7 @@ export async function deleteRoomWithRelations(roomId: string): Promise .delete() .eq("room_id", roomId); if (deleteSegmentRoomsError) { - console.error("[ERROR] deleteRoomWithRelations segment_rooms:", deleteSegmentRoomsError); - return false; + console.error("[WARN] deleteRoomWithRelations segment_rooms:", deleteSegmentRoomsError); } const { error: deleteRoomError } = await supabase.from("rooms").delete().eq("id", roomId); From 6effee37d9678377567f19037c2bf0bf185cdf43 Mon Sep 17 00:00:00 2001 From: Arpit Gupta Date: Mon, 30 Mar 2026 23:04:50 +0530 Subject: [PATCH 4/9] test: restore update chat body validation edge cases --- .../__tests__/validateUpdateChatBody.test.ts | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/lib/chats/__tests__/validateUpdateChatBody.test.ts b/lib/chats/__tests__/validateUpdateChatBody.test.ts index 376a26c0..6650959d 100644 --- a/lib/chats/__tests__/validateUpdateChatBody.test.ts +++ b/lib/chats/__tests__/validateUpdateChatBody.test.ts @@ -54,6 +54,22 @@ describe("validateUpdateChatBody", () => { expect(response.status).toBe(400); }); + it("returns 400 when chatId is not a valid UUID", async () => { + const result = await validateUpdateChatBody( + createRequest({ chatId: "invalid-uuid", topic: "Valid Topic" }), + ); + expect(result).toBeInstanceOf(NextResponse); + const response = result as NextResponse; + expect(response.status).toBe(400); + }); + + it("returns 400 when topic is missing", async () => { + const result = await validateUpdateChatBody(createRequest({ chatId })); + expect(result).toBeInstanceOf(NextResponse); + const response = result as NextResponse; + expect(response.status).toBe(400); + }); + it("returns 400 when topic is too short", async () => { const result = await validateUpdateChatBody(createRequest({ chatId, topic: "ab" })); expect(result).toBeInstanceOf(NextResponse); @@ -85,4 +101,11 @@ describe("validateUpdateChatBody", () => { const response = result as NextResponse; expect(response.status).toBe(400); }); + + it("handles empty body gracefully", async () => { + const result = await validateUpdateChatBody(createRequest({})); + expect(result).toBeInstanceOf(NextResponse); + const response = result as NextResponse; + expect(response.status).toBe(400); + }); }); From 70e0177f205d94f4ae3de23629cbc5a641561e0f Mon Sep 17 00:00:00 2001 From: Arpit Gupta Date: Mon, 30 Mar 2026 23:14:27 +0530 Subject: [PATCH 5/9] refactor: use path aliases for chat delete modules --- lib/chats/deleteChatHandler.ts | 2 +- lib/chats/validateChatAccess.ts | 2 +- lib/chats/validateDeleteChatBody.ts | 2 +- lib/chats/validateUpdateChatBody.ts | 2 +- lib/supabase/rooms/deleteRoomWithRelations.ts | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/chats/deleteChatHandler.ts b/lib/chats/deleteChatHandler.ts index 42b21679..b397ad44 100644 --- a/lib/chats/deleteChatHandler.ts +++ b/lib/chats/deleteChatHandler.ts @@ -1,7 +1,7 @@ import type { NextRequest } from "next/server"; import { NextResponse } from "next/server"; import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; -import { validateDeleteChatBody } from "./validateDeleteChatBody"; +import { validateDeleteChatBody } from "@/lib/chats/validateDeleteChatBody"; import { deleteRoomWithRelations } from "@/lib/supabase/rooms/deleteRoomWithRelations"; /** diff --git a/lib/chats/validateChatAccess.ts b/lib/chats/validateChatAccess.ts index 2743b945..02b4f080 100644 --- a/lib/chats/validateChatAccess.ts +++ b/lib/chats/validateChatAccess.ts @@ -3,7 +3,7 @@ import { NextResponse } from "next/server"; import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; import { validateAuthContext } from "@/lib/auth/validateAuthContext"; import selectRoom from "@/lib/supabase/rooms/selectRoom"; -import { buildGetChatsParams } from "./buildGetChatsParams"; +import { buildGetChatsParams } from "@/lib/chats/buildGetChatsParams"; export interface ValidatedChatAccess { roomId: string; diff --git a/lib/chats/validateDeleteChatBody.ts b/lib/chats/validateDeleteChatBody.ts index b8b6db85..c9fa8a63 100644 --- a/lib/chats/validateDeleteChatBody.ts +++ b/lib/chats/validateDeleteChatBody.ts @@ -2,7 +2,7 @@ import type { NextRequest } from "next/server"; import { NextResponse } from "next/server"; import { z } from "zod"; import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; -import { validateChatAccess } from "./validateChatAccess"; +import { validateChatAccess } from "@/lib/chats/validateChatAccess"; export const deleteChatBodySchema = z.object({ id: z.string().uuid("id must be a valid UUID"), diff --git a/lib/chats/validateUpdateChatBody.ts b/lib/chats/validateUpdateChatBody.ts index b2c9597a..8a9bfa48 100644 --- a/lib/chats/validateUpdateChatBody.ts +++ b/lib/chats/validateUpdateChatBody.ts @@ -2,7 +2,7 @@ import type { NextRequest } from "next/server"; import { NextResponse } from "next/server"; import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; import { z } from "zod"; -import { validateChatAccess } from "./validateChatAccess"; +import { validateChatAccess } from "@/lib/chats/validateChatAccess"; /** * Zod schema for PATCH /api/chats request body. diff --git a/lib/supabase/rooms/deleteRoomWithRelations.ts b/lib/supabase/rooms/deleteRoomWithRelations.ts index 6c96ea71..f38798d2 100644 --- a/lib/supabase/rooms/deleteRoomWithRelations.ts +++ b/lib/supabase/rooms/deleteRoomWithRelations.ts @@ -1,4 +1,4 @@ -import supabase from "../serverClient"; +import supabase from "@/lib/supabase/serverClient"; /** * Deletes a room and related room data. From 6f542c64cad95408ade262e224eed538172dcbd1 Mon Sep 17 00:00:00 2001 From: Arpit Gupta Date: Tue, 31 Mar 2026 01:12:08 +0530 Subject: [PATCH 6/9] refactor: centralize chat room access resolution --- .../__tests__/resolveAccessibleRoom.test.ts | 117 ++++++++++++++++++ .../__tests__/validateChatAccess.test.ts | 102 +++------------ lib/chats/resolveAccessibleRoom.ts | 68 ++++++++++ lib/chats/validateChatAccess.ts | 37 +----- 4 files changed, 208 insertions(+), 116 deletions(-) create mode 100644 lib/chats/__tests__/resolveAccessibleRoom.test.ts create mode 100644 lib/chats/resolveAccessibleRoom.ts diff --git a/lib/chats/__tests__/resolveAccessibleRoom.test.ts b/lib/chats/__tests__/resolveAccessibleRoom.test.ts new file mode 100644 index 00000000..c5f03d34 --- /dev/null +++ b/lib/chats/__tests__/resolveAccessibleRoom.test.ts @@ -0,0 +1,117 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { NextRequest, NextResponse } from "next/server"; +import { resolveAccessibleRoom } from "../resolveAccessibleRoom"; +import { validateAuthContext } from "@/lib/auth/validateAuthContext"; +import selectRoom from "@/lib/supabase/rooms/selectRoom"; +import { buildGetChatsParams } from "@/lib/chats/buildGetChatsParams"; + +vi.mock("@/lib/networking/getCorsHeaders", () => ({ + getCorsHeaders: vi.fn(() => ({ "Access-Control-Allow-Origin": "*" })), +})); + +vi.mock("@/lib/auth/validateAuthContext", () => ({ + validateAuthContext: vi.fn(), +})); + +vi.mock("@/lib/supabase/rooms/selectRoom", () => ({ + default: vi.fn(), +})); + +vi.mock("@/lib/chats/buildGetChatsParams", () => ({ + buildGetChatsParams: vi.fn(), +})); + +describe("resolveAccessibleRoom", () => { + const roomId = "123e4567-e89b-12d3-a456-426614174000"; + const accountId = "123e4567-e89b-12d3-a456-426614174001"; + const request = new NextRequest("http://localhost/api/chats", { + headers: { "x-api-key": "test-key" }, + }); + + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("returns 400 when roomId is invalid uuid", async () => { + const result = await resolveAccessibleRoom(request, "invalid-id"); + expect(result).toBeInstanceOf(NextResponse); + const response = result as NextResponse; + expect(response.status).toBe(400); + }); + + it("returns auth response when auth fails", async () => { + vi.mocked(validateAuthContext).mockResolvedValue( + NextResponse.json({ status: "error", error: "Unauthorized" }, { status: 401 }), + ); + + const result = await resolveAccessibleRoom(request, roomId); + expect(result).toBeInstanceOf(NextResponse); + const response = result as NextResponse; + expect(response.status).toBe(401); + }); + + it("returns 404 when room does not exist", async () => { + vi.mocked(validateAuthContext).mockResolvedValue({ + accountId, + orgId: null, + authToken: "test-key", + }); + vi.mocked(selectRoom).mockResolvedValue(null); + + const result = await resolveAccessibleRoom(request, roomId); + expect(result).toBeInstanceOf(NextResponse); + const response = result as NextResponse; + expect(response.status).toBe(404); + }); + + it("returns 403 when room belongs to inaccessible account", async () => { + vi.mocked(validateAuthContext).mockResolvedValue({ + accountId, + orgId: null, + authToken: "test-key", + }); + vi.mocked(selectRoom).mockResolvedValue({ + id: roomId, + account_id: "another-account", + artist_id: null, + topic: "Topic", + updated_at: "2026-03-30T00:00:00Z", + }); + vi.mocked(buildGetChatsParams).mockResolvedValue({ + params: { account_ids: [accountId] }, + error: null, + }); + + const result = await resolveAccessibleRoom(request, roomId); + expect(result).toBeInstanceOf(NextResponse); + const response = result as NextResponse; + expect(response.status).toBe(403); + }); + + it("returns room and accountId for accessible room", async () => { + const room = { + id: roomId, + account_id: accountId, + artist_id: null, + topic: "Topic", + updated_at: "2026-03-30T00:00:00Z", + }; + + vi.mocked(validateAuthContext).mockResolvedValue({ + accountId, + orgId: null, + authToken: "test-key", + }); + vi.mocked(selectRoom).mockResolvedValue(room); + vi.mocked(buildGetChatsParams).mockResolvedValue({ + params: { account_ids: [accountId] }, + error: null, + }); + + const result = await resolveAccessibleRoom(request, roomId); + expect(result).toEqual({ + room, + accountId, + }); + }); +}); diff --git a/lib/chats/__tests__/validateChatAccess.test.ts b/lib/chats/__tests__/validateChatAccess.test.ts index c369dac3..ee39adcc 100644 --- a/lib/chats/__tests__/validateChatAccess.test.ts +++ b/lib/chats/__tests__/validateChatAccess.test.ts @@ -1,29 +1,18 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; import { NextRequest, NextResponse } from "next/server"; import { validateChatAccess } from "../validateChatAccess"; -import { validateAuthContext } from "@/lib/auth/validateAuthContext"; -import selectRoom from "@/lib/supabase/rooms/selectRoom"; -import { buildGetChatsParams } from "@/lib/chats/buildGetChatsParams"; +import { resolveAccessibleRoom } from "../resolveAccessibleRoom"; vi.mock("@/lib/networking/getCorsHeaders", () => ({ getCorsHeaders: vi.fn(() => ({ "Access-Control-Allow-Origin": "*" })), })); -vi.mock("@/lib/auth/validateAuthContext", () => ({ - validateAuthContext: vi.fn(), -})); - -vi.mock("@/lib/supabase/rooms/selectRoom", () => ({ - default: vi.fn(), -})); - -vi.mock("@/lib/chats/buildGetChatsParams", () => ({ - buildGetChatsParams: vi.fn(), +vi.mock("../resolveAccessibleRoom", () => ({ + resolveAccessibleRoom: vi.fn(), })); describe("validateChatAccess", () => { const roomId = "123e4567-e89b-12d3-a456-426614174000"; - const accountId = "123e4567-e89b-12d3-a456-426614174001"; const request = new NextRequest("http://localhost/api/chats", { headers: { "x-api-key": "test-key" }, }); @@ -33,29 +22,24 @@ describe("validateChatAccess", () => { }); it("returns roomId when account has access", async () => { - vi.mocked(validateAuthContext).mockResolvedValue({ - accountId, - orgId: null, - authToken: "test-key", - }); - vi.mocked(selectRoom).mockResolvedValue({ + vi.mocked(resolveAccessibleRoom).mockResolvedValue({ id: roomId, - account_id: accountId, - artist_id: null, - topic: "Topic", - updated_at: "2026-03-30T00:00:00Z", - }); - vi.mocked(buildGetChatsParams).mockResolvedValue({ - params: { account_ids: [accountId] }, - error: null, + accountId: "123e4567-e89b-12d3-a456-426614174001", + room: { + id: roomId, + account_id: "123e4567-e89b-12d3-a456-426614174001", + artist_id: null, + topic: "Topic", + updated_at: "2026-03-30T00:00:00Z", + }, }); const result = await validateChatAccess(request, roomId); expect(result).toEqual({ roomId }); }); - it("returns auth response when auth fails", async () => { - vi.mocked(validateAuthContext).mockResolvedValue( + it("returns resolver response when access resolution fails", async () => { + vi.mocked(resolveAccessibleRoom).mockResolvedValue( NextResponse.json({ status: "error", error: "Unauthorized" }, { status: 401 }), ); @@ -65,63 +49,13 @@ describe("validateChatAccess", () => { expect(response.status).toBe(401); }); - it("returns 404 when room does not exist", async () => { - vi.mocked(validateAuthContext).mockResolvedValue({ - accountId, - orgId: null, - authToken: "test-key", - }); - vi.mocked(selectRoom).mockResolvedValue(null); - + it("returns resolver 404 response unchanged", async () => { + vi.mocked(resolveAccessibleRoom).mockResolvedValue( + NextResponse.json({ status: "error", error: "Chat room not found" }, { status: 404 }), + ); const result = await validateChatAccess(request, roomId); expect(result).toBeInstanceOf(NextResponse); const response = result as NextResponse; expect(response.status).toBe(404); }); - - it("returns 403 when room belongs to a different account", async () => { - vi.mocked(validateAuthContext).mockResolvedValue({ - accountId, - orgId: null, - authToken: "test-key", - }); - vi.mocked(selectRoom).mockResolvedValue({ - id: roomId, - account_id: "another-account", - artist_id: null, - topic: "Topic", - updated_at: "2026-03-30T00:00:00Z", - }); - vi.mocked(buildGetChatsParams).mockResolvedValue({ - params: { account_ids: [accountId] }, - error: null, - }); - - const result = await validateChatAccess(request, roomId); - expect(result).toBeInstanceOf(NextResponse); - const response = result as NextResponse; - expect(response.status).toBe(403); - }); - - it("allows admin access when account_ids is undefined", async () => { - vi.mocked(validateAuthContext).mockResolvedValue({ - accountId, - orgId: "admin-org", - authToken: "admin-key", - }); - vi.mocked(selectRoom).mockResolvedValue({ - id: roomId, - account_id: "another-account", - artist_id: null, - topic: "Topic", - updated_at: "2026-03-30T00:00:00Z", - }); - vi.mocked(buildGetChatsParams).mockResolvedValue({ - params: {}, - error: null, - }); - - const result = await validateChatAccess(request, roomId); - expect(result).toEqual({ roomId }); - }); }); diff --git a/lib/chats/resolveAccessibleRoom.ts b/lib/chats/resolveAccessibleRoom.ts new file mode 100644 index 00000000..74044ee9 --- /dev/null +++ b/lib/chats/resolveAccessibleRoom.ts @@ -0,0 +1,68 @@ +import { NextResponse } from "next/server"; +import { z } from "zod"; +import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; +import { validateAuthContext } from "@/lib/auth/validateAuthContext"; +import selectRoom from "@/lib/supabase/rooms/selectRoom"; +import { buildGetChatsParams } from "@/lib/chats/buildGetChatsParams"; +import type { Tables } from "@/types/database.types"; + +const chatIdSchema = z.string().uuid("id must be a valid UUID"); + +export interface ResolvedAccessibleRoom { + room: Tables<"rooms">; + accountId: string; +} + +/** + * Resolves a chat room and validates access for the authenticated caller. + * + * @param request - The incoming request (used for auth context) + * @param roomId - The room/chat UUID to validate access for + * @returns The resolved room + accountId, or an error NextResponse + */ +export async function resolveAccessibleRoom( + request: Request, + roomId: string, +): Promise { + const roomIdResult = chatIdSchema.safeParse(roomId); + if (!roomIdResult.success) { + return NextResponse.json( + { status: "error", error: roomIdResult.error.issues[0]?.message || "Invalid chat ID" }, + { status: 400, headers: getCorsHeaders() }, + ); + } + + const authResult = await validateAuthContext(request); + if (authResult instanceof NextResponse) { + return authResult; + } + + const { accountId } = authResult; + + const room = await selectRoom(roomIdResult.data); + if (!room) { + return NextResponse.json( + { status: "error", error: "Chat room not found" }, + { status: 404, headers: getCorsHeaders() }, + ); + } + + const { params } = await buildGetChatsParams({ + account_id: accountId, + }); + + // If params.account_ids is undefined, it means admin access (all records) + if (params.account_ids && room.account_id) { + if (!params.account_ids.includes(room.account_id)) { + return NextResponse.json( + { status: "error", error: "Access denied to this chat" }, + { status: 403, headers: getCorsHeaders() }, + ); + } + } + + return { + room, + accountId, + }; +} diff --git a/lib/chats/validateChatAccess.ts b/lib/chats/validateChatAccess.ts index 02b4f080..1989d24b 100644 --- a/lib/chats/validateChatAccess.ts +++ b/lib/chats/validateChatAccess.ts @@ -1,9 +1,6 @@ import type { NextRequest } from "next/server"; import { NextResponse } from "next/server"; -import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; -import { validateAuthContext } from "@/lib/auth/validateAuthContext"; -import selectRoom from "@/lib/supabase/rooms/selectRoom"; -import { buildGetChatsParams } from "@/lib/chats/buildGetChatsParams"; +import { resolveAccessibleRoom } from "@/lib/chats/resolveAccessibleRoom"; export interface ValidatedChatAccess { roomId: string; @@ -20,34 +17,10 @@ export async function validateChatAccess( request: NextRequest, roomId: string, ): Promise { - const authResult = await validateAuthContext(request); - if (authResult instanceof NextResponse) { - return authResult; + const roomResult = await resolveAccessibleRoom(request, roomId); + if (roomResult instanceof NextResponse) { + return roomResult; } - const { accountId } = authResult; - - const room = await selectRoom(roomId); - if (!room) { - return NextResponse.json( - { status: "error", error: "Chat room not found" }, - { status: 404, headers: getCorsHeaders() }, - ); - } - - const { params } = await buildGetChatsParams({ - account_id: accountId, - }); - - // If params.account_ids is undefined, it means admin access (all records) - if (params.account_ids && room.account_id) { - if (!params.account_ids.includes(room.account_id)) { - return NextResponse.json( - { status: "error", error: "Access denied to this chat" }, - { status: 403, headers: getCorsHeaders() }, - ); - } - } - - return { roomId }; + return { roomId: roomResult.room.id }; } From d041da3da59c0ecf6db8a6a3ed71c69ead320125 Mon Sep 17 00:00:00 2001 From: Arpit Gupta Date: Tue, 31 Mar 2026 02:00:53 +0530 Subject: [PATCH 7/9] refactor: fold room resolver into validateChatAccess --- .../__tests__/resolveAccessibleRoom.test.ts | 117 ------------------ .../__tests__/validateChatAccess.test.ts | 101 +++++++++++---- lib/chats/resolveAccessibleRoom.ts | 68 ---------- lib/chats/validateChatAccess.ts | 48 ++++++- 4 files changed, 120 insertions(+), 214 deletions(-) delete mode 100644 lib/chats/__tests__/resolveAccessibleRoom.test.ts delete mode 100644 lib/chats/resolveAccessibleRoom.ts diff --git a/lib/chats/__tests__/resolveAccessibleRoom.test.ts b/lib/chats/__tests__/resolveAccessibleRoom.test.ts deleted file mode 100644 index c5f03d34..00000000 --- a/lib/chats/__tests__/resolveAccessibleRoom.test.ts +++ /dev/null @@ -1,117 +0,0 @@ -import { describe, it, expect, vi, beforeEach } from "vitest"; -import { NextRequest, NextResponse } from "next/server"; -import { resolveAccessibleRoom } from "../resolveAccessibleRoom"; -import { validateAuthContext } from "@/lib/auth/validateAuthContext"; -import selectRoom from "@/lib/supabase/rooms/selectRoom"; -import { buildGetChatsParams } from "@/lib/chats/buildGetChatsParams"; - -vi.mock("@/lib/networking/getCorsHeaders", () => ({ - getCorsHeaders: vi.fn(() => ({ "Access-Control-Allow-Origin": "*" })), -})); - -vi.mock("@/lib/auth/validateAuthContext", () => ({ - validateAuthContext: vi.fn(), -})); - -vi.mock("@/lib/supabase/rooms/selectRoom", () => ({ - default: vi.fn(), -})); - -vi.mock("@/lib/chats/buildGetChatsParams", () => ({ - buildGetChatsParams: vi.fn(), -})); - -describe("resolveAccessibleRoom", () => { - const roomId = "123e4567-e89b-12d3-a456-426614174000"; - const accountId = "123e4567-e89b-12d3-a456-426614174001"; - const request = new NextRequest("http://localhost/api/chats", { - headers: { "x-api-key": "test-key" }, - }); - - beforeEach(() => { - vi.clearAllMocks(); - }); - - it("returns 400 when roomId is invalid uuid", async () => { - const result = await resolveAccessibleRoom(request, "invalid-id"); - expect(result).toBeInstanceOf(NextResponse); - const response = result as NextResponse; - expect(response.status).toBe(400); - }); - - it("returns auth response when auth fails", async () => { - vi.mocked(validateAuthContext).mockResolvedValue( - NextResponse.json({ status: "error", error: "Unauthorized" }, { status: 401 }), - ); - - const result = await resolveAccessibleRoom(request, roomId); - expect(result).toBeInstanceOf(NextResponse); - const response = result as NextResponse; - expect(response.status).toBe(401); - }); - - it("returns 404 when room does not exist", async () => { - vi.mocked(validateAuthContext).mockResolvedValue({ - accountId, - orgId: null, - authToken: "test-key", - }); - vi.mocked(selectRoom).mockResolvedValue(null); - - const result = await resolveAccessibleRoom(request, roomId); - expect(result).toBeInstanceOf(NextResponse); - const response = result as NextResponse; - expect(response.status).toBe(404); - }); - - it("returns 403 when room belongs to inaccessible account", async () => { - vi.mocked(validateAuthContext).mockResolvedValue({ - accountId, - orgId: null, - authToken: "test-key", - }); - vi.mocked(selectRoom).mockResolvedValue({ - id: roomId, - account_id: "another-account", - artist_id: null, - topic: "Topic", - updated_at: "2026-03-30T00:00:00Z", - }); - vi.mocked(buildGetChatsParams).mockResolvedValue({ - params: { account_ids: [accountId] }, - error: null, - }); - - const result = await resolveAccessibleRoom(request, roomId); - expect(result).toBeInstanceOf(NextResponse); - const response = result as NextResponse; - expect(response.status).toBe(403); - }); - - it("returns room and accountId for accessible room", async () => { - const room = { - id: roomId, - account_id: accountId, - artist_id: null, - topic: "Topic", - updated_at: "2026-03-30T00:00:00Z", - }; - - vi.mocked(validateAuthContext).mockResolvedValue({ - accountId, - orgId: null, - authToken: "test-key", - }); - vi.mocked(selectRoom).mockResolvedValue(room); - vi.mocked(buildGetChatsParams).mockResolvedValue({ - params: { account_ids: [accountId] }, - error: null, - }); - - const result = await resolveAccessibleRoom(request, roomId); - expect(result).toEqual({ - room, - accountId, - }); - }); -}); diff --git a/lib/chats/__tests__/validateChatAccess.test.ts b/lib/chats/__tests__/validateChatAccess.test.ts index ee39adcc..d4d8e271 100644 --- a/lib/chats/__tests__/validateChatAccess.test.ts +++ b/lib/chats/__tests__/validateChatAccess.test.ts @@ -1,18 +1,29 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; import { NextRequest, NextResponse } from "next/server"; import { validateChatAccess } from "../validateChatAccess"; -import { resolveAccessibleRoom } from "../resolveAccessibleRoom"; +import { validateAuthContext } from "@/lib/auth/validateAuthContext"; +import selectRoom from "@/lib/supabase/rooms/selectRoom"; +import { buildGetChatsParams } from "@/lib/chats/buildGetChatsParams"; vi.mock("@/lib/networking/getCorsHeaders", () => ({ getCorsHeaders: vi.fn(() => ({ "Access-Control-Allow-Origin": "*" })), })); -vi.mock("../resolveAccessibleRoom", () => ({ - resolveAccessibleRoom: vi.fn(), +vi.mock("@/lib/auth/validateAuthContext", () => ({ + validateAuthContext: vi.fn(), +})); + +vi.mock("@/lib/supabase/rooms/selectRoom", () => ({ + default: vi.fn(), +})); + +vi.mock("@/lib/chats/buildGetChatsParams", () => ({ + buildGetChatsParams: vi.fn(), })); describe("validateChatAccess", () => { const roomId = "123e4567-e89b-12d3-a456-426614174000"; + const accountId = "123e4567-e89b-12d3-a456-426614174001"; const request = new NextRequest("http://localhost/api/chats", { headers: { "x-api-key": "test-key" }, }); @@ -21,25 +32,15 @@ describe("validateChatAccess", () => { vi.clearAllMocks(); }); - it("returns roomId when account has access", async () => { - vi.mocked(resolveAccessibleRoom).mockResolvedValue({ - id: roomId, - accountId: "123e4567-e89b-12d3-a456-426614174001", - room: { - id: roomId, - account_id: "123e4567-e89b-12d3-a456-426614174001", - artist_id: null, - topic: "Topic", - updated_at: "2026-03-30T00:00:00Z", - }, - }); - - const result = await validateChatAccess(request, roomId); - expect(result).toEqual({ roomId }); + it("returns 400 when roomId is invalid uuid", async () => { + const result = await validateChatAccess(request, "invalid-id"); + expect(result).toBeInstanceOf(NextResponse); + const response = result as NextResponse; + expect(response.status).toBe(400); }); - it("returns resolver response when access resolution fails", async () => { - vi.mocked(resolveAccessibleRoom).mockResolvedValue( + it("returns auth response when auth fails", async () => { + vi.mocked(validateAuthContext).mockResolvedValue( NextResponse.json({ status: "error", error: "Unauthorized" }, { status: 401 }), ); @@ -49,13 +50,65 @@ describe("validateChatAccess", () => { expect(response.status).toBe(401); }); - it("returns resolver 404 response unchanged", async () => { - vi.mocked(resolveAccessibleRoom).mockResolvedValue( - NextResponse.json({ status: "error", error: "Chat room not found" }, { status: 404 }), - ); + it("returns 404 when room does not exist", async () => { + vi.mocked(validateAuthContext).mockResolvedValue({ + accountId, + orgId: null, + authToken: "test-key", + }); + vi.mocked(selectRoom).mockResolvedValue(null); + const result = await validateChatAccess(request, roomId); expect(result).toBeInstanceOf(NextResponse); const response = result as NextResponse; expect(response.status).toBe(404); }); + + it("returns 403 when room belongs to inaccessible account", async () => { + vi.mocked(validateAuthContext).mockResolvedValue({ + accountId, + orgId: null, + authToken: "test-key", + }); + vi.mocked(selectRoom).mockResolvedValue({ + id: roomId, + account_id: "another-account", + artist_id: null, + topic: "Topic", + updated_at: "2026-03-30T00:00:00Z", + }); + vi.mocked(buildGetChatsParams).mockResolvedValue({ + params: { account_ids: [accountId] }, + error: null, + }); + + const result = await validateChatAccess(request, roomId); + expect(result).toBeInstanceOf(NextResponse); + const response = result as NextResponse; + expect(response.status).toBe(403); + }); + + it("returns roomId for accessible room", async () => { + const room = { + id: roomId, + account_id: accountId, + artist_id: null, + topic: "Topic", + updated_at: "2026-03-30T00:00:00Z", + }; + + vi.mocked(validateAuthContext).mockResolvedValue({ + accountId, + orgId: null, + authToken: "test-key", + }); + vi.mocked(selectRoom).mockResolvedValue(room); + vi.mocked(buildGetChatsParams).mockResolvedValue({ + params: { account_ids: [accountId] }, + error: null, + }); + + const result = await validateChatAccess(request, roomId); + expect(result).toEqual({ roomId }); + }); }); diff --git a/lib/chats/resolveAccessibleRoom.ts b/lib/chats/resolveAccessibleRoom.ts deleted file mode 100644 index 74044ee9..00000000 --- a/lib/chats/resolveAccessibleRoom.ts +++ /dev/null @@ -1,68 +0,0 @@ -import { NextResponse } from "next/server"; -import { z } from "zod"; -import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; -import { validateAuthContext } from "@/lib/auth/validateAuthContext"; -import selectRoom from "@/lib/supabase/rooms/selectRoom"; -import { buildGetChatsParams } from "@/lib/chats/buildGetChatsParams"; -import type { Tables } from "@/types/database.types"; - -const chatIdSchema = z.string().uuid("id must be a valid UUID"); - -export interface ResolvedAccessibleRoom { - room: Tables<"rooms">; - accountId: string; -} - -/** - * Resolves a chat room and validates access for the authenticated caller. - * - * @param request - The incoming request (used for auth context) - * @param roomId - The room/chat UUID to validate access for - * @returns The resolved room + accountId, or an error NextResponse - */ -export async function resolveAccessibleRoom( - request: Request, - roomId: string, -): Promise { - const roomIdResult = chatIdSchema.safeParse(roomId); - if (!roomIdResult.success) { - return NextResponse.json( - { status: "error", error: roomIdResult.error.issues[0]?.message || "Invalid chat ID" }, - { status: 400, headers: getCorsHeaders() }, - ); - } - - const authResult = await validateAuthContext(request); - if (authResult instanceof NextResponse) { - return authResult; - } - - const { accountId } = authResult; - - const room = await selectRoom(roomIdResult.data); - if (!room) { - return NextResponse.json( - { status: "error", error: "Chat room not found" }, - { status: 404, headers: getCorsHeaders() }, - ); - } - - const { params } = await buildGetChatsParams({ - account_id: accountId, - }); - - // If params.account_ids is undefined, it means admin access (all records) - if (params.account_ids && room.account_id) { - if (!params.account_ids.includes(room.account_id)) { - return NextResponse.json( - { status: "error", error: "Access denied to this chat" }, - { status: 403, headers: getCorsHeaders() }, - ); - } - } - - return { - room, - accountId, - }; -} diff --git a/lib/chats/validateChatAccess.ts b/lib/chats/validateChatAccess.ts index 1989d24b..90811bee 100644 --- a/lib/chats/validateChatAccess.ts +++ b/lib/chats/validateChatAccess.ts @@ -1,11 +1,17 @@ import type { NextRequest } from "next/server"; import { NextResponse } from "next/server"; -import { resolveAccessibleRoom } from "@/lib/chats/resolveAccessibleRoom"; +import { z } from "zod"; +import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; +import { validateAuthContext } from "@/lib/auth/validateAuthContext"; +import selectRoom from "@/lib/supabase/rooms/selectRoom"; +import { buildGetChatsParams } from "@/lib/chats/buildGetChatsParams"; export interface ValidatedChatAccess { roomId: string; } +const chatIdSchema = z.string().uuid("id must be a valid UUID"); + /** * Validates that the authenticated caller can access a chat room. * @@ -17,10 +23,42 @@ export async function validateChatAccess( request: NextRequest, roomId: string, ): Promise { - const roomResult = await resolveAccessibleRoom(request, roomId); - if (roomResult instanceof NextResponse) { - return roomResult; + const roomIdResult = chatIdSchema.safeParse(roomId); + if (!roomIdResult.success) { + return NextResponse.json( + { status: "error", error: roomIdResult.error.issues[0]?.message || "Invalid chat ID" }, + { status: 400, headers: getCorsHeaders() }, + ); + } + + const authResult = await validateAuthContext(request); + if (authResult instanceof NextResponse) { + return authResult; + } + + const { accountId } = authResult; + + const room = await selectRoom(roomIdResult.data); + if (!room) { + return NextResponse.json( + { status: "error", error: "Chat room not found" }, + { status: 404, headers: getCorsHeaders() }, + ); + } + + const { params } = await buildGetChatsParams({ + account_id: accountId, + }); + + // If params.account_ids is undefined, it means admin access (all records) + if (params.account_ids && room.account_id) { + if (!params.account_ids.includes(room.account_id)) { + return NextResponse.json( + { status: "error", error: "Access denied to this chat" }, + { status: 403, headers: getCorsHeaders() }, + ); + } } - return { roomId: roomResult.room.id }; + return { roomId: room.id }; } From 7834150257e3c76f42b431c94261a2c07ca1218d Mon Sep 17 00:00:00 2001 From: Sweets Sweetman Date: Mon, 30 Mar 2026 21:06:24 -0500 Subject: [PATCH 8/9] refactor: simplify room deletion to rely on DB cascades, fix auth bypass - Replace deleteRoomWithRelations with deleteRoom (single query, ON DELETE CASCADE handles related records automatically) - Fix auth bypass in validateChatAccess when room.account_id is null - Handle null params from buildGetChatsParams - Sanitize error messages in deleteChatHandler (don't leak internals) - Add tests for null params and null account_id access denial Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/chats/__tests__/deleteChatHandler.test.ts | 20 +++--- .../__tests__/validateChatAccess.test.ts | 48 ++++++++++++++ lib/chats/deleteChatHandler.ts | 12 ++-- lib/chats/validateChatAccess.ts | 22 ++++--- lib/supabase/rooms/deleteRoom.ts | 18 +++++ lib/supabase/rooms/deleteRoomWithRelations.ts | 65 ------------------- 6 files changed, 96 insertions(+), 89 deletions(-) create mode 100644 lib/supabase/rooms/deleteRoom.ts delete mode 100644 lib/supabase/rooms/deleteRoomWithRelations.ts diff --git a/lib/chats/__tests__/deleteChatHandler.test.ts b/lib/chats/__tests__/deleteChatHandler.test.ts index 1d0ccb76..d8d79c9e 100644 --- a/lib/chats/__tests__/deleteChatHandler.test.ts +++ b/lib/chats/__tests__/deleteChatHandler.test.ts @@ -2,7 +2,7 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; import { NextRequest, NextResponse } from "next/server"; import { deleteChatHandler } from "../deleteChatHandler"; import { validateDeleteChatBody } from "../validateDeleteChatBody"; -import { deleteRoomWithRelations } from "@/lib/supabase/rooms/deleteRoomWithRelations"; +import { deleteRoom } from "@/lib/supabase/rooms/deleteRoom"; vi.mock("@/lib/networking/getCorsHeaders", () => ({ getCorsHeaders: vi.fn(() => ({ "Access-Control-Allow-Origin": "*" })), @@ -12,8 +12,8 @@ vi.mock("@/lib/chats/validateDeleteChatBody", () => ({ validateDeleteChatBody: vi.fn(), })); -vi.mock("@/lib/supabase/rooms/deleteRoomWithRelations", () => ({ - deleteRoomWithRelations: vi.fn(), +vi.mock("@/lib/supabase/rooms/deleteRoom", () => ({ + deleteRoom: vi.fn(), })); describe("deleteChatHandler", () => { @@ -31,7 +31,7 @@ describe("deleteChatHandler", () => { it("deletes chat and returns success response", async () => { vi.mocked(validateDeleteChatBody).mockResolvedValue({ id }); - vi.mocked(deleteRoomWithRelations).mockResolvedValue(true); + vi.mocked(deleteRoom).mockResolvedValue([]); const response = await deleteChatHandler(request); @@ -42,7 +42,7 @@ describe("deleteChatHandler", () => { id, message: "Chat deleted successfully", }); - expect(deleteRoomWithRelations).toHaveBeenCalledWith(id); + expect(deleteRoom).toHaveBeenCalledWith(id); }); it("returns validation response when request is invalid", async () => { @@ -52,12 +52,12 @@ describe("deleteChatHandler", () => { const response = await deleteChatHandler(request); expect(response.status).toBe(400); - expect(deleteRoomWithRelations).not.toHaveBeenCalled(); + expect(deleteRoom).not.toHaveBeenCalled(); }); it("returns 500 when deletion fails", async () => { vi.mocked(validateDeleteChatBody).mockResolvedValue({ id }); - vi.mocked(deleteRoomWithRelations).mockResolvedValue(false); + vi.mocked(deleteRoom).mockResolvedValue(null); const response = await deleteChatHandler(request); expect(response.status).toBe(500); @@ -66,14 +66,14 @@ describe("deleteChatHandler", () => { expect(body.error).toBe("Failed to delete chat"); }); - it("returns 500 when deletion throws", async () => { + it("returns 500 with generic message when deletion throws", async () => { vi.mocked(validateDeleteChatBody).mockResolvedValue({ id }); - vi.mocked(deleteRoomWithRelations).mockRejectedValue(new Error("Database down")); + vi.mocked(deleteRoom).mockRejectedValue(new Error("Database down")); const response = await deleteChatHandler(request); expect(response.status).toBe(500); const body = await response.json(); expect(body.status).toBe("error"); - expect(body.error).toBe("Database down"); + expect(body.error).toBe("Server error"); }); }); diff --git a/lib/chats/__tests__/validateChatAccess.test.ts b/lib/chats/__tests__/validateChatAccess.test.ts index d4d8e271..00bb430b 100644 --- a/lib/chats/__tests__/validateChatAccess.test.ts +++ b/lib/chats/__tests__/validateChatAccess.test.ts @@ -88,6 +88,54 @@ describe("validateChatAccess", () => { expect(response.status).toBe(403); }); + it("returns 403 when buildGetChatsParams returns null params", async () => { + vi.mocked(validateAuthContext).mockResolvedValue({ + accountId, + orgId: null, + authToken: "test-key", + }); + vi.mocked(selectRoom).mockResolvedValue({ + id: roomId, + account_id: accountId, + artist_id: null, + topic: "Topic", + updated_at: "2026-03-30T00:00:00Z", + }); + vi.mocked(buildGetChatsParams).mockResolvedValue({ + params: null, + error: "Access denied", + }); + + const result = await validateChatAccess(request, roomId); + expect(result).toBeInstanceOf(NextResponse); + const response = result as NextResponse; + expect(response.status).toBe(403); + }); + + it("returns 403 when room has null account_id", async () => { + vi.mocked(validateAuthContext).mockResolvedValue({ + accountId, + orgId: null, + authToken: "test-key", + }); + vi.mocked(selectRoom).mockResolvedValue({ + id: roomId, + account_id: null, + artist_id: null, + topic: "Topic", + updated_at: "2026-03-30T00:00:00Z", + }); + vi.mocked(buildGetChatsParams).mockResolvedValue({ + params: { account_ids: [accountId] }, + error: null, + }); + + const result = await validateChatAccess(request, roomId); + expect(result).toBeInstanceOf(NextResponse); + const response = result as NextResponse; + expect(response.status).toBe(403); + }); + it("returns roomId for accessible room", async () => { const room = { id: roomId, diff --git a/lib/chats/deleteChatHandler.ts b/lib/chats/deleteChatHandler.ts index b397ad44..066a118a 100644 --- a/lib/chats/deleteChatHandler.ts +++ b/lib/chats/deleteChatHandler.ts @@ -2,7 +2,7 @@ import type { NextRequest } from "next/server"; import { NextResponse } from "next/server"; import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; import { validateDeleteChatBody } from "@/lib/chats/validateDeleteChatBody"; -import { deleteRoomWithRelations } from "@/lib/supabase/rooms/deleteRoomWithRelations"; +import { deleteRoom } from "@/lib/supabase/rooms/deleteRoom"; /** * Handles DELETE /api/chats - Delete a chat room and related records. @@ -19,9 +19,9 @@ export async function deleteChatHandler(request: NextRequest): Promise { - const { data: memories, error: selectMemoriesError } = await supabase - .from("memories") - .select("id") - .eq("room_id", roomId); - - if (selectMemoriesError) { - console.error("[WARN] deleteRoomWithRelations select memories:", selectMemoriesError); - } - - const memoryIds = (memories || []).map(memory => memory.id); - - if (memoryIds.length > 0) { - const { error: deleteMemoryEmailsError } = await supabase - .from("memory_emails") - .delete() - .in("memory", memoryIds); - - if (deleteMemoryEmailsError) { - console.error("[WARN] deleteRoomWithRelations memory_emails:", deleteMemoryEmailsError); - } - } - - const { error: deleteMemoriesError } = await supabase - .from("memories") - .delete() - .eq("room_id", roomId); - if (deleteMemoriesError) { - console.error("[WARN] deleteRoomWithRelations memories:", deleteMemoriesError); - } - - const { error: deleteRoomReportsError } = await supabase - .from("room_reports") - .delete() - .eq("room_id", roomId); - if (deleteRoomReportsError) { - console.error("[WARN] deleteRoomWithRelations room_reports:", deleteRoomReportsError); - } - - const { error: deleteSegmentRoomsError } = await supabase - .from("segment_rooms") - .delete() - .eq("room_id", roomId); - if (deleteSegmentRoomsError) { - console.error("[WARN] deleteRoomWithRelations segment_rooms:", deleteSegmentRoomsError); - } - - const { error: deleteRoomError } = await supabase.from("rooms").delete().eq("id", roomId); - - if (deleteRoomError) { - console.error("[ERROR] deleteRoomWithRelations room:", deleteRoomError); - return false; - } - - return true; -} From 9669ed1f1f35fbc255b1cf5777bf756a25110789 Mon Sep 17 00:00:00 2001 From: Sweets Sweetman Date: Mon, 30 Mar 2026 21:10:55 -0500 Subject: [PATCH 9/9] fix: deleteRoom returns boolean instead of null data from Supabase Supabase .delete() without .select() returns null data on success, which was incorrectly treated as a failure. Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/chats/__tests__/deleteChatHandler.test.ts | 4 ++-- lib/chats/deleteChatHandler.ts | 2 +- lib/supabase/rooms/deleteRoom.ts | 10 +++++----- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/chats/__tests__/deleteChatHandler.test.ts b/lib/chats/__tests__/deleteChatHandler.test.ts index d8d79c9e..3e23cbd8 100644 --- a/lib/chats/__tests__/deleteChatHandler.test.ts +++ b/lib/chats/__tests__/deleteChatHandler.test.ts @@ -31,7 +31,7 @@ describe("deleteChatHandler", () => { it("deletes chat and returns success response", async () => { vi.mocked(validateDeleteChatBody).mockResolvedValue({ id }); - vi.mocked(deleteRoom).mockResolvedValue([]); + vi.mocked(deleteRoom).mockResolvedValue(true); const response = await deleteChatHandler(request); @@ -57,7 +57,7 @@ describe("deleteChatHandler", () => { it("returns 500 when deletion fails", async () => { vi.mocked(validateDeleteChatBody).mockResolvedValue({ id }); - vi.mocked(deleteRoom).mockResolvedValue(null); + vi.mocked(deleteRoom).mockResolvedValue(false); const response = await deleteChatHandler(request); expect(response.status).toBe(500); diff --git a/lib/chats/deleteChatHandler.ts b/lib/chats/deleteChatHandler.ts index 066a118a..c8d2fad8 100644 --- a/lib/chats/deleteChatHandler.ts +++ b/lib/chats/deleteChatHandler.ts @@ -21,7 +21,7 @@ export async function deleteChatHandler(request: NextRequest): Promise { + const { error } = await supabase.from("rooms").delete().eq("id", roomId); if (error) { console.error("Error deleting room:", error); - return null; + return false; } - return data; + return true; }