Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions app/src/app/api/compositions/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
}

Expand Down Expand Up @@ -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 });
}

Expand Down
10 changes: 8 additions & 2 deletions app/src/app/api/figures/[id]/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
}

Expand All @@ -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 });
}

Expand Down
10 changes: 8 additions & 2 deletions app/src/app/api/figures/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
}

Expand All @@ -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 });
}

Expand Down
10 changes: 8 additions & 2 deletions app/src/app/api/surfaces/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
}

Expand All @@ -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 });
}

Expand Down
52 changes: 44 additions & 8 deletions app/src/lib/api/ownership.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { ownsProject, ownsFigure } from './ownership';
type Row = Record<string, string>;
type SupabaseArg = Parameters<typeof ownsProject>[0];

function fakeSupabase(tables: Record<string, Row[]>): SupabaseArg {
function fakeSupabase(tables: Record<string, Row[]>, failTable?: string): SupabaseArg {
const client = {
from(table: string) {
const filters: Row = {};
Expand All @@ -23,6 +23,9 @@ function fakeSupabase(tables: Record<string, Row[]>): 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),
Expand All @@ -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();
});
});

Expand Down
16 changes: 10 additions & 6 deletions app/src/lib/api/ownership.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,32 +15,36 @@ import type { getApiClient } from '@/lib/supabase/api-client';

type ApiSupabase = Awaited<ReturnType<typeof getApiClient>>['supabase'];

export type OwnershipResult = { owns: boolean; error: string | null };

export async function ownsProject(
supabase: ApiSupabase,
projectId: string,
userId: string,
): Promise<boolean> {
const { data } = await supabase
): Promise<OwnershipResult> {
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<boolean> {
const { data } = await supabase
): Promise<OwnershipResult> {
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);
}
Loading