From 01232414a43773129d765ac9ae00437500dcb22d Mon Sep 17 00:00:00 2001 From: Ethan Greenfeld Date: Tue, 5 Aug 2025 15:35:32 -0700 Subject: [PATCH 1/2] introduce RLSConfig object. enable default deny. --- .../server/rowLevelSecurity.test.ts | 160 +++++++++++++++++- .../convex-helpers/server/rowLevelSecurity.ts | 33 +++- 2 files changed, 186 insertions(+), 7 deletions(-) diff --git a/packages/convex-helpers/server/rowLevelSecurity.test.ts b/packages/convex-helpers/server/rowLevelSecurity.test.ts index 0fba06c5..70433e21 100644 --- a/packages/convex-helpers/server/rowLevelSecurity.test.ts +++ b/packages/convex-helpers/server/rowLevelSecurity.test.ts @@ -1,10 +1,11 @@ import { convexTest } from "convex-test"; import { v } from "convex/values"; import { describe, expect, test } from "vitest"; -import { wrapDatabaseWriter } from "./rowLevelSecurity.js"; +import { wrapDatabaseReader, wrapDatabaseWriter } from "./rowLevelSecurity.js"; import type { Auth, DataModelFromSchemaDefinition, + GenericDatabaseReader, GenericDatabaseWriter, MutationBuilder, } from "convex/server"; @@ -26,10 +27,18 @@ const schema = defineSchema({ note: v.string(), userId: v.id("users"), }), + publicData: defineTable({ + content: v.string(), + }), + privateData: defineTable({ + content: v.string(), + ownerId: v.id("users"), + }), }); type DataModel = DataModelFromSchemaDefinition; type DatabaseWriter = GenericDatabaseWriter; +type DatabaseReader = GenericDatabaseReader; const withRLS = async (ctx: { db: DatabaseWriter; auth: Auth }) => { const tokenIdentifier = (await ctx.auth.getUserIdentity())?.tokenIdentifier; @@ -100,6 +109,155 @@ describe("row level security", () => { return rls.db.delete(noteId); }); }); + + test("default allow policy permits access to tables without rules", async () => { + const t = convexTest(schema, modules); + await t.run(async (ctx) => { + const userId = await ctx.db.insert("users", { tokenIdentifier: "Person A" }); + await ctx.db.insert("publicData", { content: "Public content" }); + await ctx.db.insert("privateData", { content: "Private content", ownerId: userId }); + }); + + const asA = t.withIdentity({ tokenIdentifier: "Person A" }); + const result = await asA.run(async (ctx) => { + const tokenIdentifier = (await ctx.auth.getUserIdentity())?.tokenIdentifier; + if (!tokenIdentifier) throw new Error("Unauthenticated"); + + // Default allow - no config specified + const db = wrapDatabaseReader({ tokenIdentifier }, ctx.db, { + notes: { + read: async ({ tokenIdentifier }, doc) => { + const author = await ctx.db.get(doc.userId); + return tokenIdentifier === author?.tokenIdentifier; + }, + }, + }); + + // Should be able to read publicData (no rules defined) + const publicData = await db.query("publicData").collect(); + // Should be able to read privateData (no rules defined) + const privateData = await db.query("privateData").collect(); + + return { publicData, privateData }; + }); + + expect(result.publicData).toHaveLength(1); + expect(result.privateData).toHaveLength(1); + }); + + test("default deny policy blocks access to tables without rules", async () => { + const t = convexTest(schema, modules); + await t.run(async (ctx) => { + const userId = await ctx.db.insert("users", { tokenIdentifier: "Person A" }); + await ctx.db.insert("publicData", { content: "Public content" }); + await ctx.db.insert("privateData", { content: "Private content", ownerId: userId }); + }); + + const asA = t.withIdentity({ tokenIdentifier: "Person A" }); + const result = await asA.run(async (ctx) => { + const tokenIdentifier = (await ctx.auth.getUserIdentity())?.tokenIdentifier; + if (!tokenIdentifier) throw new Error("Unauthenticated"); + + // Default deny policy + const db = wrapDatabaseReader({ tokenIdentifier }, ctx.db, { + notes: { + read: async ({ tokenIdentifier }, doc) => { + const author = await ctx.db.get(doc.userId); + return tokenIdentifier === author?.tokenIdentifier; + }, + }, + // Explicitly allow publicData + publicData: { + read: async () => true, + }, + }, { defaultPolicy: "deny" }); + + // Should be able to read publicData (has explicit allow rule) + const publicData = await db.query("publicData").collect(); + // Should NOT be able to read privateData (no rules, default deny) + const privateData = await db.query("privateData").collect(); + + return { publicData, privateData }; + }); + + expect(result.publicData).toHaveLength(1); + expect(result.privateData).toHaveLength(0); + }); + + test("default deny policy blocks inserts to tables without rules", async () => { + const t = convexTest(schema, modules); + await t.run(async (ctx) => { + await ctx.db.insert("users", { 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"); + + const db = wrapDatabaseWriter({ tokenIdentifier }, ctx.db, {}, { defaultPolicy: "allow" }); + + // Should be able to insert (no rules, default allow) + await db.insert("publicData", { content: "Allowed 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, {}, { defaultPolicy: "deny" }); + + // Should NOT be able to insert (no rules, default deny) + await db.insert("publicData", { content: "Blocked content" }); + }), + ).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" }); + }); + + 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"); + + const db = wrapDatabaseWriter({ tokenIdentifier }, ctx.db, { + publicData: { + read: async () => true, // Allow reads + }, + }, { 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 NOT be able to modify (no modify rule, default deny) + await db.patch(docId, { content: "Blocked modification" }); + }), + ).rejects.toThrow(/write access not allowed/); + }); }); const mutation = mutationGeneric as MutationBuilder; diff --git a/packages/convex-helpers/server/rowLevelSecurity.ts b/packages/convex-helpers/server/rowLevelSecurity.ts index 9801b7dd..8567fb5a 100644 --- a/packages/convex-helpers/server/rowLevelSecurity.ts +++ b/packages/convex-helpers/server/rowLevelSecurity.ts @@ -26,6 +26,15 @@ export type Rules = { }; }; +export type RLSConfig = { + /** + * Default policy when no rule is defined for a table. + * - "allow": Allow access by default (default behavior) + * - "deny": Deny access by default + */ + defaultPolicy?: "allow" | "deny"; +}; + /** * Apply row level security (RLS) to queries and mutations with the returned * middleware functions. @@ -153,16 +162,18 @@ export function wrapDatabaseReader( ctx: Ctx, db: GenericDatabaseReader, rules: Rules, + config?: RLSConfig, ): GenericDatabaseReader { - return new WrapReader(ctx, db, rules); + return new WrapReader(ctx, db, rules, config); } export function wrapDatabaseWriter( ctx: Ctx, db: GenericDatabaseWriter, rules: Rules, + config?: RLSConfig, ): GenericDatabaseWriter { - return new WrapWriter(ctx, db, rules); + return new WrapWriter(ctx, db, rules, config); } type ArgsArray = [] | [FunctionArgs]; @@ -178,16 +189,19 @@ class WrapReader db: GenericDatabaseReader; system: GenericDatabaseReader["system"]; rules: Rules; + config: RLSConfig; constructor( ctx: Ctx, db: GenericDatabaseReader, rules: Rules, + config?: RLSConfig, ) { this.ctx = ctx; this.db = db; this.system = db.system; this.rules = rules; + this.config = config ?? { defaultPolicy: "allow" }; } normalizeId>( @@ -213,7 +227,7 @@ class WrapReader doc: DocumentByInfo, ): Promise { if (!this.rules[tableName]?.read) { - return true; + return (this.config.defaultPolicy ?? "allow") === "allow"; } return await this.rules[tableName]!.read!(this.ctx, doc); } @@ -249,13 +263,14 @@ class WrapWriter system: GenericDatabaseWriter["system"]; reader: GenericDatabaseReader; rules: Rules; + config: RLSConfig; async modifyPredicate( tableName: string, doc: DocumentByInfo, ): Promise { if (!this.rules[tableName]?.modify) { - return true; + return (this.config.defaultPolicy ?? "allow") === "allow"; } return await this.rules[tableName]!.modify!(this.ctx, doc); } @@ -264,12 +279,14 @@ class WrapWriter ctx: Ctx, db: GenericDatabaseWriter, rules: Rules, + config?: RLSConfig, ) { this.ctx = ctx; this.db = db; this.system = db.system; - this.reader = new WrapReader(ctx, db, rules); + this.reader = new WrapReader(ctx, db, rules, config); this.rules = rules; + this.config = config ?? { defaultPolicy: "allow" }; } normalizeId>( tableName: TableName, @@ -282,7 +299,11 @@ class WrapWriter value: any, ): Promise { const rules = this.rules[table]; - if (rules?.insert && !(await rules.insert(this.ctx, value))) { + if (rules?.insert) { + if (!(await rules.insert(this.ctx, value))) { + throw new Error("insert access not allowed"); + } + } else if ((this.config.defaultPolicy ?? "allow") === "deny") { throw new Error("insert access not allowed"); } return await this.db.insert(table, value); From b98573f91bf0e757f12ebc97f688f3e5fbcb6119 Mon Sep 17 00:00:00 2001 From: Ian Macartney Date: Fri, 15 Aug 2025 11:38:57 -0700 Subject: [PATCH 2/2] lint --- .../server/rowLevelSecurity.test.ts | 139 +++++++++++------- 1 file changed, 89 insertions(+), 50 deletions(-) diff --git a/packages/convex-helpers/server/rowLevelSecurity.test.ts b/packages/convex-helpers/server/rowLevelSecurity.test.ts index 70433e21..03178113 100644 --- a/packages/convex-helpers/server/rowLevelSecurity.test.ts +++ b/packages/convex-helpers/server/rowLevelSecurity.test.ts @@ -5,7 +5,6 @@ import { wrapDatabaseReader, wrapDatabaseWriter } from "./rowLevelSecurity.js"; import type { Auth, DataModelFromSchemaDefinition, - GenericDatabaseReader, GenericDatabaseWriter, MutationBuilder, } from "convex/server"; @@ -38,7 +37,6 @@ const schema = defineSchema({ type DataModel = DataModelFromSchemaDefinition; type DatabaseWriter = GenericDatabaseWriter; -type DatabaseReader = GenericDatabaseReader; const withRLS = async (ctx: { db: DatabaseWriter; auth: Auth }) => { const tokenIdentifier = (await ctx.auth.getUserIdentity())?.tokenIdentifier; @@ -113,16 +111,22 @@ describe("row level security", () => { test("default allow policy permits access to tables without rules", async () => { const t = convexTest(schema, modules); await t.run(async (ctx) => { - const userId = await ctx.db.insert("users", { tokenIdentifier: "Person A" }); + const userId = await ctx.db.insert("users", { + tokenIdentifier: "Person A", + }); await ctx.db.insert("publicData", { content: "Public content" }); - await ctx.db.insert("privateData", { content: "Private content", ownerId: userId }); + await ctx.db.insert("privateData", { + content: "Private content", + ownerId: userId, + }); }); const asA = t.withIdentity({ tokenIdentifier: "Person A" }); const result = await asA.run(async (ctx) => { - const tokenIdentifier = (await ctx.auth.getUserIdentity())?.tokenIdentifier; + const tokenIdentifier = (await ctx.auth.getUserIdentity()) + ?.tokenIdentifier; if (!tokenIdentifier) throw new Error("Unauthenticated"); - + // Default allow - no config specified const db = wrapDatabaseReader({ tokenIdentifier }, ctx.db, { notes: { @@ -132,15 +136,15 @@ describe("row level security", () => { }, }, }); - + // Should be able to read publicData (no rules defined) const publicData = await db.query("publicData").collect(); // Should be able to read privateData (no rules defined) const privateData = await db.query("privateData").collect(); - + return { publicData, privateData }; }); - + expect(result.publicData).toHaveLength(1); expect(result.privateData).toHaveLength(1); }); @@ -148,38 +152,49 @@ describe("row level security", () => { test("default deny policy blocks access to tables without rules", async () => { const t = convexTest(schema, modules); await t.run(async (ctx) => { - const userId = await ctx.db.insert("users", { tokenIdentifier: "Person A" }); + const userId = await ctx.db.insert("users", { + tokenIdentifier: "Person A", + }); await ctx.db.insert("publicData", { content: "Public content" }); - await ctx.db.insert("privateData", { content: "Private content", ownerId: userId }); + await ctx.db.insert("privateData", { + content: "Private content", + ownerId: userId, + }); }); const asA = t.withIdentity({ tokenIdentifier: "Person A" }); const result = await asA.run(async (ctx) => { - const tokenIdentifier = (await ctx.auth.getUserIdentity())?.tokenIdentifier; + const tokenIdentifier = (await ctx.auth.getUserIdentity()) + ?.tokenIdentifier; if (!tokenIdentifier) throw new Error("Unauthenticated"); - + // Default deny policy - const db = wrapDatabaseReader({ tokenIdentifier }, ctx.db, { - notes: { - read: async ({ tokenIdentifier }, doc) => { - const author = await ctx.db.get(doc.userId); - return tokenIdentifier === author?.tokenIdentifier; + const db = wrapDatabaseReader( + { tokenIdentifier }, + ctx.db, + { + notes: { + read: async ({ tokenIdentifier }, doc) => { + const author = await ctx.db.get(doc.userId); + return tokenIdentifier === author?.tokenIdentifier; + }, + }, + // Explicitly allow publicData + publicData: { + read: async () => true, }, }, - // Explicitly allow publicData - publicData: { - read: async () => true, - }, - }, { defaultPolicy: "deny" }); - + { defaultPolicy: "deny" }, + ); + // Should be able to read publicData (has explicit allow rule) const publicData = await db.query("publicData").collect(); // Should NOT be able to read privateData (no rules, default deny) const privateData = await db.query("privateData").collect(); - + return { publicData, privateData }; }); - + expect(result.publicData).toHaveLength(1); expect(result.privateData).toHaveLength(0); }); @@ -191,14 +206,20 @@ describe("row level security", () => { }); const asA = t.withIdentity({ tokenIdentifier: "Person A" }); - + // Test with default allow await asA.run(async (ctx) => { - const tokenIdentifier = (await ctx.auth.getUserIdentity())?.tokenIdentifier; + const tokenIdentifier = (await ctx.auth.getUserIdentity()) + ?.tokenIdentifier; if (!tokenIdentifier) throw new Error("Unauthenticated"); - - const db = wrapDatabaseWriter({ tokenIdentifier }, ctx.db, {}, { defaultPolicy: "allow" }); - + + const db = wrapDatabaseWriter( + { tokenIdentifier }, + ctx.db, + {}, + { defaultPolicy: "allow" }, + ); + // Should be able to insert (no rules, default allow) await db.insert("publicData", { content: "Allowed content" }); }); @@ -206,11 +227,17 @@ describe("row level security", () => { // Test with default deny await expect(() => asA.run(async (ctx) => { - const tokenIdentifier = (await ctx.auth.getUserIdentity())?.tokenIdentifier; + const tokenIdentifier = (await ctx.auth.getUserIdentity()) + ?.tokenIdentifier; if (!tokenIdentifier) throw new Error("Unauthenticated"); - - const db = wrapDatabaseWriter({ tokenIdentifier }, ctx.db, {}, { defaultPolicy: "deny" }); - + + const db = wrapDatabaseWriter( + { tokenIdentifier }, + ctx.db, + {}, + { defaultPolicy: "deny" }, + ); + // Should NOT be able to insert (no rules, default deny) await db.insert("publicData", { content: "Blocked content" }); }), @@ -225,18 +252,24 @@ describe("row level security", () => { }); const asA = t.withIdentity({ tokenIdentifier: "Person A" }); - + // Test with default allow await asA.run(async (ctx) => { - const tokenIdentifier = (await ctx.auth.getUserIdentity())?.tokenIdentifier; + 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" }); }); @@ -244,15 +277,21 @@ describe("row level security", () => { // Test with default deny await expect(() => asA.run(async (ctx) => { - const tokenIdentifier = (await ctx.auth.getUserIdentity())?.tokenIdentifier; + 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 + + const db = wrapDatabaseWriter( + { tokenIdentifier }, + ctx.db, + { + publicData: { + read: async () => true, // Allow reads but no modify rule + }, }, - }, { defaultPolicy: "deny" }); - + { defaultPolicy: "deny" }, + ); + // Should NOT be able to modify (no modify rule, default deny) await db.patch(docId, { content: "Blocked modification" }); }),