Skip to content

Add option for default deny behavior to RLS Helpers #720

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
160 changes: 159 additions & 1 deletion packages/convex-helpers/server/rowLevelSecurity.test.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand All @@ -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<typeof schema>;
type DatabaseWriter = GenericDatabaseWriter<DataModel>;
type DatabaseReader = GenericDatabaseReader<DataModel>;

const withRLS = async (ctx: { db: DatabaseWriter; auth: Auth }) => {
const tokenIdentifier = (await ctx.auth.getUserIdentity())?.tokenIdentifier;
Expand Down Expand Up @@ -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<DataModel, "public">;
Expand Down
33 changes: 27 additions & 6 deletions packages/convex-helpers/server/rowLevelSecurity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,15 @@ export type Rules<Ctx, DataModel extends GenericDataModel> = {
};
};

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.
Expand Down Expand Up @@ -153,16 +162,18 @@ export function wrapDatabaseReader<Ctx, DataModel extends GenericDataModel>(
ctx: Ctx,
db: GenericDatabaseReader<DataModel>,
rules: Rules<Ctx, DataModel>,
config?: RLSConfig,
): GenericDatabaseReader<DataModel> {
return new WrapReader(ctx, db, rules);
return new WrapReader(ctx, db, rules, config);
}

export function wrapDatabaseWriter<Ctx, DataModel extends GenericDataModel>(
ctx: Ctx,
db: GenericDatabaseWriter<DataModel>,
rules: Rules<Ctx, DataModel>,
config?: RLSConfig,
): GenericDatabaseWriter<DataModel> {
return new WrapWriter(ctx, db, rules);
return new WrapWriter(ctx, db, rules, config);
}

type ArgsArray = [] | [FunctionArgs<any>];
Expand All @@ -178,16 +189,19 @@ class WrapReader<Ctx, DataModel extends GenericDataModel>
db: GenericDatabaseReader<DataModel>;
system: GenericDatabaseReader<DataModel>["system"];
rules: Rules<Ctx, DataModel>;
config: RLSConfig;

constructor(
ctx: Ctx,
db: GenericDatabaseReader<DataModel>,
rules: Rules<Ctx, DataModel>,
config?: RLSConfig,
) {
this.ctx = ctx;
this.db = db;
this.system = db.system;
this.rules = rules;
this.config = config ?? { defaultPolicy: "allow" };
}

normalizeId<TableName extends TableNamesInDataModel<DataModel>>(
Expand All @@ -213,7 +227,7 @@ class WrapReader<Ctx, DataModel extends GenericDataModel>
doc: DocumentByInfo<T>,
): Promise<boolean> {
if (!this.rules[tableName]?.read) {
return true;
return (this.config.defaultPolicy ?? "allow") === "allow";
}
return await this.rules[tableName]!.read!(this.ctx, doc);
}
Expand Down Expand Up @@ -249,13 +263,14 @@ class WrapWriter<Ctx, DataModel extends GenericDataModel>
system: GenericDatabaseWriter<DataModel>["system"];
reader: GenericDatabaseReader<DataModel>;
rules: Rules<Ctx, DataModel>;
config: RLSConfig;

async modifyPredicate<T extends GenericTableInfo>(
tableName: string,
doc: DocumentByInfo<T>,
): Promise<boolean> {
if (!this.rules[tableName]?.modify) {
return true;
return (this.config.defaultPolicy ?? "allow") === "allow";
}
return await this.rules[tableName]!.modify!(this.ctx, doc);
}
Expand All @@ -264,12 +279,14 @@ class WrapWriter<Ctx, DataModel extends GenericDataModel>
ctx: Ctx,
db: GenericDatabaseWriter<DataModel>,
rules: Rules<Ctx, DataModel>,
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 extends TableNamesInDataModel<DataModel>>(
tableName: TableName,
Expand All @@ -282,7 +299,11 @@ class WrapWriter<Ctx, DataModel extends GenericDataModel>
value: any,
): Promise<any> {
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);
Expand Down