diff --git a/packages/convex-helpers/server/rowLevelSecurity.test.ts b/packages/convex-helpers/server/rowLevelSecurity.test.ts index 03178113..917a4434 100644 --- a/packages/convex-helpers/server/rowLevelSecurity.test.ts +++ b/packages/convex-helpers/server/rowLevelSecurity.test.ts @@ -84,27 +84,59 @@ describe("row level security", () => { expect(notesB).toMatchObject([{ note: "Hello from Person B" }]); }); - test("cannot delete someone else's note", async () => { - const t = convexTest(schema, modules); - const noteId = await t.run(async (ctx) => { - const aId = await ctx.db.insert("users", { tokenIdentifier: "Person A" }); - await ctx.db.insert("users", { tokenIdentifier: "Person B" }); - return ctx.db.insert("notes", { - note: "Hello from Person A", - userId: aId, + describe("cannot delete someone else's note", () => { + test("implicit table names", async () => { + const t = convexTest(schema, modules); + const noteId = await t.run(async (ctx) => { + const aId = await ctx.db.insert("users", { + tokenIdentifier: "Person A", + }); + await ctx.db.insert("users", { tokenIdentifier: "Person B" }); + return ctx.db.insert("notes", { + note: "Hello from Person A", + userId: aId, + }); }); - }); - const asA = t.withIdentity({ tokenIdentifier: "Person A" }); - const asB = t.withIdentity({ tokenIdentifier: "Person B" }); - await expect(() => - asB.run(async (ctx) => { + const asA = t.withIdentity({ tokenIdentifier: "Person A" }); + const asB = t.withIdentity({ tokenIdentifier: "Person B" }); + await expect(() => + asB.run(async (ctx) => { + const rls = await withRLS(ctx); + return rls.db.delete(noteId); + }), + ).rejects.toThrow(/no read access/); + await asA.run(async (ctx) => { const rls = await withRLS(ctx); return rls.db.delete(noteId); - }), - ).rejects.toThrow(/no read access/); - await asA.run(async (ctx) => { - const rls = await withRLS(ctx); - return rls.db.delete(noteId); + }); + }); + + test("explicit table names", async () => { + const t = convexTest(schema, modules); + const noteId = await t.run(async (ctx) => { + const aId = await ctx.db.insert("users", { + tokenIdentifier: "Person A", + }); + await ctx.db.insert("users", { tokenIdentifier: "Person B" }); + return ctx.db.insert("notes", { + note: "Hello from Person A", + userId: aId, + }); + }); + const asA = t.withIdentity({ tokenIdentifier: "Person A" }); + const asB = t.withIdentity({ tokenIdentifier: "Person B" }); + await expect(() => + asB.run(async (ctx) => { + const rls = await withRLS(ctx); + // @ts-expect-error - testing new explicit table name API + return rls.db.delete("notes", noteId); + }), + ).rejects.toThrow(/no read access/); + await asA.run(async (ctx) => { + const rls = await withRLS(ctx); + // @ts-expect-error - testing new explicit table name API + return rls.db.delete("notes", noteId); + }); }); }); @@ -244,39 +276,72 @@ describe("row level security", () => { ).rejects.toThrow(/insert access not allowed/); }); - test("default deny policy blocks modifications to tables without rules", async () => { - const t = convexTest(schema, modules); - const docId = await t.run(async (ctx) => { - await ctx.db.insert("users", { tokenIdentifier: "Person A" }); - return ctx.db.insert("publicData", { content: "Initial content" }); - }); + describe("default deny policy blocks modifications to tables without rules", () => { + test("implicit table names", async () => { + const t = convexTest(schema, modules); + const docId = await t.run(async (ctx) => { + await ctx.db.insert("users", { tokenIdentifier: "Person A" }); + return ctx.db.insert("publicData", { content: "Initial content" }); + }); - const asA = t.withIdentity({ tokenIdentifier: "Person A" }); + const asA = t.withIdentity({ tokenIdentifier: "Person A" }); - // Test with default allow - await asA.run(async (ctx) => { - const tokenIdentifier = (await ctx.auth.getUserIdentity()) - ?.tokenIdentifier; - if (!tokenIdentifier) throw new Error("Unauthenticated"); + // Test with default allow + await asA.run(async (ctx) => { + const tokenIdentifier = (await ctx.auth.getUserIdentity()) + ?.tokenIdentifier; + if (!tokenIdentifier) throw new Error("Unauthenticated"); - const db = wrapDatabaseWriter( - { tokenIdentifier }, - ctx.db, - { - publicData: { - read: async () => true, // Allow reads + const db = wrapDatabaseWriter( + { tokenIdentifier }, + ctx.db, + { + publicData: { + read: async () => true, // Allow reads + }, }, - }, - { defaultPolicy: "allow" }, - ); + { defaultPolicy: "allow" }, + ); + + // Should be able to modify (no modify rule, default allow) + await db.patch(docId, { content: "Modified content" }); + }); + + // Test with default deny + await expect(() => + asA.run(async (ctx) => { + const tokenIdentifier = (await ctx.auth.getUserIdentity()) + ?.tokenIdentifier; + if (!tokenIdentifier) throw new Error("Unauthenticated"); + + const db = wrapDatabaseWriter( + { tokenIdentifier }, + ctx.db, + { + publicData: { + read: async () => true, // Allow reads but no modify rule + }, + }, + { defaultPolicy: "deny" }, + ); - // Should be able to modify (no modify rule, default allow) - await db.patch(docId, { content: "Modified content" }); + // Should NOT be able to modify (no modify rule, default deny) + await db.patch(docId, { content: "Blocked modification" }); + }), + ).rejects.toThrow(/write access not allowed/); }); - // Test with default deny - await expect(() => - asA.run(async (ctx) => { + test("explicit table names", async () => { + const t = convexTest(schema, modules); + const docId = await t.run(async (ctx) => { + await ctx.db.insert("users", { tokenIdentifier: "Person A" }); + return ctx.db.insert("publicData", { content: "Initial content" }); + }); + + const asA = t.withIdentity({ tokenIdentifier: "Person A" }); + + // Test with default allow + await asA.run(async (ctx) => { const tokenIdentifier = (await ctx.auth.getUserIdentity()) ?.tokenIdentifier; if (!tokenIdentifier) throw new Error("Unauthenticated"); @@ -286,16 +351,43 @@ describe("row level security", () => { ctx.db, { publicData: { - read: async () => true, // Allow reads but no modify rule + read: async () => true, // Allow reads }, }, - { defaultPolicy: "deny" }, + { defaultPolicy: "allow" }, ); - // Should NOT be able to modify (no modify rule, default deny) - await db.patch(docId, { content: "Blocked modification" }); - }), - ).rejects.toThrow(/write access not allowed/); + // Should be able to modify (no modify rule, default allow) + // @ts-expect-error - testing new explicit table name API + await db.patch("publicData", docId, { content: "Modified content" }); + }); + + // Test with default deny + await expect(() => + asA.run(async (ctx) => { + const tokenIdentifier = (await ctx.auth.getUserIdentity()) + ?.tokenIdentifier; + if (!tokenIdentifier) throw new Error("Unauthenticated"); + + const db = wrapDatabaseWriter( + { tokenIdentifier }, + ctx.db, + { + publicData: { + read: async () => true, // Allow reads but no modify rule + }, + }, + { defaultPolicy: "deny" }, + ); + + // Should NOT be able to modify (no modify rule, default deny) + // @ts-expect-error - testing new explicit table name API + await db.patch("publicData", docId, { + content: "Blocked modification", + }); + }), + ).rejects.toThrow(/write access not allowed/); + }); }); }); diff --git a/packages/convex-helpers/server/rowLevelSecurity.ts b/packages/convex-helpers/server/rowLevelSecurity.ts index 4743fb88..73e93812 100644 --- a/packages/convex-helpers/server/rowLevelSecurity.ts +++ b/packages/convex-helpers/server/rowLevelSecurity.ts @@ -11,6 +11,7 @@ import type { GenericQueryCtx, QueryInitializer, TableNamesInDataModel, + WithOptionalSystemFields, WithoutSystemFields, } from "convex/server"; import type { GenericId } from "convex/values"; @@ -235,12 +236,18 @@ class WrapReader return await this.rules[tableName]!.read!(this.ctx, doc); } - async get( + get>( + table: NonUnion, id: GenericId, - ): Promise | null> { + ): Promise | null>; + get>( + id: GenericId, + ): Promise | null>; + async get(arg0: any, arg1?: any): Promise { + const [tableName, id]: [string | null, GenericId] = + arg1 !== undefined ? [arg0, arg1] : [this.tableName(arg0), arg0]; const doc = await this.db.get(id); if (doc) { - const tableName = this.tableName(id); if (tableName && !(await this.predicate(tableName, doc))) { return null; } @@ -291,12 +298,14 @@ class WrapWriter this.rules = rules; this.config = config ?? { defaultPolicy: "allow" }; } + normalizeId>( tableName: TableName, id: string, ): GenericId | null { return this.db.normalizeId(tableName, id); } + async insert( table: TableName, value: any, @@ -311,6 +320,7 @@ class WrapWriter } return await this.db.insert(table, value); } + tableName( id: GenericId, ): TableName | null { @@ -321,16 +331,22 @@ class WrapWriter } return null; } - async checkAuth(id: GenericId) { + + async checkAuth( + tableNameArg: string | null, + id: GenericId, + ) { // Note all writes already do a `db.get` internally, so this isn't // an extra read; it's just populating the cache earlier. // Since we call `this.get`, read access controls apply and this may return // null even if the document exists. - const doc = await this.get(id); + const doc = tableNameArg + ? await this.get(tableNameArg as any, id) + : await this.get(id); if (doc === null) { throw new Error("no read access or doc does not exist"); } - const tableName = this.tableName(id); + const tableName = tableNameArg ?? this.tableName(id); if (tableName === null) { return; } @@ -338,28 +354,75 @@ class WrapWriter throw new Error("write access not allowed"); } } - async patch( + + patch>( + table: NonUnion, id: GenericId, - value: Partial, - ): Promise { - await this.checkAuth(id); - return await this.db.patch(id, value); + value: Partial>, + ): Promise; + patch>( + id: GenericId, + value: Partial>, + ): Promise; + async patch(arg0: any, arg1: any, arg2?: any): Promise { + const [tableName, id, value]: [string | null, GenericId, any] = + arg2 !== undefined ? [arg0, arg1, arg2] : [null, arg0, arg1]; + await this.checkAuth(tableName, id); + return tableName + ? // @ts-expect-error -- patch supports 3 args since convex@1.25.4 + this.db.patch(tableName, id, value) + : this.db.patch(id, value); } - async replace( + + replace>( + table: NonUnion, id: GenericId, - value: any, - ): Promise { - await this.checkAuth(id); - return await this.db.replace(id, value); + value: WithOptionalSystemFields>, + ): Promise; + replace>( + id: GenericId, + value: WithOptionalSystemFields>, + ): Promise; + async replace(arg0: any, arg1: any, arg2?: any): Promise { + const [tableName, id, value]: [string | null, GenericId, any] = + arg2 !== undefined ? [arg0, arg1, arg2] : [null, arg0, arg1]; + await this.checkAuth(tableName, id); + return tableName + ? // @ts-expect-error -- replace supports 3 args since convex@1.25.4 + this.db.replace(tableName, id, value) + : this.db.replace(id, value); } - async delete(id: GenericId): Promise { - await this.checkAuth(id); - return await this.db.delete(id); + + delete>( + table: NonUnion, + id: GenericId, + ): Promise; + delete(id: GenericId>): Promise; + async delete(arg0: any, arg1?: any): Promise { + const [tableName, id]: [string | null, GenericId] = + arg1 !== undefined ? [arg0, arg1] : [null, arg0]; + await this.checkAuth(tableName, id); + + return tableName + ? // @ts-expect-error -- delete supports 2 args since convex@1.25.4 + this.db.delete(tableName, id) + : this.db.delete(id); } - get(id: GenericId): Promise { - return this.reader.get(id); + + get>( + table: NonUnion, + id: GenericId, + ): Promise | null>; + get>( + id: GenericId, + ): Promise | null>; + get(arg0: any, arg1?: any): Promise { + // @ts-expect-error -- get supports 2 args since convex@1.25.4 + return this.reader.get(arg0, arg1); } query(tableName: TableName): QueryInitializer { return this.reader.query(tableName); } } + +type NonUnion = T extends never ? never : T;