From dd3b8282f27065bd04ee66f3f98e6b9127a805d7 Mon Sep 17 00:00:00 2001 From: Sweets Sweetman Date: Mon, 2 Feb 2026 20:31:19 -0500 Subject: [PATCH 1/5] feat: add PATCH /api/chats endpoint to update chat topic Migrate Recoup-Chat/app/api/room/update/route.ts to PATCH /api/chats following the new API docs at developers.recoupable.com/api-reference/chat/update - Add PATCH method to app/api/chats/route.ts - Add lib/chats/updateChatHandler.ts for business logic - Add lib/chats/validateUpdateChatBody.ts for Zod validation - Add lib/supabase/rooms/updateRoom.ts for database operations The endpoint: - Accepts chatId (UUID) and topic (3-50 chars) in request body - Supports both x-api-key and Authorization Bearer token auth - Validates access using buildGetChatsParams (same as GET /api/chats) - Returns 404 if chat not found, 403 if no access, 200 on success Co-Authored-By: Claude Opus 4.5 --- app/api/chats/route.ts | 19 ++++++ lib/chats/updateChatHandler.ts | 97 +++++++++++++++++++++++++++++ lib/chats/validateUpdateChatBody.ts | 43 +++++++++++++ lib/supabase/rooms/updateRoom.ts | 30 +++++++++ 4 files changed, 189 insertions(+) create mode 100644 lib/chats/updateChatHandler.ts create mode 100644 lib/chats/validateUpdateChatBody.ts create mode 100644 lib/supabase/rooms/updateRoom.ts diff --git a/app/api/chats/route.ts b/app/api/chats/route.ts index ca58f6cf..dbfe7cfb 100644 --- a/app/api/chats/route.ts +++ b/app/api/chats/route.ts @@ -3,6 +3,7 @@ import { NextResponse } from "next/server"; import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; import { createChatHandler } from "@/lib/chats/createChatHandler"; import { getChatsHandler } from "@/lib/chats/getChatsHandler"; +import { updateChatHandler } from "@/lib/chats/updateChatHandler"; /** * OPTIONS handler for CORS preflight requests. @@ -52,3 +53,21 @@ export async function GET(request: NextRequest): Promise { export async function POST(request: NextRequest): Promise { return createChatHandler(request); } + +/** + * PATCH /api/chats + * + * Update a chat room's topic (display name). + * + * Authentication: x-api-key header or Authorization Bearer token required. + * + * Body parameters: + * - chatId (required): UUID of the chat room to update + * - topic (required): New display name for the chat (3-50 characters) + * + * @param request - The request object + * @returns A NextResponse with the updated chat or an error + */ +export async function PATCH(request: NextRequest): Promise { + return updateChatHandler(request); +} diff --git a/lib/chats/updateChatHandler.ts b/lib/chats/updateChatHandler.ts new file mode 100644 index 00000000..b481fe98 --- /dev/null +++ b/lib/chats/updateChatHandler.ts @@ -0,0 +1,97 @@ +import type { NextRequest } from "next/server"; +import { NextResponse } from "next/server"; +import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; +import { validateAuthContext } from "@/lib/auth/validateAuthContext"; +import { safeParseJson } from "@/lib/networking/safeParseJson"; +import { validateUpdateChatBody } from "./validateUpdateChatBody"; +import selectRoom from "@/lib/supabase/rooms/selectRoom"; +import { updateRoom } from "@/lib/supabase/rooms/updateRoom"; +import { buildGetChatsParams } from "./buildGetChatsParams"; + +/** + * Handles PATCH /api/chats - Update a chat room's topic. + * + * @param request - The NextRequest object + * @returns NextResponse with updated chat data or error + */ +export async function updateChatHandler(request: NextRequest): Promise { + const body = await safeParseJson(request); + + const validated = validateUpdateChatBody(body); + if (validated instanceof NextResponse) { + return validated; + } + + const { chatId, topic } = validated; + + const authResult = await validateAuthContext(request); + if (authResult instanceof NextResponse) { + return authResult; + } + + const { accountId, orgId } = authResult; + + try { + // Verify room exists + const room = await selectRoom(chatId); + if (!room) { + return NextResponse.json( + { status: "error", error: "Chat room not found" }, + { status: 404, headers: getCorsHeaders() }, + ); + } + + // Verify access to this chat using the same logic as GET /api/chats + const { params, error } = await buildGetChatsParams({ + account_id: accountId, + org_id: orgId, + target_account_id: room.account_id ?? undefined, + }); + + if (error) { + return NextResponse.json( + { status: "error", error: "Access denied to this chat" }, + { status: 403, headers: getCorsHeaders() }, + ); + } + + // Check if the room's account_id is in the allowed account_ids + 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() }, + ); + } + } + + // Update the room + const updated = await updateRoom(chatId, { topic }); + if (!updated) { + return NextResponse.json( + { status: "error", error: "Failed to update chat" }, + { status: 500, headers: getCorsHeaders() }, + ); + } + + return NextResponse.json( + { + status: "success", + chat: { + id: updated.id, + account_id: updated.account_id, + topic: updated.topic, + updated_at: updated.updated_at, + artist_id: updated.artist_id, + }, + }, + { 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/validateUpdateChatBody.ts b/lib/chats/validateUpdateChatBody.ts new file mode 100644 index 00000000..2feb718f --- /dev/null +++ b/lib/chats/validateUpdateChatBody.ts @@ -0,0 +1,43 @@ +import { NextResponse } from "next/server"; +import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; +import { z } from "zod"; + +/** + * Zod schema for PATCH /api/chats request body. + */ +export const updateChatBodySchema = z.object({ + chatId: z.string().uuid("chatId must be a valid UUID"), + topic: z + .string({ message: "topic is required" }) + .min(3, "topic must be between 3 and 50 characters") + .max(50, "topic must be between 3 and 50 characters"), +}); + +export type UpdateChatBody = z.infer; + +/** + * Validates request body for PATCH /api/chats. + * + * @param body - The request body + * @returns A NextResponse with an error if validation fails, or the validated body if validation passes. + */ +export function validateUpdateChatBody(body: unknown): NextResponse | UpdateChatBody { + const result = updateChatBodySchema.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(), + }, + ); + } + + return result.data; +} diff --git a/lib/supabase/rooms/updateRoom.ts b/lib/supabase/rooms/updateRoom.ts new file mode 100644 index 00000000..20714531 --- /dev/null +++ b/lib/supabase/rooms/updateRoom.ts @@ -0,0 +1,30 @@ +import supabase from "../serverClient"; +import type { Tables, TablesUpdate } from "@/types/database.types"; + +type Room = Tables<"rooms">; + +/** + * Updates a room's topic by ID. + * + * @param roomId - The ID of the room to update + * @param updates - The fields to update + * @returns The updated room data or null if not found or error + */ +export async function updateRoom( + roomId: string, + updates: TablesUpdate<"rooms">, +): Promise { + const { data, error } = await supabase + .from("rooms") + .update(updates) + .eq("id", roomId) + .select("*") + .single(); + + if (error) { + console.error("[ERROR] updateRoom:", error); + return null; + } + + return data; +} From 979e447d235eb70ddca36e10fa4e75dd077e62bc Mon Sep 17 00:00:00 2001 From: Sweets Sweetman Date: Mon, 2 Feb 2026 20:39:25 -0500 Subject: [PATCH 2/5] fix: correct access control in updateChatHandler + add TDD requirement Bug fix: - Remove target_account_id from buildGetChatsParams call which was causing "Personal API keys cannot filter by account_id" error for users updating their own chats - Access control now correctly checks if room's account_id is in the user's allowed account_ids Tests: - Add comprehensive test suite for updateChatHandler covering: - Successful updates (personal key, org key) - Validation errors (invalid UUID, topic length) - Auth errors (401) - Not found errors (404) - Access denied errors (403) Documentation: - Add TDD section to CLAUDE.md requiring tests before implementation Co-Authored-By: Claude Opus 4.5 --- CLAUDE.md | 68 +++++ lib/chats/__tests__/updateChatHandler.test.ts | 281 ++++++++++++++++++ lib/chats/updateChatHandler.ts | 13 +- 3 files changed, 352 insertions(+), 10 deletions(-) create mode 100644 lib/chats/__tests__/updateChatHandler.test.ts diff --git a/CLAUDE.md b/CLAUDE.md index 4ee4cafd..a64148a9 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -144,6 +144,74 @@ export async function selectTableName({ - All API routes should have JSDoc comments - Run `pnpm lint` before committing +## Test-Driven Development (TDD) + +**CRITICAL: Always write tests BEFORE implementing new features or fixing bugs.** + +### TDD Workflow + +1. **Write failing tests first** - Create tests in `lib/[domain]/__tests__/[filename].test.ts` that describe the expected behavior +2. **Run tests to verify they fail** - `pnpm test path/to/test.ts` +3. **Implement the code** - Write the minimum code needed to make tests pass +4. **Run tests to verify they pass** - All tests should be green +5. **Refactor if needed** - Clean up while keeping tests green + +### Test File Location + +Tests live alongside the code they test: +``` +lib/ +├── chats/ +│ ├── __tests__/ +│ │ └── updateChatHandler.test.ts +│ ├── updateChatHandler.ts +│ └── validateUpdateChatBody.ts +``` + +### Test Pattern + +```typescript +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { NextRequest, NextResponse } from "next/server"; + +// Mock dependencies +vi.mock("@/lib/networking/getCorsHeaders", () => ({ + getCorsHeaders: vi.fn(() => ({ "Access-Control-Allow-Origin": "*" })), +})); + +describe("functionName", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + describe("successful cases", () => { + it("does something when condition is met", async () => { + // Arrange + vi.mocked(dependency).mockResolvedValue(mockData); + + // Act + const result = await functionName(input); + + // Assert + expect(result.status).toBe(200); + }); + }); + + describe("error cases", () => { + it("returns 400 when validation fails", async () => { + // Test error handling + }); + }); +}); +``` + +### When to Write Tests + +- **New API endpoints**: Write tests for all success and error paths +- **New handlers**: Test business logic with mocked dependencies +- **Bug fixes**: Write a failing test that reproduces the bug, then fix it +- **Validation functions**: Test all valid and invalid input combinations + ## Authentication **Never use `account_id` in request bodies or tool schemas.** Always derive the account ID from authentication: diff --git a/lib/chats/__tests__/updateChatHandler.test.ts b/lib/chats/__tests__/updateChatHandler.test.ts new file mode 100644 index 00000000..316ee582 --- /dev/null +++ b/lib/chats/__tests__/updateChatHandler.test.ts @@ -0,0 +1,281 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { NextRequest, NextResponse } from "next/server"; +import { updateChatHandler } from "../updateChatHandler"; + +vi.mock("@/lib/networking/getCorsHeaders", () => ({ + getCorsHeaders: vi.fn(() => ({ "Access-Control-Allow-Origin": "*" })), +})); + +vi.mock("@/lib/networking/safeParseJson", () => ({ + safeParseJson: vi.fn(), +})); + +vi.mock("@/lib/auth/validateAuthContext", () => ({ + validateAuthContext: vi.fn(), +})); + +vi.mock("@/lib/supabase/rooms/selectRoom", () => ({ + default: vi.fn(), +})); + +vi.mock("@/lib/supabase/rooms/updateRoom", () => ({ + updateRoom: vi.fn(), +})); + +vi.mock("@/lib/chats/buildGetChatsParams", () => ({ + buildGetChatsParams: vi.fn(), +})); + +import { safeParseJson } from "@/lib/networking/safeParseJson"; +import { validateAuthContext } from "@/lib/auth/validateAuthContext"; +import selectRoom from "@/lib/supabase/rooms/selectRoom"; +import { updateRoom } from "@/lib/supabase/rooms/updateRoom"; +import { buildGetChatsParams } from "@/lib/chats/buildGetChatsParams"; + +describe("updateChatHandler", () => { + const mockRequest = (body: object) => { + return new NextRequest("http://localhost/api/chats", { + method: "PATCH", + headers: { "x-api-key": "test-key" }, + }); + }; + + beforeEach(() => { + vi.clearAllMocks(); + }); + + describe("successful update", () => { + it("updates chat topic when user owns the chat (personal key)", async () => { + const accountId = "123e4567-e89b-12d3-a456-426614174000"; + const chatId = "123e4567-e89b-12d3-a456-426614174001"; + const newTopic = "My Updated Chat"; + + vi.mocked(safeParseJson).mockResolvedValue({ + chatId, + topic: newTopic, + }); + + vi.mocked(validateAuthContext).mockResolvedValue({ + accountId, + orgId: null, // Personal key + authToken: "test-key", + }); + + vi.mocked(selectRoom).mockResolvedValue({ + id: chatId, + account_id: accountId, // User owns this chat + artist_id: null, + topic: "Old Topic", + updated_at: "2024-01-01T00:00:00Z", + }); + + vi.mocked(buildGetChatsParams).mockResolvedValue({ + params: { account_ids: [accountId] }, + error: null, + }); + + vi.mocked(updateRoom).mockResolvedValue({ + id: chatId, + account_id: accountId, + artist_id: null, + topic: newTopic, + updated_at: "2024-01-02T00:00:00Z", + }); + + const request = mockRequest({ chatId, topic: newTopic }); + const response = await updateChatHandler(request); + + expect(response.status).toBe(200); + const body = await response.json(); + expect(body).toEqual({ + status: "success", + chat: { + id: chatId, + account_id: accountId, + topic: newTopic, + updated_at: "2024-01-02T00:00:00Z", + artist_id: null, + }, + }); + + // Verify buildGetChatsParams was called WITHOUT target_account_id + expect(buildGetChatsParams).toHaveBeenCalledWith({ + account_id: accountId, + org_id: null, + }); + }); + + it("updates chat topic when org key has access to member's chat", async () => { + const orgId = "123e4567-e89b-12d3-a456-426614174002"; + const memberAccountId = "123e4567-e89b-12d3-a456-426614174003"; + const chatId = "123e4567-e89b-12d3-a456-426614174004"; + const newTopic = "Org Chat Updated"; + + vi.mocked(safeParseJson).mockResolvedValue({ + chatId, + topic: newTopic, + }); + + vi.mocked(validateAuthContext).mockResolvedValue({ + accountId: orgId, + orgId: orgId, + authToken: "org-key", + }); + + vi.mocked(selectRoom).mockResolvedValue({ + id: chatId, + account_id: memberAccountId, + artist_id: null, + topic: "Old Topic", + updated_at: "2024-01-01T00:00:00Z", + }); + + vi.mocked(buildGetChatsParams).mockResolvedValue({ + params: { account_ids: [memberAccountId, "other-member"] }, + error: null, + }); + + vi.mocked(updateRoom).mockResolvedValue({ + id: chatId, + account_id: memberAccountId, + artist_id: null, + topic: newTopic, + updated_at: "2024-01-02T00:00:00Z", + }); + + const request = mockRequest({ chatId, topic: newTopic }); + const response = await updateChatHandler(request); + + expect(response.status).toBe(200); + }); + }); + + describe("validation errors", () => { + it("returns 400 when chatId is not a valid UUID", async () => { + vi.mocked(safeParseJson).mockResolvedValue({ + chatId: "not-a-uuid", + topic: "Valid Topic", + }); + + const request = mockRequest({}); + const response = await updateChatHandler(request); + + expect(response.status).toBe(400); + const body = await response.json(); + expect(body.status).toBe("error"); + expect(body.error).toContain("UUID"); + }); + + it("returns 400 when topic is too short", async () => { + vi.mocked(safeParseJson).mockResolvedValue({ + chatId: "123e4567-e89b-12d3-a456-426614174000", + topic: "ab", + }); + + const request = mockRequest({}); + const response = await updateChatHandler(request); + + 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 () => { + vi.mocked(safeParseJson).mockResolvedValue({ + chatId: "123e4567-e89b-12d3-a456-426614174000", + topic: "a".repeat(51), + }); + + const request = mockRequest({}); + const response = await updateChatHandler(request); + + expect(response.status).toBe(400); + const body = await response.json(); + expect(body.error).toContain("3 and 50"); + }); + }); + + describe("authentication errors", () => { + it("returns 401 when no auth provided", async () => { + vi.mocked(safeParseJson).mockResolvedValue({ + chatId: "123e4567-e89b-12d3-a456-426614174000", + topic: "Valid Topic", + }); + + vi.mocked(validateAuthContext).mockResolvedValue( + NextResponse.json( + { status: "error", error: "Unauthorized" }, + { status: 401 }, + ), + ); + + const request = mockRequest({}); + const response = await updateChatHandler(request); + + expect(response.status).toBe(401); + }); + }); + + describe("not found errors", () => { + it("returns 404 when chat does not exist", async () => { + vi.mocked(safeParseJson).mockResolvedValue({ + chatId: "123e4567-e89b-12d3-a456-426614174000", + topic: "Valid Topic", + }); + + vi.mocked(validateAuthContext).mockResolvedValue({ + accountId: "123e4567-e89b-12d3-a456-426614174008", + orgId: null, + authToken: "key", + }); + + vi.mocked(selectRoom).mockResolvedValue(null); + + const request = mockRequest({}); + const response = await updateChatHandler(request); + + expect(response.status).toBe(404); + const body = await response.json(); + expect(body.error).toContain("not found"); + }); + }); + + describe("access denied errors", () => { + it("returns 403 when user tries to update another user's chat", async () => { + const userAccountId = "123e4567-e89b-12d3-a456-426614174005"; + const otherAccountId = "123e4567-e89b-12d3-a456-426614174006"; + const chatId = "123e4567-e89b-12d3-a456-426614174007"; + + vi.mocked(safeParseJson).mockResolvedValue({ + chatId, + topic: "Valid Topic", + }); + + vi.mocked(validateAuthContext).mockResolvedValue({ + accountId: userAccountId, + orgId: null, + authToken: "key", + }); + + vi.mocked(selectRoom).mockResolvedValue({ + id: chatId, + account_id: otherAccountId, // Different user owns this + artist_id: null, + topic: "Old Topic", + updated_at: "2024-01-01T00:00:00Z", + }); + + vi.mocked(buildGetChatsParams).mockResolvedValue({ + params: { account_ids: [userAccountId] }, // Only has access to own account + error: null, + }); + + const request = mockRequest({}); + const response = await updateChatHandler(request); + + expect(response.status).toBe(403); + const body = await response.json(); + expect(body.error).toContain("Access denied"); + }); + }); +}); diff --git a/lib/chats/updateChatHandler.ts b/lib/chats/updateChatHandler.ts index b481fe98..1e295853 100644 --- a/lib/chats/updateChatHandler.ts +++ b/lib/chats/updateChatHandler.ts @@ -41,21 +41,14 @@ export async function updateChatHandler(request: NextRequest): Promise Date: Mon, 2 Feb 2026 20:44:51 -0500 Subject: [PATCH 3/5] refactor: move JSON parsing into validateUpdateChatBody - validateUpdateChatBody now takes NextRequest instead of unknown body - Handles JSON parsing internally with try/catch - Remove safeParseJson import from updateChatHandler - Add comprehensive tests for validateUpdateChatBody (10 tests) - Update handler tests to mock validateUpdateChatBody instead of safeParseJson Co-Authored-By: Claude Opus 4.5 --- lib/chats/__tests__/updateChatHandler.test.ts | 74 +++------ .../__tests__/validateUpdateChatBody.test.ts | 151 ++++++++++++++++++ lib/chats/updateChatHandler.ts | 5 +- lib/chats/validateUpdateChatBody.ts | 16 +- 4 files changed, 190 insertions(+), 56 deletions(-) create mode 100644 lib/chats/__tests__/validateUpdateChatBody.test.ts diff --git a/lib/chats/__tests__/updateChatHandler.test.ts b/lib/chats/__tests__/updateChatHandler.test.ts index 316ee582..10f15294 100644 --- a/lib/chats/__tests__/updateChatHandler.test.ts +++ b/lib/chats/__tests__/updateChatHandler.test.ts @@ -6,8 +6,8 @@ vi.mock("@/lib/networking/getCorsHeaders", () => ({ getCorsHeaders: vi.fn(() => ({ "Access-Control-Allow-Origin": "*" })), })); -vi.mock("@/lib/networking/safeParseJson", () => ({ - safeParseJson: vi.fn(), +vi.mock("@/lib/chats/validateUpdateChatBody", () => ({ + validateUpdateChatBody: vi.fn(), })); vi.mock("@/lib/auth/validateAuthContext", () => ({ @@ -26,17 +26,18 @@ vi.mock("@/lib/chats/buildGetChatsParams", () => ({ buildGetChatsParams: vi.fn(), })); -import { safeParseJson } from "@/lib/networking/safeParseJson"; +import { validateUpdateChatBody } from "@/lib/chats/validateUpdateChatBody"; import { validateAuthContext } from "@/lib/auth/validateAuthContext"; import selectRoom from "@/lib/supabase/rooms/selectRoom"; import { updateRoom } from "@/lib/supabase/rooms/updateRoom"; import { buildGetChatsParams } from "@/lib/chats/buildGetChatsParams"; describe("updateChatHandler", () => { - const mockRequest = (body: object) => { + const mockRequest = () => { return new NextRequest("http://localhost/api/chats", { method: "PATCH", - headers: { "x-api-key": "test-key" }, + headers: { "x-api-key": "test-key", "Content-Type": "application/json" }, + body: JSON.stringify({}), }); }; @@ -50,7 +51,7 @@ describe("updateChatHandler", () => { const chatId = "123e4567-e89b-12d3-a456-426614174001"; const newTopic = "My Updated Chat"; - vi.mocked(safeParseJson).mockResolvedValue({ + vi.mocked(validateUpdateChatBody).mockResolvedValue({ chatId, topic: newTopic, }); @@ -82,7 +83,7 @@ describe("updateChatHandler", () => { updated_at: "2024-01-02T00:00:00Z", }); - const request = mockRequest({ chatId, topic: newTopic }); + const request = mockRequest(); const response = await updateChatHandler(request); expect(response.status).toBe(200); @@ -111,7 +112,7 @@ describe("updateChatHandler", () => { const chatId = "123e4567-e89b-12d3-a456-426614174004"; const newTopic = "Org Chat Updated"; - vi.mocked(safeParseJson).mockResolvedValue({ + vi.mocked(validateUpdateChatBody).mockResolvedValue({ chatId, topic: newTopic, }); @@ -143,7 +144,7 @@ describe("updateChatHandler", () => { updated_at: "2024-01-02T00:00:00Z", }); - const request = mockRequest({ chatId, topic: newTopic }); + const request = mockRequest(); const response = await updateChatHandler(request); expect(response.status).toBe(200); @@ -151,53 +152,26 @@ describe("updateChatHandler", () => { }); describe("validation errors", () => { - it("returns 400 when chatId is not a valid UUID", async () => { - vi.mocked(safeParseJson).mockResolvedValue({ - chatId: "not-a-uuid", - topic: "Valid Topic", - }); + it("returns 400 when validation fails", async () => { + vi.mocked(validateUpdateChatBody).mockResolvedValue( + NextResponse.json( + { status: "error", error: "chatId must be a valid UUID" }, + { status: 400 }, + ), + ); - const request = mockRequest({}); + const request = mockRequest(); const response = await updateChatHandler(request); expect(response.status).toBe(400); const body = await response.json(); expect(body.status).toBe("error"); - expect(body.error).toContain("UUID"); - }); - - it("returns 400 when topic is too short", async () => { - vi.mocked(safeParseJson).mockResolvedValue({ - chatId: "123e4567-e89b-12d3-a456-426614174000", - topic: "ab", - }); - - const request = mockRequest({}); - const response = await updateChatHandler(request); - - 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 () => { - vi.mocked(safeParseJson).mockResolvedValue({ - chatId: "123e4567-e89b-12d3-a456-426614174000", - topic: "a".repeat(51), - }); - - const request = mockRequest({}); - const response = await updateChatHandler(request); - - expect(response.status).toBe(400); - const body = await response.json(); - expect(body.error).toContain("3 and 50"); }); }); describe("authentication errors", () => { it("returns 401 when no auth provided", async () => { - vi.mocked(safeParseJson).mockResolvedValue({ + vi.mocked(validateUpdateChatBody).mockResolvedValue({ chatId: "123e4567-e89b-12d3-a456-426614174000", topic: "Valid Topic", }); @@ -209,7 +183,7 @@ describe("updateChatHandler", () => { ), ); - const request = mockRequest({}); + const request = mockRequest(); const response = await updateChatHandler(request); expect(response.status).toBe(401); @@ -218,7 +192,7 @@ describe("updateChatHandler", () => { describe("not found errors", () => { it("returns 404 when chat does not exist", async () => { - vi.mocked(safeParseJson).mockResolvedValue({ + vi.mocked(validateUpdateChatBody).mockResolvedValue({ chatId: "123e4567-e89b-12d3-a456-426614174000", topic: "Valid Topic", }); @@ -231,7 +205,7 @@ describe("updateChatHandler", () => { vi.mocked(selectRoom).mockResolvedValue(null); - const request = mockRequest({}); + const request = mockRequest(); const response = await updateChatHandler(request); expect(response.status).toBe(404); @@ -246,7 +220,7 @@ describe("updateChatHandler", () => { const otherAccountId = "123e4567-e89b-12d3-a456-426614174006"; const chatId = "123e4567-e89b-12d3-a456-426614174007"; - vi.mocked(safeParseJson).mockResolvedValue({ + vi.mocked(validateUpdateChatBody).mockResolvedValue({ chatId, topic: "Valid Topic", }); @@ -270,7 +244,7 @@ describe("updateChatHandler", () => { error: null, }); - const request = mockRequest({}); + const request = mockRequest(); const response = await updateChatHandler(request); expect(response.status).toBe(403); diff --git a/lib/chats/__tests__/validateUpdateChatBody.test.ts b/lib/chats/__tests__/validateUpdateChatBody.test.ts new file mode 100644 index 00000000..2e1705bb --- /dev/null +++ b/lib/chats/__tests__/validateUpdateChatBody.test.ts @@ -0,0 +1,151 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { NextRequest, NextResponse } from "next/server"; +import { validateUpdateChatBody } from "../validateUpdateChatBody"; + +vi.mock("@/lib/networking/getCorsHeaders", () => ({ + getCorsHeaders: vi.fn(() => ({ "Access-Control-Allow-Origin": "*" })), +})); + +describe("validateUpdateChatBody", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + const createRequest = (body: object) => { + return new NextRequest("http://localhost/api/chats", { + method: "PATCH", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify(body), + }); + }; + + describe("successful validation", () => { + it("returns validated body when chatId and topic are valid", async () => { + const chatId = "123e4567-e89b-12d3-a456-426614174000"; + const topic = "Valid Topic"; + 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 topic = "abc"; + const request = createRequest({ chatId, topic }); + + const result = await validateUpdateChatBody(request); + + expect(result).not.toBeInstanceOf(NextResponse); + expect(result).toEqual({ chatId, topic }); + }); + + it("accepts topic at maximum length (50 chars)", async () => { + const chatId = "123e4567-e89b-12d3-a456-426614174000"; + const topic = "a".repeat(50); + const request = createRequest({ chatId, topic }); + + const result = await validateUpdateChatBody(request); + + expect(result).not.toBeInstanceOf(NextResponse); + expect(result).toEqual({ chatId, topic }); + }); + }); + + describe("validation errors", () => { + it("returns 400 when chatId is missing", async () => { + const request = createRequest({ 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.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"); + }); + }); + + describe("JSON parsing", () => { + 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); + }); + }); +}); diff --git a/lib/chats/updateChatHandler.ts b/lib/chats/updateChatHandler.ts index 1e295853..5fb49aba 100644 --- a/lib/chats/updateChatHandler.ts +++ b/lib/chats/updateChatHandler.ts @@ -2,7 +2,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 { safeParseJson } from "@/lib/networking/safeParseJson"; import { validateUpdateChatBody } from "./validateUpdateChatBody"; import selectRoom from "@/lib/supabase/rooms/selectRoom"; import { updateRoom } from "@/lib/supabase/rooms/updateRoom"; @@ -15,9 +14,7 @@ import { buildGetChatsParams } from "./buildGetChatsParams"; * @returns NextResponse with updated chat data or error */ export async function updateChatHandler(request: NextRequest): Promise { - const body = await safeParseJson(request); - - const validated = validateUpdateChatBody(body); + const validated = await validateUpdateChatBody(request); if (validated instanceof NextResponse) { return validated; } diff --git a/lib/chats/validateUpdateChatBody.ts b/lib/chats/validateUpdateChatBody.ts index 2feb718f..8d84adc1 100644 --- a/lib/chats/validateUpdateChatBody.ts +++ b/lib/chats/validateUpdateChatBody.ts @@ -1,3 +1,4 @@ +import type { NextRequest } from "next/server"; import { NextResponse } from "next/server"; import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; import { z } from "zod"; @@ -17,11 +18,22 @@ export type UpdateChatBody = z.infer; /** * Validates request body for PATCH /api/chats. + * Parses JSON from the request and validates against the schema. * - * @param body - The request body + * @param request - The NextRequest object * @returns A NextResponse with an error if validation fails, or the validated body if validation passes. */ -export function validateUpdateChatBody(body: unknown): NextResponse | UpdateChatBody { +export async function validateUpdateChatBody( + request: NextRequest, +): Promise { + let body: unknown; + + try { + body = await request.json(); + } catch { + body = {}; + } + const result = updateChatBodySchema.safeParse(body); if (!result.success) { From a737e5d03152ff311ce0bddca4b4cfd94adefad4 Mon Sep 17 00:00:00 2001 From: Sweets Sweetman Date: Mon, 2 Feb 2026 21:13:04 -0500 Subject: [PATCH 4/5] refactor: move auth and room validation into validateUpdateChatBody - validateUpdateChatBody now handles: - JSON parsing - Body schema validation - Authentication via validateAuthContext - Room existence check via selectRoom - Returns ValidatedUpdateChat with chatId, topic, room, accountId, orgId - Handler now only handles access control and update logic - Updated tests for both validation (13 tests) and handler (7 tests) Co-Authored-By: Claude Opus 4.5 --- lib/chats/__tests__/updateChatHandler.test.ts | 143 ++++++++-------- .../__tests__/validateUpdateChatBody.test.ts | 156 +++++++++++++++++- lib/chats/updateChatHandler.ts | 20 +-- lib/chats/validateUpdateChatBody.ts | 53 +++++- 4 files changed, 264 insertions(+), 108 deletions(-) diff --git a/lib/chats/__tests__/updateChatHandler.test.ts b/lib/chats/__tests__/updateChatHandler.test.ts index 10f15294..bbe591e1 100644 --- a/lib/chats/__tests__/updateChatHandler.test.ts +++ b/lib/chats/__tests__/updateChatHandler.test.ts @@ -10,14 +10,6 @@ vi.mock("@/lib/chats/validateUpdateChatBody", () => ({ validateUpdateChatBody: vi.fn(), })); -vi.mock("@/lib/auth/validateAuthContext", () => ({ - validateAuthContext: vi.fn(), -})); - -vi.mock("@/lib/supabase/rooms/selectRoom", () => ({ - default: vi.fn(), -})); - vi.mock("@/lib/supabase/rooms/updateRoom", () => ({ updateRoom: vi.fn(), })); @@ -27,8 +19,6 @@ vi.mock("@/lib/chats/buildGetChatsParams", () => ({ })); import { validateUpdateChatBody } from "@/lib/chats/validateUpdateChatBody"; -import { validateAuthContext } from "@/lib/auth/validateAuthContext"; -import selectRoom from "@/lib/supabase/rooms/selectRoom"; import { updateRoom } from "@/lib/supabase/rooms/updateRoom"; import { buildGetChatsParams } from "@/lib/chats/buildGetChatsParams"; @@ -50,24 +40,20 @@ describe("updateChatHandler", () => { const accountId = "123e4567-e89b-12d3-a456-426614174000"; const chatId = "123e4567-e89b-12d3-a456-426614174001"; const newTopic = "My Updated Chat"; + const room = { + id: chatId, + account_id: accountId, + artist_id: null, + topic: "Old Topic", + updated_at: "2024-01-01T00:00:00Z", + }; vi.mocked(validateUpdateChatBody).mockResolvedValue({ chatId, topic: newTopic, - }); - - vi.mocked(validateAuthContext).mockResolvedValue({ + room, accountId, - orgId: null, // Personal key - authToken: "test-key", - }); - - vi.mocked(selectRoom).mockResolvedValue({ - id: chatId, - account_id: accountId, // User owns this chat - artist_id: null, - topic: "Old Topic", - updated_at: "2024-01-01T00:00:00Z", + orgId: null, }); vi.mocked(buildGetChatsParams).mockResolvedValue({ @@ -99,7 +85,7 @@ describe("updateChatHandler", () => { }, }); - // Verify buildGetChatsParams was called WITHOUT target_account_id + // Verify buildGetChatsParams was called with correct params expect(buildGetChatsParams).toHaveBeenCalledWith({ account_id: accountId, org_id: null, @@ -111,24 +97,20 @@ describe("updateChatHandler", () => { const memberAccountId = "123e4567-e89b-12d3-a456-426614174003"; const chatId = "123e4567-e89b-12d3-a456-426614174004"; const newTopic = "Org Chat Updated"; - - vi.mocked(validateUpdateChatBody).mockResolvedValue({ - chatId, - topic: newTopic, - }); - - vi.mocked(validateAuthContext).mockResolvedValue({ - accountId: orgId, - orgId: orgId, - authToken: "org-key", - }); - - vi.mocked(selectRoom).mockResolvedValue({ + const room = { id: chatId, account_id: memberAccountId, artist_id: null, topic: "Old Topic", updated_at: "2024-01-01T00:00:00Z", + }; + + vi.mocked(validateUpdateChatBody).mockResolvedValue({ + chatId, + topic: newTopic, + room, + accountId: orgId, + orgId, }); vi.mocked(buildGetChatsParams).mockResolvedValue({ @@ -152,7 +134,7 @@ describe("updateChatHandler", () => { }); describe("validation errors", () => { - it("returns 400 when validation fails", async () => { + it("returns error response from validation", async () => { vi.mocked(validateUpdateChatBody).mockResolvedValue( NextResponse.json( { status: "error", error: "chatId must be a valid UUID" }, @@ -167,16 +149,9 @@ describe("updateChatHandler", () => { const body = await response.json(); expect(body.status).toBe("error"); }); - }); - - describe("authentication errors", () => { - it("returns 401 when no auth provided", async () => { - vi.mocked(validateUpdateChatBody).mockResolvedValue({ - chatId: "123e4567-e89b-12d3-a456-426614174000", - topic: "Valid Topic", - }); - vi.mocked(validateAuthContext).mockResolvedValue( + it("returns 401 from validation when auth fails", async () => { + vi.mocked(validateUpdateChatBody).mockResolvedValue( NextResponse.json( { status: "error", error: "Unauthorized" }, { status: 401 }, @@ -188,29 +163,19 @@ describe("updateChatHandler", () => { expect(response.status).toBe(401); }); - }); - - describe("not found errors", () => { - it("returns 404 when chat does not exist", async () => { - vi.mocked(validateUpdateChatBody).mockResolvedValue({ - chatId: "123e4567-e89b-12d3-a456-426614174000", - topic: "Valid Topic", - }); - - vi.mocked(validateAuthContext).mockResolvedValue({ - accountId: "123e4567-e89b-12d3-a456-426614174008", - orgId: null, - authToken: "key", - }); - vi.mocked(selectRoom).mockResolvedValue(null); + it("returns 404 from validation when chat not found", async () => { + vi.mocked(validateUpdateChatBody).mockResolvedValue( + NextResponse.json( + { status: "error", error: "Chat room not found" }, + { status: 404 }, + ), + ); const request = mockRequest(); const response = await updateChatHandler(request); expect(response.status).toBe(404); - const body = await response.json(); - expect(body.error).toContain("not found"); }); }); @@ -219,37 +184,69 @@ describe("updateChatHandler", () => { const userAccountId = "123e4567-e89b-12d3-a456-426614174005"; const otherAccountId = "123e4567-e89b-12d3-a456-426614174006"; const chatId = "123e4567-e89b-12d3-a456-426614174007"; + const room = { + id: chatId, + account_id: otherAccountId, + artist_id: null, + topic: "Old Topic", + updated_at: "2024-01-01T00:00:00Z", + }; vi.mocked(validateUpdateChatBody).mockResolvedValue({ chatId, topic: "Valid Topic", - }); - - vi.mocked(validateAuthContext).mockResolvedValue({ + room, accountId: userAccountId, orgId: null, - authToken: "key", }); - vi.mocked(selectRoom).mockResolvedValue({ + vi.mocked(buildGetChatsParams).mockResolvedValue({ + params: { account_ids: [userAccountId] }, + error: null, + }); + + const request = mockRequest(); + const response = await updateChatHandler(request); + + expect(response.status).toBe(403); + const body = await response.json(); + expect(body.error).toContain("Access denied"); + }); + }); + + describe("update errors", () => { + it("returns 500 when updateRoom fails", async () => { + const accountId = "123e4567-e89b-12d3-a456-426614174000"; + const chatId = "123e4567-e89b-12d3-a456-426614174001"; + const room = { id: chatId, - account_id: otherAccountId, // Different user owns this + account_id: accountId, artist_id: null, topic: "Old Topic", updated_at: "2024-01-01T00:00:00Z", + }; + + vi.mocked(validateUpdateChatBody).mockResolvedValue({ + chatId, + topic: "New Topic", + room, + accountId, + orgId: null, }); vi.mocked(buildGetChatsParams).mockResolvedValue({ - params: { account_ids: [userAccountId] }, // Only has access to own account + params: { account_ids: [accountId] }, error: null, }); + vi.mocked(updateRoom).mockResolvedValue(null); + const request = mockRequest(); const response = await updateChatHandler(request); - expect(response.status).toBe(403); + expect(response.status).toBe(500); const body = await response.json(); - expect(body.error).toContain("Access denied"); + expect(body.error).toContain("Failed to update"); }); }); }); diff --git a/lib/chats/__tests__/validateUpdateChatBody.test.ts b/lib/chats/__tests__/validateUpdateChatBody.test.ts index 2e1705bb..a005dd12 100644 --- a/lib/chats/__tests__/validateUpdateChatBody.test.ts +++ b/lib/chats/__tests__/validateUpdateChatBody.test.ts @@ -6,6 +6,17 @@ 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(), +})); + +import { validateAuthContext } from "@/lib/auth/validateAuthContext"; +import selectRoom from "@/lib/supabase/rooms/selectRoom"; + describe("validateUpdateChatBody", () => { beforeEach(() => { vi.clearAllMocks(); @@ -14,47 +25,129 @@ describe("validateUpdateChatBody", () => { const createRequest = (body: object) => { return new NextRequest("http://localhost/api/chats", { method: "PATCH", - headers: { "Content-Type": "application/json" }, + headers: { "Content-Type": "application/json", "x-api-key": "test-key" }, body: JSON.stringify(body), }); }; describe("successful validation", () => { - it("returns validated body when chatId and topic are valid", async () => { + it("returns validated data with room and auth context", async () => { const chatId = "123e4567-e89b-12d3-a456-426614174000"; + const accountId = "123e4567-e89b-12d3-a456-426614174001"; const topic = "Valid Topic"; - const request = createRequest({ chatId, 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); + + const request = createRequest({ chatId, topic }); const result = await validateUpdateChatBody(request); expect(result).not.toBeInstanceOf(NextResponse); - expect(result).toEqual({ chatId, topic }); + expect(result).toEqual({ + chatId, + topic, + room, + accountId, + orgId: null, + }); }); 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 request = createRequest({ chatId, topic }); + 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); + const request = createRequest({ chatId, topic }); const result = await validateUpdateChatBody(request); expect(result).not.toBeInstanceOf(NextResponse); - expect(result).toEqual({ chatId, topic }); + expect(result).toMatchObject({ chatId, topic }); }); 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); + const request = createRequest({ chatId, topic }); + const result = await validateUpdateChatBody(request); + + expect(result).not.toBeInstanceOf(NextResponse); + expect(result).toMatchObject({ chatId, topic }); + }); + + it("includes orgId when using org key", async () => { + const chatId = "123e4567-e89b-12d3-a456-426614174000"; + const accountId = "123e4567-e89b-12d3-a456-426614174001"; + const orgId = "123e4567-e89b-12d3-a456-426614174002"; + const topic = "Valid Topic"; + 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, + authToken: "org-key", + }); + + vi.mocked(selectRoom).mockResolvedValue(room); + const request = createRequest({ chatId, topic }); const result = await validateUpdateChatBody(request); expect(result).not.toBeInstanceOf(NextResponse); - expect(result).toEqual({ chatId, topic }); + expect(result).toMatchObject({ accountId, orgId }); }); }); - describe("validation errors", () => { + describe("body validation errors", () => { it("returns 400 when chatId is missing", async () => { const request = createRequest({ topic: "Valid Topic" }); @@ -120,7 +213,7 @@ describe("validateUpdateChatBody", () => { }); }); - describe("JSON parsing", () => { + describe("JSON parsing errors", () => { it("handles invalid JSON gracefully", async () => { const request = new NextRequest("http://localhost/api/chats", { method: "PATCH", @@ -148,4 +241,49 @@ describe("validateUpdateChatBody", () => { expect(response.status).toBe(400); }); }); + + 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); + }); + }); + + 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"); + }); + }); }); diff --git a/lib/chats/updateChatHandler.ts b/lib/chats/updateChatHandler.ts index 5fb49aba..7090dd80 100644 --- a/lib/chats/updateChatHandler.ts +++ b/lib/chats/updateChatHandler.ts @@ -1,9 +1,7 @@ import type { NextRequest } from "next/server"; import { NextResponse } from "next/server"; import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; -import { validateAuthContext } from "@/lib/auth/validateAuthContext"; import { validateUpdateChatBody } from "./validateUpdateChatBody"; -import selectRoom from "@/lib/supabase/rooms/selectRoom"; import { updateRoom } from "@/lib/supabase/rooms/updateRoom"; import { buildGetChatsParams } from "./buildGetChatsParams"; @@ -19,25 +17,9 @@ export async function updateChatHandler(request: NextRequest): Promise; /** - * Validates request body for PATCH /api/chats. - * Parses JSON from the request and validates against the schema. + * Validated update chat request data. + */ +export interface ValidatedUpdateChat { + chatId: string; + topic: string; + room: Tables<"rooms">; + accountId: string; + orgId: string | null; +} + +/** + * Validates request for PATCH /api/chats. + * Parses JSON, validates schema, authenticates, and verifies room exists. * * @param request - The NextRequest object - * @returns A NextResponse with an error if validation fails, or the validated body if validation passes. + * @returns A NextResponse with an error if validation fails, or the validated data if validation passes. */ export async function validateUpdateChatBody( request: NextRequest, -): Promise { +): Promise { + // Parse JSON body let body: unknown; - try { body = await request.json(); } catch { body = {}; } + // Validate body schema const result = updateChatBodySchema.safeParse(body); - if (!result.success) { const firstError = result.error.issues[0]; return NextResponse.json( @@ -51,5 +65,30 @@ export async function validateUpdateChatBody( ); } - return result.data; + 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() }, + ); + } + + return { + chatId, + topic, + room, + accountId, + orgId, + }; } From f9be8fd5686e17f296d59b577587da463db5bed8 Mon Sep 17 00:00:00 2001 From: Sweets Sweetman Date: Mon, 2 Feb 2026 21:33:12 -0500 Subject: [PATCH 5/5] refactor: move access control into validateUpdateChatBody - validateUpdateChatBody now handles complete request validation: - JSON parsing - Body schema validation (chatId UUID, topic 3-50 chars) - Authentication via validateAuthContext - Room existence check via selectRoom - Access control via buildGetChatsParams - Returns only { chatId, topic } on success (simpler interface) - Handler now only calls updateRoom and formats response - Validation tests: 16 tests (including 2 new access denied tests) - Handler tests: 7 tests (simplified, validation errors pass through) Co-Authored-By: Claude Opus 4.5 --- lib/chats/__tests__/updateChatHandler.test.ts | 149 ++++------------ .../__tests__/validateUpdateChatBody.test.ts | 159 ++++++++++++++++-- lib/chats/updateChatHandler.ts | 21 +-- lib/chats/validateUpdateChatBody.ts | 26 ++- 4 files changed, 195 insertions(+), 160 deletions(-) diff --git a/lib/chats/__tests__/updateChatHandler.test.ts b/lib/chats/__tests__/updateChatHandler.test.ts index bbe591e1..cbb33603 100644 --- a/lib/chats/__tests__/updateChatHandler.test.ts +++ b/lib/chats/__tests__/updateChatHandler.test.ts @@ -14,13 +14,8 @@ vi.mock("@/lib/supabase/rooms/updateRoom", () => ({ updateRoom: vi.fn(), })); -vi.mock("@/lib/chats/buildGetChatsParams", () => ({ - buildGetChatsParams: vi.fn(), -})); - import { validateUpdateChatBody } from "@/lib/chats/validateUpdateChatBody"; import { updateRoom } from "@/lib/supabase/rooms/updateRoom"; -import { buildGetChatsParams } from "@/lib/chats/buildGetChatsParams"; describe("updateChatHandler", () => { const mockRequest = () => { @@ -36,29 +31,14 @@ describe("updateChatHandler", () => { }); describe("successful update", () => { - it("updates chat topic when user owns the chat (personal key)", async () => { - const accountId = "123e4567-e89b-12d3-a456-426614174000"; + it("updates chat topic and returns success response", async () => { const chatId = "123e4567-e89b-12d3-a456-426614174001"; + const accountId = "123e4567-e89b-12d3-a456-426614174000"; const newTopic = "My Updated Chat"; - const room = { - id: chatId, - account_id: accountId, - artist_id: null, - topic: "Old Topic", - updated_at: "2024-01-01T00:00:00Z", - }; vi.mocked(validateUpdateChatBody).mockResolvedValue({ chatId, topic: newTopic, - room, - accountId, - orgId: null, - }); - - vi.mocked(buildGetChatsParams).mockResolvedValue({ - params: { account_ids: [accountId] }, - error: null, }); vi.mocked(updateRoom).mockResolvedValue({ @@ -85,56 +65,12 @@ describe("updateChatHandler", () => { }, }); - // Verify buildGetChatsParams was called with correct params - expect(buildGetChatsParams).toHaveBeenCalledWith({ - account_id: accountId, - org_id: null, - }); - }); - - it("updates chat topic when org key has access to member's chat", async () => { - const orgId = "123e4567-e89b-12d3-a456-426614174002"; - const memberAccountId = "123e4567-e89b-12d3-a456-426614174003"; - const chatId = "123e4567-e89b-12d3-a456-426614174004"; - const newTopic = "Org Chat Updated"; - const room = { - id: chatId, - account_id: memberAccountId, - artist_id: null, - topic: "Old Topic", - updated_at: "2024-01-01T00:00:00Z", - }; - - vi.mocked(validateUpdateChatBody).mockResolvedValue({ - chatId, - topic: newTopic, - room, - accountId: orgId, - orgId, - }); - - vi.mocked(buildGetChatsParams).mockResolvedValue({ - params: { account_ids: [memberAccountId, "other-member"] }, - error: null, - }); - - vi.mocked(updateRoom).mockResolvedValue({ - id: chatId, - account_id: memberAccountId, - artist_id: null, - topic: newTopic, - updated_at: "2024-01-02T00:00:00Z", - }); - - const request = mockRequest(); - const response = await updateChatHandler(request); - - expect(response.status).toBe(200); + expect(updateRoom).toHaveBeenCalledWith(chatId, { topic: newTopic }); }); }); describe("validation errors", () => { - it("returns error response from validation", async () => { + it("returns 400 from validation when body is invalid", async () => { vi.mocked(validateUpdateChatBody).mockResolvedValue( NextResponse.json( { status: "error", error: "chatId must be a valid UUID" }, @@ -146,8 +82,7 @@ describe("updateChatHandler", () => { const response = await updateChatHandler(request); expect(response.status).toBe(400); - const body = await response.json(); - expect(body.status).toBe("error"); + expect(updateRoom).not.toHaveBeenCalled(); }); it("returns 401 from validation when auth fails", async () => { @@ -162,6 +97,7 @@ describe("updateChatHandler", () => { const response = await updateChatHandler(request); expect(response.status).toBe(401); + expect(updateRoom).not.toHaveBeenCalled(); }); it("returns 404 from validation when chat not found", async () => { @@ -176,67 +112,30 @@ describe("updateChatHandler", () => { const response = await updateChatHandler(request); expect(response.status).toBe(404); + expect(updateRoom).not.toHaveBeenCalled(); }); - }); - describe("access denied errors", () => { - it("returns 403 when user tries to update another user's chat", async () => { - const userAccountId = "123e4567-e89b-12d3-a456-426614174005"; - const otherAccountId = "123e4567-e89b-12d3-a456-426614174006"; - const chatId = "123e4567-e89b-12d3-a456-426614174007"; - const room = { - id: chatId, - account_id: otherAccountId, - artist_id: null, - topic: "Old Topic", - updated_at: "2024-01-01T00:00:00Z", - }; - - vi.mocked(validateUpdateChatBody).mockResolvedValue({ - chatId, - topic: "Valid Topic", - room, - accountId: userAccountId, - orgId: null, - }); - - vi.mocked(buildGetChatsParams).mockResolvedValue({ - params: { account_ids: [userAccountId] }, - error: null, - }); + it("returns 403 from validation when access denied", async () => { + vi.mocked(validateUpdateChatBody).mockResolvedValue( + NextResponse.json( + { status: "error", error: "Access denied to this chat" }, + { status: 403 }, + ), + ); const request = mockRequest(); const response = await updateChatHandler(request); expect(response.status).toBe(403); - const body = await response.json(); - expect(body.error).toContain("Access denied"); + expect(updateRoom).not.toHaveBeenCalled(); }); }); describe("update errors", () => { it("returns 500 when updateRoom fails", async () => { - const accountId = "123e4567-e89b-12d3-a456-426614174000"; - const chatId = "123e4567-e89b-12d3-a456-426614174001"; - const room = { - id: chatId, - account_id: accountId, - artist_id: null, - topic: "Old Topic", - updated_at: "2024-01-01T00:00:00Z", - }; - vi.mocked(validateUpdateChatBody).mockResolvedValue({ - chatId, + chatId: "123e4567-e89b-12d3-a456-426614174001", topic: "New Topic", - room, - accountId, - orgId: null, - }); - - vi.mocked(buildGetChatsParams).mockResolvedValue({ - params: { account_ids: [accountId] }, - error: null, }); vi.mocked(updateRoom).mockResolvedValue(null); @@ -248,5 +147,21 @@ describe("updateChatHandler", () => { const body = await response.json(); expect(body.error).toContain("Failed to update"); }); + + it("returns 500 when updateRoom throws", async () => { + vi.mocked(validateUpdateChatBody).mockResolvedValue({ + chatId: "123e4567-e89b-12d3-a456-426614174001", + topic: "New Topic", + }); + + vi.mocked(updateRoom).mockRejectedValue(new Error("Database error")); + + const request = mockRequest(); + const response = await updateChatHandler(request); + + expect(response.status).toBe(500); + const body = await response.json(); + expect(body.error).toBe("Database error"); + }); }); }); diff --git a/lib/chats/__tests__/validateUpdateChatBody.test.ts b/lib/chats/__tests__/validateUpdateChatBody.test.ts index a005dd12..d3328d98 100644 --- a/lib/chats/__tests__/validateUpdateChatBody.test.ts +++ b/lib/chats/__tests__/validateUpdateChatBody.test.ts @@ -14,8 +14,13 @@ vi.mock("@/lib/supabase/rooms/selectRoom", () => ({ default: vi.fn(), })); +vi.mock("@/lib/chats/buildGetChatsParams", () => ({ + buildGetChatsParams: vi.fn(), +})); + import { validateAuthContext } from "@/lib/auth/validateAuthContext"; import selectRoom from "@/lib/supabase/rooms/selectRoom"; +import { buildGetChatsParams } from "@/lib/chats/buildGetChatsParams"; describe("validateUpdateChatBody", () => { beforeEach(() => { @@ -31,7 +36,7 @@ describe("validateUpdateChatBody", () => { }; describe("successful validation", () => { - it("returns validated data with room and auth context", async () => { + 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"; @@ -51,17 +56,16 @@ describe("validateUpdateChatBody", () => { 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, - room, - accountId, - orgId: null, - }); + expect(result).toEqual({ chatId, topic }); }); it("accepts topic at minimum length (3 chars)", async () => { @@ -84,11 +88,16 @@ describe("validateUpdateChatBody", () => { 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).toMatchObject({ chatId, topic }); + expect(result).toEqual({ chatId, topic }); }); it("accepts topic at maximum length (50 chars)", async () => { @@ -111,39 +120,81 @@ describe("validateUpdateChatBody", () => { 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).toMatchObject({ chatId, topic }); + expect(result).toEqual({ chatId, topic }); }); - it("includes orgId when using org key", async () => { + it("allows org key to access member's chat", async () => { const chatId = "123e4567-e89b-12d3-a456-426614174000"; - const accountId = "123e4567-e89b-12d3-a456-426614174001"; + const memberAccountId = "123e4567-e89b-12d3-a456-426614174001"; const orgId = "123e4567-e89b-12d3-a456-426614174002"; const topic = "Valid Topic"; const room = { id: chatId, - account_id: accountId, + account_id: memberAccountId, artist_id: null, topic: "Old", updated_at: "2024-01-01T00:00:00Z", }; vi.mocked(validateAuthContext).mockResolvedValue({ - accountId, + 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).toMatchObject({ accountId, orgId }); + 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 }); }); }); @@ -286,4 +337,82 @@ describe("validateUpdateChatBody", () => { expect(body.error).toContain("not found"); }); }); + + 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", + }); + + vi.mocked(selectRoom).mockResolvedValue(room); + + vi.mocked(buildGetChatsParams).mockResolvedValue({ + params: { account_ids: ["member-1", "member-2"] }, // non-member not in list + 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"); + }); + }); }); diff --git a/lib/chats/updateChatHandler.ts b/lib/chats/updateChatHandler.ts index 7090dd80..dba51a74 100644 --- a/lib/chats/updateChatHandler.ts +++ b/lib/chats/updateChatHandler.ts @@ -3,7 +3,6 @@ import { NextResponse } from "next/server"; import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; import { validateUpdateChatBody } from "./validateUpdateChatBody"; import { updateRoom } from "@/lib/supabase/rooms/updateRoom"; -import { buildGetChatsParams } from "./buildGetChatsParams"; /** * Handles PATCH /api/chats - Update a chat room's topic. @@ -17,27 +16,9 @@ export async function updateChatHandler(request: NextRequest): Promise; export interface ValidatedUpdateChat { chatId: string; topic: string; - room: Tables<"rooms">; - accountId: string; - orgId: string | null; } /** * Validates request for PATCH /api/chats. - * Parses JSON, validates schema, authenticates, and verifies room exists. + * 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 the validated data if validation passes. @@ -84,11 +81,24 @@ export async function validateUpdateChatBody( ); } + // Check access control + const { params } = await buildGetChatsParams({ + account_id: accountId, + org_id: orgId, + }); + + // 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, topic, - room, - accountId, - orgId, }; }