diff --git a/app/src/app/api/compositions/route.ts b/app/src/app/api/compositions/route.ts index 3688b99..83411d3 100644 --- a/app/src/app/api/compositions/route.ts +++ b/app/src/app/api/compositions/route.ts @@ -9,7 +9,10 @@ export async function GET(request: NextRequest) { const projectId = request.nextUrl.searchParams.get('project_id'); if (!projectId) return NextResponse.json({ success: false, error: 'project_id required' }, { status: 400 }); - if (!(await ownsProject(supabase, projectId, userId))) { + const ownership = await ownsProject(supabase, projectId, userId); + if (ownership.error) + return NextResponse.json({ success: false, error: ownership.error }, { status: 500 }); + if (!ownership.owns) { return NextResponse.json({ success: false, error: 'Not found' }, { status: 404 }); } @@ -37,7 +40,10 @@ export async function POST(request: NextRequest) { ); } - if (!(await ownsProject(supabase, parsed.data.project_id, userId))) { + const ownership = await ownsProject(supabase, parsed.data.project_id, userId); + if (ownership.error) + return NextResponse.json({ success: false, error: ownership.error }, { status: 500 }); + if (!ownership.owns) { return NextResponse.json({ success: false, error: 'Not found' }, { status: 404 }); } diff --git a/app/src/app/api/figures/[id]/route.ts b/app/src/app/api/figures/[id]/route.ts index 9b48e23..3a30d30 100644 --- a/app/src/app/api/figures/[id]/route.ts +++ b/app/src/app/api/figures/[id]/route.ts @@ -17,7 +17,10 @@ export async function PATCH(request: NextRequest, context: RouteContext) { { status: 400 }, ); } - if (!(await ownsFigure(supabase, id, userId))) { + const ownership = await ownsFigure(supabase, id, userId); + if (ownership.error) + return NextResponse.json({ success: false, error: ownership.error }, { status: 500 }); + if (!ownership.owns) { return NextResponse.json({ success: false, error: 'Not found' }, { status: 404 }); } @@ -36,7 +39,10 @@ export async function DELETE(_request: NextRequest, context: RouteContext) { const { id } = await context.params; const { supabase, userId } = await getApiClient(); - if (!(await ownsFigure(supabase, id, userId))) { + const ownership = await ownsFigure(supabase, id, userId); + if (ownership.error) + return NextResponse.json({ success: false, error: ownership.error }, { status: 500 }); + if (!ownership.owns) { return NextResponse.json({ success: false, error: 'Not found' }, { status: 404 }); } diff --git a/app/src/app/api/figures/route.ts b/app/src/app/api/figures/route.ts index 7678c8e..a8bc449 100644 --- a/app/src/app/api/figures/route.ts +++ b/app/src/app/api/figures/route.ts @@ -9,7 +9,10 @@ export async function GET(request: NextRequest) { const projectId = request.nextUrl.searchParams.get('project_id'); if (!projectId) return NextResponse.json({ success: false, error: 'project_id required' }, { status: 400 }); - if (!(await ownsProject(supabase, projectId, userId))) { + const ownership = await ownsProject(supabase, projectId, userId); + if (ownership.error) + return NextResponse.json({ success: false, error: ownership.error }, { status: 500 }); + if (!ownership.owns) { return NextResponse.json({ success: false, error: 'Not found' }, { status: 404 }); } @@ -34,7 +37,10 @@ export async function POST(request: NextRequest) { { status: 400 }, ); } - if (!(await ownsProject(supabase, parsed.data.project_id, userId))) { + const ownership = await ownsProject(supabase, parsed.data.project_id, userId); + if (ownership.error) + return NextResponse.json({ success: false, error: ownership.error }, { status: 500 }); + if (!ownership.owns) { return NextResponse.json({ success: false, error: 'Not found' }, { status: 404 }); } diff --git a/app/src/app/api/surfaces/route.ts b/app/src/app/api/surfaces/route.ts index 1168ba5..df00243 100644 --- a/app/src/app/api/surfaces/route.ts +++ b/app/src/app/api/surfaces/route.ts @@ -9,7 +9,10 @@ export async function GET(request: NextRequest) { const projectId = request.nextUrl.searchParams.get('project_id'); if (!projectId) return NextResponse.json({ success: false, error: 'project_id required' }, { status: 400 }); - if (!(await ownsProject(supabase, projectId, userId))) { + const ownership = await ownsProject(supabase, projectId, userId); + if (ownership.error) + return NextResponse.json({ success: false, error: ownership.error }, { status: 500 }); + if (!ownership.owns) { return NextResponse.json({ success: false, error: 'Not found' }, { status: 404 }); } @@ -35,7 +38,10 @@ export async function POST(request: NextRequest) { ); } - if (!(await ownsProject(supabase, parsed.data.project_id, userId))) { + const ownership = await ownsProject(supabase, parsed.data.project_id, userId); + if (ownership.error) + return NextResponse.json({ success: false, error: ownership.error }, { status: 500 }); + if (!ownership.owns) { return NextResponse.json({ success: false, error: 'Not found' }, { status: 404 }); } diff --git a/app/src/lib/api/ownership.test.ts b/app/src/lib/api/ownership.test.ts index bd99bc6..1aed7a0 100644 --- a/app/src/lib/api/ownership.test.ts +++ b/app/src/lib/api/ownership.test.ts @@ -12,7 +12,7 @@ import { ownsProject, ownsFigure } from './ownership'; type Row = Record; type SupabaseArg = Parameters[0]; -function fakeSupabase(tables: Record): SupabaseArg { +function fakeSupabase(tables: Record, failTable?: string): SupabaseArg { const client = { from(table: string) { const filters: Row = {}; @@ -23,6 +23,9 @@ function fakeSupabase(tables: Record): SupabaseArg { return builder; }, maybeSingle: async () => { + if (table === failTable) { + return { data: null, error: { message: `${table} query failed` } }; + } const rows = tables[table] ?? []; const match = rows.find((row) => Object.entries(filters).every(([column, value]) => row[column] === value), @@ -46,34 +49,67 @@ const db = { describe('ownsProject', () => { it('accepts the owner', async () => { - expect(await ownsProject(fakeSupabase(db), 'project-a', OWNER)).toBe(true); + expect(await ownsProject(fakeSupabase(db), 'project-a', OWNER)).toEqual({ + owns: true, + error: null, + }); }); it('refuses someone else holding a real project id', async () => { - expect(await ownsProject(fakeSupabase(db), 'project-a', STRANGER)).toBe(false); + expect(await ownsProject(fakeSupabase(db), 'project-a', STRANGER)).toEqual({ + owns: false, + error: null, + }); }); it('refuses an id that does not exist', async () => { - expect(await ownsProject(fakeSupabase(db), 'project-nope', OWNER)).toBe(false); + expect(await ownsProject(fakeSupabase(db), 'project-nope', OWNER)).toEqual({ + owns: false, + error: null, + }); + }); + + it('surfaces a query failure instead of silently reporting not-owned', async () => { + const result = await ownsProject(fakeSupabase(db, 'projects'), 'project-a', OWNER); + expect(result.owns).toBe(false); + expect(result.error).not.toBeNull(); }); }); describe('ownsFigure', () => { it("accepts a figure inside the caller's own project", async () => { - expect(await ownsFigure(fakeSupabase(db), 'figure-a', OWNER)).toBe(true); + expect(await ownsFigure(fakeSupabase(db), 'figure-a', OWNER)).toEqual({ + owns: true, + error: null, + }); }); it("refuses a figure inside somebody else's project", async () => { - expect(await ownsFigure(fakeSupabase(db), 'figure-a', STRANGER)).toBe(false); + expect(await ownsFigure(fakeSupabase(db), 'figure-a', STRANGER)).toEqual({ + owns: false, + error: null, + }); }); it('refuses a figure that does not exist', async () => { - expect(await ownsFigure(fakeSupabase(db), 'figure-nope', OWNER)).toBe(false); + expect(await ownsFigure(fakeSupabase(db), 'figure-nope', OWNER)).toEqual({ + owns: false, + error: null, + }); }); it('refuses an orphaned figure rather than defaulting to allow', async () => { const orphan = { projects: db.projects, figures: [{ id: 'figure-orphan' }] }; - expect(await ownsFigure(fakeSupabase(orphan), 'figure-orphan', OWNER)).toBe(false); + expect(await ownsFigure(fakeSupabase(orphan), 'figure-orphan', OWNER)).toEqual({ + owns: false, + error: null, + }); + }); + + it('surfaces a figure-query failure instead of silently reporting not-owned', async () => { + const result = await ownsFigure(fakeSupabase(db, 'figures'), 'figure-a', OWNER); + expect(result.owns).toBe(false); + expect(result.error).not.toBeNull(); }); }); diff --git a/app/src/lib/api/ownership.ts b/app/src/lib/api/ownership.ts index eb76a04..855ac8d 100644 --- a/app/src/lib/api/ownership.ts +++ b/app/src/lib/api/ownership.ts @@ -15,32 +15,36 @@ import type { getApiClient } from '@/lib/supabase/api-client'; type ApiSupabase = Awaited>['supabase']; +export type OwnershipResult = { owns: boolean; error: string | null }; + export async function ownsProject( supabase: ApiSupabase, projectId: string, userId: string, -): Promise { - const { data } = await supabase +): Promise { + const { data, error } = await supabase .from('projects') .select('id') .eq('id', projectId) .eq('user_id', userId) .maybeSingle(); - return Boolean(data); + if (error) return { owns: false, error: error.message }; + return { owns: Boolean(data), error: null }; } export async function ownsFigure( supabase: ApiSupabase, figureId: string, userId: string, -): Promise { - const { data } = await supabase +): Promise { + const { data, error } = await supabase .from('figures') .select('project_id') .eq('id', figureId) .maybeSingle(); - if (!data?.project_id) return false; + if (error) return { owns: false, error: error.message }; + if (!data?.project_id) return { owns: false, error: null }; return ownsProject(supabase, data.project_id, userId); }