From bd51111506e52e933c2bcb637f6cab7319d53ac5 Mon Sep 17 00:00:00 2001 From: pradipthaadhi Date: Mon, 6 Apr 2026 18:56:45 +0700 Subject: [PATCH 1/2] feat: enhance scheduled action deletion with authentication and validation - Implemented header validation and authentication checks in the DELETE route for scheduled actions. - Added Zod schema validation for incoming request body to ensure valid UUID format. - Introduced comprehensive test cases for various scenarios including unauthorized access, invalid IDs, and successful deletion. - Updated deleteTask function to include access token in the request headers for secure deletion. - Enhanced useDeleteScheduledAction hook to retrieve access token before performing delete operation. --- .../delete/__tests__/route.test.ts | 129 ++++++++++++++++++ app/api/scheduled-actions/delete/route.ts | 54 +++++++- hooks/useDeleteScheduledAction.ts | 9 +- lib/tasks/deleteTask.ts | 15 +- 4 files changed, 200 insertions(+), 7 deletions(-) create mode 100644 app/api/scheduled-actions/delete/__tests__/route.test.ts diff --git a/app/api/scheduled-actions/delete/__tests__/route.test.ts b/app/api/scheduled-actions/delete/__tests__/route.test.ts new file mode 100644 index 000000000..a1199734f --- /dev/null +++ b/app/api/scheduled-actions/delete/__tests__/route.test.ts @@ -0,0 +1,129 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { NextRequest } from "next/server"; +import { DELETE } from "../route"; +import supabase from "@/lib/supabase/serverClient"; +import { validateHeaders } from "@/lib/chat/validateHeaders"; +import { checkAccountArtistAccess } from "@/lib/supabase/account_artist_ids/checkAccountArtistAccess"; + +vi.mock("@/lib/supabase/serverClient", () => ({ + default: { + from: vi.fn(), + }, +})); + +vi.mock("@/lib/chat/validateHeaders", () => ({ + validateHeaders: vi.fn(), +})); + +vi.mock("@/lib/supabase/account_artist_ids/checkAccountArtistAccess", () => ({ + checkAccountArtistAccess: vi.fn(), +})); + +function makeRequest(body: unknown): NextRequest { + return new Request("http://localhost/api/scheduled-actions/delete", { + method: "DELETE", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify(body), + }) as NextRequest; +} + +describe("DELETE /api/scheduled-actions/delete", () => { + const mockFrom = vi.mocked(supabase.from); + const mockValidateHeaders = vi.mocked(validateHeaders); + const mockCheckAccountArtistAccess = vi.mocked(checkAccountArtistAccess); + + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("returns 401 when caller is unauthenticated", async () => { + mockValidateHeaders.mockResolvedValueOnce({}); + + const response = await DELETE(makeRequest({ id: "1e632dc8-94aa-4f85-9f85-241213d0d2f9" })); + const data = await response.json(); + + expect(response.status).toBe(401); + expect(data).toEqual({ error: "Unauthorized" }); + expect(mockFrom).not.toHaveBeenCalled(); + }); + + it("returns 400 when id is missing or invalid", async () => { + mockValidateHeaders.mockResolvedValueOnce({ accountId: "account-123" }); + + const response = await DELETE(makeRequest({ id: "not-a-uuid" })); + const data = await response.json(); + + expect(response.status).toBe(400); + expect(data.error).toContain("UUID"); + expect(mockFrom).not.toHaveBeenCalled(); + }); + + it("returns 404 when task does not exist", async () => { + mockValidateHeaders.mockResolvedValueOnce({ accountId: "account-123" }); + + const maybeSingle = vi.fn().mockResolvedValueOnce({ data: null, error: null }); + const eq = vi.fn().mockReturnValue({ maybeSingle }); + const select = vi.fn().mockReturnValue({ eq }); + mockFrom.mockReturnValueOnce({ select } as never); + + const response = await DELETE(makeRequest({ id: "1e632dc8-94aa-4f85-9f85-241213d0d2f9" })); + const data = await response.json(); + + expect(response.status).toBe(404); + expect(data).toEqual({ error: "Task not found" }); + }); + + it("returns 403 when caller is not owner and lacks artist access", async () => { + mockValidateHeaders.mockResolvedValueOnce({ accountId: "account-123" }); + mockCheckAccountArtistAccess.mockResolvedValueOnce(false); + + const maybeSingle = vi.fn().mockResolvedValueOnce({ + data: { + id: "1e632dc8-94aa-4f85-9f85-241213d0d2f9", + account_id: "other-account", + artist_account_id: "artist-456", + }, + error: null, + }); + const eqForSelect = vi.fn().mockReturnValue({ maybeSingle }); + const select = vi.fn().mockReturnValue({ eq: eqForSelect }); + mockFrom.mockReturnValueOnce({ select } as never); + + const response = await DELETE(makeRequest({ id: "1e632dc8-94aa-4f85-9f85-241213d0d2f9" })); + const data = await response.json(); + + expect(response.status).toBe(403); + expect(data).toEqual({ error: "Forbidden" }); + }); + + it("returns 200 and deletes when caller owns the task", async () => { + mockValidateHeaders.mockResolvedValueOnce({ accountId: "account-123" }); + + const maybeSingle = vi.fn().mockResolvedValueOnce({ + data: { + id: "1e632dc8-94aa-4f85-9f85-241213d0d2f9", + account_id: "account-123", + artist_account_id: "artist-456", + }, + error: null, + }); + const eqForSelect = vi.fn().mockReturnValue({ maybeSingle }); + const select = vi.fn().mockReturnValue({ eq: eqForSelect }); + + const deleteEq = vi.fn().mockResolvedValueOnce({ error: null }); + const deleteFn = vi.fn().mockReturnValue({ eq: deleteEq }); + + mockFrom + .mockReturnValueOnce({ select } as never) + .mockReturnValueOnce({ delete: deleteFn } as never); + + const response = await DELETE(makeRequest({ id: "1e632dc8-94aa-4f85-9f85-241213d0d2f9" })); + const data = await response.json(); + + expect(response.status).toBe(200); + expect(data).toEqual({ success: true }); + expect(mockCheckAccountArtistAccess).not.toHaveBeenCalled(); + expect(deleteFn).toHaveBeenCalledOnce(); + expect(deleteEq).toHaveBeenCalledWith("id", "1e632dc8-94aa-4f85-9f85-241213d0d2f9"); + }); +}); diff --git a/app/api/scheduled-actions/delete/route.ts b/app/api/scheduled-actions/delete/route.ts index 24e5eec34..b2a6e654c 100644 --- a/app/api/scheduled-actions/delete/route.ts +++ b/app/api/scheduled-actions/delete/route.ts @@ -1,14 +1,62 @@ import { NextRequest, NextResponse } from "next/server"; import supabase from "@/lib/supabase/serverClient"; +import { z } from "zod"; +import { validateHeaders } from "@/lib/chat/validateHeaders"; +import { checkAccountArtistAccess } from "@/lib/supabase/account_artist_ids/checkAccountArtistAccess"; + +const deleteScheduledActionBodySchema = z.object({ + id: z.string().uuid("id must be a valid UUID"), +}); export async function DELETE(req: NextRequest) { - const { id } = await req.json(); + const auth = await validateHeaders(req); + if (auth instanceof Response) { + return auth; + } - if (!id) { - return NextResponse.json({ error: "Missing task id" }, { status: 400 }); + const accountId = auth.accountId; + if (!accountId) { + return NextResponse.json({ error: "Unauthorized" }, { status: 401 }); } + let body: unknown; try { + body = await req.json(); + } catch { + return NextResponse.json({ error: "Invalid JSON body" }, { status: 400 }); + } + + const parsed = deleteScheduledActionBodySchema.safeParse(body); + if (!parsed.success) { + return NextResponse.json({ error: parsed.error.issues[0]?.message ?? "Invalid id" }, { status: 400 }); + } + + try { + const { id } = parsed.data; + + const { data: scheduledAction, error: selectError } = await supabase + .from("scheduled_actions") + .select("id, account_id, artist_account_id") + .eq("id", id) + .maybeSingle(); + + if (selectError) { + throw new Error(`Failed to load task: ${selectError.message}`); + } + + if (!scheduledAction) { + return NextResponse.json({ error: "Task not found" }, { status: 404 }); + } + + const canDeleteAsOwner = scheduledAction.account_id === accountId; + const canDeleteAsArtistAccess = canDeleteAsOwner + ? true + : await checkAccountArtistAccess(accountId, scheduledAction.artist_account_id); + + if (!canDeleteAsOwner && !canDeleteAsArtistAccess) { + return NextResponse.json({ error: "Forbidden" }, { status: 403 }); + } + const { error } = await supabase .from("scheduled_actions") .delete() diff --git a/hooks/useDeleteScheduledAction.ts b/hooks/useDeleteScheduledAction.ts index cf2d043bd..5974b6cf8 100644 --- a/hooks/useDeleteScheduledAction.ts +++ b/hooks/useDeleteScheduledAction.ts @@ -1,6 +1,7 @@ import { useState } from "react"; import { toast } from "react-toastify"; import { useQueryClient } from "@tanstack/react-query"; +import { usePrivy } from "@privy-io/react-auth"; import { deleteTask } from "@/lib/tasks/deleteTask"; interface DeleteScheduledActionParams { @@ -12,6 +13,7 @@ interface DeleteScheduledActionParams { export const useDeleteScheduledAction = () => { const [isLoading, setIsLoading] = useState(false); const queryClient = useQueryClient(); + const { getAccessToken } = usePrivy(); const deleteAction = async ({ actionId, @@ -20,7 +22,12 @@ export const useDeleteScheduledAction = () => { }: DeleteScheduledActionParams) => { setIsLoading(true); try { - await deleteTask({ id: actionId }); + const accessToken = await getAccessToken(); + if (!accessToken) { + throw new Error("Please sign in to delete scheduled actions"); + } + + await deleteTask({ id: actionId, accessToken }); onSuccess?.(); toast.success(successMessage); diff --git a/lib/tasks/deleteTask.ts b/lib/tasks/deleteTask.ts index 06466865e..a7327c458 100644 --- a/lib/tasks/deleteTask.ts +++ b/lib/tasks/deleteTask.ts @@ -3,6 +3,7 @@ import { TASKS_API_URL } from "@/lib/consts"; export interface DeleteTaskParams { id: string; + accessToken: string; } const SCHEDULE_NOT_FOUND_MSG = "Schedule not found"; @@ -18,10 +19,13 @@ function isScheduleNotFoundError(errorText: string): boolean { /** * Delete task record from database when scheduler deletion isn't possible */ -async function deleteTaskFromDatabase(taskId: string): Promise { +async function deleteTaskFromDatabase(taskId: string, accessToken: string): Promise { await fetch("/api/scheduled-actions/delete", { method: "DELETE", - headers: { "Content-Type": "application/json" }, + headers: { + "Content-Type": "application/json", + Authorization: `Bearer ${accessToken}`, + }, body: JSON.stringify({ id: taskId }), }); } @@ -32,10 +36,15 @@ async function deleteTaskFromDatabase(taskId: string): Promise { */ export async function deleteTask(params: DeleteTaskParams): Promise { try { + if (!params.accessToken) { + throw new Error("Please sign in to delete scheduled actions"); + } + const response = await fetch(TASKS_API_URL, { method: "DELETE", headers: { "Content-Type": "application/json", + Authorization: `Bearer ${params.accessToken}`, }, body: JSON.stringify({ id: params.id, @@ -46,7 +55,7 @@ export async function deleteTask(params: DeleteTaskParams): Promise { const errorText = await response.text(); if (isScheduleNotFoundError(errorText)) { - await deleteTaskFromDatabase(params.id); + await deleteTaskFromDatabase(params.id, params.accessToken); return; } From 2790400dd3812542b6fa4914d3eca1c2c98981e8 Mon Sep 17 00:00:00 2001 From: pradipthaadhi Date: Tue, 7 Apr 2026 10:09:11 +0700 Subject: [PATCH 2/2] refactor: modularize scheduled action deletion logic - Replaced direct Supabase calls in the DELETE route with dedicated functions for selecting and deleting scheduled actions. - Introduced `selectScheduledActionById` and `deleteScheduledActionById` utility functions for improved code organization and readability. - Updated tests to mock new utility functions, ensuring proper validation and error handling during deletion process. --- .../delete/__tests__/route.test.ts | 65 +++++++++---------- app/api/scheduled-actions/delete/route.ts | 15 ++--- .../deleteScheduledActionById.ts | 5 ++ .../selectScheduledActionById.ts | 15 +++++ 4 files changed, 54 insertions(+), 46 deletions(-) create mode 100644 lib/supabase/scheduled_actions/deleteScheduledActionById.ts create mode 100644 lib/supabase/scheduled_actions/selectScheduledActionById.ts diff --git a/app/api/scheduled-actions/delete/__tests__/route.test.ts b/app/api/scheduled-actions/delete/__tests__/route.test.ts index a1199734f..4fc6fe109 100644 --- a/app/api/scheduled-actions/delete/__tests__/route.test.ts +++ b/app/api/scheduled-actions/delete/__tests__/route.test.ts @@ -1,15 +1,10 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { NextRequest } from "next/server"; import { DELETE } from "../route"; -import supabase from "@/lib/supabase/serverClient"; import { validateHeaders } from "@/lib/chat/validateHeaders"; import { checkAccountArtistAccess } from "@/lib/supabase/account_artist_ids/checkAccountArtistAccess"; - -vi.mock("@/lib/supabase/serverClient", () => ({ - default: { - from: vi.fn(), - }, -})); +import { deleteScheduledActionById } from "@/lib/supabase/scheduled_actions/deleteScheduledActionById"; +import { selectScheduledActionById } from "@/lib/supabase/scheduled_actions/selectScheduledActionById"; vi.mock("@/lib/chat/validateHeaders", () => ({ validateHeaders: vi.fn(), @@ -19,6 +14,14 @@ vi.mock("@/lib/supabase/account_artist_ids/checkAccountArtistAccess", () => ({ checkAccountArtistAccess: vi.fn(), })); +vi.mock("@/lib/supabase/scheduled_actions/selectScheduledActionById", () => ({ + selectScheduledActionById: vi.fn(), +})); + +vi.mock("@/lib/supabase/scheduled_actions/deleteScheduledActionById", () => ({ + deleteScheduledActionById: vi.fn(), +})); + function makeRequest(body: unknown): NextRequest { return new Request("http://localhost/api/scheduled-actions/delete", { method: "DELETE", @@ -28,9 +31,10 @@ function makeRequest(body: unknown): NextRequest { } describe("DELETE /api/scheduled-actions/delete", () => { - const mockFrom = vi.mocked(supabase.from); const mockValidateHeaders = vi.mocked(validateHeaders); const mockCheckAccountArtistAccess = vi.mocked(checkAccountArtistAccess); + const mockSelectScheduledActionById = vi.mocked(selectScheduledActionById); + const mockDeleteScheduledActionById = vi.mocked(deleteScheduledActionById); beforeEach(() => { vi.clearAllMocks(); @@ -44,7 +48,7 @@ describe("DELETE /api/scheduled-actions/delete", () => { expect(response.status).toBe(401); expect(data).toEqual({ error: "Unauthorized" }); - expect(mockFrom).not.toHaveBeenCalled(); + expect(mockSelectScheduledActionById).not.toHaveBeenCalled(); }); it("returns 400 when id is missing or invalid", async () => { @@ -55,16 +59,15 @@ describe("DELETE /api/scheduled-actions/delete", () => { expect(response.status).toBe(400); expect(data.error).toContain("UUID"); - expect(mockFrom).not.toHaveBeenCalled(); + expect(mockSelectScheduledActionById).not.toHaveBeenCalled(); }); it("returns 404 when task does not exist", async () => { mockValidateHeaders.mockResolvedValueOnce({ accountId: "account-123" }); - - const maybeSingle = vi.fn().mockResolvedValueOnce({ data: null, error: null }); - const eq = vi.fn().mockReturnValue({ maybeSingle }); - const select = vi.fn().mockReturnValue({ eq }); - mockFrom.mockReturnValueOnce({ select } as never); + mockSelectScheduledActionById.mockResolvedValueOnce({ + data: null, + error: null, + } as Awaited>); const response = await DELETE(makeRequest({ id: "1e632dc8-94aa-4f85-9f85-241213d0d2f9" })); const data = await response.json(); @@ -76,46 +79,36 @@ describe("DELETE /api/scheduled-actions/delete", () => { it("returns 403 when caller is not owner and lacks artist access", async () => { mockValidateHeaders.mockResolvedValueOnce({ accountId: "account-123" }); mockCheckAccountArtistAccess.mockResolvedValueOnce(false); - - const maybeSingle = vi.fn().mockResolvedValueOnce({ + mockSelectScheduledActionById.mockResolvedValueOnce({ data: { id: "1e632dc8-94aa-4f85-9f85-241213d0d2f9", account_id: "other-account", artist_account_id: "artist-456", }, error: null, - }); - const eqForSelect = vi.fn().mockReturnValue({ maybeSingle }); - const select = vi.fn().mockReturnValue({ eq: eqForSelect }); - mockFrom.mockReturnValueOnce({ select } as never); + } as Awaited>); const response = await DELETE(makeRequest({ id: "1e632dc8-94aa-4f85-9f85-241213d0d2f9" })); const data = await response.json(); expect(response.status).toBe(403); expect(data).toEqual({ error: "Forbidden" }); + expect(mockDeleteScheduledActionById).not.toHaveBeenCalled(); }); it("returns 200 and deletes when caller owns the task", async () => { mockValidateHeaders.mockResolvedValueOnce({ accountId: "account-123" }); - - const maybeSingle = vi.fn().mockResolvedValueOnce({ + mockSelectScheduledActionById.mockResolvedValueOnce({ data: { id: "1e632dc8-94aa-4f85-9f85-241213d0d2f9", account_id: "account-123", artist_account_id: "artist-456", }, error: null, - }); - const eqForSelect = vi.fn().mockReturnValue({ maybeSingle }); - const select = vi.fn().mockReturnValue({ eq: eqForSelect }); - - const deleteEq = vi.fn().mockResolvedValueOnce({ error: null }); - const deleteFn = vi.fn().mockReturnValue({ eq: deleteEq }); - - mockFrom - .mockReturnValueOnce({ select } as never) - .mockReturnValueOnce({ delete: deleteFn } as never); + } as Awaited>); + mockDeleteScheduledActionById.mockResolvedValueOnce({ + error: null, + } as Awaited>); const response = await DELETE(makeRequest({ id: "1e632dc8-94aa-4f85-9f85-241213d0d2f9" })); const data = await response.json(); @@ -123,7 +116,9 @@ describe("DELETE /api/scheduled-actions/delete", () => { expect(response.status).toBe(200); expect(data).toEqual({ success: true }); expect(mockCheckAccountArtistAccess).not.toHaveBeenCalled(); - expect(deleteFn).toHaveBeenCalledOnce(); - expect(deleteEq).toHaveBeenCalledWith("id", "1e632dc8-94aa-4f85-9f85-241213d0d2f9"); + expect(mockDeleteScheduledActionById).toHaveBeenCalledTimes(1); + expect(mockDeleteScheduledActionById).toHaveBeenCalledWith( + "1e632dc8-94aa-4f85-9f85-241213d0d2f9" + ); }); }); diff --git a/app/api/scheduled-actions/delete/route.ts b/app/api/scheduled-actions/delete/route.ts index b2a6e654c..f2d1c8cf6 100644 --- a/app/api/scheduled-actions/delete/route.ts +++ b/app/api/scheduled-actions/delete/route.ts @@ -1,8 +1,9 @@ import { NextRequest, NextResponse } from "next/server"; -import supabase from "@/lib/supabase/serverClient"; import { z } from "zod"; import { validateHeaders } from "@/lib/chat/validateHeaders"; import { checkAccountArtistAccess } from "@/lib/supabase/account_artist_ids/checkAccountArtistAccess"; +import { deleteScheduledActionById } from "@/lib/supabase/scheduled_actions/deleteScheduledActionById"; +import { selectScheduledActionById } from "@/lib/supabase/scheduled_actions/selectScheduledActionById"; const deleteScheduledActionBodySchema = z.object({ id: z.string().uuid("id must be a valid UUID"), @@ -34,11 +35,7 @@ export async function DELETE(req: NextRequest) { try { const { id } = parsed.data; - const { data: scheduledAction, error: selectError } = await supabase - .from("scheduled_actions") - .select("id, account_id, artist_account_id") - .eq("id", id) - .maybeSingle(); + const { data: scheduledAction, error: selectError } = await selectScheduledActionById(id); if (selectError) { throw new Error(`Failed to load task: ${selectError.message}`); @@ -57,10 +54,7 @@ export async function DELETE(req: NextRequest) { return NextResponse.json({ error: "Forbidden" }, { status: 403 }); } - const { error } = await supabase - .from("scheduled_actions") - .delete() - .eq("id", id); + const { error } = await deleteScheduledActionById(id); if (error) { throw new Error(`Failed to delete task: ${error.message}`); @@ -76,4 +70,3 @@ export async function DELETE(req: NextRequest) { } export const dynamic = "force-dynamic"; - diff --git a/lib/supabase/scheduled_actions/deleteScheduledActionById.ts b/lib/supabase/scheduled_actions/deleteScheduledActionById.ts new file mode 100644 index 000000000..6826426f6 --- /dev/null +++ b/lib/supabase/scheduled_actions/deleteScheduledActionById.ts @@ -0,0 +1,5 @@ +import supabase from "@/lib/supabase/serverClient"; + +export async function deleteScheduledActionById(id: string) { + return supabase.from("scheduled_actions").delete().eq("id", id); +} diff --git a/lib/supabase/scheduled_actions/selectScheduledActionById.ts b/lib/supabase/scheduled_actions/selectScheduledActionById.ts new file mode 100644 index 000000000..75fdd913f --- /dev/null +++ b/lib/supabase/scheduled_actions/selectScheduledActionById.ts @@ -0,0 +1,15 @@ +import supabase from "@/lib/supabase/serverClient"; +import type { Tables } from "@/types/database.types"; + +export type ScheduledActionAuthRow = Pick< + Tables<"scheduled_actions">, + "id" | "account_id" | "artist_account_id" +>; + +export async function selectScheduledActionById(id: string) { + return supabase + .from("scheduled_actions") + .select("id, account_id, artist_account_id") + .eq("id", id) + .maybeSingle(); +}