Skip to content

Commit 9b5e59c

Browse files
NicolappsNicolapps
authored
RLS: support the explicit table name APIs (#873)
1 parent 6933cbb commit 9b5e59c

File tree

2 files changed

+226
-71
lines changed

2 files changed

+226
-71
lines changed

‎packages/convex-helpers/server/rowLevelSecurity.test.ts‎

Lines changed: 142 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -84,27 +84,59 @@ describe("row level security", () => {
8484
expect(notesB).toMatchObject([{ note: "Hello from Person B" }]);
8585
});
8686

87-
test("cannot delete someone else's note", async () => {
88-
const t = convexTest(schema, modules);
89-
const noteId = await t.run(async (ctx) => {
90-
const aId = await ctx.db.insert("users", { tokenIdentifier: "Person A" });
91-
await ctx.db.insert("users", { tokenIdentifier: "Person B" });
92-
return ctx.db.insert("notes", {
93-
note: "Hello from Person A",
94-
userId: aId,
87+
describe("cannot delete someone else's note", () => {
88+
test("implicit table names", async () => {
89+
const t = convexTest(schema, modules);
90+
const noteId = await t.run(async (ctx) => {
91+
const aId = await ctx.db.insert("users", {
92+
tokenIdentifier: "Person A",
93+
});
94+
await ctx.db.insert("users", { tokenIdentifier: "Person B" });
95+
return ctx.db.insert("notes", {
96+
note: "Hello from Person A",
97+
userId: aId,
98+
});
9599
});
96-
});
97-
const asA = t.withIdentity({ tokenIdentifier: "Person A" });
98-
const asB = t.withIdentity({ tokenIdentifier: "Person B" });
99-
await expect(() =>
100-
asB.run(async (ctx) => {
100+
const asA = t.withIdentity({ tokenIdentifier: "Person A" });
101+
const asB = t.withIdentity({ tokenIdentifier: "Person B" });
102+
await expect(() =>
103+
asB.run(async (ctx) => {
104+
const rls = await withRLS(ctx);
105+
return rls.db.delete(noteId);
106+
}),
107+
).rejects.toThrow(/no read access/);
108+
await asA.run(async (ctx) => {
101109
const rls = await withRLS(ctx);
102110
return rls.db.delete(noteId);
103-
}),
104-
).rejects.toThrow(/no read access/);
105-
await asA.run(async (ctx) => {
106-
const rls = await withRLS(ctx);
107-
return rls.db.delete(noteId);
111+
});
112+
});
113+
114+
test("explicit table names", async () => {
115+
const t = convexTest(schema, modules);
116+
const noteId = await t.run(async (ctx) => {
117+
const aId = await ctx.db.insert("users", {
118+
tokenIdentifier: "Person A",
119+
});
120+
await ctx.db.insert("users", { tokenIdentifier: "Person B" });
121+
return ctx.db.insert("notes", {
122+
note: "Hello from Person A",
123+
userId: aId,
124+
});
125+
});
126+
const asA = t.withIdentity({ tokenIdentifier: "Person A" });
127+
const asB = t.withIdentity({ tokenIdentifier: "Person B" });
128+
await expect(() =>
129+
asB.run(async (ctx) => {
130+
const rls = await withRLS(ctx);
131+
// @ts-expect-error - testing new explicit table name API
132+
return rls.db.delete("notes", noteId);
133+
}),
134+
).rejects.toThrow(/no read access/);
135+
await asA.run(async (ctx) => {
136+
const rls = await withRLS(ctx);
137+
// @ts-expect-error - testing new explicit table name API
138+
return rls.db.delete("notes", noteId);
139+
});
108140
});
109141
});
110142

@@ -244,39 +276,72 @@ describe("row level security", () => {
244276
).rejects.toThrow(/insert access not allowed/);
245277
});
246278

247-
test("default deny policy blocks modifications to tables without rules", async () => {
248-
const t = convexTest(schema, modules);
249-
const docId = await t.run(async (ctx) => {
250-
await ctx.db.insert("users", { tokenIdentifier: "Person A" });
251-
return ctx.db.insert("publicData", { content: "Initial content" });
252-
});
279+
describe("default deny policy blocks modifications to tables without rules", () => {
280+
test("implicit table names", async () => {
281+
const t = convexTest(schema, modules);
282+
const docId = await t.run(async (ctx) => {
283+
await ctx.db.insert("users", { tokenIdentifier: "Person A" });
284+
return ctx.db.insert("publicData", { content: "Initial content" });
285+
});
253286

254-
const asA = t.withIdentity({ tokenIdentifier: "Person A" });
287+
const asA = t.withIdentity({ tokenIdentifier: "Person A" });
255288

256-
// Test with default allow
257-
await asA.run(async (ctx) => {
258-
const tokenIdentifier = (await ctx.auth.getUserIdentity())
259-
?.tokenIdentifier;
260-
if (!tokenIdentifier) throw new Error("Unauthenticated");
289+
// Test with default allow
290+
await asA.run(async (ctx) => {
291+
const tokenIdentifier = (await ctx.auth.getUserIdentity())
292+
?.tokenIdentifier;
293+
if (!tokenIdentifier) throw new Error("Unauthenticated");
261294

262-
const db = wrapDatabaseWriter(
263-
{ tokenIdentifier },
264-
ctx.db,
265-
{
266-
publicData: {
267-
read: async () => true, // Allow reads
295+
const db = wrapDatabaseWriter(
296+
{ tokenIdentifier },
297+
ctx.db,
298+
{
299+
publicData: {
300+
read: async () => true, // Allow reads
301+
},
268302
},
269-
},
270-
{ defaultPolicy: "allow" },
271-
);
303+
{ defaultPolicy: "allow" },
304+
);
305+
306+
// Should be able to modify (no modify rule, default allow)
307+
await db.patch(docId, { content: "Modified content" });
308+
});
309+
310+
// Test with default deny
311+
await expect(() =>
312+
asA.run(async (ctx) => {
313+
const tokenIdentifier = (await ctx.auth.getUserIdentity())
314+
?.tokenIdentifier;
315+
if (!tokenIdentifier) throw new Error("Unauthenticated");
316+
317+
const db = wrapDatabaseWriter(
318+
{ tokenIdentifier },
319+
ctx.db,
320+
{
321+
publicData: {
322+
read: async () => true, // Allow reads but no modify rule
323+
},
324+
},
325+
{ defaultPolicy: "deny" },
326+
);
272327

273-
// Should be able to modify (no modify rule, default allow)
274-
await db.patch(docId, { content: "Modified content" });
328+
// Should NOT be able to modify (no modify rule, default deny)
329+
await db.patch(docId, { content: "Blocked modification" });
330+
}),
331+
).rejects.toThrow(/write access not allowed/);
275332
});
276333

277-
// Test with default deny
278-
await expect(() =>
279-
asA.run(async (ctx) => {
334+
test("explicit table names", async () => {
335+
const t = convexTest(schema, modules);
336+
const docId = await t.run(async (ctx) => {
337+
await ctx.db.insert("users", { tokenIdentifier: "Person A" });
338+
return ctx.db.insert("publicData", { content: "Initial content" });
339+
});
340+
341+
const asA = t.withIdentity({ tokenIdentifier: "Person A" });
342+
343+
// Test with default allow
344+
await asA.run(async (ctx) => {
280345
const tokenIdentifier = (await ctx.auth.getUserIdentity())
281346
?.tokenIdentifier;
282347
if (!tokenIdentifier) throw new Error("Unauthenticated");
@@ -286,16 +351,43 @@ describe("row level security", () => {
286351
ctx.db,
287352
{
288353
publicData: {
289-
read: async () => true, // Allow reads but no modify rule
354+
read: async () => true, // Allow reads
290355
},
291356
},
292-
{ defaultPolicy: "deny" },
357+
{ defaultPolicy: "allow" },
293358
);
294359

295-
// Should NOT be able to modify (no modify rule, default deny)
296-
await db.patch(docId, { content: "Blocked modification" });
297-
}),
298-
).rejects.toThrow(/write access not allowed/);
360+
// Should be able to modify (no modify rule, default allow)
361+
// @ts-expect-error - testing new explicit table name API
362+
await db.patch("publicData", docId, { content: "Modified content" });
363+
});
364+
365+
// Test with default deny
366+
await expect(() =>
367+
asA.run(async (ctx) => {
368+
const tokenIdentifier = (await ctx.auth.getUserIdentity())
369+
?.tokenIdentifier;
370+
if (!tokenIdentifier) throw new Error("Unauthenticated");
371+
372+
const db = wrapDatabaseWriter(
373+
{ tokenIdentifier },
374+
ctx.db,
375+
{
376+
publicData: {
377+
read: async () => true, // Allow reads but no modify rule
378+
},
379+
},
380+
{ defaultPolicy: "deny" },
381+
);
382+
383+
// Should NOT be able to modify (no modify rule, default deny)
384+
// @ts-expect-error - testing new explicit table name API
385+
await db.patch("publicData", docId, {
386+
content: "Blocked modification",
387+
});
388+
}),
389+
).rejects.toThrow(/write access not allowed/);
390+
});
299391
});
300392
});
301393

‎packages/convex-helpers/server/rowLevelSecurity.ts‎

Lines changed: 84 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import type {
1111
GenericQueryCtx,
1212
QueryInitializer,
1313
TableNamesInDataModel,
14+
WithOptionalSystemFields,
1415
WithoutSystemFields,
1516
} from "convex/server";
1617
import type { GenericId } from "convex/values";
@@ -235,12 +236,18 @@ class WrapReader<Ctx, DataModel extends GenericDataModel>
235236
return await this.rules[tableName]!.read!(this.ctx, doc);
236237
}
237238

238-
async get<TableName extends string>(
239+
get<TableName extends TableNamesInDataModel<DataModel>>(
240+
table: NonUnion<TableName>,
239241
id: GenericId<TableName>,
240-
): Promise<DocumentByName<DataModel, TableName> | null> {
242+
): Promise<DocumentByName<DataModel, TableName> | null>;
243+
get<TableName extends TableNamesInDataModel<DataModel>>(
244+
id: GenericId<TableName>,
245+
): Promise<DocumentByName<DataModel, TableName> | null>;
246+
async get(arg0: any, arg1?: any): Promise<any> {
247+
const [tableName, id]: [string | null, GenericId<string>] =
248+
arg1 !== undefined ? [arg0, arg1] : [this.tableName(arg0), arg0];
241249
const doc = await this.db.get(id);
242250
if (doc) {
243-
const tableName = this.tableName(id);
244251
if (tableName && !(await this.predicate(tableName, doc))) {
245252
return null;
246253
}
@@ -291,12 +298,14 @@ class WrapWriter<Ctx, DataModel extends GenericDataModel>
291298
this.rules = rules;
292299
this.config = config ?? { defaultPolicy: "allow" };
293300
}
301+
294302
normalizeId<TableName extends TableNamesInDataModel<DataModel>>(
295303
tableName: TableName,
296304
id: string,
297305
): GenericId<TableName> | null {
298306
return this.db.normalizeId(tableName, id);
299307
}
308+
300309
async insert<TableName extends string>(
301310
table: TableName,
302311
value: any,
@@ -311,6 +320,7 @@ class WrapWriter<Ctx, DataModel extends GenericDataModel>
311320
}
312321
return await this.db.insert(table, value);
313322
}
323+
314324
tableName<TableName extends string>(
315325
id: GenericId<TableName>,
316326
): TableName | null {
@@ -321,45 +331,98 @@ class WrapWriter<Ctx, DataModel extends GenericDataModel>
321331
}
322332
return null;
323333
}
324-
async checkAuth<TableName extends string>(id: GenericId<TableName>) {
334+
335+
async checkAuth<TableName extends string>(
336+
tableNameArg: string | null,
337+
id: GenericId<TableName>,
338+
) {
325339
// Note all writes already do a `db.get` internally, so this isn't
326340
// an extra read; it's just populating the cache earlier.
327341
// Since we call `this.get`, read access controls apply and this may return
328342
// null even if the document exists.
329-
const doc = await this.get(id);
343+
const doc = tableNameArg
344+
? await this.get(tableNameArg as any, id)
345+
: await this.get(id);
330346
if (doc === null) {
331347
throw new Error("no read access or doc does not exist");
332348
}
333-
const tableName = this.tableName(id);
349+
const tableName = tableNameArg ?? this.tableName(id);
334350
if (tableName === null) {
335351
return;
336352
}
337353
if (!(await this.modifyPredicate(tableName, doc))) {
338354
throw new Error("write access not allowed");
339355
}
340356
}
341-
async patch<TableName extends string>(
357+
358+
patch<TableName extends TableNamesInDataModel<DataModel>>(
359+
table: NonUnion<TableName>,
342360
id: GenericId<TableName>,
343-
value: Partial<any>,
344-
): Promise<void> {
345-
await this.checkAuth(id);
346-
return await this.db.patch(id, value);
361+
value: Partial<DocumentByName<DataModel, TableName>>,
362+
): Promise<void>;
363+
patch<TableName extends TableNamesInDataModel<DataModel>>(
364+
id: GenericId<TableName>,
365+
value: Partial<DocumentByName<DataModel, TableName>>,
366+
): Promise<void>;
367+
async patch(arg0: any, arg1: any, arg2?: any): Promise<void> {
368+
const [tableName, id, value]: [string | null, GenericId<string>, any] =
369+
arg2 !== undefined ? [arg0, arg1, arg2] : [null, arg0, arg1];
370+
await this.checkAuth(tableName, id);
371+
return tableName
372+
? // @ts-expect-error -- patch supports 3 args since convex@1.25.4
373+
this.db.patch(tableName, id, value)
374+
: this.db.patch(id, value);
347375
}
348-
async replace<TableName extends string>(
376+
377+
replace<TableName extends TableNamesInDataModel<DataModel>>(
378+
table: NonUnion<TableName>,
349379
id: GenericId<TableName>,
350-
value: any,
351-
): Promise<void> {
352-
await this.checkAuth(id);
353-
return await this.db.replace(id, value);
380+
value: WithOptionalSystemFields<DocumentByName<DataModel, TableName>>,
381+
): Promise<void>;
382+
replace<TableName extends TableNamesInDataModel<DataModel>>(
383+
id: GenericId<TableName>,
384+
value: WithOptionalSystemFields<DocumentByName<DataModel, TableName>>,
385+
): Promise<void>;
386+
async replace(arg0: any, arg1: any, arg2?: any): Promise<void> {
387+
const [tableName, id, value]: [string | null, GenericId<string>, any] =
388+
arg2 !== undefined ? [arg0, arg1, arg2] : [null, arg0, arg1];
389+
await this.checkAuth(tableName, id);
390+
return tableName
391+
? // @ts-expect-error -- replace supports 3 args since convex@1.25.4
392+
this.db.replace(tableName, id, value)
393+
: this.db.replace(id, value);
354394
}
355-
async delete(id: GenericId<string>): Promise<void> {
356-
await this.checkAuth(id);
357-
return await this.db.delete(id);
395+
396+
delete<TableName extends TableNamesInDataModel<DataModel>>(
397+
table: NonUnion<TableName>,
398+
id: GenericId<TableName>,
399+
): Promise<void>;
400+
delete(id: GenericId<TableNamesInDataModel<DataModel>>): Promise<void>;
401+
async delete(arg0: any, arg1?: any): Promise<void> {
402+
const [tableName, id]: [string | null, GenericId<string>] =
403+
arg1 !== undefined ? [arg0, arg1] : [null, arg0];
404+
await this.checkAuth(tableName, id);
405+
406+
return tableName
407+
? // @ts-expect-error -- delete supports 2 args since convex@1.25.4
408+
this.db.delete(tableName, id)
409+
: this.db.delete(id);
358410
}
359-
get<TableName extends string>(id: GenericId<TableName>): Promise<any> {
360-
return this.reader.get(id);
411+
412+
get<TableName extends TableNamesInDataModel<DataModel>>(
413+
table: NonUnion<TableName>,
414+
id: GenericId<TableName>,
415+
): Promise<DocumentByName<DataModel, TableName> | null>;
416+
get<TableName extends TableNamesInDataModel<DataModel>>(
417+
id: GenericId<TableName>,
418+
): Promise<DocumentByName<DataModel, TableName> | null>;
419+
get(arg0: any, arg1?: any): Promise<any> {
420+
// @ts-expect-error -- get supports 2 args since convex@1.25.4
421+
return this.reader.get(arg0, arg1);
361422
}
362423
query<TableName extends string>(tableName: TableName): QueryInitializer<any> {
363424
return this.reader.query(tableName);
364425
}
365426
}
427+
428+
type NonUnion<T> = T extends never ? never : T;

0 commit comments

Comments
 (0)