From 90f93caff4ca7919e11f1bf07c20fda1ad06cd17 Mon Sep 17 00:00:00 2001 From: dswbx Date: Fri, 3 Oct 2025 20:22:42 +0200 Subject: [PATCH 01/14] refactor: enhance permission handling and introduce new Permission and Policy classes - Updated the `Guard` class to improve permission checking by utilizing the new `Permission` class. - Refactored tests in `authorize.spec.ts` to use `Permission` instances instead of strings for better type safety. - Introduced a new `permissions.spec.ts` file to test the functionality of the `Permission` and `Policy` classes. - Enhanced the `recursivelyReplacePlaceholders` utility function to support various object structures and types. - Updated middleware and controller files to align with the new permission handling structure. --- app/__test__/auth/authorize/authorize.spec.ts | 32 ++--- .../auth/authorize/permissions.spec.ts | 93 +++++++++++++++ app/__test__/core/utils.spec.ts | 110 ++++++++++++++++++ app/src/auth/api/AuthController.ts | 3 +- app/src/auth/authorize/Guard.ts | 25 +++- app/src/auth/middlewares.ts | 20 ++-- app/src/core/security/Permission.ts | 90 +++++++++++++- app/src/core/utils/objects.ts | 35 ++++++ app/src/core/utils/schema/index.ts | 2 + app/src/media/api/MediaController.ts | 3 +- app/src/modules/ModuleHelper.ts | 2 +- app/src/modules/permissions/index.ts | 25 +++- app/src/modules/server/AdminController.tsx | 21 ++-- app/src/modules/server/SystemController.ts | 22 +++- 14 files changed, 432 insertions(+), 51 deletions(-) create mode 100644 app/__test__/auth/authorize/permissions.spec.ts diff --git a/app/__test__/auth/authorize/authorize.spec.ts b/app/__test__/auth/authorize/authorize.spec.ts index c0e04ff..5510e73 100644 --- a/app/__test__/auth/authorize/authorize.spec.ts +++ b/app/__test__/auth/authorize/authorize.spec.ts @@ -1,7 +1,11 @@ import { describe, expect, test } from "bun:test"; -import { Guard } from "../../../src/auth/authorize/Guard"; +import { Guard } from "auth/authorize/Guard"; +import { Permission } from "core/security/Permission"; describe("authorize", () => { + const read = new Permission("read"); + const write = new Permission("write"); + test("basic", async () => { const guard = Guard.create( ["read", "write"], @@ -16,10 +20,10 @@ describe("authorize", () => { role: "admin", }; - expect(guard.granted("read", user)).toBe(true); - expect(guard.granted("write", user)).toBe(true); + expect(guard.granted(read, user)).toBe(true); + expect(guard.granted(write, user)).toBe(true); - expect(() => guard.granted("something")).toThrow(); + expect(() => guard.granted(new Permission("something"))).toThrow(); }); test("with default", async () => { @@ -37,22 +41,22 @@ describe("authorize", () => { { enabled: true }, ); - expect(guard.granted("read")).toBe(true); - expect(guard.granted("write")).toBe(false); + expect(guard.granted(read)).toBe(true); + expect(guard.granted(write)).toBe(false); const user = { role: "admin", }; - expect(guard.granted("read", user)).toBe(true); - expect(guard.granted("write", user)).toBe(true); + expect(guard.granted(read, user)).toBe(true); + expect(guard.granted(write, user)).toBe(true); }); test("guard implicit allow", async () => { const guard = Guard.create([], {}, { enabled: false }); - expect(guard.granted("read")).toBe(true); - expect(guard.granted("write")).toBe(true); + expect(guard.granted(read)).toBe(true); + expect(guard.granted(write)).toBe(true); }); test("role implicit allow", async () => { @@ -66,8 +70,8 @@ describe("authorize", () => { role: "admin", }; - expect(guard.granted("read", user)).toBe(true); - expect(guard.granted("write", user)).toBe(true); + expect(guard.granted(read, user)).toBe(true); + expect(guard.granted(write, user)).toBe(true); }); test("guard with guest role implicit allow", async () => { @@ -79,7 +83,7 @@ describe("authorize", () => { }); expect(guard.getUserRole()?.name).toBe("guest"); - expect(guard.granted("read")).toBe(true); - expect(guard.granted("write")).toBe(true); + expect(guard.granted(read)).toBe(true); + expect(guard.granted(write)).toBe(true); }); }); diff --git a/app/__test__/auth/authorize/permissions.spec.ts b/app/__test__/auth/authorize/permissions.spec.ts new file mode 100644 index 0000000..c6e58fb --- /dev/null +++ b/app/__test__/auth/authorize/permissions.spec.ts @@ -0,0 +1,93 @@ +import { describe, it, expect } from "bun:test"; +import { s } from "bknd/utils"; +import { Permission, Policy } from "core/security/Permission"; + +describe("Permission", () => { + it("works with minimal schema", () => { + expect(() => new Permission("test")).not.toThrow(); + }); + + it("parses context", () => { + const p = new Permission( + "test3", + { + filterable: true, + }, + s.object({ + a: s.string(), + }), + ); + + // @ts-expect-error + expect(() => p.parseContext({ a: [] })).toThrow(); + expect(p.parseContext({ a: "test" })).toEqual({ a: "test" }); + // @ts-expect-error + expect(p.parseContext({ a: 1 })).toEqual({ a: "1" }); + }); +}); + +describe("Policy", () => { + it("works with minimal schema", () => { + expect(() => new Policy().toJSON()).not.toThrow(); + }); + + it("checks condition", () => { + const p = new Policy({ + condition: { + a: 1, + }, + }); + + expect(p.meetsCondition({ a: 1 })).toBe(true); + expect(p.meetsCondition({ a: 2 })).toBe(false); + expect(p.meetsCondition({ a: 1, b: 1 })).toBe(true); + expect(p.meetsCondition({})).toBe(false); + + const p2 = new Policy({ + condition: { + a: { $gt: 1 }, + $or: { + b: { $lt: 2 }, + }, + }, + }); + + expect(p2.meetsCondition({ a: 2 })).toBe(true); + expect(p2.meetsCondition({ a: 1 })).toBe(false); + expect(p2.meetsCondition({ a: 1, b: 1 })).toBe(true); + }); + + it("filters", () => { + const p = new Policy({ + filter: { + age: { $gt: 18 }, + }, + }); + const subjects = [{ age: 19 }, { age: 17 }, { age: 12 }]; + + expect(p.getFiltered(subjects)).toEqual([{ age: 19 }]); + + expect(p.meetsFilter({ age: 19 })).toBe(true); + expect(p.meetsFilter({ age: 17 })).toBe(false); + expect(p.meetsFilter({ age: 12 })).toBe(false); + }); + + it("replaces placeholders", () => { + const p = new Policy({ + condition: { + a: "@auth.username", + }, + filter: { + a: "@auth.username", + }, + }); + const vars = { auth: { username: "test" } }; + + expect(p.meetsCondition({ a: "test" }, vars)).toBe(true); + expect(p.meetsCondition({ a: "test2" }, vars)).toBe(false); + expect(p.meetsCondition({ a: "test2" })).toBe(false); + expect(p.meetsFilter({ a: "test" }, vars)).toBe(true); + expect(p.meetsFilter({ a: "test2" }, vars)).toBe(false); + expect(p.meetsFilter({ a: "test2" })).toBe(false); + }); +}); diff --git a/app/__test__/core/utils.spec.ts b/app/__test__/core/utils.spec.ts index 36b4969..b7d4c96 100644 --- a/app/__test__/core/utils.spec.ts +++ b/app/__test__/core/utils.spec.ts @@ -194,6 +194,116 @@ describe("Core Utils", async () => { expect(result).toEqual(expected); } }); + + test("recursivelyReplacePlaceholders", () => { + // test basic replacement with simple pattern + const obj1 = { a: "Hello, {$name}!", b: { c: "Hello, {$name}!" } }; + const variables1 = { name: "John" }; + const result1 = utils.recursivelyReplacePlaceholders(obj1, /\{\$(\w+)\}/g, variables1); + expect(result1).toEqual({ a: "Hello, John!", b: { c: "Hello, John!" } }); + + // test the specific example from the user request + const obj2 = { some: "value", here: "@auth.user" }; + const variables2 = { auth: { user: "what" } }; + const result2 = utils.recursivelyReplacePlaceholders(obj2, /^@([a-z\.]+)$/, variables2); + expect(result2).toEqual({ some: "value", here: "what" }); + + // test with arrays + const obj3 = { items: ["@config.name", "static", "@config.version"] }; + const variables3 = { config: { name: "MyApp", version: "1.0.0" } }; + const result3 = utils.recursivelyReplacePlaceholders(obj3, /^@([a-z\.]+)$/, variables3); + expect(result3).toEqual({ items: ["MyApp", "static", "1.0.0"] }); + + // test with nested objects and deep paths + const obj4 = { + user: "@auth.user.name", + settings: { + theme: "@ui.theme", + nested: { + value: "@deep.nested.value", + }, + }, + }; + const variables4 = { + auth: { user: { name: "Alice" } }, + ui: { theme: "dark" }, + deep: { nested: { value: "found" } }, + }; + const result4 = utils.recursivelyReplacePlaceholders(obj4, /^@([a-z\.]+)$/, variables4); + expect(result4).toEqual({ + user: "Alice", + settings: { + theme: "dark", + nested: { + value: "found", + }, + }, + }); + + // test with missing paths (should return original match) + const obj5 = { value: "@missing.path" }; + const variables5 = { existing: "value" }; + const result5 = utils.recursivelyReplacePlaceholders(obj5, /^@([a-z\.]+)$/, variables5); + expect(result5).toEqual({ value: "@missing.path" }); + + // test with non-matching strings (should remain unchanged) + const obj6 = { value: "normal string", other: "not@matching" }; + const variables6 = { some: "value" }; + const result6 = utils.recursivelyReplacePlaceholders(obj6, /^@([a-z\.]+)$/, variables6); + expect(result6).toEqual({ value: "normal string", other: "not@matching" }); + + // test with primitive values (should handle gracefully) + expect( + utils.recursivelyReplacePlaceholders("@test.value", /^@([a-z\.]+)$/, { + test: { value: "replaced" }, + }), + ).toBe("replaced"); + expect(utils.recursivelyReplacePlaceholders(123, /^@([a-z\.]+)$/, {})).toBe(123); + expect(utils.recursivelyReplacePlaceholders(null, /^@([a-z\.]+)$/, {})).toBe(null); + + // test type preservation for full string matches + const variables7 = { test: { value: 123, flag: true, data: null, arr: [1, 2, 3] } }; + const result7 = utils.recursivelyReplacePlaceholders( + { + number: "@test.value", + boolean: "@test.flag", + nullValue: "@test.data", + array: "@test.arr", + }, + /^@([a-z\.]+)$/, + variables7, + ); + expect(result7).toEqual({ + number: 123, + boolean: true, + nullValue: null, + array: [1, 2, 3], + }); + + // test partial string replacement (should convert to string) + const result8 = utils.recursivelyReplacePlaceholders( + { message: "The value is @test.value!" }, + /@([a-z\.]+)/g, + variables7, + ); + expect(result8).toEqual({ message: "The value is 123!" }); + + // test mixed scenarios + const result9 = utils.recursivelyReplacePlaceholders( + { + fullMatch: "@test.value", // should preserve number type + partialMatch: "Value: @test.value", // should convert to string + noMatch: "static text", + }, + /^@([a-z\.]+)$/, + variables7, + ); + expect(result9).toEqual({ + fullMatch: 123, // number preserved + partialMatch: "Value: @test.value", // no replacement (pattern requires full match) + noMatch: "static text", + }); + }); }); describe("file", async () => { diff --git a/app/src/auth/api/AuthController.ts b/app/src/auth/api/AuthController.ts index ba12d4a..15448be 100644 --- a/app/src/auth/api/AuthController.ts +++ b/app/src/auth/api/AuthController.ts @@ -60,7 +60,8 @@ export class AuthController extends Controller { if (create) { hono.post( "/create", - permission([AuthPermissions.createUser, DataPermissions.entityCreate]), + permission(AuthPermissions.createUser), + permission(DataPermissions.entityCreate), describeRoute({ summary: "Create a new user", tags: ["auth"], diff --git a/app/src/auth/authorize/Guard.ts b/app/src/auth/authorize/Guard.ts index a89b98d..5576d4b 100644 --- a/app/src/auth/authorize/Guard.ts +++ b/app/src/auth/authorize/Guard.ts @@ -1,5 +1,5 @@ import { Exception } from "core/errors"; -import { $console, objectTransform } from "bknd/utils"; +import { $console, objectTransform, type s } from "bknd/utils"; import { Permission } from "core/security/Permission"; import type { Context } from "hono"; import type { ServerEnv } from "modules/Controller"; @@ -12,6 +12,7 @@ export type GuardUserContext = { export type GuardConfig = { enabled?: boolean; + context?: string; }; export type GuardContext = Context | GuardUserContext; @@ -26,6 +27,9 @@ export class Guard { this.config = config; } + /** + * @deprecated + */ static create( permissionNames: string[], roles?: Record< @@ -156,12 +160,25 @@ export class Guard { return !!rolePermission; } - granted(permission: Permission | string, c?: GuardContext): boolean { + granted

( + permission: P, + c?: GuardContext, + context: s.Static = {} as s.Static, + ): boolean { const user = c && "get" in c ? c.get("auth")?.user : c; - return this.hasPermission(permission as any, user); + const ctx = { + ...context, + user, + context: this.config?.context, + }; + return this.hasPermission(permission, user); } - throwUnlessGranted(permission: Permission | string, c: GuardContext) { + throwUnlessGranted

( + permission: P, + c: GuardContext, + context: s.Static, + ) { if (!this.granted(permission, c)) { throw new Exception( `Permission "${typeof permission === "string" ? permission : permission.name}" not granted`, diff --git a/app/src/auth/middlewares.ts b/app/src/auth/middlewares.ts index 702023b..685a9bd 100644 --- a/app/src/auth/middlewares.ts +++ b/app/src/auth/middlewares.ts @@ -1,8 +1,9 @@ import type { Permission } from "core/security/Permission"; -import { $console, patternMatch } from "bknd/utils"; +import { $console, patternMatch, type s } from "bknd/utils"; import type { Context } from "hono"; import { createMiddleware } from "hono/factory"; import type { ServerEnv } from "modules/Controller"; +import type { MaybePromise } from "core/types"; function getPath(reqOrCtx: Request | Context) { const req = reqOrCtx instanceof Request ? reqOrCtx : reqOrCtx.req.raw; @@ -49,7 +50,7 @@ export const auth = (options?: { // make sure to only register once if (authCtx.registered) { skipped = true; - $console.warn(`auth middleware already registered for ${getPath(c)}`); + $console.debug(`auth middleware already registered for ${getPath(c)}`); } else { authCtx.registered = true; @@ -68,11 +69,12 @@ export const auth = (options?: { authCtx.user = undefined; }); -export const permission = ( - permission: Permission | Permission[], +export const permission =

( + permission: P, options?: { - onGranted?: (c: Context) => Promise; - onDenied?: (c: Context) => Promise; + onGranted?: (c: Context) => MaybePromise; + onDenied?: (c: Context) => MaybePromise; + context?: (c: Context) => MaybePromise>; }, ) => // @ts-ignore @@ -93,11 +95,11 @@ export const permission = ( } } else if (!authCtx.skip) { const guard = app.modules.ctx().guard; - const permissions = Array.isArray(permission) ? permission : [permission]; + const context = (await options?.context?.(c)) ?? ({} as any); if (options?.onGranted || options?.onDenied) { let returned: undefined | void | Response; - if (permissions.every((p) => guard.granted(p, c))) { + if (guard.granted(permission, c, context)) { returned = await options?.onGranted?.(c); } else { returned = await options?.onDenied?.(c); @@ -106,7 +108,7 @@ export const permission = ( return returned; } } else { - permissions.some((p) => guard.throwUnlessGranted(p, c)); + guard.throwUnlessGranted(permission, c, context); } } diff --git a/app/src/core/security/Permission.ts b/app/src/core/security/Permission.ts index 86cf46b..5ca4d84 100644 --- a/app/src/core/security/Permission.ts +++ b/app/src/core/security/Permission.ts @@ -1,11 +1,95 @@ -export class Permission { - constructor(public name: Name) { - this.name = name; +import { + s, + type ParseOptions, + parse, + InvalidSchemaError, + recursivelyReplacePlaceholders, +} from "bknd/utils"; +import * as query from "core/object/query/object-query"; + +export const permissionOptionsSchema = s + .strictObject({ + description: s.string(), + filterable: s.boolean(), + }) + .partial(); + +export type PermissionOptions = s.Static; + +export class InvalidPermissionContextError extends InvalidSchemaError { + override name = "InvalidPermissionContextError"; + + static from(e: InvalidSchemaError) { + return new InvalidPermissionContextError(e.schema, e.value, e.errors); + } +} + +export class Permission< + Name extends string = string, + Options extends PermissionOptions = {}, + Context extends s.ObjectSchema = s.ObjectSchema, +> { + constructor( + public name: Name, + public options: Options = {} as Options, + public context: Context = s.object({}) as Context, + ) {} + + parseContext(ctx: s.Static, opts?: ParseOptions) { + try { + return parse(this.context, ctx, opts); + } catch (e) { + if (e instanceof InvalidSchemaError) { + throw InvalidPermissionContextError.from(e); + } + + throw e; + } } toJSON() { return { name: this.name, + ...this.options, + context: this.context, }; } } + +export const policySchema = s + .strictObject({ + description: s.string(), + condition: s.object({}, { default: {} }) as s.Schema<{}, query.ObjectQuery>, + effect: s.string({ enum: ["allow", "deny", "filter"], default: "deny" }), + filter: s.object({}, { default: {} }) as s.Schema<{}, query.ObjectQuery>, + }) + .partial(); +export type PolicySchema = s.Static; + +export class Policy { + public content: Schema; + + constructor(content?: Schema) { + this.content = parse(policySchema, content ?? {}) as Schema; + } + + replace(context: object, vars?: Record) { + return vars ? recursivelyReplacePlaceholders(context, /^@([a-zA-Z_\.]+)$/, vars) : context; + } + + meetsCondition(context: object, vars?: Record) { + return query.validate(this.replace(this.content.condition!, vars), context); + } + + meetsFilter(subject: object, vars?: Record) { + return query.validate(this.replace(this.content.filter!, vars), subject); + } + + getFiltered(given: Given): Given { + return given.filter((item) => this.meetsFilter(item)) as Given; + } + + toJSON() { + return this.content; + } +} diff --git a/app/src/core/utils/objects.ts b/app/src/core/utils/objects.ts index 41902a9..65f18f9 100644 --- a/app/src/core/utils/objects.ts +++ b/app/src/core/utils/objects.ts @@ -512,3 +512,38 @@ export function convertNumberedObjectToArray(obj: object): any[] | object { } return obj; } + +export function recursivelyReplacePlaceholders( + obj: any, + pattern: RegExp, + variables: Record, +) { + if (typeof obj === "string") { + // check if the entire string matches the pattern + const match = obj.match(pattern); + if (match && match[0] === obj && match[1]) { + // full string match - replace with the actual value (preserving type) + const key = match[1]; + const value = getPath(variables, key); + return value !== undefined ? value : obj; + } + // partial match - use string replacement + if (pattern.test(obj)) { + return obj.replace(pattern, (match, key) => { + const value = getPath(variables, key); + // convert to string for partial replacements + return value !== undefined ? String(value) : match; + }); + } + } + if (Array.isArray(obj)) { + return obj.map((item) => recursivelyReplacePlaceholders(item, pattern, variables)); + } + if (obj && typeof obj === "object") { + return Object.entries(obj).reduce((acc, [key, value]) => { + acc[key] = recursivelyReplacePlaceholders(value, pattern, variables); + return acc; + }, {} as object); + } + return obj; +} diff --git a/app/src/core/utils/schema/index.ts b/app/src/core/utils/schema/index.ts index 3d3692c..d30aae1 100644 --- a/app/src/core/utils/schema/index.ts +++ b/app/src/core/utils/schema/index.ts @@ -59,6 +59,8 @@ export const stringIdentifier = s.string({ }); export class InvalidSchemaError extends Error { + override name = "InvalidSchemaError"; + constructor( public schema: s.Schema, public value: unknown, diff --git a/app/src/media/api/MediaController.ts b/app/src/media/api/MediaController.ts index 6a72048..e20fa2e 100644 --- a/app/src/media/api/MediaController.ts +++ b/app/src/media/api/MediaController.ts @@ -186,7 +186,8 @@ export class MediaController extends Controller { }), ), jsc("query", s.object({ overwrite: s.boolean().optional() })), - permission([DataPermissions.entityCreate, MediaPermissions.uploadFile]), + permission(DataPermissions.entityCreate), + permission(MediaPermissions.uploadFile), async (c) => { const { entity: entity_name, id: entity_id, field: field_name } = c.req.valid("param"); diff --git a/app/src/modules/ModuleHelper.ts b/app/src/modules/ModuleHelper.ts index 60a6dfc..d671065 100644 --- a/app/src/modules/ModuleHelper.ts +++ b/app/src/modules/ModuleHelper.ts @@ -115,7 +115,7 @@ export class ModuleHelper { } async throwUnlessGranted( - permission: Permission | string, + permission: Permission, c: { context: ModuleBuildContextMcpContext; raw?: unknown }, ) { invariant(c.context.app, "app is not available in mcp context"); diff --git a/app/src/modules/permissions/index.ts b/app/src/modules/permissions/index.ts index b6fbead..5f5ccd1 100644 --- a/app/src/modules/permissions/index.ts +++ b/app/src/modules/permissions/index.ts @@ -1,10 +1,29 @@ import { Permission } from "core/security/Permission"; +import { s } from "bknd/utils"; export const accessAdmin = new Permission("system.access.admin"); export const accessApi = new Permission("system.access.api"); -export const configRead = new Permission("system.config.read"); -export const configReadSecrets = new Permission("system.config.read.secrets"); -export const configWrite = new Permission("system.config.write"); +export const configRead = new Permission( + "system.config.read", + {}, + s.object({ + module: s.string().optional(), + }), +); +export const configReadSecrets = new Permission( + "system.config.read.secrets", + {}, + s.object({ + module: s.string().optional(), + }), +); +export const configWrite = new Permission( + "system.config.write", + {}, + s.object({ + module: s.string().optional(), + }), +); export const schemaRead = new Permission("system.schema.read"); export const build = new Permission("system.build"); export const mcp = new Permission("system.mcp"); diff --git a/app/src/modules/server/AdminController.tsx b/app/src/modules/server/AdminController.tsx index 2800781..2cdb2a7 100644 --- a/app/src/modules/server/AdminController.tsx +++ b/app/src/modules/server/AdminController.tsx @@ -139,17 +139,18 @@ export class AdminController extends Controller { } if (auth_enabled) { + const options = { + onGranted: async (c) => { + // @todo: add strict test to permissions middleware? + if (c.get("auth")?.user) { + $console.log("redirecting to success"); + return c.redirect(authRoutes.success); + } + }, + }; const redirectRouteParams = [ - permission([SystemPermissions.accessAdmin, SystemPermissions.schemaRead], { - // @ts-ignore - onGranted: async (c) => { - // @todo: add strict test to permissions middleware? - if (c.get("auth")?.user) { - $console.log("redirecting to success"); - return c.redirect(authRoutes.success); - } - }, - }), + permission(SystemPermissions.accessAdmin, options), + permission(SystemPermissions.schemaRead, options), async (c) => { return c.html(c.get("html")!); }, diff --git a/app/src/modules/server/SystemController.ts b/app/src/modules/server/SystemController.ts index 93533a2..43d688e 100644 --- a/app/src/modules/server/SystemController.ts +++ b/app/src/modules/server/SystemController.ts @@ -130,7 +130,7 @@ export class SystemController extends Controller { summary: "Get the raw config", tags: ["system"], }), - permission([SystemPermissions.configReadSecrets]), + permission(SystemPermissions.configReadSecrets), async (c) => { // @ts-expect-error "fetch" is private return c.json(await this.app.modules.fetch().then((r) => r?.configs)); @@ -295,7 +295,11 @@ export class SystemController extends Controller { const { secrets } = c.req.valid("query"); const { module } = c.req.valid("param"); - secrets && this.ctx.guard.throwUnlessGranted(SystemPermissions.configReadSecrets, c); + if (secrets) { + this.ctx.guard.throwUnlessGranted(SystemPermissions.configReadSecrets, c, { + module, + }); + } const config = this.app.toJSON(secrets); @@ -342,8 +346,16 @@ export class SystemController extends Controller { const { config, secrets, fresh } = c.req.valid("query"); const readonly = this.app.isReadOnly(); - config && this.ctx.guard.throwUnlessGranted(SystemPermissions.configRead, c); - secrets && this.ctx.guard.throwUnlessGranted(SystemPermissions.configReadSecrets, c); + if (config) { + this.ctx.guard.throwUnlessGranted(SystemPermissions.configRead, c, { + module, + }); + } + if (secrets) { + this.ctx.guard.throwUnlessGranted(SystemPermissions.configReadSecrets, c, { + module, + }); + } const { version, ...schema } = this.app.getSchema(); @@ -383,7 +395,7 @@ export class SystemController extends Controller { jsc("query", s.object({ sync: s.boolean().optional(), fetch: s.boolean().optional() })), async (c) => { const options = c.req.valid("query") as Record; - this.ctx.guard.throwUnlessGranted(SystemPermissions.build, c); + this.ctx.guard.throwUnlessGranted(SystemPermissions.build, c, {}); await this.app.build(options); return c.json({ From 2f88c2216cd39350d48ef9f992be578553f4a645 Mon Sep 17 00:00:00 2001 From: dswbx Date: Mon, 13 Oct 2025 18:20:46 +0200 Subject: [PATCH 02/14] refactor: restructure permission handling and enhance Guard functionality - Introduced a new `createGuard` function to streamline the creation of Guard instances with permissions and roles. - Updated tests in `authorize.spec.ts` to reflect changes in permission checks, ensuring they now return undefined for denied permissions. - Added new `Permission` and `Policy` classes to improve type safety and flexibility in permission management. - Refactored middleware and controller files to utilize the updated permission structure, including context handling for permissions. - Created a new `SystemController.spec.ts` file to test the integration of the new permission system within the SystemController. - Removed legacy permission handling from core security files, consolidating permission logic within the new structure. --- .../auth/authorize/SystemController.spec.ts | 20 ++ app/__test__/auth/authorize/authorize.spec.ts | 63 ++-- .../auth/authorize/permissions.spec.ts | 337 +++++++++++++++++- .../integration/auth.integration.test.ts | 2 +- app/src/auth/api/AuthController.ts | 12 +- app/src/auth/auth-permissions.ts | 2 +- app/src/auth/authorize/Guard.ts | 250 ++++++++----- app/src/auth/authorize/Permission.ts | 68 ++++ app/src/auth/authorize/Policy.ts | 42 +++ app/src/auth/authorize/Role.ts | 70 ++-- .../auth.middleware.ts} | 50 +-- .../auth/middlewares/permission.middleware.ts | 93 +++++ app/src/core/security/Permission.ts | 95 ----- app/src/core/utils/runtime.ts | 9 + app/src/core/utils/schema/index.ts | 5 +- app/src/data/api/DataController.ts | 32 +- app/src/data/permissions/index.ts | 2 +- app/src/index.ts | 2 +- app/src/media/api/MediaController.ts | 12 +- app/src/media/media-permissions.ts | 2 +- app/src/modules/ModuleHelper.ts | 26 +- app/src/modules/middlewares/index.ts | 3 +- app/src/modules/permissions/index.ts | 10 +- app/src/modules/server/AdminController.tsx | 4 +- app/src/modules/server/AppServer.ts | 4 + app/src/modules/server/SystemController.ts | 106 ++++-- 26 files changed, 954 insertions(+), 367 deletions(-) create mode 100644 app/__test__/auth/authorize/SystemController.spec.ts create mode 100644 app/src/auth/authorize/Permission.ts create mode 100644 app/src/auth/authorize/Policy.ts rename app/src/auth/{middlewares.ts => middlewares/auth.middleware.ts} (52%) create mode 100644 app/src/auth/middlewares/permission.middleware.ts delete mode 100644 app/src/core/security/Permission.ts diff --git a/app/__test__/auth/authorize/SystemController.spec.ts b/app/__test__/auth/authorize/SystemController.spec.ts new file mode 100644 index 0000000..8400f46 --- /dev/null +++ b/app/__test__/auth/authorize/SystemController.spec.ts @@ -0,0 +1,20 @@ +import { describe, it, expect } from "bun:test"; +import { SystemController } from "modules/server/SystemController"; +import { createApp } from "core/test/utils"; +import type { CreateAppConfig } from "App"; +import { getPermissionRoutes } from "auth/middlewares/permission.middleware"; + +async function makeApp(config: Partial = {}) { + const app = createApp(config); + await app.build(); + return app; +} + +describe("SystemController", () => { + it("...", async () => { + const app = await makeApp(); + const controller = new SystemController(app); + const hono = controller.getController(); + console.log(getPermissionRoutes(hono)); + }); +}); diff --git a/app/__test__/auth/authorize/authorize.spec.ts b/app/__test__/auth/authorize/authorize.spec.ts index 5510e73..fbf787f 100644 --- a/app/__test__/auth/authorize/authorize.spec.ts +++ b/app/__test__/auth/authorize/authorize.spec.ts @@ -1,13 +1,36 @@ import { describe, expect, test } from "bun:test"; -import { Guard } from "auth/authorize/Guard"; -import { Permission } from "core/security/Permission"; +import { Guard, type GuardConfig } from "auth/authorize/Guard"; +import { Permission } from "auth/authorize/Permission"; +import { Role } from "auth/authorize/Role"; +import { objectTransform } from "bknd/utils"; + +function createGuard( + permissionNames: string[], + roles?: Record< + string, + { + permissions?: string[]; + is_default?: boolean; + implicit_allow?: boolean; + } + >, + config?: GuardConfig, +) { + const _roles = roles + ? objectTransform(roles, ({ permissions = [], is_default, implicit_allow }, name) => { + return Role.create({ name, permissions, is_default, implicit_allow }); + }) + : {}; + const _permissions = permissionNames.map((name) => new Permission(name)); + return new Guard(_permissions, Object.values(_roles), config); +} describe("authorize", () => { const read = new Permission("read"); const write = new Permission("write"); test("basic", async () => { - const guard = Guard.create( + const guard = createGuard( ["read", "write"], { admin: { @@ -20,14 +43,14 @@ describe("authorize", () => { role: "admin", }; - expect(guard.granted(read, user)).toBe(true); - expect(guard.granted(write, user)).toBe(true); + expect(guard.granted(read, user)).toBeUndefined(); + expect(guard.granted(write, user)).toBeUndefined(); - expect(() => guard.granted(new Permission("something"))).toThrow(); + expect(() => guard.granted(new Permission("something"), {})).toThrow(); }); test("with default", async () => { - const guard = Guard.create( + const guard = createGuard( ["read", "write"], { admin: { @@ -41,26 +64,26 @@ describe("authorize", () => { { enabled: true }, ); - expect(guard.granted(read)).toBe(true); - expect(guard.granted(write)).toBe(false); + expect(guard.granted(read, {})).toBeUndefined(); + expect(() => guard.granted(write, {})).toThrow(); const user = { role: "admin", }; - expect(guard.granted(read, user)).toBe(true); - expect(guard.granted(write, user)).toBe(true); + expect(guard.granted(read, user)).toBeUndefined(); + expect(guard.granted(write, user)).toBeUndefined(); }); test("guard implicit allow", async () => { - const guard = Guard.create([], {}, { enabled: false }); + const guard = createGuard([], {}, { enabled: false }); - expect(guard.granted(read)).toBe(true); - expect(guard.granted(write)).toBe(true); + expect(guard.granted(read, {})).toBeUndefined(); + expect(guard.granted(write, {})).toBeUndefined(); }); test("role implicit allow", async () => { - const guard = Guard.create(["read", "write"], { + const guard = createGuard(["read", "write"], { admin: { implicit_allow: true, }, @@ -70,12 +93,12 @@ describe("authorize", () => { role: "admin", }; - expect(guard.granted(read, user)).toBe(true); - expect(guard.granted(write, user)).toBe(true); + expect(guard.granted(read, user)).toBeUndefined(); + expect(guard.granted(write, user)).toBeUndefined(); }); test("guard with guest role implicit allow", async () => { - const guard = Guard.create(["read", "write"], { + const guard = createGuard(["read", "write"], { guest: { implicit_allow: true, is_default: true, @@ -83,7 +106,7 @@ describe("authorize", () => { }); expect(guard.getUserRole()?.name).toBe("guest"); - expect(guard.granted(read)).toBe(true); - expect(guard.granted(write)).toBe(true); + expect(guard.granted(read, {})).toBeUndefined(); + expect(guard.granted(write, {})).toBeUndefined(); }); }); diff --git a/app/__test__/auth/authorize/permissions.spec.ts b/app/__test__/auth/authorize/permissions.spec.ts index c6e58fb..2b3a5ce 100644 --- a/app/__test__/auth/authorize/permissions.spec.ts +++ b/app/__test__/auth/authorize/permissions.spec.ts @@ -1,6 +1,13 @@ import { describe, it, expect } from "bun:test"; import { s } from "bknd/utils"; -import { Permission, Policy } from "core/security/Permission"; +import { Permission } from "auth/authorize/Permission"; +import { Policy } from "auth/authorize/Policy"; +import { Hono } from "hono"; +import { permission } from "auth/middlewares/permission.middleware"; +import { auth } from "auth/middlewares/auth.middleware"; +import { Guard, type GuardConfig } from "auth/authorize/Guard"; +import { Role, RolePermission } from "auth/authorize/Role"; +import { Exception } from "bknd"; describe("Permission", () => { it("works with minimal schema", () => { @@ -91,3 +98,331 @@ describe("Policy", () => { expect(p.meetsFilter({ a: "test2" })).toBe(false); }); }); + +describe("Guard", () => { + it("collects filters", () => { + const p = new Permission( + "test", + { + filterable: true, + }, + s.object({ + a: s.number(), + }), + ); + const r = new Role("test", [ + new RolePermission(p, [ + new Policy({ + filter: { a: { $eq: 1 } }, + effect: "filter", + }), + ]), + ]); + const guard = new Guard([p], [r], { + enabled: true, + }); + expect( + guard.getPolicyFilter( + p, + { + role: r.name, + }, + { a: 1 }, + ), + ).toEqual({ a: { $eq: 1 } }); + expect( + guard.getPolicyFilter( + p, + { + role: r.name, + }, + { a: 2 }, + ), + ).toBeUndefined(); + // if no user context given, filter cannot be applied + expect(guard.getPolicyFilter(p, {}, { a: 1 })).toBeUndefined(); + }); + + it("collects filters for default role", () => { + const p = new Permission( + "test", + { + filterable: true, + }, + s.object({ + a: s.number(), + }), + ); + const r = new Role( + "test", + [ + new RolePermission(p, [ + new Policy({ + filter: { a: { $eq: 1 } }, + effect: "filter", + }), + ]), + ], + true, + ); + const guard = new Guard([p], [r], { + enabled: true, + }); + + expect( + guard.getPolicyFilter( + p, + { + role: r.name, + }, + { a: 1 }, + ), + ).toEqual({ a: { $eq: 1 } }); + expect( + guard.getPolicyFilter( + p, + { + role: r.name, + }, + { a: 2 }, + ), + ).toBeUndefined(); + // if no user context given, the default role is applied + // hence it can be found + expect(guard.getPolicyFilter(p, {}, { a: 1 })).toEqual({ a: { $eq: 1 } }); + }); +}); + +describe("permission middleware", () => { + const makeApp = ( + permissions: Permission[], + roles: Role[] = [], + config: Partial = {}, + ) => { + const app = { + module: { + auth: { + enabled: true, + }, + }, + modules: { + ctx: () => ({ + guard: new Guard(permissions, roles, { + enabled: true, + ...config, + }), + }), + }, + }; + return new Hono() + .use(async (c, next) => { + // @ts-expect-error + c.set("app", app); + await next(); + }) + .use(auth()) + .onError((err, c) => { + if (err instanceof Exception) { + return c.json(err.toJSON(), err.code as any); + } + return c.json({ error: err.message }, "code" in err ? (err.code as any) : 500); + }); + }; + + it("allows if guard is disabled", async () => { + const p = new Permission("test"); + const hono = makeApp([p], [], { enabled: false }).get("/test", permission(p, {}), async (c) => + c.text("test"), + ); + + const res = await hono.request("/test"); + expect(res.status).toBe(200); + expect(await res.text()).toBe("test"); + }); + + it("denies if guard is enabled", async () => { + const p = new Permission("test"); + const hono = makeApp([p]).get("/test", permission(p, {}), async (c) => c.text("test")); + + const res = await hono.request("/test"); + expect(res.status).toBe(403); + }); + + it("allows if user has (plain) role", async () => { + const p = new Permission("test"); + const r = Role.create({ name: "test", permissions: [p.name] }); + const hono = makeApp([p], [r]) + .use(async (c, next) => { + // @ts-expect-error + c.set("auth", { registered: true, user: { id: 0, role: r.name } }); + await next(); + }) + .get("/test", permission(p, {}), async (c) => c.text("test")); + + const res = await hono.request("/test"); + expect(res.status).toBe(200); + }); + + it("allows if user has role with policy", async () => { + const p = new Permission("test"); + const r = new Role("test", [ + new RolePermission(p, [ + new Policy({ + condition: { + a: { $gte: 1 }, + }, + }), + ]), + ]); + const hono = makeApp([p], [r], { + context: { + a: 1, + }, + }) + .use(async (c, next) => { + // @ts-expect-error + c.set("auth", { registered: true, user: { id: 0, role: r.name } }); + await next(); + }) + .get("/test", permission(p, {}), async (c) => c.text("test")); + + const res = await hono.request("/test"); + expect(res.status).toBe(200); + }); + + it("denies if user with role doesn't meet condition", async () => { + const p = new Permission("test"); + const r = new Role("test", [ + new RolePermission(p, [ + new Policy({ + condition: { + a: { $lt: 1 }, + }, + }), + ]), + ]); + const hono = makeApp([p], [r], { + context: { + a: 1, + }, + }) + .use(async (c, next) => { + // @ts-expect-error + c.set("auth", { registered: true, user: { id: 0, role: r.name } }); + await next(); + }) + .get("/test", permission(p, {}), async (c) => c.text("test")); + + const res = await hono.request("/test"); + expect(res.status).toBe(403); + }); + + it("allows if user with role doesn't meet condition (from middleware)", async () => { + const p = new Permission( + "test", + {}, + s.object({ + a: s.number(), + }), + ); + const r = new Role("test", [ + new RolePermission(p, [ + new Policy({ + condition: { + a: { $eq: 1 }, + }, + }), + ]), + ]); + const hono = makeApp([p], [r]) + .use(async (c, next) => { + // @ts-expect-error + c.set("auth", { registered: true, user: { id: 0, role: r.name } }); + await next(); + }) + .get( + "/test", + permission(p, { + context: (c) => ({ + a: 1, + }), + }), + async (c) => c.text("test"), + ); + + const res = await hono.request("/test"); + expect(res.status).toBe(200); + }); + + it("throws if permission context is invalid", async () => { + const p = new Permission( + "test", + {}, + s.object({ + a: s.number({ minimum: 2 }), + }), + ); + const r = new Role("test", [ + new RolePermission(p, [ + new Policy({ + condition: { + a: { $eq: 1 }, + }, + }), + ]), + ]); + const hono = makeApp([p], [r]) + .use(async (c, next) => { + // @ts-expect-error + c.set("auth", { registered: true, user: { id: 0, role: r.name } }); + await next(); + }) + .get( + "/test", + permission(p, { + context: (c) => ({ + a: 1, + }), + }), + async (c) => c.text("test"), + ); + + const res = await hono.request("/test"); + // expecting 500 because bknd should have handled it correctly + expect(res.status).toBe(500); + }); +}); + +describe("Role", () => { + it("serializes and deserializes", () => { + const p = new Permission( + "test", + { + filterable: true, + }, + s.object({ + a: s.number({ minimum: 2 }), + }), + ); + const r = new Role( + "test", + [ + new RolePermission(p, [ + new Policy({ + condition: { + a: { $eq: 1 }, + }, + effect: "deny", + filter: { + b: { $lt: 1 }, + }, + }), + ]), + ], + true, + ); + const json = JSON.parse(JSON.stringify(r.toJSON())); + const r2 = Role.create(json); + expect(r2.toJSON()).toEqual(r.toJSON()); + }); +}); diff --git a/app/__test__/integration/auth.integration.test.ts b/app/__test__/integration/auth.integration.test.ts index 340ccaf..477951f 100644 --- a/app/__test__/integration/auth.integration.test.ts +++ b/app/__test__/integration/auth.integration.test.ts @@ -1,6 +1,6 @@ import { afterAll, beforeAll, describe, expect, it } from "bun:test"; import { App, createApp, type AuthResponse } from "../../src"; -import { auth } from "../../src/auth/middlewares"; +import { auth } from "../../src/modules/middlewares"; import { randomString, secureRandomString, withDisabledConsole } from "../../src/core/utils"; import { disableConsoleLog, enableConsoleLog } from "core/utils/test"; import { getDummyConnection } from "../helper"; diff --git a/app/src/auth/api/AuthController.ts b/app/src/auth/api/AuthController.ts index 15448be..b94aa4b 100644 --- a/app/src/auth/api/AuthController.ts +++ b/app/src/auth/api/AuthController.ts @@ -60,8 +60,8 @@ export class AuthController extends Controller { if (create) { hono.post( "/create", - permission(AuthPermissions.createUser), - permission(DataPermissions.entityCreate), + permission(AuthPermissions.createUser, {}), + permission(DataPermissions.entityCreate, {}), describeRoute({ summary: "Create a new user", tags: ["auth"], @@ -239,7 +239,7 @@ export class AuthController extends Controller { }), }, async (params, c) => { - await c.context.ctx().helper.throwUnlessGranted(AuthPermissions.createUser, c); + await c.context.ctx().helper.granted(c, AuthPermissions.createUser); return c.json(await this.auth.createUser(params)); }, @@ -256,7 +256,7 @@ export class AuthController extends Controller { }), }, async (params, c) => { - await c.context.ctx().helper.throwUnlessGranted(AuthPermissions.createToken, c); + await c.context.ctx().helper.granted(c, AuthPermissions.createToken); const user = await getUser(params); return c.json({ user, token: await this.auth.authenticator.jwt(user) }); @@ -275,7 +275,7 @@ export class AuthController extends Controller { }), }, async (params, c) => { - await c.context.ctx().helper.throwUnlessGranted(AuthPermissions.changePassword, c); + await c.context.ctx().helper.granted(c, AuthPermissions.changePassword); const user = await getUser(params); if (!(await this.auth.changePassword(user.id, params.password))) { @@ -296,7 +296,7 @@ export class AuthController extends Controller { }), }, async (params, c) => { - await c.context.ctx().helper.throwUnlessGranted(AuthPermissions.testPassword, c); + await c.context.ctx().helper.granted(c, AuthPermissions.testPassword); const pw = this.auth.authenticator.strategy("password") as PasswordStrategy; const controller = pw.getController(this.auth.authenticator); diff --git a/app/src/auth/auth-permissions.ts b/app/src/auth/auth-permissions.ts index 8b097e7..dce59f5 100644 --- a/app/src/auth/auth-permissions.ts +++ b/app/src/auth/auth-permissions.ts @@ -1,4 +1,4 @@ -import { Permission } from "core/security/Permission"; +import { Permission } from "auth/authorize/Permission"; export const createUser = new Permission("auth.user.create"); //export const updateUser = new Permission("auth.user.update"); diff --git a/app/src/auth/authorize/Guard.ts b/app/src/auth/authorize/Guard.ts index 5576d4b..ac97c63 100644 --- a/app/src/auth/authorize/Guard.ts +++ b/app/src/auth/authorize/Guard.ts @@ -1,9 +1,11 @@ import { Exception } from "core/errors"; -import { $console, objectTransform, type s } from "bknd/utils"; -import { Permission } from "core/security/Permission"; +import { $console, type s } from "bknd/utils"; +import type { Permission, PermissionContext } from "auth/authorize/Permission"; import type { Context } from "hono"; import type { ServerEnv } from "modules/Controller"; -import { Role } from "./Role"; +import type { Role } from "./Role"; +import { HttpStatus } from "bknd/utils"; +import type { Policy, PolicySchema } from "./Policy"; export type GuardUserContext = { role?: string | null; @@ -12,45 +14,43 @@ export type GuardUserContext = { export type GuardConfig = { enabled?: boolean; - context?: string; + context?: object; }; export type GuardContext = Context | GuardUserContext; -export class Guard { - permissions: Permission[]; - roles?: Role[]; - config?: GuardConfig; +export class GuardPermissionsException extends Exception { + override name = "PermissionsException"; + override code = HttpStatus.FORBIDDEN; - constructor(permissions: Permission[] = [], roles: Role[] = [], config?: GuardConfig) { + constructor( + public permission: Permission, + public policy?: Policy, + public description?: string, + ) { + super(`Permission "${permission.name}" not granted`); + } + + override toJSON(): any { + return { + ...super.toJSON(), + description: this.description, + permission: this.permission.name, + policy: this.policy?.toJSON(), + }; + } +} + +export class Guard { + constructor( + public permissions: Permission[] = [], + public roles: Role[] = [], + public config?: GuardConfig, + ) { this.permissions = permissions; this.roles = roles; this.config = config; } - /** - * @deprecated - */ - static create( - permissionNames: string[], - roles?: Record< - string, - { - permissions?: string[]; - is_default?: boolean; - implicit_allow?: boolean; - } - >, - config?: GuardConfig, - ) { - const _roles = roles - ? objectTransform(roles, ({ permissions = [], is_default, implicit_allow }, name) => { - return Role.createWithPermissionNames(name, permissions, is_default, implicit_allow); - }) - : {}; - const _permissions = permissionNames.map((name) => new Permission(name)); - return new Guard(_permissions, Object.values(_roles), config); - } - getPermissionNames(): string[] { return this.permissions.map((permission) => permission.name); } @@ -77,7 +77,7 @@ export class Guard { return this; } - registerPermission(permission: Permission) { + registerPermission(permission: Permission) { if (this.permissions.find((p) => p.name === permission.name)) { throw new Error(`Permission ${permission.name} already exists`); } @@ -86,9 +86,13 @@ export class Guard { return this; } - registerPermissions(permissions: Record); - registerPermissions(permissions: Permission[]); - registerPermissions(permissions: Permission[] | Record) { + registerPermissions(permissions: Record>); + registerPermissions(permissions: Permission[]); + registerPermissions( + permissions: + | Permission[] + | Record>, + ) { const p = Array.isArray(permissions) ? permissions : Object.values(permissions); for (const permission of p) { @@ -121,69 +125,133 @@ export class Guard { return this.config?.enabled === true; } - hasPermission(permission: Permission, user?: GuardUserContext): boolean; - hasPermission(name: string, user?: GuardUserContext): boolean; - hasPermission(permissionOrName: Permission | string, user?: GuardUserContext): boolean { - if (!this.isEnabled()) { - return true; - } - - const name = typeof permissionOrName === "string" ? permissionOrName : permissionOrName.name; - $console.debug("guard: checking permission", { - name, - user: { id: user?.id, role: user?.role }, - }); - const exists = this.permissionExists(name); - if (!exists) { - throw new Error(`Permission ${name} does not exist`); - } - + private collect(permission: Permission, c: GuardContext, context: any) { + const user = c && "get" in c ? c.get("auth")?.user : c; + const ctx = { + ...((context ?? {}) as any), + ...this.config?.context, + user, + }; + const exists = this.permissionExists(permission.name); const role = this.getUserRole(user); + const rolePermission = role?.permissions.find( + (rolePermission) => rolePermission.permission.name === permission.name, + ); + return { + ctx, + user, + exists, + role, + rolePermission, + }; + } + + granted

>( + permission: P, + c: GuardContext, + context: PermissionContext

, + ): void; + granted

>(permission: P, c: GuardContext): void; + granted

>( + permission: P, + c: GuardContext, + context?: PermissionContext

, + ): void { + if (!this.isEnabled()) { + return; + } + const { ctx, user, exists, role, rolePermission } = this.collect(permission, c, context); + + $console.debug("guard: checking permission", { + name: permission.name, + context: ctx, + }); + if (!exists) { + throw new GuardPermissionsException( + permission, + undefined, + `Permission ${permission.name} does not exist`, + ); + } if (!role) { $console.debug("guard: user has no role, denying"); - return false; + throw new GuardPermissionsException(permission, undefined, "User has no role"); } else if (role.implicit_allow === true) { $console.debug(`guard: role "${role.name}" has implicit allow, allowing`); - return true; + return; } - const rolePermission = role.permissions.find( - (rolePermission) => rolePermission.permission.name === name, - ); - - $console.debug("guard: rolePermission, allowing?", { - permission: name, - role: role.name, - allowing: !!rolePermission, - }); - return !!rolePermission; - } - - granted

( - permission: P, - c?: GuardContext, - context: s.Static = {} as s.Static, - ): boolean { - const user = c && "get" in c ? c.get("auth")?.user : c; - const ctx = { - ...context, - user, - context: this.config?.context, - }; - return this.hasPermission(permission, user); - } - - throwUnlessGranted

( - permission: P, - c: GuardContext, - context: s.Static, - ) { - if (!this.granted(permission, c)) { - throw new Exception( - `Permission "${typeof permission === "string" ? permission : permission.name}" not granted`, - 403, + if (!rolePermission) { + $console.debug("guard: rolePermission not found, denying"); + throw new GuardPermissionsException( + permission, + undefined, + "Role does not have required permission", ); } + + // validate context + let ctx2 = Object.assign({}, ctx); + if (permission.context) { + ctx2 = permission.parseContext(ctx2); + } + + if (rolePermission?.policies.length > 0) { + $console.debug("guard: rolePermission has policies, checking"); + for (const policy of rolePermission.policies) { + // skip filter policies + if (policy.content.effect === "filter") continue; + + // if condition unmet or effect is deny, throw + const meets = policy.meetsCondition(ctx2); + if (!meets || (meets && policy.content.effect === "deny")) { + throw new GuardPermissionsException( + permission, + policy, + "Policy does not meet condition", + ); + } + } + } + + $console.debug("guard allowing", { + permission: permission.name, + role: role.name, + }); + } + + getPolicyFilter

>( + permission: P, + c: GuardContext, + context: PermissionContext

, + ): PolicySchema["filter"] | undefined; + getPolicyFilter

>( + permission: P, + c: GuardContext, + ): PolicySchema["filter"] | undefined; + getPolicyFilter

>( + permission: P, + c: GuardContext, + context?: PermissionContext

, + ): PolicySchema["filter"] | undefined { + if (!permission.isFilterable()) return; + + const { ctx, exists, role, rolePermission } = this.collect(permission, c, context); + + // validate context + let ctx2 = Object.assign({}, ctx); + if (permission.context) { + ctx2 = permission.parseContext(ctx2); + } + + if (exists && role && rolePermission && rolePermission.policies.length > 0) { + for (const policy of rolePermission.policies) { + if (policy.content.effect === "filter") { + return policy.meetsFilter(ctx2) ? policy.content.filter : undefined; + } + } + } + return; } } diff --git a/app/src/auth/authorize/Permission.ts b/app/src/auth/authorize/Permission.ts new file mode 100644 index 0000000..cd7b51b --- /dev/null +++ b/app/src/auth/authorize/Permission.ts @@ -0,0 +1,68 @@ +import { s, type ParseOptions, parse, InvalidSchemaError, HttpStatus } from "bknd/utils"; + +export const permissionOptionsSchema = s + .strictObject({ + description: s.string(), + filterable: s.boolean(), + }) + .partial(); + +export type PermissionOptions = s.Static; +export type PermissionContext

> = P extends Permission< + any, + any, + infer Context, + any +> + ? Context extends s.ObjectSchema + ? s.Static + : never + : never; + +export class InvalidPermissionContextError extends InvalidSchemaError { + override name = "InvalidPermissionContextError"; + + // changing to internal server error because it's an unexpected behavior + override code = HttpStatus.INTERNAL_SERVER_ERROR; + + static from(e: InvalidSchemaError) { + return new InvalidPermissionContextError(e.schema, e.value, e.errors); + } +} + +export class Permission< + Name extends string = string, + Options extends PermissionOptions = {}, + Context extends s.ObjectSchema | undefined = undefined, + ContextValue = Context extends s.ObjectSchema ? s.Static : undefined, +> { + constructor( + public name: Name, + public options: Options = {} as Options, + public context: Context = undefined as Context, + ) {} + + isFilterable() { + return this.options.filterable === true; + } + + parseContext(ctx: ContextValue, opts?: ParseOptions) { + try { + return this.context ? parse(this.context!, ctx, opts) : undefined; + } catch (e) { + if (e instanceof InvalidSchemaError) { + throw InvalidPermissionContextError.from(e); + } + + throw e; + } + } + + toJSON() { + return { + name: this.name, + ...this.options, + context: this.context, + }; + } +} diff --git a/app/src/auth/authorize/Policy.ts b/app/src/auth/authorize/Policy.ts new file mode 100644 index 0000000..fd873af --- /dev/null +++ b/app/src/auth/authorize/Policy.ts @@ -0,0 +1,42 @@ +import { s, parse, recursivelyReplacePlaceholders } from "bknd/utils"; +import * as query from "core/object/query/object-query"; + +export const policySchema = s + .strictObject({ + description: s.string(), + condition: s.object({}).optional() as s.Schema<{}, query.ObjectQuery | undefined>, + effect: s.string({ enum: ["allow", "deny", "filter"], default: "allow" }), + filter: s.object({}).optional() as s.Schema<{}, query.ObjectQuery | undefined>, + }) + .partial(); +export type PolicySchema = s.Static; + +export class Policy { + public content: Schema; + + constructor(content?: Schema) { + this.content = parse(policySchema, content ?? {}, { + withDefaults: true, + }) as Schema; + } + + replace(context: object, vars?: Record) { + return vars ? recursivelyReplacePlaceholders(context, /^@([a-zA-Z_\.]+)$/, vars) : context; + } + + meetsCondition(context: object, vars?: Record) { + return query.validate(this.replace(this.content.condition!, vars), context); + } + + meetsFilter(subject: object, vars?: Record) { + return query.validate(this.replace(this.content.filter!, vars), subject); + } + + getFiltered(given: Given): Given { + return given.filter((item) => this.meetsFilter(item)) as Given; + } + + toJSON() { + return this.content; + } +} diff --git a/app/src/auth/authorize/Role.ts b/app/src/auth/authorize/Role.ts index 54efaf1..0cf038b 100644 --- a/app/src/auth/authorize/Role.ts +++ b/app/src/auth/authorize/Role.ts @@ -1,10 +1,33 @@ -import { Permission } from "core/security/Permission"; +import { parse, s } from "bknd/utils"; +import { Permission } from "./Permission"; +import { Policy, policySchema } from "./Policy"; + +export const rolePermissionSchema = s.strictObject({ + permission: s.string(), + policies: s.array(policySchema).optional(), +}); +export type RolePermissionSchema = s.Static; + +export const roleSchema = s.strictObject({ + name: s.string(), + permissions: s.anyOf([s.array(s.string()), s.array(rolePermissionSchema)]).optional(), + is_default: s.boolean().optional(), + implicit_allow: s.boolean().optional(), +}); +export type RoleSchema = s.Static; export class RolePermission { constructor( - public permission: Permission, - public config?: any, + public permission: Permission, + public policies: Policy[] = [], ) {} + + toJSON() { + return { + permission: this.permission.name, + policies: this.policies.map((p) => p.toJSON()), + }; + } } export class Role { @@ -15,31 +38,24 @@ export class Role { public implicit_allow: boolean = false, ) {} - static createWithPermissionNames( - name: string, - permissionNames: string[], - is_default: boolean = false, - implicit_allow: boolean = false, - ) { - return new Role( - name, - permissionNames.map((name) => new RolePermission(new Permission(name))), - is_default, - implicit_allow, - ); + static create(config: RoleSchema) { + const permissions = + config.permissions?.map((p: string | RolePermissionSchema) => { + if (typeof p === "string") { + return new RolePermission(new Permission(p), []); + } + const policies = p.policies?.map((policy) => new Policy(policy)); + return new RolePermission(new Permission(p.permission), policies); + }) ?? []; + return new Role(config.name, permissions, config.is_default, config.implicit_allow); } - static create(config: { - name: string; - permissions?: string[]; - is_default?: boolean; - implicit_allow?: boolean; - }) { - return new Role( - config.name, - config.permissions?.map((name) => new RolePermission(new Permission(name))) ?? [], - config.is_default, - config.implicit_allow, - ); + toJSON() { + return { + name: this.name, + permissions: this.permissions.map((p) => p.toJSON()), + is_default: this.is_default, + implicit_allow: this.implicit_allow, + }; } } diff --git a/app/src/auth/middlewares.ts b/app/src/auth/middlewares/auth.middleware.ts similarity index 52% rename from app/src/auth/middlewares.ts rename to app/src/auth/middlewares/auth.middleware.ts index 685a9bd..eeebe45 100644 --- a/app/src/auth/middlewares.ts +++ b/app/src/auth/middlewares/auth.middleware.ts @@ -1,9 +1,7 @@ -import type { Permission } from "core/security/Permission"; -import { $console, patternMatch, type s } from "bknd/utils"; +import { $console, patternMatch } from "bknd/utils"; import type { Context } from "hono"; import { createMiddleware } from "hono/factory"; import type { ServerEnv } from "modules/Controller"; -import type { MaybePromise } from "core/types"; function getPath(reqOrCtx: Request | Context) { const req = reqOrCtx instanceof Request ? reqOrCtx : reqOrCtx.req.raw; @@ -68,49 +66,3 @@ export const auth = (options?: { authCtx.resolved = false; authCtx.user = undefined; }); - -export const permission =

( - permission: P, - options?: { - onGranted?: (c: Context) => MaybePromise; - onDenied?: (c: Context) => MaybePromise; - context?: (c: Context) => MaybePromise>; - }, -) => - // @ts-ignore - createMiddleware(async (c, next) => { - const app = c.get("app"); - const authCtx = c.get("auth"); - if (!authCtx) { - throw new Error("auth ctx not found"); - } - - // in tests, app is not defined - if (!authCtx.registered || !app) { - const msg = `auth middleware not registered, cannot check permissions for ${getPath(c)}`; - if (app?.module.auth.enabled) { - throw new Error(msg); - } else { - $console.warn(msg); - } - } else if (!authCtx.skip) { - const guard = app.modules.ctx().guard; - const context = (await options?.context?.(c)) ?? ({} as any); - - if (options?.onGranted || options?.onDenied) { - let returned: undefined | void | Response; - if (guard.granted(permission, c, context)) { - returned = await options?.onGranted?.(c); - } else { - returned = await options?.onDenied?.(c); - } - if (returned instanceof Response) { - return returned; - } - } else { - guard.throwUnlessGranted(permission, c, context); - } - } - - await next(); - }); diff --git a/app/src/auth/middlewares/permission.middleware.ts b/app/src/auth/middlewares/permission.middleware.ts new file mode 100644 index 0000000..ac38a08 --- /dev/null +++ b/app/src/auth/middlewares/permission.middleware.ts @@ -0,0 +1,93 @@ +import type { Permission, PermissionContext } from "auth/authorize/Permission"; +import { $console, threw } from "bknd/utils"; +import type { Context, Hono } from "hono"; +import type { RouterRoute } from "hono/types"; +import { createMiddleware } from "hono/factory"; +import type { ServerEnv } from "modules/Controller"; +import type { MaybePromise } from "core/types"; + +function getPath(reqOrCtx: Request | Context) { + const req = reqOrCtx instanceof Request ? reqOrCtx : reqOrCtx.req.raw; + return new URL(req.url).pathname; +} + +const permissionSymbol = Symbol.for("permission"); + +type PermissionMiddlewareOptions

> = { + onGranted?: (c: Context) => MaybePromise; + onDenied?: (c: Context) => MaybePromise; +} & (P extends Permission + ? PC extends undefined + ? { + context?: never; + } + : { + context: (c: Context) => MaybePromise>; + } + : { + context?: never; + }); + +export function permission

>( + permission: P, + options: PermissionMiddlewareOptions

, +) { + // @ts-ignore (middlewares do not always return) + const handler = createMiddleware(async (c, next) => { + const app = c.get("app"); + const authCtx = c.get("auth"); + if (!authCtx) { + throw new Error("auth ctx not found"); + } + + // in tests, app is not defined + if (!authCtx.registered || !app) { + const msg = `auth middleware not registered, cannot check permissions for ${getPath(c)}`; + if (app?.module.auth.enabled) { + throw new Error(msg); + } else { + $console.warn(msg); + } + } else if (!authCtx.skip) { + const guard = app.modules.ctx().guard; + const context = (await options?.context?.(c)) ?? ({} as any); + + if (options?.onGranted || options?.onDenied) { + let returned: undefined | void | Response; + if (threw(() => guard.granted(permission, c, context))) { + returned = await options?.onDenied?.(c); + } else { + returned = await options?.onGranted?.(c); + } + if (returned instanceof Response) { + return returned; + } + } else { + guard.granted(permission, c, context); + } + } + + await next(); + }); + + return Object.assign(handler, { + [permissionSymbol]: { permission, options }, + }); +} + +export function getPermissionRoutes(hono: Hono) { + const routes: { + route: RouterRoute; + permission: Permission; + options: PermissionMiddlewareOptions; + }[] = []; + for (const route of hono.routes) { + if (permissionSymbol in route.handler) { + routes.push({ + route, + ...(route.handler[permissionSymbol] as any), + }); + } + } + return routes; +} diff --git a/app/src/core/security/Permission.ts b/app/src/core/security/Permission.ts deleted file mode 100644 index 5ca4d84..0000000 --- a/app/src/core/security/Permission.ts +++ /dev/null @@ -1,95 +0,0 @@ -import { - s, - type ParseOptions, - parse, - InvalidSchemaError, - recursivelyReplacePlaceholders, -} from "bknd/utils"; -import * as query from "core/object/query/object-query"; - -export const permissionOptionsSchema = s - .strictObject({ - description: s.string(), - filterable: s.boolean(), - }) - .partial(); - -export type PermissionOptions = s.Static; - -export class InvalidPermissionContextError extends InvalidSchemaError { - override name = "InvalidPermissionContextError"; - - static from(e: InvalidSchemaError) { - return new InvalidPermissionContextError(e.schema, e.value, e.errors); - } -} - -export class Permission< - Name extends string = string, - Options extends PermissionOptions = {}, - Context extends s.ObjectSchema = s.ObjectSchema, -> { - constructor( - public name: Name, - public options: Options = {} as Options, - public context: Context = s.object({}) as Context, - ) {} - - parseContext(ctx: s.Static, opts?: ParseOptions) { - try { - return parse(this.context, ctx, opts); - } catch (e) { - if (e instanceof InvalidSchemaError) { - throw InvalidPermissionContextError.from(e); - } - - throw e; - } - } - - toJSON() { - return { - name: this.name, - ...this.options, - context: this.context, - }; - } -} - -export const policySchema = s - .strictObject({ - description: s.string(), - condition: s.object({}, { default: {} }) as s.Schema<{}, query.ObjectQuery>, - effect: s.string({ enum: ["allow", "deny", "filter"], default: "deny" }), - filter: s.object({}, { default: {} }) as s.Schema<{}, query.ObjectQuery>, - }) - .partial(); -export type PolicySchema = s.Static; - -export class Policy { - public content: Schema; - - constructor(content?: Schema) { - this.content = parse(policySchema, content ?? {}) as Schema; - } - - replace(context: object, vars?: Record) { - return vars ? recursivelyReplacePlaceholders(context, /^@([a-zA-Z_\.]+)$/, vars) : context; - } - - meetsCondition(context: object, vars?: Record) { - return query.validate(this.replace(this.content.condition!, vars), context); - } - - meetsFilter(subject: object, vars?: Record) { - return query.validate(this.replace(this.content.filter!, vars), subject); - } - - getFiltered(given: Given): Given { - return given.filter((item) => this.meetsFilter(item)) as Given; - } - - toJSON() { - return this.content; - } -} diff --git a/app/src/core/utils/runtime.ts b/app/src/core/utils/runtime.ts index 0772abd..9b8e385 100644 --- a/app/src/core/utils/runtime.ts +++ b/app/src/core/utils/runtime.ts @@ -61,3 +61,12 @@ export function invariant(condition: boolean | any, message: string) { throw new Error(message); } } + +export function threw(fn: () => any) { + try { + fn(); + return false; + } catch (e) { + return true; + } +} diff --git a/app/src/core/utils/schema/index.ts b/app/src/core/utils/schema/index.ts index d30aae1..ff8190c 100644 --- a/app/src/core/utils/schema/index.ts +++ b/app/src/core/utils/schema/index.ts @@ -1,3 +1,5 @@ +import { Exception } from "core/errors"; +import { HttpStatus } from "bknd/utils"; import * as s from "jsonv-ts"; export { validator as jsc, type Options } from "jsonv-ts/hono"; @@ -58,8 +60,9 @@ export const stringIdentifier = s.string({ maxLength: 150, }); -export class InvalidSchemaError extends Error { +export class InvalidSchemaError extends Exception { override name = "InvalidSchemaError"; + override code = HttpStatus.UNPROCESSABLE_ENTITY; constructor( public schema: s.Schema, diff --git a/app/src/data/api/DataController.ts b/app/src/data/api/DataController.ts index 163f0af..d4f9cdf 100644 --- a/app/src/data/api/DataController.ts +++ b/app/src/data/api/DataController.ts @@ -42,7 +42,7 @@ export class DataController extends Controller { override getController() { const { permission, auth } = this.middlewares; - const hono = this.create().use(auth(), permission(SystemPermissions.accessApi)); + const hono = this.create().use(auth(), permission(SystemPermissions.accessApi, {})); const entitiesEnum = this.getEntitiesEnum(this.em); // info @@ -58,7 +58,7 @@ export class DataController extends Controller { // sync endpoint hono.get( "/sync", - permission(DataPermissions.databaseSync), + permission(DataPermissions.databaseSync, {}), mcpTool("data_sync", { // @todo: should be removed if readonly annotations: { @@ -95,7 +95,7 @@ export class DataController extends Controller { // read entity schema hono.get( "/schema.json", - permission(DataPermissions.entityRead), + permission(DataPermissions.entityRead, {}), describeRoute({ summary: "Retrieve data schema", tags: ["data"], @@ -121,7 +121,7 @@ export class DataController extends Controller { // read schema hono.get( "/schemas/:entity/:context?", - permission(DataPermissions.entityRead), + permission(DataPermissions.entityRead, {}), describeRoute({ summary: "Retrieve entity schema", tags: ["data"], @@ -161,7 +161,7 @@ export class DataController extends Controller { */ hono.get( "/info/:entity", - permission(DataPermissions.entityRead), + permission(DataPermissions.entityRead, {}), describeRoute({ summary: "Retrieve entity info", tags: ["data"], @@ -213,7 +213,7 @@ export class DataController extends Controller { // fn: count hono.post( "/:entity/fn/count", - permission(DataPermissions.entityRead), + permission(DataPermissions.entityRead, {}), describeRoute({ summary: "Count entities", tags: ["data"], @@ -236,7 +236,7 @@ export class DataController extends Controller { // fn: exists hono.post( "/:entity/fn/exists", - permission(DataPermissions.entityRead), + permission(DataPermissions.entityRead, {}), describeRoute({ summary: "Check if entity exists", tags: ["data"], @@ -285,7 +285,7 @@ export class DataController extends Controller { parameters: saveRepoQueryParams(["limit", "offset", "sort", "select", "join"]), tags: ["data"], }), - permission(DataPermissions.entityRead), + permission(DataPermissions.entityRead, {}), jsc("param", s.object({ entity: entitiesEnum })), jsc("query", repoQuery, { skipOpenAPI: true }), async (c) => { @@ -308,7 +308,7 @@ export class DataController extends Controller { parameters: saveRepoQueryParams(["offset", "sort", "select"]), tags: ["data"], }), - permission(DataPermissions.entityRead), + permission(DataPermissions.entityRead, {}), mcpTool("data_entity_read_one", { inputSchema: { param: s.object({ entity: entitiesEnum, id: idType }), @@ -344,7 +344,7 @@ export class DataController extends Controller { parameters: saveRepoQueryParams(), tags: ["data"], }), - permission(DataPermissions.entityRead), + permission(DataPermissions.entityRead, {}), jsc( "param", s.object({ @@ -390,7 +390,7 @@ export class DataController extends Controller { }, tags: ["data"], }), - permission(DataPermissions.entityRead), + permission(DataPermissions.entityRead, {}), mcpTool("data_entity_read_many", { inputSchema: { param: s.object({ entity: entitiesEnum }), @@ -421,7 +421,7 @@ export class DataController extends Controller { summary: "Insert one or many", tags: ["data"], }), - permission(DataPermissions.entityCreate), + permission(DataPermissions.entityCreate, {}), mcpTool("data_entity_insert"), jsc("param", s.object({ entity: entitiesEnum })), jsc("json", s.anyOf([s.object({}), s.array(s.object({}))])), @@ -455,7 +455,7 @@ export class DataController extends Controller { summary: "Update many", tags: ["data"], }), - permission(DataPermissions.entityUpdate), + permission(DataPermissions.entityUpdate, {}), mcpTool("data_entity_update_many", { inputSchema: { param: s.object({ entity: entitiesEnum }), @@ -495,7 +495,7 @@ export class DataController extends Controller { summary: "Update one", tags: ["data"], }), - permission(DataPermissions.entityUpdate), + permission(DataPermissions.entityUpdate, {}), mcpTool("data_entity_update_one"), jsc("param", s.object({ entity: entitiesEnum, id: idType })), jsc("json", s.object({})), @@ -518,7 +518,7 @@ export class DataController extends Controller { summary: "Delete one", tags: ["data"], }), - permission(DataPermissions.entityDelete), + permission(DataPermissions.entityDelete, {}), mcpTool("data_entity_delete_one"), jsc("param", s.object({ entity: entitiesEnum, id: idType })), async (c) => { @@ -539,7 +539,7 @@ export class DataController extends Controller { summary: "Delete many", tags: ["data"], }), - permission(DataPermissions.entityDelete), + permission(DataPermissions.entityDelete, {}), mcpTool("data_entity_delete_many", { inputSchema: { param: s.object({ entity: entitiesEnum }), diff --git a/app/src/data/permissions/index.ts b/app/src/data/permissions/index.ts index 3db75ed..124980e 100644 --- a/app/src/data/permissions/index.ts +++ b/app/src/data/permissions/index.ts @@ -1,4 +1,4 @@ -import { Permission } from "core/security/Permission"; +import { Permission } from "auth/authorize/Permission"; export const entityRead = new Permission("data.entity.read"); export const entityCreate = new Permission("data.entity.create"); diff --git a/app/src/index.ts b/app/src/index.ts index ae01151..0f3d980 100644 --- a/app/src/index.ts +++ b/app/src/index.ts @@ -45,7 +45,7 @@ export type { MaybePromise } from "core/types"; export { Exception, BkndError } from "core/errors"; export { isDebug, env } from "core/env"; export { type PrimaryFieldType, config, type DB, type AppEntity } from "core/config"; -export { Permission } from "core/security/Permission"; +export { Permission } from "auth/authorize/Permission"; export { getFlashMessage } from "core/server/flash"; export * from "core/drivers"; export { Event, InvalidEventReturn } from "core/events/Event"; diff --git a/app/src/media/api/MediaController.ts b/app/src/media/api/MediaController.ts index 3c67bc7..e1f795b 100644 --- a/app/src/media/api/MediaController.ts +++ b/app/src/media/api/MediaController.ts @@ -36,7 +36,7 @@ export class MediaController extends Controller { summary: "Get the list of files", tags: ["media"], }), - permission(MediaPermissions.listFiles), + permission(MediaPermissions.listFiles, {}), async (c) => { const files = await this.getStorageAdapter().listObjects(); return c.json(files); @@ -51,7 +51,7 @@ export class MediaController extends Controller { summary: "Get a file by name", tags: ["media"], }), - permission(MediaPermissions.readFile), + permission(MediaPermissions.readFile, {}), async (c) => { const { filename } = c.req.param(); if (!filename) { @@ -81,7 +81,7 @@ export class MediaController extends Controller { summary: "Delete a file by name", tags: ["media"], }), - permission(MediaPermissions.deleteFile), + permission(MediaPermissions.deleteFile, {}), async (c) => { const { filename } = c.req.param(); if (!filename) { @@ -149,7 +149,7 @@ export class MediaController extends Controller { requestBody, }), jsc("param", s.object({ filename: s.string().optional() })), - permission(MediaPermissions.uploadFile), + permission(MediaPermissions.uploadFile, {}), async (c) => { const reqname = c.req.param("filename"); @@ -189,8 +189,8 @@ export class MediaController extends Controller { }), ), jsc("query", s.object({ overwrite: s.boolean().optional() })), - permission(DataPermissions.entityCreate), - permission(MediaPermissions.uploadFile), + permission(DataPermissions.entityCreate, {}), + permission(MediaPermissions.uploadFile, {}), async (c) => { const { entity: entity_name, id: entity_id, field: field_name } = c.req.valid("param"); diff --git a/app/src/media/media-permissions.ts b/app/src/media/media-permissions.ts index 527ce28..0ae0017 100644 --- a/app/src/media/media-permissions.ts +++ b/app/src/media/media-permissions.ts @@ -1,4 +1,4 @@ -import { Permission } from "core/security/Permission"; +import { Permission } from "auth/authorize/Permission"; export const readFile = new Permission("media.file.read"); export const listFiles = new Permission("media.file.list"); diff --git a/app/src/modules/ModuleHelper.ts b/app/src/modules/ModuleHelper.ts index d671065..29c4172 100644 --- a/app/src/modules/ModuleHelper.ts +++ b/app/src/modules/ModuleHelper.ts @@ -5,7 +5,7 @@ import { entityTypes } from "data/entities/Entity"; import { isEqual } from "lodash-es"; import type { ModuleBuildContext, ModuleBuildContextMcpContext } from "./Module"; import type { EntityRelation } from "data/relations"; -import type { Permission } from "core/security/Permission"; +import type { Permission, PermissionContext } from "auth/authorize/Permission"; import { Exception } from "core/errors"; import { invariant, isPlainObject } from "bknd/utils"; @@ -114,10 +114,20 @@ export class ModuleHelper { entity.__replaceField(name, newField); } - async throwUnlessGranted( - permission: Permission, + async granted

>( c: { context: ModuleBuildContextMcpContext; raw?: unknown }, - ) { + permission: P, + context: PermissionContext

, + ): Promise; + async granted

>( + c: { context: ModuleBuildContextMcpContext; raw?: unknown }, + permission: P, + ): Promise; + async granted

>( + c: { context: ModuleBuildContextMcpContext; raw?: unknown }, + permission: P, + context?: PermissionContext

, + ): Promise { invariant(c.context.app, "app is not available in mcp context"); const auth = c.context.app.module.auth; if (!auth.enabled) return; @@ -127,12 +137,6 @@ export class ModuleHelper { } const user = await auth.authenticator?.resolveAuthFromRequest(c.raw as any); - - if (!this.ctx.guard.granted(permission, user)) { - throw new Exception( - `Permission "${typeof permission === "string" ? permission : permission.name}" not granted`, - 403, - ); - } + this.ctx.guard.granted(permission, { user }, context as any); } } diff --git a/app/src/modules/middlewares/index.ts b/app/src/modules/middlewares/index.ts index be1ad59..213eb7e 100644 --- a/app/src/modules/middlewares/index.ts +++ b/app/src/modules/middlewares/index.ts @@ -1 +1,2 @@ -export { auth, permission } from "auth/middlewares"; +export { auth } from "auth/middlewares/auth.middleware"; +export { permission } from "auth/middlewares/permission.middleware"; diff --git a/app/src/modules/permissions/index.ts b/app/src/modules/permissions/index.ts index 5f5ccd1..152072d 100644 --- a/app/src/modules/permissions/index.ts +++ b/app/src/modules/permissions/index.ts @@ -1,4 +1,4 @@ -import { Permission } from "core/security/Permission"; +import { Permission } from "auth/authorize/Permission"; import { s } from "bknd/utils"; export const accessAdmin = new Permission("system.access.admin"); @@ -24,6 +24,12 @@ export const configWrite = new Permission( module: s.string().optional(), }), ); -export const schemaRead = new Permission("system.schema.read"); +export const schemaRead = new Permission( + "system.schema.read", + {}, + s.object({ + module: s.string().optional(), + }), +); export const build = new Permission("system.build"); export const mcp = new Permission("system.mcp"); diff --git a/app/src/modules/server/AdminController.tsx b/app/src/modules/server/AdminController.tsx index 2cdb2a7..454ad40 100644 --- a/app/src/modules/server/AdminController.tsx +++ b/app/src/modules/server/AdminController.tsx @@ -116,6 +116,7 @@ export class AdminController extends Controller { onDenied: async (c) => { addFlashMessage(c, "You not allowed to read the schema", "warning"); }, + context: (c) => ({}), }), async (c) => { const obj: AdminBkndWindowContext = { @@ -147,9 +148,10 @@ export class AdminController extends Controller { return c.redirect(authRoutes.success); } }, + context: (c) => ({}), }; const redirectRouteParams = [ - permission(SystemPermissions.accessAdmin, options), + permission(SystemPermissions.accessAdmin, options as any), permission(SystemPermissions.schemaRead, options), async (c) => { return c.html(c.get("html")!); diff --git a/app/src/modules/server/AppServer.ts b/app/src/modules/server/AppServer.ts index b9fb531..e774d1e 100644 --- a/app/src/modules/server/AppServer.ts +++ b/app/src/modules/server/AppServer.ts @@ -87,6 +87,10 @@ export class AppServer extends Module { } if (err instanceof AuthException) { + if (isDebug()) { + return c.json(err.toJSON(), err.code); + } + return c.json(err.toJSON(), err.getSafeErrorAndCode().code); } diff --git a/app/src/modules/server/SystemController.ts b/app/src/modules/server/SystemController.ts index 43d688e..4469c7e 100644 --- a/app/src/modules/server/SystemController.ts +++ b/app/src/modules/server/SystemController.ts @@ -119,7 +119,7 @@ export class SystemController extends Controller { private registerConfigController(client: Hono): void { const { permission } = this.middlewares; // don't add auth again, it's already added in getController - const hono = this.create().use(permission(SystemPermissions.configRead)); + const hono = this.create(); /* .use(permission(SystemPermissions.configRead)); */ if (!this.app.isReadOnly()) { const manager = this.app.modules as DbModuleManager; @@ -130,7 +130,11 @@ export class SystemController extends Controller { summary: "Get the raw config", tags: ["system"], }), - permission(SystemPermissions.configReadSecrets), + permission(SystemPermissions.configReadSecrets, { + context: (c) => ({ + module: c.req.param("module"), + }), + }), async (c) => { // @ts-expect-error "fetch" is private return c.json(await this.app.modules.fetch().then((r) => r?.configs)); @@ -165,7 +169,11 @@ export class SystemController extends Controller { hono.post( "/set/:module", - permission(SystemPermissions.configWrite), + permission(SystemPermissions.configWrite, { + context: (c) => ({ + module: c.req.param("module"), + }), + }), jsc("query", s.object({ force: s.boolean().optional() }), { skipOpenAPI: true }), async (c) => { const module = c.req.param("module") as any; @@ -194,32 +202,44 @@ export class SystemController extends Controller { }, ); - hono.post("/add/:module/:path", permission(SystemPermissions.configWrite), async (c) => { - // @todo: require auth (admin) - const module = c.req.param("module") as any; - const value = await c.req.json(); - const path = c.req.param("path") as string; + hono.post( + "/add/:module/:path", + permission(SystemPermissions.configWrite, { + context: (c) => ({ + module: c.req.param("module"), + }), + }), + async (c) => { + // @todo: require auth (admin) + const module = c.req.param("module") as any; + const value = await c.req.json(); + const path = c.req.param("path") as string; - if (this.app.modules.get(module).schema().has(path)) { - return c.json( - { success: false, path, error: "Path already exists" }, - { status: 400 }, - ); - } + if (this.app.modules.get(module).schema().has(path)) { + return c.json( + { success: false, path, error: "Path already exists" }, + { status: 400 }, + ); + } - return await handleConfigUpdateResponse(c, async () => { - await manager.mutateConfigSafe(module).patch(path, value); - return { - success: true, - module, - config: this.app.module[module].config, - }; - }); - }); + return await handleConfigUpdateResponse(c, async () => { + await manager.mutateConfigSafe(module).patch(path, value); + return { + success: true, + module, + config: this.app.module[module].config, + }; + }); + }, + ); hono.patch( "/patch/:module/:path", - permission(SystemPermissions.configWrite), + permission(SystemPermissions.configWrite, { + context: (c) => ({ + module: c.req.param("module"), + }), + }), async (c) => { // @todo: require auth (admin) const module = c.req.param("module") as any; @@ -239,7 +259,11 @@ export class SystemController extends Controller { hono.put( "/overwrite/:module/:path", - permission(SystemPermissions.configWrite), + permission(SystemPermissions.configWrite, { + context: (c) => ({ + module: c.req.param("module"), + }), + }), async (c) => { // @todo: require auth (admin) const module = c.req.param("module") as any; @@ -259,7 +283,11 @@ export class SystemController extends Controller { hono.delete( "/remove/:module/:path", - permission(SystemPermissions.configWrite), + permission(SystemPermissions.configWrite, { + context: (c) => ({ + module: c.req.param("module"), + }), + }), async (c) => { // @todo: require auth (admin) const module = c.req.param("module") as any; @@ -296,7 +324,7 @@ export class SystemController extends Controller { const { module } = c.req.valid("param"); if (secrets) { - this.ctx.guard.throwUnlessGranted(SystemPermissions.configReadSecrets, c, { + this.ctx.guard.granted(SystemPermissions.configReadSecrets, c, { module, }); } @@ -330,7 +358,11 @@ export class SystemController extends Controller { summary: "Get the schema for a module", tags: ["system"], }), - permission(SystemPermissions.schemaRead), + permission(SystemPermissions.schemaRead, { + context: (c) => ({ + module: c.req.param("module"), + }), + }), jsc( "query", s @@ -347,12 +379,12 @@ export class SystemController extends Controller { const readonly = this.app.isReadOnly(); if (config) { - this.ctx.guard.throwUnlessGranted(SystemPermissions.configRead, c, { + this.ctx.guard.granted(SystemPermissions.configRead, c, { module, }); } if (secrets) { - this.ctx.guard.throwUnlessGranted(SystemPermissions.configReadSecrets, c, { + this.ctx.guard.granted(SystemPermissions.configReadSecrets, c, { module, }); } @@ -395,7 +427,7 @@ export class SystemController extends Controller { jsc("query", s.object({ sync: s.boolean().optional(), fetch: s.boolean().optional() })), async (c) => { const options = c.req.valid("query") as Record; - this.ctx.guard.throwUnlessGranted(SystemPermissions.build, c, {}); + this.ctx.guard.granted(SystemPermissions.build, c); await this.app.build(options); return c.json({ @@ -467,7 +499,7 @@ export class SystemController extends Controller { const { version, ...appConfig } = this.app.toJSON(); mcp.resource("system_config", "bknd://system/config", async (c) => { - await c.context.ctx().helper.throwUnlessGranted(SystemPermissions.configRead, c); + await c.context.ctx().helper.granted(c, SystemPermissions.configRead, {}); return c.json(this.app.toJSON(), { title: "System Config", @@ -477,7 +509,9 @@ export class SystemController extends Controller { "system_config_module", "bknd://system/config/{module}", async (c, { module }) => { - await this.ctx.helper.throwUnlessGranted(SystemPermissions.configRead, c); + await this.ctx.helper.granted(c, SystemPermissions.configRead, { + module, + }); const m = this.app.modules.get(module as any) as Module; return c.json(m.toJSON(), { @@ -489,7 +523,7 @@ export class SystemController extends Controller { }, ) .resource("system_schema", "bknd://system/schema", async (c) => { - await this.ctx.helper.throwUnlessGranted(SystemPermissions.schemaRead, c); + await this.ctx.helper.granted(c, SystemPermissions.schemaRead, {}); return c.json(this.app.getSchema(), { title: "System Schema", @@ -499,7 +533,9 @@ export class SystemController extends Controller { "system_schema_module", "bknd://system/schema/{module}", async (c, { module }) => { - await this.ctx.helper.throwUnlessGranted(SystemPermissions.schemaRead, c); + await this.ctx.helper.granted(c, SystemPermissions.schemaRead, { + module, + }); const m = this.app.modules.get(module as any); return c.json(m.getSchema().toJSON(), { From 7e5c28d62196f67b4ba1f00baac2a8ee8e22aa24 Mon Sep 17 00:00:00 2001 From: dswbx Date: Mon, 13 Oct 2025 21:03:49 +0200 Subject: [PATCH 03/14] enhance Guard and permission handling with new test cases - Updated the `Guard` class to improve context validation and permission checks, ensuring clearer error messages for unmet conditions. - Refactored the `Policy` and `RolePermission` classes to support default effects and better handle conditions and filters. - Enhanced tests in `authorize.spec.ts` and `permissions.spec.ts` to cover new permission scenarios, including guest and member role behaviors. - Added new tests for context validation in permission middleware, ensuring robust error handling for invalid contexts. - Improved utility functions for better integration with the updated permission structure. --- app/__test__/auth/authorize/authorize.spec.ts | 153 ++++++++++++++++-- .../auth/authorize/permissions.spec.ts | 116 +++++++++++-- app/__test__/core/object/object-query.spec.ts | 10 ++ app/src/auth/authorize/Guard.ts | 57 ++++--- app/src/auth/authorize/Policy.ts | 3 + app/src/auth/authorize/Role.ts | 10 +- .../auth/middlewares/permission.middleware.ts | 3 +- app/src/core/object/query/query.ts | 8 +- app/src/core/utils/runtime.ts | 9 +- 9 files changed, 317 insertions(+), 52 deletions(-) diff --git a/app/__test__/auth/authorize/authorize.spec.ts b/app/__test__/auth/authorize/authorize.spec.ts index fbf787f..5e39fb8 100644 --- a/app/__test__/auth/authorize/authorize.spec.ts +++ b/app/__test__/auth/authorize/authorize.spec.ts @@ -1,19 +1,12 @@ import { describe, expect, test } from "bun:test"; import { Guard, type GuardConfig } from "auth/authorize/Guard"; import { Permission } from "auth/authorize/Permission"; -import { Role } from "auth/authorize/Role"; -import { objectTransform } from "bknd/utils"; +import { Role, type RoleSchema } from "auth/authorize/Role"; +import { objectTransform, s } from "bknd/utils"; function createGuard( permissionNames: string[], - roles?: Record< - string, - { - permissions?: string[]; - is_default?: boolean; - implicit_allow?: boolean; - } - >, + roles?: Record>, config?: GuardConfig, ) { const _roles = roles @@ -26,7 +19,9 @@ function createGuard( } describe("authorize", () => { - const read = new Permission("read"); + const read = new Permission("read", { + filterable: true, + }); const write = new Permission("write"); test("basic", async () => { @@ -109,4 +104,140 @@ describe("authorize", () => { expect(guard.granted(read, {})).toBeUndefined(); expect(guard.granted(write, {})).toBeUndefined(); }); + + describe("cases", () => { + test("guest none, member deny if user.enabled is false", () => { + const guard = createGuard( + ["read"], + { + guest: { + is_default: true, + }, + member: { + permissions: [ + { + permission: "read", + policies: [ + { + condition: {}, + effect: "filter", + filter: { + type: "member", + }, + }, + { + condition: { + "user.enabled": false, + }, + effect: "deny", + }, + ], + }, + ], + }, + }, + { enabled: true }, + ); + + expect(() => guard.granted(read, { role: "guest" })).toThrow(); + + // member is allowed, because default role permission effect is allow + // and no deny policy is met + expect(guard.granted(read, { role: "member" })).toBeUndefined(); + + // member is allowed, because deny policy is not met + expect(guard.granted(read, { role: "member", enabled: true })).toBeUndefined(); + + // member is denied, because deny policy is met + expect(() => guard.granted(read, { role: "member", enabled: false })).toThrow(); + + // get the filter for member role + expect(guard.getPolicyFilter(read, { role: "member" })).toEqual({ + type: "member", + }); + + // get filter for guest + expect(guard.getPolicyFilter(read, {})).toBeUndefined(); + }); + + test("guest should only read posts that are public", () => { + const read = new Permission( + "read", + { + // make this permission filterable + // without this, `filter` policies have no effect + filterable: true, + }, + // expect the context to match this schema + // otherwise exit with 500 to ensure proper policy checking + s.object({ + entity: s.string(), + }), + ); + const guard = createGuard( + ["read"], + { + guest: { + // this permission is applied if no (or invalid) role is provided + is_default: true, + permissions: [ + { + permission: "read", + // effect deny means only having this permission, doesn't guarantee access + effect: "deny", + policies: [ + { + // only if this condition is met + condition: { + entity: { + $in: ["posts"], + }, + }, + // the effect is allow + effect: "allow", + }, + { + condition: { + entity: "posts", + }, + effect: "filter", + filter: { + public: true, + }, + }, + ], + }, + ], + }, + // members should be allowed to read all + member: { + permissions: [ + { + permission: "read", + }, + ], + }, + }, + { enabled: true }, + ); + + // guest can only read posts + expect(guard.granted(read, {}, { entity: "posts" })).toBeUndefined(); + expect(() => guard.granted(read, {}, { entity: "users" })).toThrow(); + + // and guests can only read public posts + expect(guard.getPolicyFilter(read, {}, { entity: "posts" })).toEqual({ + public: true, + }); + + // member can read posts and users + expect(guard.granted(read, { role: "member" }, { entity: "posts" })).toBeUndefined(); + expect(guard.granted(read, { role: "member" }, { entity: "users" })).toBeUndefined(); + + // member should not have a filter + expect( + guard.getPolicyFilter(read, { role: "member" }, { entity: "posts" }), + ).toBeUndefined(); + }); + }); }); diff --git a/app/__test__/auth/authorize/permissions.spec.ts b/app/__test__/auth/authorize/permissions.spec.ts index 2b3a5ce..f885d6e 100644 --- a/app/__test__/auth/authorize/permissions.spec.ts +++ b/app/__test__/auth/authorize/permissions.spec.ts @@ -3,7 +3,7 @@ import { s } from "bknd/utils"; import { Permission } from "auth/authorize/Permission"; import { Policy } from "auth/authorize/Policy"; import { Hono } from "hono"; -import { permission } from "auth/middlewares/permission.middleware"; +import { getPermissionRoutes, permission } from "auth/middlewares/permission.middleware"; import { auth } from "auth/middlewares/auth.middleware"; import { Guard, type GuardConfig } from "auth/authorize/Guard"; import { Role, RolePermission } from "auth/authorize/Role"; @@ -113,7 +113,8 @@ describe("Guard", () => { const r = new Role("test", [ new RolePermission(p, [ new Policy({ - filter: { a: { $eq: 1 } }, + condition: { a: { $eq: 1 } }, + filter: { foo: "bar" }, effect: "filter", }), ]), @@ -129,7 +130,7 @@ describe("Guard", () => { }, { a: 1 }, ), - ).toEqual({ a: { $eq: 1 } }); + ).toEqual({ foo: "bar" }); expect( guard.getPolicyFilter( p, @@ -158,7 +159,8 @@ describe("Guard", () => { [ new RolePermission(p, [ new Policy({ - filter: { a: { $eq: 1 } }, + condition: { a: { $eq: 1 } }, + filter: { foo: "bar" }, effect: "filter", }), ]), @@ -177,7 +179,7 @@ describe("Guard", () => { }, { a: 1 }, ), - ).toEqual({ a: { $eq: 1 } }); + ).toEqual({ foo: "bar" }); expect( guard.getPolicyFilter( p, @@ -189,7 +191,7 @@ describe("Guard", () => { ).toBeUndefined(); // if no user context given, the default role is applied // hence it can be found - expect(guard.getPolicyFilter(p, {}, { a: 1 })).toEqual({ a: { $eq: 1 } }); + expect(guard.getPolicyFilter(p, {}, { a: 1 })).toEqual({ foo: "bar" }); }); }); @@ -293,13 +295,19 @@ describe("permission middleware", () => { it("denies if user with role doesn't meet condition", async () => { const p = new Permission("test"); const r = new Role("test", [ - new RolePermission(p, [ - new Policy({ - condition: { - a: { $lt: 1 }, - }, - }), - ]), + new RolePermission( + p, + [ + new Policy({ + condition: { + a: { $lt: 1 }, + }, + // default effect is allow + }), + ], + // change default effect to deny if no condition is met + "deny", + ), ]); const hono = makeApp([p], [r], { context: { @@ -391,6 +399,88 @@ describe("permission middleware", () => { // expecting 500 because bknd should have handled it correctly expect(res.status).toBe(500); }); + + it("checks context on routes with permissions", async () => { + const make = (user: any) => { + const p = new Permission( + "test", + {}, + s.object({ + a: s.number(), + }), + ); + const r = new Role("test", [ + new RolePermission(p, [ + new Policy({ + condition: { + a: { $eq: 1 }, + }, + }), + ]), + ]); + return makeApp([p], [r]) + .use(async (c, next) => { + // @ts-expect-error + c.set("auth", { registered: true, user }); + await next(); + }) + .get( + "/valid", + permission(p, { + context: (c) => ({ + a: 1, + }), + }), + async (c) => c.text("test"), + ) + .get( + "/invalid", + permission(p, { + // @ts-expect-error + context: (c) => ({ + b: "1", + }), + }), + async (c) => c.text("test"), + ) + .get( + "/invalid2", + permission(p, { + // @ts-expect-error + context: (c) => ({}), + }), + async (c) => c.text("test"), + ) + .get( + "/invalid3", + // @ts-expect-error + permission(p), + async (c) => c.text("test"), + ); + }; + + const hono = make({ id: 0, role: "test" }); + const valid = await hono.request("/valid"); + expect(valid.status).toBe(200); + const invalid = await hono.request("/invalid"); + expect(invalid.status).toBe(500); + const invalid2 = await hono.request("/invalid2"); + expect(invalid2.status).toBe(500); + const invalid3 = await hono.request("/invalid3"); + expect(invalid3.status).toBe(500); + + { + const hono = make(null); + const valid = await hono.request("/valid"); + expect(valid.status).toBe(403); + const invalid = await hono.request("/invalid"); + expect(invalid.status).toBe(500); + const invalid2 = await hono.request("/invalid2"); + expect(invalid2.status).toBe(500); + const invalid3 = await hono.request("/invalid3"); + expect(invalid3.status).toBe(500); + } + }); }); describe("Role", () => { diff --git a/app/__test__/core/object/object-query.spec.ts b/app/__test__/core/object/object-query.spec.ts index dc03fb6..215adf8 100644 --- a/app/__test__/core/object/object-query.spec.ts +++ b/app/__test__/core/object/object-query.spec.ts @@ -66,4 +66,14 @@ describe("object-query", () => { expect(result).toBe(expected); } }); + + test("paths", () => { + const result = validate({ "user.age": { $lt: 18 } }, { user: { age: 17 } }); + expect(result).toBe(true); + }); + + test("empty filters", () => { + const result = validate({}, { user: { age: 17 } }); + expect(result).toBe(true); + }); }); diff --git a/app/src/auth/authorize/Guard.ts b/app/src/auth/authorize/Guard.ts index ac97c63..37ee842 100644 --- a/app/src/auth/authorize/Guard.ts +++ b/app/src/auth/authorize/Guard.ts @@ -160,7 +160,13 @@ export class Guard { if (!this.isEnabled()) { return; } - const { ctx, user, exists, role, rolePermission } = this.collect(permission, c, context); + const { ctx: _ctx, exists, role, rolePermission } = this.collect(permission, c, context); + + // validate context + let ctx = Object.assign({}, _ctx); + if (permission.context) { + ctx = permission.parseContext(ctx); + } $console.debug("guard: checking permission", { name: permission.name, @@ -187,32 +193,37 @@ export class Guard { throw new GuardPermissionsException( permission, undefined, - "Role does not have required permission", + `Role "${role.name}" does not have required permission`, ); } - // validate context - let ctx2 = Object.assign({}, ctx); - if (permission.context) { - ctx2 = permission.parseContext(ctx2); - } - if (rolePermission?.policies.length > 0) { $console.debug("guard: rolePermission has policies, checking"); + + // set the default effect of the role permission + let allowed = rolePermission.effect === "allow"; for (const policy of rolePermission.policies) { // skip filter policies if (policy.content.effect === "filter") continue; - // if condition unmet or effect is deny, throw - const meets = policy.meetsCondition(ctx2); - if (!meets || (meets && policy.content.effect === "deny")) { - throw new GuardPermissionsException( - permission, - policy, - "Policy does not meet condition", - ); + // if condition is met, check the effect + const meets = policy.meetsCondition(ctx); + if (meets) { + // if deny, then break early + if (policy.content.effect === "deny") { + allowed = false; + break; + + // if allow, set allow but continue checking + } else if (policy.content.effect === "allow") { + allowed = true; + } } } + + if (!allowed) { + throw new GuardPermissionsException(permission, undefined, "Policy condition unmet"); + } } $console.debug("guard allowing", { @@ -235,20 +246,24 @@ export class Guard { c: GuardContext, context?: PermissionContext

, ): PolicySchema["filter"] | undefined { - if (!permission.isFilterable()) return; + if (!permission.isFilterable()) { + $console.debug("getPolicyFilter: permission is not filterable, returning undefined"); + return; + } - const { ctx, exists, role, rolePermission } = this.collect(permission, c, context); + const { ctx: _ctx, exists, role, rolePermission } = this.collect(permission, c, context); // validate context - let ctx2 = Object.assign({}, ctx); + let ctx = Object.assign({}, _ctx); if (permission.context) { - ctx2 = permission.parseContext(ctx2); + ctx = permission.parseContext(ctx); } if (exists && role && rolePermission && rolePermission.policies.length > 0) { for (const policy of rolePermission.policies) { if (policy.content.effect === "filter") { - return policy.meetsFilter(ctx2) ? policy.content.filter : undefined; + const meets = policy.meetsCondition(ctx); + return meets ? policy.content.filter : undefined; } } } diff --git a/app/src/auth/authorize/Policy.ts b/app/src/auth/authorize/Policy.ts index fd873af..9995e20 100644 --- a/app/src/auth/authorize/Policy.ts +++ b/app/src/auth/authorize/Policy.ts @@ -5,6 +5,7 @@ export const policySchema = s .strictObject({ description: s.string(), condition: s.object({}).optional() as s.Schema<{}, query.ObjectQuery | undefined>, + // @todo: potentially remove this, and invert from rolePermission.effect effect: s.string({ enum: ["allow", "deny", "filter"], default: "allow" }), filter: s.object({}).optional() as s.Schema<{}, query.ObjectQuery | undefined>, }) @@ -25,10 +26,12 @@ export class Policy { } meetsCondition(context: object, vars?: Record) { + if (!this.content.condition) return true; return query.validate(this.replace(this.content.condition!, vars), context); } meetsFilter(subject: object, vars?: Record) { + if (!this.content.filter) return true; return query.validate(this.replace(this.content.filter!, vars), subject); } diff --git a/app/src/auth/authorize/Role.ts b/app/src/auth/authorize/Role.ts index 0cf038b..3f072a1 100644 --- a/app/src/auth/authorize/Role.ts +++ b/app/src/auth/authorize/Role.ts @@ -1,9 +1,13 @@ -import { parse, s } from "bknd/utils"; +import { s } from "bknd/utils"; import { Permission } from "./Permission"; import { Policy, policySchema } from "./Policy"; +// default effect is allow for backward compatibility +const defaultEffect = "allow"; + export const rolePermissionSchema = s.strictObject({ permission: s.string(), + effect: s.string({ enum: ["allow", "deny"], default: defaultEffect }).optional(), policies: s.array(policySchema).optional(), }); export type RolePermissionSchema = s.Static; @@ -20,12 +24,14 @@ export class RolePermission { constructor( public permission: Permission, public policies: Policy[] = [], + public effect: "allow" | "deny" = defaultEffect, ) {} toJSON() { return { permission: this.permission.name, policies: this.policies.map((p) => p.toJSON()), + effect: this.effect, }; } } @@ -45,7 +51,7 @@ export class Role { return new RolePermission(new Permission(p), []); } const policies = p.policies?.map((policy) => new Policy(policy)); - return new RolePermission(new Permission(p.permission), policies); + return new RolePermission(new Permission(p.permission), policies, p.effect); }) ?? []; return new Role(config.name, permissions, config.is_default, config.implicit_allow); } diff --git a/app/src/auth/middlewares/permission.middleware.ts b/app/src/auth/middlewares/permission.middleware.ts index ac38a08..c6e53f4 100644 --- a/app/src/auth/middlewares/permission.middleware.ts +++ b/app/src/auth/middlewares/permission.middleware.ts @@ -5,6 +5,7 @@ import type { RouterRoute } from "hono/types"; import { createMiddleware } from "hono/factory"; import type { ServerEnv } from "modules/Controller"; import type { MaybePromise } from "core/types"; +import { GuardPermissionsException } from "auth/authorize/Guard"; function getPath(reqOrCtx: Request | Context) { const req = reqOrCtx instanceof Request ? reqOrCtx : reqOrCtx.req.raw; @@ -54,7 +55,7 @@ export function permission

>( if (options?.onGranted || options?.onDenied) { let returned: undefined | void | Response; - if (threw(() => guard.granted(permission, c, context))) { + if (threw(() => guard.granted(permission, c, context), GuardPermissionsException)) { returned = await options?.onDenied?.(c); } else { returned = await options?.onGranted?.(c); diff --git a/app/src/core/object/query/query.ts b/app/src/core/object/query/query.ts index e90921d..24352c4 100644 --- a/app/src/core/object/query/query.ts +++ b/app/src/core/object/query/query.ts @@ -1,4 +1,5 @@ import type { PrimaryFieldType } from "core/config"; +import { getPath, invariant } from "bknd/utils"; export type Primitive = PrimaryFieldType | string | number | boolean; export function isPrimitive(value: any): value is Primitive { @@ -67,8 +68,9 @@ function _convert( expressions: Exps, path: string[] = [], ): FilterQuery { + invariant(typeof $query === "object", "$query must be an object"); const ExpressionConditionKeys = expressions.map((e) => e.key); - const keys = Object.keys($query); + const keys = Object.keys($query ?? {}); const operands = [OperandOr] as const; const newQuery: FilterQuery = {}; @@ -157,7 +159,7 @@ function _build( // check $and for (const [key, value] of Object.entries($and)) { for (const [$op, $v] of Object.entries(value)) { - const objValue = options.value_is_kv ? key : options.object[key]; + const objValue = options.value_is_kv ? key : getPath(options.object, key); result.$and.push(__validate($op, $v, objValue, [key])); result.keys.add(key); } @@ -165,7 +167,7 @@ function _build( // check $or for (const [key, value] of Object.entries($or ?? {})) { - const objValue = options.value_is_kv ? key : options.object[key]; + const objValue = options.value_is_kv ? key : getPath(options.object, key); for (const [$op, $v] of Object.entries(value)) { result.$or.push(__validate($op, $v, objValue, [key])); diff --git a/app/src/core/utils/runtime.ts b/app/src/core/utils/runtime.ts index 9b8e385..5b943ff 100644 --- a/app/src/core/utils/runtime.ts +++ b/app/src/core/utils/runtime.ts @@ -62,11 +62,18 @@ export function invariant(condition: boolean | any, message: string) { } } -export function threw(fn: () => any) { +export function threw(fn: () => any, instance?: new (...args: any[]) => Error) { try { fn(); return false; } catch (e) { + if (instance) { + if (e instanceof instance) { + return true; + } + // if instance given but not what expected, throw + throw e; + } return true; } } From 6624927286f8cfe5459878d6bfb6ab834ce8fea5 Mon Sep 17 00:00:00 2001 From: dswbx Date: Tue, 14 Oct 2025 16:36:16 +0200 Subject: [PATCH 04/14] enhance form field components and add JsonEditor support - Updated `ObjectField`, `ArrayField`, and `FieldWrapper` components to improve flexibility and integration options by supporting additional props like `wrapperProps`. - Added `JsonEditor` for enhanced object editing capabilities with state management and safety checks. - Refactored utility functions and error handling for improved stability and developer experience. - Introduced new test cases to validate `JsonEditor` functionality and schema-based forms handling. --- app/src/ui/components/code/JsonEditor.tsx | 35 +++++++- .../ui/components/form/Formy/components.tsx | 3 +- .../form/json-schema-form/ArrayField.tsx | 83 ++++++++++++------- .../form/json-schema-form/Field.tsx | 2 +- .../form/json-schema-form/FieldWrapper.tsx | 7 +- .../form/json-schema-form/ObjectField.tsx | 10 ++- .../components/form/json-schema-form/utils.ts | 15 ++-- .../ui/routes/test/tests/code-editor-test.tsx | 13 +++ .../routes/test/tests/json-schema-form3.tsx | 49 ++++++++++- 9 files changed, 172 insertions(+), 45 deletions(-) create mode 100644 app/src/ui/routes/test/tests/code-editor-test.tsx diff --git a/app/src/ui/components/code/JsonEditor.tsx b/app/src/ui/components/code/JsonEditor.tsx index ec96811..c65e59a 100644 --- a/app/src/ui/components/code/JsonEditor.tsx +++ b/app/src/ui/components/code/JsonEditor.tsx @@ -1,9 +1,37 @@ -import { Suspense, lazy } from "react"; +import { Suspense, lazy, useState } from "react"; import { twMerge } from "tailwind-merge"; import type { CodeEditorProps } from "./CodeEditor"; const CodeEditor = lazy(() => import("./CodeEditor")); -export function JsonEditor({ editable, className, ...props }: CodeEditorProps) { +export type JsonEditorProps = Omit & { + value?: object; + onChange?: (value: object) => void; + emptyAs?: "null" | "undefined"; +}; + +export function JsonEditor({ + editable, + className, + value, + onChange, + onBlur, + emptyAs = "undefined", + ...props +}: JsonEditorProps) { + const [editorValue, setEditorValue] = useState( + JSON.stringify(value, null, 2), + ); + const handleChange = (given: string) => { + const value = given === "" ? (emptyAs === "null" ? null : undefined) : given; + try { + setEditorValue(value); + onChange?.(value ? JSON.parse(value) : value); + } catch (e) {} + }; + const handleBlur = (e) => { + setEditorValue(JSON.stringify(value, null, 2)); + onBlur?.(e); + }; return ( diff --git a/app/src/ui/components/form/Formy/components.tsx b/app/src/ui/components/form/Formy/components.tsx index 502a844..3ad9146 100644 --- a/app/src/ui/components/form/Formy/components.tsx +++ b/app/src/ui/components/form/Formy/components.tsx @@ -28,8 +28,9 @@ export const Group = ({ return ( { +export type ArrayFieldProps = { + path?: string; + labelAdd?: string; + wrapperProps?: Omit; +}; + +export const ArrayField = ({ + path = "", + labelAdd = "Add", + wrapperProps = { wrapper: "fieldset" }, +}: ArrayFieldProps) => { const { setValue, pointer, required, schema, ...ctx } = useDerivedFieldContext(path); if (!schema || typeof schema === "undefined") return `ArrayField(${path}): no schema ${pointer}`; // if unique items with enum if (schema.uniqueItems && typeof schema.items === "object" && "enum" in schema.items) { return ( - + { } return ( - + {({ value }) => value?.map((v, index: number) => ( @@ -44,17 +54,21 @@ export const ArrayField = ({ path = "" }: { path?: string }) => { }

- +
); }; const ArrayItem = memo(({ path, index, schema }: any) => { - const { value, ...ctx } = useDerivedFieldContext(path, (ctx) => { + const { + value, + path: absolutePath, + ...ctx + } = useDerivedFieldContext(path, (ctx) => { return ctx.value?.[index]; }); - const itemPath = suffixPath(path, index); + const itemPath = suffixPath(absolutePath, index); let subschema = schema.items; const itemsMultiSchema = getMultiSchema(schema.items); if (itemsMultiSchema) { @@ -62,10 +76,6 @@ const ArrayItem = memo(({ path, index, schema }: any) => { subschema = _subschema; } - const handleUpdate = useEvent((pointer: string, value: any) => { - ctx.setValue(pointer, value); - }); - const handleDelete = useEvent((pointer: string) => { ctx.deleteValue(pointer); }); @@ -76,21 +86,26 @@ const ArrayItem = memo(({ path, index, schema }: any) => { ); return ( -
- { - handleUpdate(itemPath, coerce(e.target.value, subschema!)); - }} - className="w-full" - /> - {DeleteButton} -
+ +
+ {/* another wrap is required for primitive schemas */} + + {DeleteButton} +
+
); }, isEqual); +const AnotherField = (props: Partial) => { + const { value } = useFormValue(""); + + const inputProps = { + // @todo: check, potentially just provide value + value: ["string", "number", "boolean"].includes(typeof value) ? value : undefined, + }; + return ; +}; + const ArrayIterator = memo( ({ name, children }: any) => { return children(useFormValue(name)); @@ -98,19 +113,25 @@ const ArrayIterator = memo( (prev, next) => prev.value?.length === next.value?.length, ); -const ArrayAdd = ({ schema, path }: { schema: JsonSchema; path: string }) => { +const ArrayAdd = ({ + schema, + path: _path, + label = "Add", +}: { schema: JsonSchema; path: string; label?: string }) => { const { setValue, value: { currentIndex }, + path, ...ctx - } = useDerivedFieldContext(path, (ctx) => { + } = useDerivedFieldContext(_path, (ctx) => { return { currentIndex: ctx.value?.length ?? 0 }; }); const itemsMultiSchema = getMultiSchema(schema.items); + const options = { addOptionalProps: true }; function handleAdd(template?: any) { const newPath = suffixPath(path, currentIndex); - setValue(newPath, template ?? ctx.lib.getTemplate(undefined, schema!.items)); + setValue(newPath, template ?? ctx.lib.getTemplate(undefined, schema!.items, options)); } if (itemsMultiSchema) { @@ -121,14 +142,14 @@ const ArrayAdd = ({ schema, path }: { schema: JsonSchema; path: string }) => { }} items={itemsMultiSchema.map((s, i) => ({ label: s!.title ?? `Option ${i + 1}`, - onClick: () => handleAdd(ctx.lib.getTemplate(undefined, s!)), + onClick: () => handleAdd(ctx.lib.getTemplate(undefined, s!, options)), }))} onClickItem={console.log} > - + ); } - return ; + return ; }; diff --git a/app/src/ui/components/form/json-schema-form/Field.tsx b/app/src/ui/components/form/json-schema-form/Field.tsx index 60351ca..bc81a83 100644 --- a/app/src/ui/components/form/json-schema-form/Field.tsx +++ b/app/src/ui/components/form/json-schema-form/Field.tsx @@ -72,7 +72,7 @@ const FieldImpl = ({ ); if (isType(schema.type, "object")) { - return ; + return ; } if (isType(schema.type, "array")) { diff --git a/app/src/ui/components/form/json-schema-form/FieldWrapper.tsx b/app/src/ui/components/form/json-schema-form/FieldWrapper.tsx index 784db35..af4607c 100644 --- a/app/src/ui/components/form/json-schema-form/FieldWrapper.tsx +++ b/app/src/ui/components/form/json-schema-form/FieldWrapper.tsx @@ -11,6 +11,7 @@ import { } from "ui/components/form/json-schema-form/Form"; import { Popover } from "ui/components/overlay/Popover"; import { getLabel } from "./utils"; +import { twMerge } from "tailwind-merge"; export type FieldwrapperProps = { name: string; @@ -25,6 +26,7 @@ export type FieldwrapperProps = { description?: string; descriptionPlacement?: "top" | "bottom"; fieldId?: string; + className?: string; }; export function FieldWrapper({ @@ -38,6 +40,7 @@ export function FieldWrapper({ descriptionPlacement = "bottom", children, fieldId, + className, ...props }: FieldwrapperProps) { const errors = useFormError(name, { strict: true }); @@ -60,7 +63,7 @@ export function FieldWrapper({ 0} as={wrapper === "fieldset" ? "fieldset" : "div"} - className={hidden ? "hidden" : "relative"} + className={twMerge(hidden ? "hidden" : "relative", className)} > {errorPlacement === "top" && Errors} @@ -76,7 +79,7 @@ export function FieldWrapper({ )} {descriptionPlacement === "top" && Description} -
+
{Children.count(children) === 1 && isValidElement(children) ? cloneElement(children, { diff --git a/app/src/ui/components/form/json-schema-form/ObjectField.tsx b/app/src/ui/components/form/json-schema-form/ObjectField.tsx index 59deceb..3cf920b 100644 --- a/app/src/ui/components/form/json-schema-form/ObjectField.tsx +++ b/app/src/ui/components/form/json-schema-form/ObjectField.tsx @@ -11,7 +11,7 @@ export type ObjectFieldProps = { }; export const ObjectField = ({ path = "", label: _label, wrapperProps = {} }: ObjectFieldProps) => { - const { schema, ...ctx } = useDerivedFieldContext(path); + const { schema } = useDerivedFieldContext(path); if (!isTypeSchema(schema)) return `ObjectField "${path}": no schema`; const properties = Object.entries(schema.properties ?? {}) as [string, JSONSchema][]; @@ -24,7 +24,7 @@ export const ObjectField = ({ path = "", label: _label, wrapperProps = {} }: Obj {...wrapperProps} > {properties.length === 0 ? ( - No properties + ) : ( properties.map(([prop, schema]) => { const name = [path, prop].filter(Boolean).join("."); @@ -40,3 +40,9 @@ export const ObjectField = ({ path = "", label: _label, wrapperProps = {} }: Obj ); }; + +export const ObjectJsonField = ({ path }: { path: string }) => { + const { value } = useFormValue(path); + const { setValue, path: absolutePath } = useDerivedFieldContext(path); + return setValue(absolutePath, value)} />; +}; diff --git a/app/src/ui/components/form/json-schema-form/utils.ts b/app/src/ui/components/form/json-schema-form/utils.ts index 333bba3..7b755cf 100644 --- a/app/src/ui/components/form/json-schema-form/utils.ts +++ b/app/src/ui/components/form/json-schema-form/utils.ts @@ -67,18 +67,23 @@ export function isRequired(lib: Draft, pointer: string, schema: JsonSchema, data return false; } - const childSchema = lib.getSchema({ pointer, data, schema }); - if (typeof childSchema === "object" && "const" in childSchema) { - return true; + try { + const childSchema = lib.getSchema({ pointer, data, schema }); + if (typeof childSchema === "object" && "const" in childSchema) { + return true; + } + } catch (e) { + return false; } const parentPointer = getParentPointer(pointer); const parentSchema = lib.getSchema({ pointer: parentPointer, data }); - const required = parentSchema?.required?.includes(pointer.split("/").pop()!); + const l = pointer.split("/").pop(); + const required = parentSchema?.required?.includes(l); return !!required; } catch (e) { - console.error("isRequired", { pointer, schema, data, e }); + console.warn("isRequired", { pointer, schema, data, e }); return false; } } diff --git a/app/src/ui/routes/test/tests/code-editor-test.tsx b/app/src/ui/routes/test/tests/code-editor-test.tsx new file mode 100644 index 0000000..99bcee1 --- /dev/null +++ b/app/src/ui/routes/test/tests/code-editor-test.tsx @@ -0,0 +1,13 @@ +import { useState } from "react"; +import { JsonEditor } from "ui/components/code/JsonEditor"; +import { JsonViewer } from "ui/components/code/JsonViewer"; + +export default function CodeEditorTest() { + const [value, setValue] = useState({}); + return ( +
+ + +
+ ); +} diff --git a/app/src/ui/routes/test/tests/json-schema-form3.tsx b/app/src/ui/routes/test/tests/json-schema-form3.tsx index 401ab1f..f1d219c 100644 --- a/app/src/ui/routes/test/tests/json-schema-form3.tsx +++ b/app/src/ui/routes/test/tests/json-schema-form3.tsx @@ -56,6 +56,14 @@ const authSchema = { }, } as const satisfies JSONSchema; +const objectCodeSchema = { + type: "object", + properties: { + name: { type: "string" }, + config: { type: "object", properties: {} }, + }, +}; + const formOptions = { debug: true, }; @@ -77,6 +85,45 @@ export default function JsonSchemaForm3() { {/*
*/} + + + + + {/* + /> */} {/* console.log("change", data)} From 1b8ce41837362d0fc48c4eb901d9c11d547ad7dd Mon Sep 17 00:00:00 2001 From: dswbx Date: Tue, 14 Oct 2025 16:36:42 +0200 Subject: [PATCH 05/14] role and permission handling in auth module - Updated the `Role` class to change the `create` method signature for improved clarity and flexibility. - Refactored the `guardRoleSchema` to utilize the new `roleSchema` for better consistency. - Introduced a new `TPermission` type to enhance type safety in permission handling across the application. - Updated various components and forms to accommodate the new permission structure, ensuring backward compatibility. - Enhanced the `AuthRolesEdit` and `AuthRolesList` components to improve role management and permissions display. - Added new API endpoints for fetching permissions, improving the overall functionality of the auth module. --- app/src/auth/AppAuth.ts | 5 +- app/src/auth/auth-schema.ts | 10 +- app/src/auth/authorize/Permission.ts | 7 + app/src/auth/authorize/Role.ts | 7 +- app/src/modules/server/SystemController.ts | 19 +- app/src/ui/client/BkndProvider.tsx | 3 +- .../form/json-schema-form/ObjectField.tsx | 3 +- .../ui/routes/auth/auth.roles.edit.$role.tsx | 217 +++++++++++++++--- app/src/ui/routes/auth/auth.roles.tsx | 17 +- app/src/ui/routes/auth/forms/role.form.tsx | 12 +- .../routes/settings/routes/auth.settings.tsx | 4 +- app/src/ui/routes/test/index.tsx | 2 + 12 files changed, 254 insertions(+), 52 deletions(-) diff --git a/app/src/auth/AppAuth.ts b/app/src/auth/AppAuth.ts index a0c6072..cead597 100644 --- a/app/src/auth/AppAuth.ts +++ b/app/src/auth/AppAuth.ts @@ -61,7 +61,7 @@ export class AppAuth extends Module { // register roles const roles = transformObject(this.config.roles ?? {}, (role, name) => { - return Role.create({ name, ...role }); + return Role.create(name, role); }); this.ctx.guard.setRoles(Object.values(roles)); this.ctx.guard.setConfig(this.config.guard ?? {}); @@ -210,10 +210,13 @@ export class AppAuth extends Module { } const strategies = this.authenticator.getStrategies(); + const roles = Object.fromEntries(this.ctx.guard.getRoles().map((r) => [r.name, r.toJSON()])); + console.log("roles", roles); return { ...this.config, ...this.authenticator.toJSON(secrets), + roles: secrets ? roles : undefined, strategies: transformObject(strategies, (strategy) => ({ enabled: this.isStrategyEnabled(strategy), ...strategy.toJSON(secrets), diff --git a/app/src/auth/auth-schema.ts b/app/src/auth/auth-schema.ts index 4fd40a4..e479ea1 100644 --- a/app/src/auth/auth-schema.ts +++ b/app/src/auth/auth-schema.ts @@ -1,6 +1,7 @@ import { cookieConfig, jwtConfig } from "auth/authenticate/Authenticator"; import { CustomOAuthStrategy, OAuthStrategy, PasswordStrategy } from "auth/authenticate/strategies"; -import { objectTransform, s } from "bknd/utils"; +import { roleSchema } from "auth/authorize/Role"; +import { objectTransform, omitKeys, pick, s } from "bknd/utils"; import { $object, $record } from "modules/mcp"; export const Strategies = { @@ -40,11 +41,8 @@ export type AppAuthCustomOAuthStrategy = s.Static; export type PermissionContext

> = P extends Permission< any, diff --git a/app/src/auth/authorize/Role.ts b/app/src/auth/authorize/Role.ts index 3f072a1..7506fc7 100644 --- a/app/src/auth/authorize/Role.ts +++ b/app/src/auth/authorize/Role.ts @@ -13,7 +13,7 @@ export const rolePermissionSchema = s.strictObject({ export type RolePermissionSchema = s.Static; export const roleSchema = s.strictObject({ - name: s.string(), + // @todo: remove anyOf, add migration permissions: s.anyOf([s.array(s.string()), s.array(rolePermissionSchema)]).optional(), is_default: s.boolean().optional(), implicit_allow: s.boolean().optional(), @@ -44,7 +44,7 @@ export class Role { public implicit_allow: boolean = false, ) {} - static create(config: RoleSchema) { + static create(name: string, config: RoleSchema) { const permissions = config.permissions?.map((p: string | RolePermissionSchema) => { if (typeof p === "string") { @@ -53,12 +53,11 @@ export class Role { const policies = p.policies?.map((policy) => new Policy(policy)); return new RolePermission(new Permission(p.permission), policies, p.effect); }) ?? []; - return new Role(config.name, permissions, config.is_default, config.implicit_allow); + return new Role(name, permissions, config.is_default, config.implicit_allow); } toJSON() { return { - name: this.name, permissions: this.permissions.map((p) => p.toJSON()), is_default: this.is_default, implicit_allow: this.implicit_allow, diff --git a/app/src/modules/server/SystemController.ts b/app/src/modules/server/SystemController.ts index 4469c7e..45787bf 100644 --- a/app/src/modules/server/SystemController.ts +++ b/app/src/modules/server/SystemController.ts @@ -32,6 +32,7 @@ import { getVersion } from "core/env"; import type { Module } from "modules/Module"; import { getSystemMcp } from "modules/mcp/system-mcp"; import type { DbModuleManager } from "modules/db/DbModuleManager"; +import type { TPermission } from "auth/authorize/Permission"; export type ConfigUpdate = { success: true; @@ -46,7 +47,8 @@ export type SchemaResponse = { schema: ModuleSchemas; readonly: boolean; config: ModuleConfigs; - permissions: string[]; + //permissions: string[]; + permissions: TPermission[]; }; export class SystemController extends Controller { @@ -412,11 +414,24 @@ export class SystemController extends Controller { readonly, schema, config: config ? this.app.toJSON(secrets) : undefined, - permissions: this.app.modules.ctx().guard.getPermissionNames(), + permissions: this.app.modules.ctx().guard.getPermissions(), + //permissions: this.app.modules.ctx().guard.getPermissionNames(), }); }, ); + hono.get( + "/permissions", + describeRoute({ + summary: "Get the permissions", + tags: ["system"], + }), + (c) => { + const permissions = this.app.modules.ctx().guard.getPermissions(); + return c.json({ permissions }); + }, + ); + hono.post( "/build", describeRoute({ diff --git a/app/src/ui/client/BkndProvider.tsx b/app/src/ui/client/BkndProvider.tsx index abb0020..c37d182 100644 --- a/app/src/ui/client/BkndProvider.tsx +++ b/app/src/ui/client/BkndProvider.tsx @@ -15,13 +15,14 @@ import { AppReduced } from "./utils/AppReduced"; import { Message } from "ui/components/display/Message"; import { useNavigate } from "ui/lib/routes"; import type { BkndAdminProps } from "ui/Admin"; +import type { TPermission } from "auth/authorize/Permission"; export type BkndContext = { version: number; readonly: boolean; schema: ModuleSchemas; config: ModuleConfigs; - permissions: string[]; + permissions: TPermission[]; hasSecrets: boolean; requireSecrets: () => Promise; actions: ReturnType; diff --git a/app/src/ui/components/form/json-schema-form/ObjectField.tsx b/app/src/ui/components/form/json-schema-form/ObjectField.tsx index 3cf920b..748bf23 100644 --- a/app/src/ui/components/form/json-schema-form/ObjectField.tsx +++ b/app/src/ui/components/form/json-schema-form/ObjectField.tsx @@ -2,7 +2,8 @@ import { isTypeSchema } from "ui/components/form/json-schema-form/utils"; import { AnyOfField } from "./AnyOfField"; import { Field } from "./Field"; import { FieldWrapper, type FieldwrapperProps } from "./FieldWrapper"; -import { type JSONSchema, useDerivedFieldContext } from "./Form"; +import { type JSONSchema, useDerivedFieldContext, useFormValue } from "./Form"; +import { JsonEditor } from "ui/components/code/JsonEditor"; export type ObjectFieldProps = { path?: string; diff --git a/app/src/ui/routes/auth/auth.roles.edit.$role.tsx b/app/src/ui/routes/auth/auth.roles.edit.$role.tsx index 7ee4ea6..ff7c39d 100644 --- a/app/src/ui/routes/auth/auth.roles.edit.$role.tsx +++ b/app/src/ui/routes/auth/auth.roles.edit.$role.tsx @@ -1,17 +1,38 @@ -import { useRef } from "react"; -import { TbDots } from "react-icons/tb"; import { useBknd } from "ui/client/bknd"; -import { useBkndAuth } from "ui/client/schema/auth/use-bknd-auth"; -import { Button } from "ui/components/buttons/Button"; -import { IconButton } from "ui/components/buttons/IconButton"; import { Message } from "ui/components/display/Message"; +import { useBkndAuth } from "ui/client/schema/auth/use-bknd-auth"; +import { useBrowserTitle } from "ui/hooks/use-browser-title"; +import { useRef, useState } from "react"; +import { useNavigate } from "ui/lib/routes"; +import { isDebug } from "core/env"; import { Dropdown } from "ui/components/overlay/Dropdown"; -import * as AppShell from "ui/layouts/AppShell/AppShell"; +import { IconButton } from "ui/components/buttons/IconButton"; +import { TbAdjustments, TbDots, TbLock, TbLockOpen, TbLockOpen2 } from "react-icons/tb"; +import { Button } from "ui/components/buttons/Button"; import { Breadcrumbs2 } from "ui/layouts/AppShell/Breadcrumbs2"; -import { routes, useNavigate } from "ui/lib/routes"; -import { AuthRoleForm, type AuthRoleFormRef } from "ui/routes/auth/forms/role.form"; +import { routes } from "ui/lib/routes"; +import * as AppShell from "ui/layouts/AppShell/AppShell"; +import * as Formy from "ui/components/form/Formy"; + +import { ucFirst, type s } from "bknd/utils"; +import type { ModuleSchemas } from "bknd"; +import { + ArrayField, + Field, + Form, + FormDebug, + Subscribe, + useFormContext, + useFormValue, +} from "ui/components/form/json-schema-form"; +import type { TPermission } from "auth/authorize/Permission"; +import type { RoleSchema } from "auth/authorize/Role"; +import { SegmentedControl, Tooltip } from "@mantine/core"; +import { cn } from "ui/lib/utils"; export function AuthRolesEdit(props) { + useBrowserTitle(["Auth", "Roles", props.params.role]); + const { hasSecrets } = useBknd({ withSecrets: true }); if (!hasSecrets) { return ; @@ -20,32 +41,46 @@ export function AuthRolesEdit(props) { return ; } +// currently for backward compatibility +function getSchema(authSchema: ModuleSchemas["auth"]) { + const roles = authSchema.properties.roles.additionalProperties; + return { + ...roles, + properties: { + ...roles.properties, + permissions: { + ...roles.properties.permissions.anyOf[1], + }, + }, + }; +} + +const formConfig = { + options: { + debug: isDebug(), + }, +}; + function AuthRolesEditInternal({ params }) { const [navigate] = useNavigate(); - const { config, actions } = useBkndAuth(); + const { config, schema: authSchema, actions } = useBkndAuth(); const roleName = params.role; const role = config.roles?.[roleName]; - const formRef = useRef(null); const { readonly } = useBknd(); + const schema = getSchema(authSchema); - async function handleUpdate() { - console.log("data", formRef.current?.isValid()); - if (!formRef.current?.isValid()) return; - const data = formRef.current?.getData(); + async function handleDelete() {} + async function handleUpdate(data: any) { + console.log("data", data); const success = await actions.roles.patch(roleName, data); - if (success) { + console.log("success", success); + /* if (success) { navigate(routes.auth.roles.list()); - } - } - - async function handleDelete() { - if (await actions.roles.delete(roleName)) { - navigate(routes.auth.roles.list()); - } + } */ } return ( - <> + @@ -69,9 +104,23 @@ function AuthRolesEditInternal({ params }) { {!readonly && ( - + ({ + dirty: state.dirty, + errors: state.errors.length > 0, + submitting: state.submitting, + })} + > + {({ dirty, errors, submitting }) => ( + + )} + )} } @@ -85,8 +134,120 @@ function AuthRolesEditInternal({ params }) { /> - +

+
+ +
+ +
+ + +
+
+ - + ); } + +type PermissionsData = Exclude; +type PermissionData = PermissionsData[number]; + +const Permissions = () => { + const { permissions } = useBknd(); + + const grouped = permissions.reduce( + (acc, permission, index) => { + const [group, name] = permission.name.split(".") as [string, string]; + if (!acc[group]) acc[group] = []; + acc[group].push({ index, permission }); + return acc; + }, + {} as Record, + ); + + return ( +
+ {Object.entries(grouped).map(([group, rows]) => { + return ( +
+

{ucFirst(group)} Permissions

+
+ {rows.map(({ index, permission }) => ( + + ))} +
+
+ ); + })} +
+ ); +}; + +const Permission = ({ permission, index }: { permission: TPermission; index?: number }) => { + const path = `permissions.${index}`; + const { value } = useFormValue(path); + const { setValue, deleteValue } = useFormContext(); + const [open, setOpen] = useState(false); + const data = value as PermissionData | undefined; + + async function handleSwitch() { + if (data) { + deleteValue(path); + } else { + setValue(path, { + permission: permission.name, + policies: [], + effect: "allow", + }); + } + } + + return ( + <> +
+
+
{permission.name}
+
+ + + setOpen((o) => !o)} + /> + +
+
+ {open && ( +
+ +
+ )} +
+ + ); +}; diff --git a/app/src/ui/routes/auth/auth.roles.tsx b/app/src/ui/routes/auth/auth.roles.tsx index 59c7e22..15596bf 100644 --- a/app/src/ui/routes/auth/auth.roles.tsx +++ b/app/src/ui/routes/auth/auth.roles.tsx @@ -12,8 +12,21 @@ import { CellValue, DataTable } from "../../components/table/DataTable"; import * as AppShell from "../../layouts/AppShell/AppShell"; import { routes, useNavigate } from "../../lib/routes"; import { useBknd } from "ui/client/bknd"; +import { useBrowserTitle } from "ui/hooks/use-browser-title"; +import { Message } from "ui/components/display/Message"; -export function AuthRolesList() { +export function AuthRolesList(props) { + useBrowserTitle(["Auth", "Roles"]); + + const { hasSecrets } = useBknd({ withSecrets: true }); + if (!hasSecrets) { + return ; + } + + return ; +} + +function AuthRolesListInternal() { const [navigate] = useNavigate(); const { config, actions } = useBkndAuth(); const { readonly } = useBknd(); @@ -21,7 +34,7 @@ export function AuthRolesList() { const data = Object.values( transformObject(config.roles ?? {}, (role, name) => ({ role: name, - permissions: role.permissions, + permissions: role.permissions?.map((p) => p.permission) as string[], is_default: role.is_default ?? false, implicit_allow: role.implicit_allow ?? false, })), diff --git a/app/src/ui/routes/auth/forms/role.form.tsx b/app/src/ui/routes/auth/forms/role.form.tsx index 0a16d2d..d1b7f51 100644 --- a/app/src/ui/routes/auth/forms/role.form.tsx +++ b/app/src/ui/routes/auth/forms/role.form.tsx @@ -34,7 +34,11 @@ export const AuthRoleForm = forwardRef< getValues, } = useForm({ resolver: standardSchemaResolver(schema), - defaultValues: role, + defaultValues: { + ...role, + // compat + permissions: role?.permissions?.map((p) => p.permission), + }, }); useImperativeHandle(ref, () => ({ @@ -47,7 +51,7 @@ export const AuthRoleForm = forwardRef<
{/*

Role Permissions

*/} - + p.name)} />
, ); - console.log("grouped", grouped); - //console.log("fieldState", fieldState, value); return (
{Object.entries(grouped).map(([group, permissions]) => { @@ -121,7 +123,7 @@ const Permissions = ({

{ucFirst(group)} Permissions

{permissions.map((permission) => { - const selected = data.includes(permission); + const selected = data.includes(permission as any); return (
diff --git a/app/src/ui/routes/settings/routes/auth.settings.tsx b/app/src/ui/routes/settings/routes/auth.settings.tsx index 6432570..a5faf5f 100644 --- a/app/src/ui/routes/settings/routes/auth.settings.tsx +++ b/app/src/ui/routes/settings/routes/auth.settings.tsx @@ -63,10 +63,10 @@ export const AuthSettings = ({ schema: _unsafe_copy, config }) => { } catch (e) {} console.log("_s", _s); const roleSchema = _schema.properties.roles?.additionalProperties ?? { type: "object" }; - if (_s.permissions) { + /* if (_s.permissions) { roleSchema.properties.permissions.items.enum = _s.permissions; roleSchema.properties.permissions.uniqueItems = true; - } + } */ return ( diff --git a/app/src/ui/routes/test/index.tsx b/app/src/ui/routes/test/index.tsx index 71bb87f..95681fd 100644 --- a/app/src/ui/routes/test/index.tsx +++ b/app/src/ui/routes/test/index.tsx @@ -27,6 +27,7 @@ import SortableTest from "./tests/sortable-test"; import { SqlAiTest } from "./tests/sql-ai-test"; import Themes from "./tests/themes"; import ErrorBoundary from "ui/components/display/ErrorBoundary"; +import CodeEditorTest from "./tests/code-editor-test"; const tests = { DropdownTest, @@ -52,6 +53,7 @@ const tests = { JsonSchemaForm3, FormyTest, HtmlFormTest, + CodeEditorTest, } as const; export default function TestRoutes() { From 0347efa592d16743544619ae4d49551808e81b77 Mon Sep 17 00:00:00 2001 From: dswbx Date: Tue, 14 Oct 2025 16:49:42 +0200 Subject: [PATCH 06/14] fix Role creation method and permission checks in tests --- app/__test__/app/mcp/mcp.auth.test.ts | 11 ++++++++--- app/__test__/auth/authorize/authorize.spec.ts | 2 +- app/__test__/auth/authorize/permissions.spec.ts | 4 ++-- app/src/auth/AppAuth.ts | 5 ++--- 4 files changed, 13 insertions(+), 9 deletions(-) diff --git a/app/__test__/app/mcp/mcp.auth.test.ts b/app/__test__/app/mcp/mcp.auth.test.ts index e7658b4..2636730 100644 --- a/app/__test__/app/mcp/mcp.auth.test.ts +++ b/app/__test__/app/mcp/mcp.auth.test.ts @@ -201,7 +201,10 @@ describe("mcp auth", async () => { }, return_config: true, }); - expect(addGuestRole.config.guest.permissions).toEqual(["read", "write"]); + expect(addGuestRole.config.guest.permissions.map((p) => p.permission)).toEqual([ + "read", + "write", + ]); // update role await tool(server, "config_auth_roles_update", { @@ -210,13 +213,15 @@ describe("mcp auth", async () => { permissions: ["read"], }, }); - expect(app.toJSON().auth.roles?.guest?.permissions).toEqual(["read"]); + expect(app.toJSON().auth.roles?.guest?.permissions?.map((p) => p.permission)).toEqual([ + "read", + ]); // get role const getGuestRole = await tool(server, "config_auth_roles_get", { key: "guest", }); - expect(getGuestRole.value.permissions).toEqual(["read"]); + expect(getGuestRole.value.permissions.map((p) => p.permission)).toEqual(["read"]); // remove role await tool(server, "config_auth_roles_remove", { diff --git a/app/__test__/auth/authorize/authorize.spec.ts b/app/__test__/auth/authorize/authorize.spec.ts index 5e39fb8..b13935a 100644 --- a/app/__test__/auth/authorize/authorize.spec.ts +++ b/app/__test__/auth/authorize/authorize.spec.ts @@ -11,7 +11,7 @@ function createGuard( ) { const _roles = roles ? objectTransform(roles, ({ permissions = [], is_default, implicit_allow }, name) => { - return Role.create({ name, permissions, is_default, implicit_allow }); + return Role.create(name, { permissions, is_default, implicit_allow }); }) : {}; const _permissions = permissionNames.map((name) => new Permission(name)); diff --git a/app/__test__/auth/authorize/permissions.spec.ts b/app/__test__/auth/authorize/permissions.spec.ts index f885d6e..78abdd0 100644 --- a/app/__test__/auth/authorize/permissions.spec.ts +++ b/app/__test__/auth/authorize/permissions.spec.ts @@ -252,7 +252,7 @@ describe("permission middleware", () => { it("allows if user has (plain) role", async () => { const p = new Permission("test"); - const r = Role.create({ name: "test", permissions: [p.name] }); + const r = Role.create("test", { permissions: [p.name] }); const hono = makeApp([p], [r]) .use(async (c, next) => { // @ts-expect-error @@ -512,7 +512,7 @@ describe("Role", () => { true, ); const json = JSON.parse(JSON.stringify(r.toJSON())); - const r2 = Role.create(json); + const r2 = Role.create(p.name, json); expect(r2.toJSON()).toEqual(r.toJSON()); }); }); diff --git a/app/src/auth/AppAuth.ts b/app/src/auth/AppAuth.ts index cead597..a73f9ec 100644 --- a/app/src/auth/AppAuth.ts +++ b/app/src/auth/AppAuth.ts @@ -2,7 +2,7 @@ import type { DB, PrimaryFieldType } from "bknd"; import * as AuthPermissions from "auth/auth-permissions"; import type { AuthStrategy } from "auth/authenticate/strategies/Strategy"; import type { PasswordStrategy } from "auth/authenticate/strategies/PasswordStrategy"; -import { $console, secureRandomString, transformObject } from "bknd/utils"; +import { $console, secureRandomString, transformObject, pick } from "bknd/utils"; import type { Entity, EntityManager } from "data/entities"; import { em, entity, enumm, type FieldSchema } from "data/prototype"; import { Module } from "modules/Module"; @@ -211,12 +211,11 @@ export class AppAuth extends Module { const strategies = this.authenticator.getStrategies(); const roles = Object.fromEntries(this.ctx.guard.getRoles().map((r) => [r.name, r.toJSON()])); - console.log("roles", roles); return { ...this.config, ...this.authenticator.toJSON(secrets), - roles: secrets ? roles : undefined, + roles, strategies: transformObject(strategies, (strategy) => ({ enabled: this.isStrategyEnabled(strategy), ...strategy.toJSON(secrets), From 38902ebcbae89a78399214770981387a94637a85 Mon Sep 17 00:00:00 2001 From: dswbx Date: Tue, 21 Oct 2025 16:44:08 +0200 Subject: [PATCH 07/14] Update permissions handling and enhance Guard functionality - Bump `jsonv-ts` dependency to 0.8.6. - Refactor permission checks in the `Guard` class to improve context validation and error handling. - Update tests to reflect changes in permission handling, ensuring robust coverage for new scenarios. - Introduce new test cases for data permissions, enhancing overall test coverage and reliability. --- app/__test__/auth/authorize/authorize.spec.ts | 8 +- .../auth/authorize/data.permissions.test.ts | 327 ++++++++++++++++++ .../{ => http}/SystemController.spec.ts | 2 +- .../auth/authorize/permissions.spec.ts | 32 +- app/__test__/core/utils.spec.ts | 88 ++++- app/__test__/data/data.test.ts | 4 +- app/package.json | 2 +- app/src/auth/api/AuthController.ts | 8 +- app/src/auth/authorize/Guard.ts | 76 +++- app/src/auth/authorize/Permission.ts | 2 + app/src/auth/authorize/Policy.ts | 11 +- app/src/core/utils/objects.ts | 19 +- app/src/data/api/DataController.ts | 141 ++++++-- app/src/data/entities/query/Repository.ts | 41 ++- app/src/data/permissions/index.ts | 50 ++- .../form/json-schema-form/Field.tsx | 8 +- .../components/form/json-schema-form/Form.tsx | 7 +- .../ui/routes/auth/auth.roles.edit.$role.tsx | 166 +++++++-- app/src/ui/routes/auth/auth.roles.tsx | 6 + bun.lock | 14 +- 20 files changed, 859 insertions(+), 153 deletions(-) create mode 100644 app/__test__/auth/authorize/data.permissions.test.ts rename app/__test__/auth/authorize/{ => http}/SystemController.spec.ts (93%) diff --git a/app/__test__/auth/authorize/authorize.spec.ts b/app/__test__/auth/authorize/authorize.spec.ts index b13935a..caa5566 100644 --- a/app/__test__/auth/authorize/authorize.spec.ts +++ b/app/__test__/auth/authorize/authorize.spec.ts @@ -152,12 +152,12 @@ describe("authorize", () => { expect(() => guard.granted(read, { role: "member", enabled: false })).toThrow(); // get the filter for member role - expect(guard.getPolicyFilter(read, { role: "member" })).toEqual({ + expect(guard.filters(read, { role: "member" }).filter).toEqual({ type: "member", }); // get filter for guest - expect(guard.getPolicyFilter(read, {})).toBeUndefined(); + expect(guard.filters(read, {}).filter).toBeUndefined(); }); test("guest should only read posts that are public", () => { @@ -226,7 +226,7 @@ describe("authorize", () => { expect(() => guard.granted(read, {}, { entity: "users" })).toThrow(); // and guests can only read public posts - expect(guard.getPolicyFilter(read, {}, { entity: "posts" })).toEqual({ + expect(guard.filters(read, {}, { entity: "posts" }).filter).toEqual({ public: true, }); @@ -236,7 +236,7 @@ describe("authorize", () => { // member should not have a filter expect( - guard.getPolicyFilter(read, { role: "member" }, { entity: "posts" }), + guard.filters(read, { role: "member" }, { entity: "posts" }).filter, ).toBeUndefined(); }); }); diff --git a/app/__test__/auth/authorize/data.permissions.test.ts b/app/__test__/auth/authorize/data.permissions.test.ts new file mode 100644 index 0000000..6ff0c3e --- /dev/null +++ b/app/__test__/auth/authorize/data.permissions.test.ts @@ -0,0 +1,327 @@ +import { describe, it, expect, beforeAll, afterAll } from "bun:test"; +import { createApp } from "core/test/utils"; +import type { CreateAppConfig } from "App"; +import * as proto from "data/prototype"; +import { mergeObject } from "core/utils/objects"; +import type { App, DB } from "bknd"; +import type { CreateUserPayload } from "auth/AppAuth"; +import { disableConsoleLog, enableConsoleLog } from "core/utils/test"; + +beforeAll(() => disableConsoleLog()); +afterAll(() => enableConsoleLog()); + +async function makeApp(config: Partial = {}) { + const app = createApp({ + config: mergeObject( + { + data: proto + .em( + { + users: proto.systemEntity("users", {}), + posts: proto.entity("posts", { + title: proto.text(), + content: proto.text(), + }), + comments: proto.entity("comments", { + content: proto.text(), + }), + }, + ({ relation }, { users, posts, comments }) => { + relation(posts).manyToOne(users); + relation(comments).manyToOne(posts); + }, + ) + .toJSON(), + auth: { + enabled: true, + jwt: { + secret: "secret", + }, + }, + }, + config, + ), + }); + await app.build(); + + return app; +} + +async function createUsers(app: App, users: CreateUserPayload[]) { + return Promise.all( + users.map(async (user) => { + return await app.createUser(user); + }), + ); +} + +async function loadFixtures(app: App, fixtures: Record = {}) { + const results = {} as any; + for (const [entity, data] of Object.entries(fixtures)) { + results[entity] = await app.em + .mutator(entity as any) + .insertMany(data) + .then((result) => result.data); + } + return results; +} + +describe("data permissions", async () => { + const app = await makeApp({ + server: { + mcp: { + enabled: true, + }, + }, + auth: { + guard: { + enabled: true, + }, + roles: { + guest: { + is_default: true, + permissions: [ + { + permission: "system.access.api", + }, + { + permission: "data.entity.read", + policies: [ + { + condition: { + entity: "posts", + }, + effect: "filter", + filter: { + users_id: { $isnull: 1 }, + }, + }, + ], + }, + { + permission: "data.entity.create", + policies: [ + { + condition: { + entity: "posts", + }, + effect: "filter", + filter: { + users_id: { $isnull: 1 }, + }, + }, + ], + }, + { + permission: "data.entity.update", + policies: [ + { + condition: { + entity: "posts", + }, + effect: "filter", + filter: { + users_id: { $isnull: 1 }, + }, + }, + ], + }, + { + permission: "data.entity.delete", + policies: [ + { + condition: { entity: "posts" }, + }, + { + condition: { entity: "posts" }, + effect: "filter", + filter: { + users_id: { $isnull: 1 }, + }, + }, + ], + }, + ], + }, + }, + }, + }); + const users = [ + { email: "foo@example.com", password: "password" }, + { email: "bar@example.com", password: "password" }, + ]; + const fixtures = { + posts: [ + { content: "post 1", users_id: 1 }, + { content: "post 2", users_id: 2 }, + { content: "post 3", users_id: null }, + ], + comments: [ + { content: "comment 1", posts_id: 1 }, + { content: "comment 2", posts_id: 2 }, + { content: "comment 3", posts_id: 3 }, + ], + }; + await createUsers(app, users); + const results = await loadFixtures(app, fixtures); + + describe("http", async () => { + it("read many", async () => { + // many only includes posts with users_id is null + const res = await app.server.request("/api/data/entity/posts"); + const data = await res.json().then((r: any) => r.data); + expect(data).toEqual([results.posts[2]]); + + // same with /query + { + const res = await app.server.request("/api/data/entity/posts/query", { + method: "POST", + }); + const data = await res.json().then((r: any) => r.data); + expect(data).toEqual([results.posts[2]]); + } + }); + + it("read one", async () => { + // one only includes posts with users_id is null + { + const res = await app.server.request("/api/data/entity/posts/1"); + const data = await res.json().then((r: any) => r.data); + expect(res.status).toBe(404); + expect(data).toBeUndefined(); + } + + // read one by allowed id + { + const res = await app.server.request("/api/data/entity/posts/3"); + const data = await res.json().then((r: any) => r.data); + expect(res.status).toBe(200); + expect(data).toEqual(results.posts[2]); + } + }); + + it("read many by reference", async () => { + const res = await app.server.request("/api/data/entity/posts/1/comments"); + const data = await res.json().then((r: any) => r.data); + expect(res.status).toBe(200); + expect(data).toEqual(results.comments.filter((c: any) => c.posts_id === 1)); + }); + + it("mutation create one", async () => { + // not allowed + { + const res = await app.server.request("/api/data/entity/posts", { + method: "POST", + body: JSON.stringify({ content: "post 4" }), + }); + expect(res.status).toBe(403); + } + // allowed + { + const res = await app.server.request("/api/data/entity/posts", { + method: "POST", + body: JSON.stringify({ content: "post 4", users_id: null }), + }); + expect(res.status).toBe(201); + } + }); + + it("mutation update one", async () => { + // update one: not allowed + const res = await app.server.request("/api/data/entity/posts/1", { + method: "PATCH", + body: JSON.stringify({ content: "post 4" }), + }); + expect(res.status).toBe(403); + + { + // update one: allowed + const res = await app.server.request("/api/data/entity/posts/3", { + method: "PATCH", + body: JSON.stringify({ content: "post 3 (updated)" }), + }); + expect(res.status).toBe(200); + expect(await res.json().then((r: any) => r.data.content)).toBe("post 3 (updated)"); + } + }); + + it("mutation update many", async () => { + // update many: not allowed + const res = await app.server.request("/api/data/entity/posts", { + method: "PATCH", + headers: { + "Content-Type": "application/json", + }, + body: JSON.stringify({ + update: { content: "post 4" }, + where: { users_id: { $isnull: 0 } }, + }), + }); + expect(res.status).toBe(200); // because filtered + const _data = await res.json().then((r: any) => r.data.map((p: any) => p.users_id)); + expect(_data.every((u: any) => u === null)).toBe(true); + + // verify + const data = await app.em + .repo("posts") + .findMany({ select: ["content", "users_id"] }) + .then((r) => r.data); + + // expect non null users_id to not have content "post 4" + expect( + data.filter((p: any) => p.users_id !== null).every((p: any) => p.content !== "post 4"), + ).toBe(true); + // expect null users_id to have content "post 4" + expect( + data.filter((p: any) => p.users_id === null).every((p: any) => p.content === "post 4"), + ).toBe(true); + }); + + const count = async () => { + const { + data: { count: _count }, + } = await app.em.repo("posts").count(); + return _count; + }; + it("mutation delete one", async () => { + const initial = await count(); + + // delete one: not allowed + const res = await app.server.request("/api/data/entity/posts/1", { + method: "DELETE", + }); + expect(res.status).toBe(403); + expect(await count()).toBe(initial); + + { + // delete one: allowed + const res = await app.server.request("/api/data/entity/posts/3", { + method: "DELETE", + }); + expect(res.status).toBe(200); + expect(await count()).toBe(initial - 1); + } + }); + + it("mutation delete many", async () => { + // delete many: not allowed + const res = await app.server.request("/api/data/entity/posts", { + method: "DELETE", + headers: { + "Content-Type": "application/json", + }, + body: JSON.stringify({ + where: {}, + }), + }); + expect(res.status).toBe(200); + + // only deleted posts with users_id is null + const remaining = await app.em + .repo("posts") + .findMany() + .then((r) => r.data); + expect(remaining.every((p: any) => p.users_id !== null)).toBe(true); + }); + }); +}); diff --git a/app/__test__/auth/authorize/SystemController.spec.ts b/app/__test__/auth/authorize/http/SystemController.spec.ts similarity index 93% rename from app/__test__/auth/authorize/SystemController.spec.ts rename to app/__test__/auth/authorize/http/SystemController.spec.ts index 8400f46..40e6493 100644 --- a/app/__test__/auth/authorize/SystemController.spec.ts +++ b/app/__test__/auth/authorize/http/SystemController.spec.ts @@ -10,7 +10,7 @@ async function makeApp(config: Partial = {}) { return app; } -describe("SystemController", () => { +describe.skip("SystemController", () => { it("...", async () => { const app = await makeApp(); const controller = new SystemController(app); diff --git a/app/__test__/auth/authorize/permissions.spec.ts b/app/__test__/auth/authorize/permissions.spec.ts index 78abdd0..14a01ee 100644 --- a/app/__test__/auth/authorize/permissions.spec.ts +++ b/app/__test__/auth/authorize/permissions.spec.ts @@ -122,26 +122,10 @@ describe("Guard", () => { const guard = new Guard([p], [r], { enabled: true, }); - expect( - guard.getPolicyFilter( - p, - { - role: r.name, - }, - { a: 1 }, - ), - ).toEqual({ foo: "bar" }); - expect( - guard.getPolicyFilter( - p, - { - role: r.name, - }, - { a: 2 }, - ), - ).toBeUndefined(); + expect(guard.filters(p, { role: r.name }, { a: 1 }).filter).toEqual({ foo: "bar" }); + expect(guard.filters(p, { role: r.name }, { a: 2 }).filter).toBeUndefined(); // if no user context given, filter cannot be applied - expect(guard.getPolicyFilter(p, {}, { a: 1 })).toBeUndefined(); + expect(guard.filters(p, {}, { a: 1 }).filter).toBeUndefined(); }); it("collects filters for default role", () => { @@ -172,26 +156,26 @@ describe("Guard", () => { }); expect( - guard.getPolicyFilter( + guard.filters( p, { role: r.name, }, { a: 1 }, - ), + ).filter, ).toEqual({ foo: "bar" }); expect( - guard.getPolicyFilter( + guard.filters( p, { role: r.name, }, { a: 2 }, - ), + ).filter, ).toBeUndefined(); // if no user context given, the default role is applied // hence it can be found - expect(guard.getPolicyFilter(p, {}, { a: 1 })).toEqual({ foo: "bar" }); + expect(guard.filters(p, {}, { a: 1 }).filter).toEqual({ foo: "bar" }); }); }); diff --git a/app/__test__/core/utils.spec.ts b/app/__test__/core/utils.spec.ts index b7d4c96..8957e9e 100644 --- a/app/__test__/core/utils.spec.ts +++ b/app/__test__/core/utils.spec.ts @@ -272,6 +272,7 @@ describe("Core Utils", async () => { }, /^@([a-z\.]+)$/, variables7, + null, ); expect(result7).toEqual({ number: 123, @@ -288,20 +289,85 @@ describe("Core Utils", async () => { ); expect(result8).toEqual({ message: "The value is 123!" }); - // test mixed scenarios + // test with fallback parameter + const obj9 = { user: "@user.id", config: "@config.theme" }; + const variables9 = {}; // empty context const result9 = utils.recursivelyReplacePlaceholders( - { - fullMatch: "@test.value", // should preserve number type - partialMatch: "Value: @test.value", // should convert to string - noMatch: "static text", - }, + obj9, /^@([a-z\.]+)$/, - variables7, + variables9, + null, ); - expect(result9).toEqual({ - fullMatch: 123, // number preserved - partialMatch: "Value: @test.value", // no replacement (pattern requires full match) - noMatch: "static text", + expect(result9).toEqual({ user: null, config: null }); + + // test with fallback for partial matches + const obj10 = { message: "Hello @user.name, welcome!" }; + const variables10 = {}; // empty context + const result10 = utils.recursivelyReplacePlaceholders( + obj10, + /@([a-z\.]+)/g, + variables10, + "Guest", + ); + expect(result10).toEqual({ message: "Hello Guest, welcome!" }); + + // test with different fallback types + const obj11 = { + stringFallback: "@missing.string", + numberFallback: "@missing.number", + booleanFallback: "@missing.boolean", + objectFallback: "@missing.object", + }; + const variables11 = {}; + const result11 = utils.recursivelyReplacePlaceholders( + obj11, + /^@([a-z\.]+)$/, + variables11, + "default", + ); + expect(result11).toEqual({ + stringFallback: "default", + numberFallback: "default", + booleanFallback: "default", + objectFallback: "default", + }); + + // test fallback with arrays + const obj12 = { items: ["@item1", "@item2", "static"] }; + const variables12 = { item1: "found" }; // item2 is missing + const result12 = utils.recursivelyReplacePlaceholders( + obj12, + /^@([a-zA-Z0-9\.]+)$/, + variables12, + "missing", + ); + expect(result12).toEqual({ items: ["found", "missing", "static"] }); + + // test fallback with nested objects + const obj13 = { + user: "@user.id", + settings: { + theme: "@theme.name", + nested: { + value: "@deep.value", + }, + }, + }; + const variables13 = {}; // empty context + const result13 = utils.recursivelyReplacePlaceholders( + obj13, + /^@([a-z\.]+)$/, + variables13, + null, + ); + expect(result13).toEqual({ + user: null, + settings: { + theme: null, + nested: { + value: null, + }, + }, }); }); }); diff --git a/app/__test__/data/data.test.ts b/app/__test__/data/data.test.ts index 10cf8ac..416b5c0 100644 --- a/app/__test__/data/data.test.ts +++ b/app/__test__/data/data.test.ts @@ -30,9 +30,9 @@ describe("some tests", async () => { const query = await em.repository(users).findId(1); expect(query.sql).toBe( - 'select "users"."id" as "id", "users"."username" as "username", "users"."email" as "email" from "users" where "id" = ? limit ?', + 'select "users"."id" as "id", "users"."username" as "username", "users"."email" as "email" from "users" where "id" = ? order by "users"."id" asc limit ? offset ?', ); - expect(query.parameters).toEqual([1, 1]); + expect(query.parameters).toEqual([1, 1, 0]); expect(query.data).toBeUndefined(); }); diff --git a/app/package.json b/app/package.json index f932580..c40f3b1 100644 --- a/app/package.json +++ b/app/package.json @@ -65,7 +65,7 @@ "hono": "4.8.3", "json-schema-library": "10.0.0-rc7", "json-schema-to-ts": "^3.1.1", - "jsonv-ts": "0.8.5", + "jsonv-ts": "0.8.6", "kysely": "0.27.6", "lodash-es": "^4.17.21", "oauth4webapi": "^2.11.1", diff --git a/app/src/auth/api/AuthController.ts b/app/src/auth/api/AuthController.ts index b94aa4b..99f1000 100644 --- a/app/src/auth/api/AuthController.ts +++ b/app/src/auth/api/AuthController.ts @@ -61,7 +61,9 @@ export class AuthController extends Controller { hono.post( "/create", permission(AuthPermissions.createUser, {}), - permission(DataPermissions.entityCreate, {}), + permission(DataPermissions.entityCreate, { + context: (c) => ({ entity: this.auth.config.entity_name }), + }), describeRoute({ summary: "Create a new user", tags: ["auth"], @@ -224,7 +226,6 @@ export class AuthController extends Controller { const roles = Object.keys(this.auth.config.roles ?? {}); mcp.tool( - // @todo: needs permission "auth_user_create", { description: "Create a new user", @@ -246,7 +247,6 @@ export class AuthController extends Controller { ); mcp.tool( - // @todo: needs permission "auth_user_token", { description: "Get a user token", @@ -264,7 +264,6 @@ export class AuthController extends Controller { ); mcp.tool( - // @todo: needs permission "auth_user_password_change", { description: "Change a user's password", @@ -286,7 +285,6 @@ export class AuthController extends Controller { ); mcp.tool( - // @todo: needs permission "auth_user_password_test", { description: "Test a user's password", diff --git a/app/src/auth/authorize/Guard.ts b/app/src/auth/authorize/Guard.ts index 37ee842..85349a9 100644 --- a/app/src/auth/authorize/Guard.ts +++ b/app/src/auth/authorize/Guard.ts @@ -1,5 +1,5 @@ import { Exception } from "core/errors"; -import { $console, type s } from "bknd/utils"; +import { $console, mergeObject, type s } from "bknd/utils"; import type { Permission, PermissionContext } from "auth/authorize/Permission"; import type { Context } from "hono"; import type { ServerEnv } from "modules/Controller"; @@ -232,41 +232,85 @@ export class Guard { }); } - getPolicyFilter

>( + filters

>( permission: P, c: GuardContext, context: PermissionContext

, - ): PolicySchema["filter"] | undefined; - getPolicyFilter

>( - permission: P, - c: GuardContext, - ): PolicySchema["filter"] | undefined; - getPolicyFilter

>( + ); + filters

>(permission: P, c: GuardContext); + filters

>( permission: P, c: GuardContext, context?: PermissionContext

, - ): PolicySchema["filter"] | undefined { + ) { if (!permission.isFilterable()) { - $console.debug("getPolicyFilter: permission is not filterable, returning undefined"); - return; + throw new GuardPermissionsException(permission, undefined, "Permission is not filterable"); } - const { ctx: _ctx, exists, role, rolePermission } = this.collect(permission, c, context); + const { + ctx: _ctx, + exists, + role, + user, + rolePermission, + } = this.collect(permission, c, context); // validate context - let ctx = Object.assign({}, _ctx); + let ctx = Object.assign( + { + user, + }, + _ctx, + ); + if (permission.context) { - ctx = permission.parseContext(ctx); + ctx = permission.parseContext(ctx, { + coerceDropUnknown: false, + }); } + const filters: PolicySchema["filter"][] = []; + const policies: Policy[] = []; if (exists && role && rolePermission && rolePermission.policies.length > 0) { for (const policy of rolePermission.policies) { if (policy.content.effect === "filter") { const meets = policy.meetsCondition(ctx); - return meets ? policy.content.filter : undefined; + if (meets) { + policies.push(policy); + filters.push(policy.getReplacedFilter(ctx)); + } } } } - return; + + const filter = filters.length > 0 ? mergeObject({}, ...filters) : undefined; + return { + filters, + filter, + policies, + merge: (givenFilter: object | undefined) => { + return mergeObject(givenFilter ?? {}, filter ?? {}); + }, + matches: (subject: object | object[], opts?: { throwOnError?: boolean }) => { + const subjects = Array.isArray(subject) ? subject : [subject]; + if (policies.length > 0) { + for (const policy of policies) { + for (const subject of subjects) { + if (!policy.meetsFilter(subject, ctx)) { + if (opts?.throwOnError) { + throw new GuardPermissionsException( + permission, + policy, + "Policy filter not met", + ); + } + return false; + } + } + } + } + return true; + }, + }; } } diff --git a/app/src/auth/authorize/Permission.ts b/app/src/auth/authorize/Permission.ts index e18a98c..cfd5963 100644 --- a/app/src/auth/authorize/Permission.ts +++ b/app/src/auth/authorize/Permission.ts @@ -54,6 +54,8 @@ export class Permission< } parseContext(ctx: ContextValue, opts?: ParseOptions) { + // @todo: allow additional properties + if (!this.context) return ctx; try { return this.context ? parse(this.context!, ctx, opts) : undefined; } catch (e) { diff --git a/app/src/auth/authorize/Policy.ts b/app/src/auth/authorize/Policy.ts index 9995e20..06357f1 100644 --- a/app/src/auth/authorize/Policy.ts +++ b/app/src/auth/authorize/Policy.ts @@ -21,8 +21,15 @@ export class Policy { }) as Schema; } - replace(context: object, vars?: Record) { - return vars ? recursivelyReplacePlaceholders(context, /^@([a-zA-Z_\.]+)$/, vars) : context; + replace(context: object, vars?: Record, fallback?: any) { + return vars + ? recursivelyReplacePlaceholders(context, /^@([a-zA-Z_\.]+)$/, vars, fallback) + : context; + } + + getReplacedFilter(context: object, fallback?: any) { + if (!this.content.filter) return context; + return this.replace(this.content.filter!, context, fallback); } meetsCondition(context: object, vars?: Record) { diff --git a/app/src/core/utils/objects.ts b/app/src/core/utils/objects.ts index 65f18f9..33c6a43 100644 --- a/app/src/core/utils/objects.ts +++ b/app/src/core/utils/objects.ts @@ -372,7 +372,7 @@ export function isEqual(value1: any, value2: any): boolean { export function getPath( object: object, _path: string | (string | number)[], - defaultValue = undefined, + defaultValue: any = undefined, ): any { const path = typeof _path === "string" ? _path.split(/[.\[\]\"]+/).filter((x) => x) : _path; @@ -517,6 +517,7 @@ export function recursivelyReplacePlaceholders( obj: any, pattern: RegExp, variables: Record, + fallback?: any, ) { if (typeof obj === "string") { // check if the entire string matches the pattern @@ -524,24 +525,28 @@ export function recursivelyReplacePlaceholders( if (match && match[0] === obj && match[1]) { // full string match - replace with the actual value (preserving type) const key = match[1]; - const value = getPath(variables, key); - return value !== undefined ? value : obj; + const value = getPath(variables, key, null); + return value !== null ? value : fallback !== undefined ? fallback : obj; } // partial match - use string replacement if (pattern.test(obj)) { return obj.replace(pattern, (match, key) => { - const value = getPath(variables, key); + const value = getPath(variables, key, null); // convert to string for partial replacements - return value !== undefined ? String(value) : match; + return value !== null + ? String(value) + : fallback !== undefined + ? String(fallback) + : match; }); } } if (Array.isArray(obj)) { - return obj.map((item) => recursivelyReplacePlaceholders(item, pattern, variables)); + return obj.map((item) => recursivelyReplacePlaceholders(item, pattern, variables, fallback)); } if (obj && typeof obj === "object") { return Object.entries(obj).reduce((acc, [key, value]) => { - acc[key] = recursivelyReplacePlaceholders(value, pattern, variables); + acc[key] = recursivelyReplacePlaceholders(value, pattern, variables, fallback); return acc; }, {} as object); } diff --git a/app/src/data/api/DataController.ts b/app/src/data/api/DataController.ts index d4f9cdf..adceffa 100644 --- a/app/src/data/api/DataController.ts +++ b/app/src/data/api/DataController.ts @@ -9,6 +9,7 @@ import { pickKeys, mcpTool, convertNumberedObjectToArray, + mergeObject, } from "bknd/utils"; import * as SystemPermissions from "modules/permissions"; import type { AppDataConfig } from "../data-schema"; @@ -95,7 +96,9 @@ export class DataController extends Controller { // read entity schema hono.get( "/schema.json", - permission(DataPermissions.entityRead, {}), + permission(DataPermissions.entityRead, { + context: (c) => ({ entity: c.req.param("entity") }), + }), describeRoute({ summary: "Retrieve data schema", tags: ["data"], @@ -121,7 +124,9 @@ export class DataController extends Controller { // read schema hono.get( "/schemas/:entity/:context?", - permission(DataPermissions.entityRead, {}), + permission(DataPermissions.entityRead, { + context: (c) => ({ entity: c.req.param("entity") }), + }), describeRoute({ summary: "Retrieve entity schema", tags: ["data"], @@ -161,7 +166,9 @@ export class DataController extends Controller { */ hono.get( "/info/:entity", - permission(DataPermissions.entityRead, {}), + permission(DataPermissions.entityRead, { + context: (c) => ({ entity: c.req.param("entity") }), + }), describeRoute({ summary: "Retrieve entity info", tags: ["data"], @@ -213,7 +220,9 @@ export class DataController extends Controller { // fn: count hono.post( "/:entity/fn/count", - permission(DataPermissions.entityRead, {}), + permission(DataPermissions.entityRead, { + context: (c) => ({ entity: c.req.param("entity") }), + }), describeRoute({ summary: "Count entities", tags: ["data"], @@ -236,7 +245,9 @@ export class DataController extends Controller { // fn: exists hono.post( "/:entity/fn/exists", - permission(DataPermissions.entityRead, {}), + permission(DataPermissions.entityRead, { + context: (c) => ({ entity: c.req.param("entity") }), + }), describeRoute({ summary: "Check if entity exists", tags: ["data"], @@ -285,16 +296,26 @@ export class DataController extends Controller { parameters: saveRepoQueryParams(["limit", "offset", "sort", "select", "join"]), tags: ["data"], }), - permission(DataPermissions.entityRead, {}), jsc("param", s.object({ entity: entitiesEnum })), jsc("query", repoQuery, { skipOpenAPI: true }), + permission(DataPermissions.entityRead, { + context: (c) => ({ entity: c.req.param("entity") }), + }), async (c) => { const { entity } = c.req.valid("param"); if (!this.entityExists(entity)) { return this.notFound(c); } + + const { merge } = this.ctx.guard.filters(DataPermissions.entityRead, c, { + entity, + }); + const options = c.req.valid("query") as RepoQuery; - const result = await this.em.repository(entity).findMany(options); + const result = await this.em.repository(entity).findMany({ + ...options, + where: merge(options.where), + }); return c.json(result, { status: result.data ? 200 : 404 }); }, @@ -308,7 +329,9 @@ export class DataController extends Controller { parameters: saveRepoQueryParams(["offset", "sort", "select"]), tags: ["data"], }), - permission(DataPermissions.entityRead, {}), + permission(DataPermissions.entityRead, { + context: (c) => ({ ...c.req.param() }) as any, + }), mcpTool("data_entity_read_one", { inputSchema: { param: s.object({ entity: entitiesEnum, id: idType }), @@ -326,11 +349,19 @@ export class DataController extends Controller { jsc("query", repoQuery, { skipOpenAPI: true }), async (c) => { const { entity, id } = c.req.valid("param"); - if (!this.entityExists(entity)) { + if (!this.entityExists(entity) || !id) { return this.notFound(c); } const options = c.req.valid("query") as RepoQuery; - const result = await this.em.repository(entity).findId(id, options); + const { merge } = this.ctx.guard.filters( + DataPermissions.entityRead, + c, + c.req.valid("param"), + ); + const id_name = this.em.entity(entity).getPrimaryField().name; + const result = await this.em + .repository(entity) + .findOne(merge({ [id_name]: id }), options); return c.json(result, { status: result.data ? 200 : 404 }); }, @@ -344,7 +375,9 @@ export class DataController extends Controller { parameters: saveRepoQueryParams(), tags: ["data"], }), - permission(DataPermissions.entityRead, {}), + permission(DataPermissions.entityRead, { + context: (c) => ({ ...c.req.param() }) as any, + }), jsc( "param", s.object({ @@ -361,9 +394,20 @@ export class DataController extends Controller { } const options = c.req.valid("query") as RepoQuery; - const result = await this.em + const { entity: newEntity } = this.em .repository(entity) - .findManyByReference(id, reference, options); + .getEntityByReference(reference); + + const { merge } = this.ctx.guard.filters(DataPermissions.entityRead, c, { + entity: newEntity.name, + id, + reference, + }); + + const result = await this.em.repository(entity).findManyByReference(id, reference, { + ...options, + where: merge(options.where), + }); return c.json(result, { status: result.data ? 200 : 404 }); }, @@ -390,7 +434,9 @@ export class DataController extends Controller { }, tags: ["data"], }), - permission(DataPermissions.entityRead, {}), + permission(DataPermissions.entityRead, { + context: (c) => ({ entity: c.req.param("entity") }), + }), mcpTool("data_entity_read_many", { inputSchema: { param: s.object({ entity: entitiesEnum }), @@ -405,7 +451,13 @@ export class DataController extends Controller { return this.notFound(c); } const options = c.req.valid("json") as RepoQuery; - const result = await this.em.repository(entity).findMany(options); + const { merge } = this.ctx.guard.filters(DataPermissions.entityRead, c, { + entity, + }); + const result = await this.em.repository(entity).findMany({ + ...options, + where: merge(options.where), + }); return c.json(result, { status: result.data ? 200 : 404 }); }, @@ -421,7 +473,9 @@ export class DataController extends Controller { summary: "Insert one or many", tags: ["data"], }), - permission(DataPermissions.entityCreate, {}), + permission(DataPermissions.entityCreate, { + context: (c) => ({ ...c.req.param() }) as any, + }), mcpTool("data_entity_insert"), jsc("param", s.object({ entity: entitiesEnum })), jsc("json", s.anyOf([s.object({}), s.array(s.object({}))])), @@ -438,6 +492,12 @@ export class DataController extends Controller { // to transform all validation targets into a single object const body = convertNumberedObjectToArray(_body); + this.ctx.guard + .filters(DataPermissions.entityCreate, c, { + entity, + }) + .matches(body, { throwOnError: true }); + if (Array.isArray(body)) { const result = await this.em.mutator(entity).insertMany(body); return c.json(result, 201); @@ -455,7 +515,9 @@ export class DataController extends Controller { summary: "Update many", tags: ["data"], }), - permission(DataPermissions.entityUpdate, {}), + permission(DataPermissions.entityUpdate, { + context: (c) => ({ ...c.req.param() }) as any, + }), mcpTool("data_entity_update_many", { inputSchema: { param: s.object({ entity: entitiesEnum }), @@ -482,7 +544,10 @@ export class DataController extends Controller { update: EntityData; where: RepoQuery["where"]; }; - const result = await this.em.mutator(entity).updateWhere(update, where); + const { merge } = this.ctx.guard.filters(DataPermissions.entityUpdate, c, { + entity, + }); + const result = await this.em.mutator(entity).updateWhere(update, merge(where)); return c.json(result); }, @@ -495,7 +560,9 @@ export class DataController extends Controller { summary: "Update one", tags: ["data"], }), - permission(DataPermissions.entityUpdate, {}), + permission(DataPermissions.entityUpdate, { + context: (c) => ({ ...c.req.param() }) as any, + }), mcpTool("data_entity_update_one"), jsc("param", s.object({ entity: entitiesEnum, id: idType })), jsc("json", s.object({})), @@ -505,6 +572,17 @@ export class DataController extends Controller { return this.notFound(c); } const body = (await c.req.json()) as EntityData; + const fns = this.ctx.guard.filters(DataPermissions.entityUpdate, c, { + entity, + id, + }); + + // if it has filters attached, fetch entry and make the check + if (fns.filters.length > 0) { + const { data } = await this.em.repository(entity).findId(id); + fns.matches(data, { throwOnError: true }); + } + const result = await this.em.mutator(entity).updateOne(id, body); return c.json(result); @@ -518,7 +596,9 @@ export class DataController extends Controller { summary: "Delete one", tags: ["data"], }), - permission(DataPermissions.entityDelete, {}), + permission(DataPermissions.entityDelete, { + context: (c) => ({ ...c.req.param() }) as any, + }), mcpTool("data_entity_delete_one"), jsc("param", s.object({ entity: entitiesEnum, id: idType })), async (c) => { @@ -526,6 +606,18 @@ export class DataController extends Controller { if (!this.entityExists(entity)) { return this.notFound(c); } + + const fns = this.ctx.guard.filters(DataPermissions.entityDelete, c, { + entity, + id, + }); + + // if it has filters attached, fetch entry and make the check + if (fns.filters.length > 0) { + const { data } = await this.em.repository(entity).findId(id); + fns.matches(data, { throwOnError: true }); + } + const result = await this.em.mutator(entity).deleteOne(id); return c.json(result); @@ -539,7 +631,9 @@ export class DataController extends Controller { summary: "Delete many", tags: ["data"], }), - permission(DataPermissions.entityDelete, {}), + permission(DataPermissions.entityDelete, { + context: (c) => ({ ...c.req.param() }) as any, + }), mcpTool("data_entity_delete_many", { inputSchema: { param: s.object({ entity: entitiesEnum }), @@ -554,7 +648,10 @@ export class DataController extends Controller { return this.notFound(c); } const where = (await c.req.json()) as RepoQuery["where"]; - const result = await this.em.mutator(entity).deleteWhere(where); + const { merge } = this.ctx.guard.filters(DataPermissions.entityDelete, c, { + entity, + }); + const result = await this.em.mutator(entity).deleteWhere(merge(where)); return c.json(result); }, diff --git a/app/src/data/entities/query/Repository.ts b/app/src/data/entities/query/Repository.ts index 13554a6..3d8f432 100644 --- a/app/src/data/entities/query/Repository.ts +++ b/app/src/data/entities/query/Repository.ts @@ -1,4 +1,4 @@ -import type { DB as DefaultDB, PrimaryFieldType } from "bknd"; +import type { DB as DefaultDB, EntityRelation, PrimaryFieldType } from "bknd"; import { $console } from "bknd/utils"; import { type EmitsEvents, EventManager } from "core/events"; import { type SelectQueryBuilder, sql } from "kysely"; @@ -280,16 +280,11 @@ export class Repository>, ): Promise> { - const { qb, options } = this.buildQuery( - { - ..._options, - where: { [this.entity.getPrimaryField().name]: id }, - limit: 1, - }, - ["offset", "sort"], - ); + if (typeof id === "undefined" || id === null) { + throw new InvalidSearchParamsException("id is required"); + } - return this.single(qb, options) as any; + return this.findOne({ [this.entity.getPrimaryField().name]: id }, _options); } async findOne( @@ -315,23 +310,27 @@ export class Repository r.ref(reference).reference === reference); + if (!relation) { + throw new Error( + `Relation "${reference}" not found or not listable on entity "${this.entity.name}"`, + ); + } + return { + entity: relation.other(this.entity).entity, + relation, + }; + } + // @todo: add unit tests, specially for many to many async findManyByReference( id: PrimaryFieldType, reference: string, _options?: Partial>, ): Promise> { - const entity = this.entity; - const listable_relations = this.em.relations.listableRelationsOf(entity); - const relation = listable_relations.find((r) => r.ref(reference).reference === reference); - - if (!relation) { - throw new Error( - `Relation "${reference}" not found or not listable on entity "${entity.name}"`, - ); - } - - const newEntity = relation.other(entity).entity; + const { entity: newEntity, relation } = this.getEntityByReference(reference); const refQueryOptions = relation.getReferenceQuery(newEntity, id as number, reference); if (!("where" in refQueryOptions) || Object.keys(refQueryOptions.where as any).length === 0) { throw new Error( diff --git a/app/src/data/permissions/index.ts b/app/src/data/permissions/index.ts index 124980e..f832716 100644 --- a/app/src/data/permissions/index.ts +++ b/app/src/data/permissions/index.ts @@ -1,9 +1,51 @@ import { Permission } from "auth/authorize/Permission"; +import { s } from "bknd/utils"; -export const entityRead = new Permission("data.entity.read"); -export const entityCreate = new Permission("data.entity.create"); -export const entityUpdate = new Permission("data.entity.update"); -export const entityDelete = new Permission("data.entity.delete"); +export const entityRead = new Permission( + "data.entity.read", + { + filterable: true, + }, + s.object({ + entity: s.string(), + id: s.anyOf([s.number(), s.string()]).optional(), + }), +); +/** + * Filter filters content given + */ +export const entityCreate = new Permission( + "data.entity.create", + { + filterable: true, + }, + s.object({ + entity: s.string(), + }), +); +/** + * Filter filters where clause + */ +export const entityUpdate = new Permission( + "data.entity.update", + { + filterable: true, + }, + s.object({ + entity: s.string(), + id: s.anyOf([s.number(), s.string()]).optional(), + }), +); +export const entityDelete = new Permission( + "data.entity.delete", + { + filterable: true, + }, + s.object({ + entity: s.string(), + id: s.anyOf([s.number(), s.string()]).optional(), + }), +); export const databaseSync = new Permission("data.database.sync"); export const rawQuery = new Permission("data.raw.query"); export const rawMutate = new Permission("data.raw.mutate"); diff --git a/app/src/ui/components/form/json-schema-form/Field.tsx b/app/src/ui/components/form/json-schema-form/Field.tsx index bc81a83..4669022 100644 --- a/app/src/ui/components/form/json-schema-form/Field.tsx +++ b/app/src/ui/components/form/json-schema-form/Field.tsx @@ -217,14 +217,14 @@ export type CustomFieldProps = { ) => React.ReactNode; }; -export const CustomField = ({ +export function CustomField({ path: _path, valueStrict = true, deriveFn, children, -}: CustomFieldProps) => { +}: CustomFieldProps) { const ctx = useDerivedFieldContext(_path, deriveFn); - const $value = useFormValue(ctx.path, { strict: valueStrict }); + const $value = useFormValue(_path, { strict: valueStrict }); const setValue = (value: any) => ctx.setValue(ctx.path, value); return children({ ...ctx, ...$value, setValue, _setValue: ctx.setValue }); -}; +} diff --git a/app/src/ui/components/form/json-schema-form/Form.tsx b/app/src/ui/components/form/json-schema-form/Form.tsx index 274c162..acfa25b 100644 --- a/app/src/ui/components/form/json-schema-form/Form.tsx +++ b/app/src/ui/components/form/json-schema-form/Form.tsx @@ -80,6 +80,7 @@ export function Form< onInvalidSubmit, validateOn = "submit", hiddenSubmit = true, + beforeSubmit, ignoreKeys = [], options = {}, readOnly = false, @@ -90,6 +91,7 @@ export function Form< initialOpts?: LibTemplateOptions; ignoreKeys?: string[]; onChange?: (data: Partial, name: string, value: any, context: FormContext) => void; + beforeSubmit?: (data: Data) => Data; onSubmit?: (data: Data) => void | Promise; onInvalidSubmit?: (errors: JsonError[], data: Partial) => void; hiddenSubmit?: boolean; @@ -177,7 +179,8 @@ export function Form< }); const validate = useEvent((_data?: Partial) => { - const actual = _data ?? getCurrentState()?.data; + const before = beforeSubmit ?? ((a: any) => a); + const actual = before((_data as any) ?? getCurrentState()?.data); const errors = lib.validate(actual, schema); setFormState((prev) => ({ ...prev, errors })); return { data: actual, errors }; @@ -378,5 +381,5 @@ export function FormDebug({ force = false }: { force?: boolean }) { if (options?.debug !== true && force !== true) return null; const ctx = useFormStateSelector((s) => s); - return ; + return ; } diff --git a/app/src/ui/routes/auth/auth.roles.edit.$role.tsx b/app/src/ui/routes/auth/auth.roles.edit.$role.tsx index ff7c39d..54a1bfd 100644 --- a/app/src/ui/routes/auth/auth.roles.edit.$role.tsx +++ b/app/src/ui/routes/auth/auth.roles.edit.$role.tsx @@ -2,12 +2,12 @@ import { useBknd } from "ui/client/bknd"; import { Message } from "ui/components/display/Message"; import { useBkndAuth } from "ui/client/schema/auth/use-bknd-auth"; import { useBrowserTitle } from "ui/hooks/use-browser-title"; -import { useRef, useState } from "react"; +import { useState } from "react"; import { useNavigate } from "ui/lib/routes"; import { isDebug } from "core/env"; import { Dropdown } from "ui/components/overlay/Dropdown"; import { IconButton } from "ui/components/buttons/IconButton"; -import { TbAdjustments, TbDots, TbLock, TbLockOpen, TbLockOpen2 } from "react-icons/tb"; +import { TbAdjustments, TbDots, TbFilter, TbTrash } from "react-icons/tb"; import { Button } from "ui/components/buttons/Button"; import { Breadcrumbs2 } from "ui/layouts/AppShell/Breadcrumbs2"; import { routes } from "ui/lib/routes"; @@ -18,17 +18,23 @@ import { ucFirst, type s } from "bknd/utils"; import type { ModuleSchemas } from "bknd"; import { ArrayField, + CustomField, Field, + FieldWrapper, Form, + FormContextOverride, FormDebug, + ObjectField, Subscribe, + useDerivedFieldContext, useFormContext, useFormValue, } from "ui/components/form/json-schema-form"; import type { TPermission } from "auth/authorize/Permission"; import type { RoleSchema } from "auth/authorize/Role"; -import { SegmentedControl, Tooltip } from "@mantine/core"; +import { Indicator, SegmentedControl, Tooltip } from "@mantine/core"; import { cn } from "ui/lib/utils"; +import type { PolicySchema } from "auth/authorize/Policy"; export function AuthRolesEdit(props) { useBrowserTitle(["Auth", "Roles", props.params.role]); @@ -66,21 +72,39 @@ function AuthRolesEditInternal({ params }) { const { config, schema: authSchema, actions } = useBkndAuth(); const roleName = params.role; const role = config.roles?.[roleName]; - const { readonly } = useBknd(); + const { readonly, permissions } = useBknd(); const schema = getSchema(authSchema); + const data = { + ...role, + // this is to maintain array structure + permissions: permissions.map((p) => { + return role?.permissions?.find((v: any) => v.permission === p.name); + }), + }; - async function handleDelete() {} - async function handleUpdate(data: any) { - console.log("data", data); - const success = await actions.roles.patch(roleName, data); - console.log("success", success); - /* if (success) { + async function handleDelete() { + const success = await actions.roles.delete(roleName); + if (success) { navigate(routes.auth.roles.list()); - } */ + } + } + async function handleUpdate(data: any) { + await actions.roles.patch(roleName, data); } return ( -

+ { + return { + ...data, + permissions: [...Object.values(data.permissions)], + }; + }} + onSubmit={handleUpdate} + > @@ -196,14 +220,21 @@ const Permissions = () => { const Permission = ({ permission, index }: { permission: TPermission; index?: number }) => { const path = `permissions.${index}`; - const { value } = useFormValue(path); + const { value } = useDerivedFieldContext("permissions", (ctx) => { + const v = ctx.value; + if (!Array.isArray(v)) return undefined; + return v.find((v) => v && v.permission === permission.name); + }); const { setValue, deleteValue } = useFormContext(); const [open, setOpen] = useState(false); const data = value as PermissionData | undefined; + const policiesCount = data?.policies?.length ?? 0; + const hasContext = !!permission.context; async function handleSwitch() { if (data) { - deleteValue(path); + setValue(path, undefined); + setOpen(false); } else { setValue(path, { permission: permission.name, @@ -220,34 +251,125 @@ const Permission = ({ permission, index }: { permission: TPermission; index?: nu className={cn("flex flex-col border border-muted", open && "border-primary/20")} >
-
{permission.name}
+
+ {permission.name} + {permission.filterable && ( + + + + )} +
+
- - +
+ {policiesCount > 0 && ( +
+ {policiesCount} +
+ )} setOpen((o) => !o)} /> - +
+
{open && (
- + {/* + /> */}
)}
); }; + +const Policies = ({ path, permission }: { path: string; permission: TPermission }) => { + const { value: _value } = useFormValue(path); + const { setValue, schema: policySchema, lib, deleteValue } = useDerivedFieldContext(path); + const value = _value ?? []; + + function handleAdd() { + setValue( + `${path}.${value.length}`, + lib.getTemplate(undefined, policySchema!.items, { + addOptionalProps: true, + }), + ); + } + + function handleDelete(index: number) { + deleteValue(`${path}.${index}`); + } + + return ( +
0 && "gap-8")}> +
+ {value.map((policy, i) => ( + + {i > 0 &&
} +
+
+ +
+ handleDelete(i)} size="sm" /> +
+ + ))} +
+
+ +
+
+ ); +}; + +const Policy = ({ + permission, +}: { + permission: TPermission; +}) => { + const { value } = useFormValue(""); + return ( +
+ + + + {({ value, setValue }) => ( + + setValue(value)} + data={ + ["allow", "deny", permission.filterable ? "filter" : undefined] + .filter(Boolean) + .map((effect) => ({ + label: ucFirst(effect ?? ""), + value: effect, + })) as any + } + /> + + )} + + + {value?.effect === "filter" && ( + + )} +
+ ); +}; diff --git a/app/src/ui/routes/auth/auth.roles.tsx b/app/src/ui/routes/auth/auth.roles.tsx index 15596bf..75793fe 100644 --- a/app/src/ui/routes/auth/auth.roles.tsx +++ b/app/src/ui/routes/auth/auth.roles.tsx @@ -35,6 +35,9 @@ function AuthRolesListInternal() { transformObject(config.roles ?? {}, (role, name) => ({ role: name, permissions: role.permissions?.map((p) => p.permission) as string[], + policies: role.permissions + ?.flatMap((p) => p.policies?.length ?? 0) + .reduce((acc, curr) => acc + curr, 0), is_default: role.is_default ?? false, implicit_allow: role.implicit_allow ?? false, })), @@ -107,6 +110,9 @@ const renderValue = ({ value, property }) => { if (["is_default", "implicit_allow"].includes(property)) { return value ? Yes : No; } + if (property === "policies") { + return value ? {value} : 0; + } if (property === "permissions") { const max = 3; diff --git a/bun.lock b/bun.lock index 8b5995e..ab63856 100644 --- a/bun.lock +++ b/bun.lock @@ -15,7 +15,7 @@ }, "app": { "name": "bknd", - "version": "0.18.0-rc.6", + "version": "0.18.1", "bin": "./dist/cli/index.js", "dependencies": { "@cfworker/json-schema": "^4.1.1", @@ -35,7 +35,7 @@ "hono": "4.8.3", "json-schema-library": "10.0.0-rc7", "json-schema-to-ts": "^3.1.1", - "jsonv-ts": "0.8.4", + "jsonv-ts": "0.8.6", "kysely": "0.27.6", "lodash-es": "^4.17.21", "oauth4webapi": "^2.11.1", @@ -1243,7 +1243,7 @@ "@types/babel__traverse": ["@types/babel__traverse@7.20.6", "", { "dependencies": { "@babel/types": "^7.20.7" } }, "sha512-r1bzfrm0tomOI8g1SzvCaQHo6Lcv6zu0EA+W2kHrt8dyrHQxGzBBL4kdkzIS+jBMV+EYcMAEAqXqYaLJq5rOZg=="], - "@types/bun": ["@types/bun@1.2.21", "", { "dependencies": { "bun-types": "1.2.21" } }, "sha512-NiDnvEqmbfQ6dmZ3EeUO577s4P5bf4HCTXtI6trMc6f6RzirY5IrF3aIookuSpyslFzrnvv2lmEWv5HyC1X79A=="], + "@types/bun": ["@types/bun@1.3.0", "", { "dependencies": { "bun-types": "1.3.0" } }, "sha512-+lAGCYjXjip2qY375xX/scJeVRmZ5cY0wyHYyCYxNcdEXrQ4AOe3gACgd4iQ8ksOslJtW4VNxBJ8llUwc3a6AA=="], "@types/cookie": ["@types/cookie@0.6.0", "", {}, "sha512-4Kh9a6B2bQciAhf7FSuMRRkUWecJgJu9nPnx3yzpsfXX/c50REIqpHY4C82bXP90qrLtXtkDxTZosYO3UpOwlA=="], @@ -2529,7 +2529,7 @@ "jsonpointer": ["jsonpointer@5.0.1", "", {}, "sha512-p/nXbhSEcu3pZRdkW1OfJhpsVtW1gd4Wa1fnQc9YLiTfAjn0312eMKimbdIQzuZl9aa9xUGaRlP9T/CJE/ditQ=="], - "jsonv-ts": ["jsonv-ts@0.8.4", "", { "optionalDependencies": { "hono": "*" }, "peerDependencies": { "typescript": "^5.0.0" } }, "sha512-TZOyAVGBZxHuzk09NgJCx2dbeh0XqVWVKHU1PtIuvjT9XO7zhvAD02RcVisJoUdt2rJNt3zlyeNQ2b8MMPc+ug=="], + "jsonv-ts": ["jsonv-ts@0.8.6", "", { "optionalDependencies": { "hono": "*" }, "peerDependencies": { "typescript": "^5.0.0" } }, "sha512-z5jJ017LFOvAFFVodAIiCY024yW72RWc/K0Sct+OtuiLN+lKy+g0pI0jaz5JmuXaMIePc6HyopeeYHi8ffbYhw=="], "jsonwebtoken": ["jsonwebtoken@9.0.2", "", { "dependencies": { "jws": "^3.2.2", "lodash.includes": "^4.3.0", "lodash.isboolean": "^3.0.3", "lodash.isinteger": "^4.0.4", "lodash.isnumber": "^3.0.3", "lodash.isplainobject": "^4.0.6", "lodash.isstring": "^4.0.1", "lodash.once": "^4.0.0", "ms": "^2.1.1", "semver": "^7.5.4" } }, "sha512-PRp66vJ865SSqOlgqS8hujT5U4AOgMfhrwYIuIhfKaoSCZcirrmASQr8CX7cUg+RMih+hgznrjp99o+W4pJLHQ=="], @@ -3847,6 +3847,8 @@ "@bknd/plasmic/typescript": ["typescript@5.8.3", "", { "bin": { "tsc": "bin/tsc", "tsserver": "bin/tsserver" } }, "sha512-p1diW6TqL9L07nNxvRMM7hMMw4c5XOo/1ibL4aAIGmSAt9slTE1Xgw5KWuof2uTOvCg9BY7ZRi+GaF+7sfgPeQ=="], + "@bknd/postgres/@types/bun": ["@types/bun@1.2.21", "", { "dependencies": { "bun-types": "1.2.21" } }, "sha512-NiDnvEqmbfQ6dmZ3EeUO577s4P5bf4HCTXtI6trMc6f6RzirY5IrF3aIookuSpyslFzrnvv2lmEWv5HyC1X79A=="], + "@bknd/sqlocal/typescript": ["typescript@5.8.3", "", { "bin": { "tsc": "bin/tsc", "tsserver": "bin/tsserver" } }, "sha512-p1diW6TqL9L07nNxvRMM7hMMw4c5XOo/1ibL4aAIGmSAt9slTE1Xgw5KWuof2uTOvCg9BY7ZRi+GaF+7sfgPeQ=="], "@bundled-es-modules/cookie/cookie": ["cookie@0.7.2", "", {}, "sha512-yki5XnKuf750l50uGTllt6kKILY4nQ1eNIQatoXEByZ5dWgnKqbnqmTrBE5B4N7lrMJKQ2ytWMiTO2o0v6Ew/w=="], @@ -4093,7 +4095,7 @@ "@testing-library/jest-dom/chalk": ["chalk@3.0.0", "", { "dependencies": { "ansi-styles": "^4.1.0", "supports-color": "^7.1.0" } }, "sha512-4D3B6Wf41KOYRFdszmDqMCGq5VV/uMAB273JILmO+3jAlh8X4qDtdtgCR3fxtbLEMzSx22QdhnDcJvu2u1fVwg=="], - "@types/bun/bun-types": ["bun-types@1.2.21", "", { "dependencies": { "@types/node": "*" }, "peerDependencies": { "@types/react": "^19" } }, "sha512-sa2Tj77Ijc/NTLS0/Odjq/qngmEPZfbfnOERi0KRUYhT9R8M4VBioWVmMWE5GrYbKMc+5lVybXygLdibHaqVqw=="], + "@types/bun/bun-types": ["bun-types@1.3.0", "", { "dependencies": { "@types/node": "*" }, "peerDependencies": { "@types/react": "^19" } }, "sha512-u8X0thhx+yJ0KmkxuEo9HAtdfgCBaM/aI9K90VQcQioAmkVp3SG3FkwWGibUFz3WdXAdcsqOcbU40lK7tbHdkQ=="], "@typescript-eslint/experimental-utils/eslint-utils": ["eslint-utils@2.1.0", "", { "dependencies": { "eslint-visitor-keys": "^1.1.0" } }, "sha512-w94dQYoauyvlDc43XnGB8lU3Zt713vNChgt4EWwhXAP2XkBvndfxF0AgIqKOOasjPIPzj9JqgwkwbCYD0/V3Zg=="], @@ -4701,6 +4703,8 @@ "@babel/preset-env/babel-plugin-polyfill-regenerator/@babel/helper-define-polyfill-provider": ["@babel/helper-define-polyfill-provider@0.6.3", "", { "dependencies": { "@babel/helper-compilation-targets": "^7.22.6", "@babel/helper-plugin-utils": "^7.22.5", "debug": "^4.1.1", "lodash.debounce": "^4.0.8", "resolve": "^1.14.2" }, "peerDependencies": { "@babel/core": "^7.4.0 || ^8.0.0-0 <8.0.0" } }, "sha512-HK7Bi+Hj6H+VTHA3ZvBis7V/6hu9QuTrnMXNybfUf2iiuU/N97I8VjB+KbhFF8Rld/Lx5MzoCwPCpPjfK+n8Cg=="], + "@bknd/postgres/@types/bun/bun-types": ["bun-types@1.2.21", "", { "dependencies": { "@types/node": "*" }, "peerDependencies": { "@types/react": "^19" } }, "sha512-sa2Tj77Ijc/NTLS0/Odjq/qngmEPZfbfnOERi0KRUYhT9R8M4VBioWVmMWE5GrYbKMc+5lVybXygLdibHaqVqw=="], + "@bundled-es-modules/tough-cookie/tough-cookie/universalify": ["universalify@0.2.0", "", {}, "sha512-CJ1QgKmNg3CwvAv/kOFmtnEN05f0D/cn9QntgNOQlQF9dgvVTHj3t+8JPdjqawCHk7V/KA+fbUqzZ9XWhcqPUg=="], "@cloudflare/unenv-preset/workerd/@cloudflare/workerd-darwin-64": ["@cloudflare/workerd-darwin-64@1.20250917.0", "", { "os": "darwin", "cpu": "x64" }, "sha512-0kL/kFnKUSycoo7b3PgM0nRyZ+1MGQAKaXtE6a2+SAeUkZ2FLnuFWmASi0s4rlWGsf/rlTw4AwXROePir9dUcQ=="], From eb0822bbffa765125a383035b3a2b7ace197543e Mon Sep 17 00:00:00 2001 From: dswbx Date: Fri, 24 Oct 2025 09:14:31 +0200 Subject: [PATCH 08/14] Enhance authentication and authorization components - Refactored `AppAuth` to introduce `getGuardContextSchema` for improved user context handling. - Updated `Authenticator` to utilize `pickKeys` for user data extraction in JWT generation. - Enhanced `Guard` class to improve permission checks and error handling. - Modified `SystemController` to return context schema alongside permissions in API responses. - Added new `permissions` method in `SystemApi` for fetching permissions. - Improved UI components with additional props and tooltip support for better user experience. --- app/package.json | 2 +- app/src/auth/AppAuth.ts | 15 +- app/src/auth/authenticate/Authenticator.ts | 8 +- app/src/auth/authorize/Guard.ts | 12 +- app/src/modules/ModuleHelper.ts | 2 +- app/src/modules/SystemApi.ts | 5 + app/src/modules/server/SystemController.ts | 9 +- app/src/ui/client/api/use-api.ts | 24 ++- app/src/ui/components/buttons/Button.tsx | 4 +- app/src/ui/components/code/CodePreview.tsx | 73 +++++++++ .../form/json-schema-form/FieldWrapper.tsx | 24 ++- .../ui/routes/auth/auth.roles.edit.$role.tsx | 152 +++++++++++++++--- app/src/ui/routes/tools/mcp/mcp.tsx | 2 +- app/src/ui/routes/tools/mcp/tools.tsx | 7 +- bun.lock | 8 +- 15 files changed, 290 insertions(+), 57 deletions(-) create mode 100644 app/src/ui/components/code/CodePreview.tsx diff --git a/app/package.json b/app/package.json index c40f3b1..c256cc9 100644 --- a/app/package.json +++ b/app/package.json @@ -65,7 +65,7 @@ "hono": "4.8.3", "json-schema-library": "10.0.0-rc7", "json-schema-to-ts": "^3.1.1", - "jsonv-ts": "0.8.6", + "jsonv-ts": "0.9.1", "kysely": "0.27.6", "lodash-es": "^4.17.21", "oauth4webapi": "^2.11.1", diff --git a/app/src/auth/AppAuth.ts b/app/src/auth/AppAuth.ts index a73f9ec..4b23919 100644 --- a/app/src/auth/AppAuth.ts +++ b/app/src/auth/AppAuth.ts @@ -2,7 +2,7 @@ import type { DB, PrimaryFieldType } from "bknd"; import * as AuthPermissions from "auth/auth-permissions"; import type { AuthStrategy } from "auth/authenticate/strategies/Strategy"; import type { PasswordStrategy } from "auth/authenticate/strategies/PasswordStrategy"; -import { $console, secureRandomString, transformObject, pick } from "bknd/utils"; +import { $console, secureRandomString, transformObject, pickKeys } from "bknd/utils"; import type { Entity, EntityManager } from "data/entities"; import { em, entity, enumm, type FieldSchema } from "data/prototype"; import { Module } from "modules/Module"; @@ -113,6 +113,19 @@ export class AppAuth extends Module { return authConfigSchema; } + getGuardContextSchema() { + const userschema = this.getUsersEntity().toSchema() as any; + return { + type: "object", + properties: { + user: { + type: "object", + properties: pickKeys(userschema.properties, this.config.jwt.fields as any), + }, + }, + }; + } + get authenticator(): Authenticator { this.throwIfNotBuilt(); return this._authenticator!; diff --git a/app/src/auth/authenticate/Authenticator.ts b/app/src/auth/authenticate/Authenticator.ts index 711099b..b7ececc 100644 --- a/app/src/auth/authenticate/Authenticator.ts +++ b/app/src/auth/authenticate/Authenticator.ts @@ -6,10 +6,8 @@ import { deleteCookie, getSignedCookie, setSignedCookie } from "hono/cookie"; import { sign, verify } from "hono/jwt"; import { type CookieOptions, serializeSigned } from "hono/utils/cookie"; import type { ServerEnv } from "modules/Controller"; -import { pick } from "lodash-es"; import { InvalidConditionsException } from "auth/errors"; -import { s, parse, secret, runtimeSupports, truncate, $console } from "bknd/utils"; -import { $object } from "modules/mcp"; +import { s, parse, secret, runtimeSupports, truncate, $console, pickKeys } from "bknd/utils"; import type { AuthStrategy } from "./strategies/Strategy"; type Input = any; // workaround @@ -229,7 +227,7 @@ export class Authenticator< // @todo: add jwt tests async jwt(_user: SafeUser | ProfileExchange): Promise { - const user = pick(_user, this.config.jwt.fields); + const user = pickKeys(_user, this.config.jwt.fields as any); const payload: JWTPayload = { ...user, @@ -255,7 +253,7 @@ export class Authenticator< } async safeAuthResponse(_user: User): Promise { - const user = pick(_user, this.config.jwt.fields) as SafeUser; + const user = pickKeys(_user, this.config.jwt.fields as any) as SafeUser; return { user, token: await this.jwt(user), diff --git a/app/src/auth/authorize/Guard.ts b/app/src/auth/authorize/Guard.ts index 85349a9..e66dba1 100644 --- a/app/src/auth/authorize/Guard.ts +++ b/app/src/auth/authorize/Guard.ts @@ -125,7 +125,7 @@ export class Guard { return this.config?.enabled === true; } - private collect(permission: Permission, c: GuardContext, context: any) { + private collect(permission: Permission, c: GuardContext | undefined, context: any) { const user = c && "get" in c ? c.get("auth")?.user : c; const ctx = { ...((context ?? {}) as any), @@ -181,15 +181,15 @@ export class Guard { } if (!role) { - $console.debug("guard: user has no role, denying"); throw new GuardPermissionsException(permission, undefined, "User has no role"); - } else if (role.implicit_allow === true) { - $console.debug(`guard: role "${role.name}" has implicit allow, allowing`); - return; } if (!rolePermission) { - $console.debug("guard: rolePermission not found, denying"); + if (role.implicit_allow === true) { + $console.debug(`guard: role "${role.name}" has implicit allow, allowing`); + return; + } + throw new GuardPermissionsException( permission, undefined, diff --git a/app/src/modules/ModuleHelper.ts b/app/src/modules/ModuleHelper.ts index 29c4172..c227ba1 100644 --- a/app/src/modules/ModuleHelper.ts +++ b/app/src/modules/ModuleHelper.ts @@ -137,6 +137,6 @@ export class ModuleHelper { } const user = await auth.authenticator?.resolveAuthFromRequest(c.raw as any); - this.ctx.guard.granted(permission, { user }, context as any); + this.ctx.guard.granted(permission, user as any, context as any); } } diff --git a/app/src/modules/SystemApi.ts b/app/src/modules/SystemApi.ts index dc2e5c6..ab26bae 100644 --- a/app/src/modules/SystemApi.ts +++ b/app/src/modules/SystemApi.ts @@ -1,6 +1,7 @@ import type { ConfigUpdateResponse } from "modules/server/SystemController"; import { ModuleApi } from "./ModuleApi"; import type { ModuleConfigs, ModuleKey, ModuleSchemas } from "./ModuleManager"; +import type { TPermission } from "auth/authorize/Permission"; export type ApiSchemaResponse = { version: number; @@ -54,4 +55,8 @@ export class SystemApi extends ModuleApi { removeConfig(module: Module, path: string) { return this.delete(["config", "remove", module, path]); } + + permissions() { + return this.get<{ permissions: TPermission[]; context: object }>("permissions"); + } } diff --git a/app/src/modules/server/SystemController.ts b/app/src/modules/server/SystemController.ts index 45787bf..1190d55 100644 --- a/app/src/modules/server/SystemController.ts +++ b/app/src/modules/server/SystemController.ts @@ -69,10 +69,13 @@ export class SystemController extends Controller { if (!config.mcp.enabled) { return; } + const { permission } = this.middlewares; this.registerMcp(); - app.server.use( + app.server.all( + config.mcp.path, + permission(SystemPermissions.mcp, {}), mcpMiddleware({ setup: async () => { if (!this._mcpServer) { @@ -110,7 +113,6 @@ export class SystemController extends Controller { explainEndpoint: true, }, endpoint: { - path: config.mcp.path as any, // @ts-ignore _init: isNode() ? { duplex: "half" } : {}, }, @@ -415,7 +417,6 @@ export class SystemController extends Controller { schema, config: config ? this.app.toJSON(secrets) : undefined, permissions: this.app.modules.ctx().guard.getPermissions(), - //permissions: this.app.modules.ctx().guard.getPermissionNames(), }); }, ); @@ -428,7 +429,7 @@ export class SystemController extends Controller { }), (c) => { const permissions = this.app.modules.ctx().guard.getPermissions(); - return c.json({ permissions }); + return c.json({ permissions, context: this.app.module.auth.getGuardContextSchema() }); }, ); diff --git a/app/src/ui/client/api/use-api.ts b/app/src/ui/client/api/use-api.ts index 6b6d546..573b990 100644 --- a/app/src/ui/client/api/use-api.ts +++ b/app/src/ui/client/api/use-api.ts @@ -1,6 +1,6 @@ import type { Api } from "Api"; import { FetchPromise, type ModuleApi, type ResponseObject } from "modules/ModuleApi"; -import useSWR, { type SWRConfiguration, useSWRConfig } from "swr"; +import useSWR, { type SWRConfiguration, useSWRConfig, type Middleware, type SWRHook } from "swr"; import useSWRInfinite from "swr/infinite"; import { useApi } from "ui/client"; import { useState } from "react"; @@ -89,3 +89,25 @@ export const useInvalidate = (options?: { exact?: boolean }) => { return mutate((k) => typeof k === "string" && k.startsWith(key)); }; }; + +const mountOnceCache = new Map(); + +/** + * Simple middleware to only load on first mount. + */ +export const mountOnce: Middleware = (useSWRNext: SWRHook) => (key, fetcher, config) => { + if (typeof key === "string") { + if (mountOnceCache.has(key)) { + return useSWRNext(key, fetcher, { + ...config, + revalidateOnMount: false, + }); + } + const swr = useSWRNext(key, fetcher, config); + if (swr.data) { + mountOnceCache.set(key, true); + } + return swr; + } + return useSWRNext(key, fetcher, config); +}; diff --git a/app/src/ui/components/buttons/Button.tsx b/app/src/ui/components/buttons/Button.tsx index b80f006..79ee3cc 100644 --- a/app/src/ui/components/buttons/Button.tsx +++ b/app/src/ui/components/buttons/Button.tsx @@ -5,13 +5,15 @@ import { twMerge } from "tailwind-merge"; import { Link } from "ui/components/wouter/Link"; const sizes = { + smaller: "px-1.5 py-1 rounded-md gap-1 !text-xs", small: "px-2 py-1.5 rounded-md gap-1 text-sm", default: "px-3 py-2.5 rounded-md gap-1.5", large: "px-4 py-3 rounded-md gap-2.5 text-lg", }; const iconSizes = { - small: 12, + smaller: 12, + small: 14, default: 16, large: 20, }; diff --git a/app/src/ui/components/code/CodePreview.tsx b/app/src/ui/components/code/CodePreview.tsx new file mode 100644 index 0000000..9796e93 --- /dev/null +++ b/app/src/ui/components/code/CodePreview.tsx @@ -0,0 +1,73 @@ +import { useEffect, useState } from "react"; +import { useTheme } from "ui/client/use-theme"; +import { cn } from "ui/lib/utils"; + +export type CodePreviewProps = { + code: string; + className?: string; + lang?: string; + theme?: string; + enabled?: boolean; +}; + +export const CodePreview = ({ + code, + className, + lang = "typescript", + theme: _theme, + enabled = true, +}: CodePreviewProps) => { + const [highlightedHtml, setHighlightedHtml] = useState(null); + const $theme = useTheme(); + const theme = (_theme ?? $theme.theme === "dark") ? "github-dark" : "github-light"; + + useEffect(() => { + if (!enabled) return; + + let cancelled = false; + setHighlightedHtml(null); + + async function highlightCode() { + try { + // Dynamically import Shiki from CDN + // @ts-expect-error - Dynamic CDN import + const { codeToHtml } = await import("https://esm.sh/shiki@3.13.0"); + + if (cancelled) return; + + const html = await codeToHtml(code, { + lang, + theme, + structure: "inline", + }); + + if (cancelled) return; + + setHighlightedHtml(html); + } catch (error) { + console.error("Failed to load Shiki:", error); + // Fallback to plain text if Shiki fails to load + if (!cancelled) { + setHighlightedHtml(code); + } + } + } + + highlightCode(); + + return () => { + cancelled = true; + }; + }, [code, enabled]); + + if (!highlightedHtml) { + return
{code}
; + } + + return ( +
+   );
+};
diff --git a/app/src/ui/components/form/json-schema-form/FieldWrapper.tsx b/app/src/ui/components/form/json-schema-form/FieldWrapper.tsx
index af4607c..334dfe5 100644
--- a/app/src/ui/components/form/json-schema-form/FieldWrapper.tsx
+++ b/app/src/ui/components/form/json-schema-form/FieldWrapper.tsx
@@ -1,4 +1,4 @@
-import { IconBug } from "@tabler/icons-react";
+import { IconBug, IconInfoCircle } from "@tabler/icons-react";
 import type { JsonSchema } from "json-schema-library";
 import { Children, type ReactElement, type ReactNode, cloneElement, isValidElement } from "react";
 import { IconButton } from "ui/components/buttons/IconButton";
@@ -12,6 +12,7 @@ import {
 import { Popover } from "ui/components/overlay/Popover";
 import { getLabel } from "./utils";
 import { twMerge } from "tailwind-merge";
+import { Tooltip } from "@mantine/core";
 
 export type FieldwrapperProps = {
    name: string;
@@ -24,7 +25,7 @@ export type FieldwrapperProps = {
    children: ReactElement | ReactNode;
    errorPlacement?: "top" | "bottom";
    description?: string;
-   descriptionPlacement?: "top" | "bottom";
+   descriptionPlacement?: "top" | "bottom" | "label";
    fieldId?: string;
    className?: string;
 };
@@ -53,11 +54,17 @@ export function FieldWrapper({
       {errors.map((e) => e.message).join(", ")}
    );
 
-   const Description = description && (
-      
-         {description}
-      
-   );
+   const Description = description ? (
+      ["top", "bottom"].includes(descriptionPlacement) ? (
+         
+            {description}
+         
+      ) : (
+         
+            
+         
+      )
+   ) : null;
 
    return (
       
                {label} {required && *}
+               {descriptionPlacement === "label" && Description}
             
          )}
          {descriptionPlacement === "top" && Description}
diff --git a/app/src/ui/routes/auth/auth.roles.edit.$role.tsx b/app/src/ui/routes/auth/auth.roles.edit.$role.tsx
index 54a1bfd..9637ed1 100644
--- a/app/src/ui/routes/auth/auth.roles.edit.$role.tsx
+++ b/app/src/ui/routes/auth/auth.roles.edit.$role.tsx
@@ -7,34 +7,36 @@ import { useNavigate } from "ui/lib/routes";
 import { isDebug } from "core/env";
 import { Dropdown } from "ui/components/overlay/Dropdown";
 import { IconButton } from "ui/components/buttons/IconButton";
-import { TbAdjustments, TbDots, TbFilter, TbTrash } from "react-icons/tb";
+import { TbAdjustments, TbDots, TbFilter, TbTrash, TbInfoCircle, TbCodeDots } from "react-icons/tb";
 import { Button } from "ui/components/buttons/Button";
 import { Breadcrumbs2 } from "ui/layouts/AppShell/Breadcrumbs2";
 import { routes } from "ui/lib/routes";
 import * as AppShell from "ui/layouts/AppShell/AppShell";
 import * as Formy from "ui/components/form/Formy";
-
-import { ucFirst, type s } from "bknd/utils";
+import { ucFirst, s, transformObject, isObject } from "bknd/utils";
 import type { ModuleSchemas } from "bknd";
 import {
-   ArrayField,
    CustomField,
    Field,
    FieldWrapper,
    Form,
    FormContextOverride,
    FormDebug,
-   ObjectField,
+   ObjectJsonField,
    Subscribe,
    useDerivedFieldContext,
    useFormContext,
+   useFormError,
    useFormValue,
 } from "ui/components/form/json-schema-form";
 import type { TPermission } from "auth/authorize/Permission";
 import type { RoleSchema } from "auth/authorize/Role";
-import { Indicator, SegmentedControl, Tooltip } from "@mantine/core";
+import { SegmentedControl, Tooltip } from "@mantine/core";
+import { Popover } from "ui/components/overlay/Popover";
 import { cn } from "ui/lib/utils";
-import type { PolicySchema } from "auth/authorize/Policy";
+import { JsonViewer } from "ui/components/code/JsonViewer";
+import { mountOnce, useApiQuery } from "ui/client";
+import { CodePreview } from "ui/components/code/CodePreview";
 
 export function AuthRolesEdit(props) {
    useBrowserTitle(["Auth", "Roles", props.params.role]);
@@ -67,7 +69,7 @@ const formConfig = {
    },
 };
 
-function AuthRolesEditInternal({ params }) {
+function AuthRolesEditInternal({ params }: { params: { role: string } }) {
    const [navigate] = useNavigate();
    const { config, schema: authSchema, actions } = useBkndAuth();
    const roleName = params.role;
@@ -225,11 +227,10 @@ const Permission = ({ permission, index }: { permission: TPermission; index?: nu
       if (!Array.isArray(v)) return undefined;
       return v.find((v) => v && v.permission === permission.name);
    });
-   const { setValue, deleteValue } = useFormContext();
+   const { setValue } = useFormContext();
    const [open, setOpen] = useState(false);
    const data = value as PermissionData | undefined;
    const policiesCount = data?.policies?.length ?? 0;
-   const hasContext = !!permission.context;
 
    async function handleSwitch() {
       if (data) {
@@ -270,9 +271,9 @@ const Permission = ({ permission, index }: { permission: TPermission; index?: nu
                       setOpen((o) => !o)}
                      />
                   
@@ -282,14 +283,6 @@ const Permission = ({ permission, index }: { permission: TPermission; index?: nu {open && (
- {/* */}
)}
@@ -337,19 +330,68 @@ const Policies = ({ path, permission }: { path: string; permission: TPermission ); }; +const mergeSchemas = (...schemas: object[]) => { + const schema = s.allOf(schemas.filter(Boolean).map(s.fromSchema)); + return s.toTypes(schema, "Context"); +}; + +function replaceEntitiesEnum(schema: Record, entities: string[]) { + if (!isObject(schema) || !Array.isArray(entities) || entities.length === 0) return schema; + return transformObject(schema, (sub, name) => { + if (name === "properties") { + return transformObject(sub as Record, (propConfig, propKey) => { + if (propKey === "entity" && propConfig.type === "string") { + return { + ...propConfig, + enum: entities, + }; + } + return propConfig; + }); + } + return sub; + }); +} + const Policy = ({ permission, }: { permission: TPermission; }) => { const { value } = useFormValue(""); + const $bknd = useBknd(); + const $permissions = useApiQuery((api) => api.system.permissions(), { + use: [mountOnce], + }); + const entities = Object.keys($bknd.config.data.entities ?? {}); + const ctx = $permissions.data + ? mergeSchemas( + $permissions.data.context, + replaceEntitiesEnum(permission.context ?? null, entities), + ) + : undefined; + return (
- + + + + + {({ value, setValue }) => ( - + {value?.effect === "filter" && ( - + + + )}
); }; + +const CustomFieldWrapper = ({ children, name, label, description, schema }: any) => { + const errors = useFormError(name, { strict: true }); + const Errors = errors.length > 0 && ( + {errors.map((e) => e.message).join(", ")} + ); + + return ( + + +
+ {label} + {description && ( + + + + )} +
+ {schema && ( +
+ + typeof schema === "object" ? ( + + ) : ( + + ) + } + > + + +
+ )} +
+ {children} + {Errors} +
+ ); +}; diff --git a/app/src/ui/routes/tools/mcp/mcp.tsx b/app/src/ui/routes/tools/mcp/mcp.tsx index c06668b..ccde4c6 100644 --- a/app/src/ui/routes/tools/mcp/mcp.tsx +++ b/app/src/ui/routes/tools/mcp/mcp.tsx @@ -39,7 +39,7 @@ export default function ToolsMcp() {
- + {window.location.origin + mcpPath}
diff --git a/app/src/ui/routes/tools/mcp/tools.tsx b/app/src/ui/routes/tools/mcp/tools.tsx index d439fc1..6c475dd 100644 --- a/app/src/ui/routes/tools/mcp/tools.tsx +++ b/app/src/ui/routes/tools/mcp/tools.tsx @@ -12,6 +12,7 @@ import * as Formy from "ui/components/form/Formy"; import { appShellStore } from "ui/store"; import { Icon } from "ui/components/display/Icon"; import { useMcpClient } from "./hooks/use-mcp-client"; +import { Tooltip } from "@mantine/core"; export function Sidebar({ open, toggle }) { const client = useMcpClient(); @@ -48,7 +49,11 @@ export function Sidebar({ open, toggle }) { toggle={toggle} renderHeaderRight={() => (
- {error && } + {error && ( + + + + )} {tools.length} diff --git a/bun.lock b/bun.lock index ab63856..fa63422 100644 --- a/bun.lock +++ b/bun.lock @@ -35,7 +35,7 @@ "hono": "4.8.3", "json-schema-library": "10.0.0-rc7", "json-schema-to-ts": "^3.1.1", - "jsonv-ts": "0.8.6", + "jsonv-ts": "0.9.1", "kysely": "0.27.6", "lodash-es": "^4.17.21", "oauth4webapi": "^2.11.1", @@ -1243,7 +1243,7 @@ "@types/babel__traverse": ["@types/babel__traverse@7.20.6", "", { "dependencies": { "@babel/types": "^7.20.7" } }, "sha512-r1bzfrm0tomOI8g1SzvCaQHo6Lcv6zu0EA+W2kHrt8dyrHQxGzBBL4kdkzIS+jBMV+EYcMAEAqXqYaLJq5rOZg=="], - "@types/bun": ["@types/bun@1.3.0", "", { "dependencies": { "bun-types": "1.3.0" } }, "sha512-+lAGCYjXjip2qY375xX/scJeVRmZ5cY0wyHYyCYxNcdEXrQ4AOe3gACgd4iQ8ksOslJtW4VNxBJ8llUwc3a6AA=="], + "@types/bun": ["@types/bun@1.3.1", "", { "dependencies": { "bun-types": "1.3.1" } }, "sha512-4jNMk2/K9YJtfqwoAa28c8wK+T7nvJFOjxI4h/7sORWcypRNxBpr+TPNaCfVWq70tLCJsqoFwcf0oI0JU/fvMQ=="], "@types/cookie": ["@types/cookie@0.6.0", "", {}, "sha512-4Kh9a6B2bQciAhf7FSuMRRkUWecJgJu9nPnx3yzpsfXX/c50REIqpHY4C82bXP90qrLtXtkDxTZosYO3UpOwlA=="], @@ -2529,7 +2529,7 @@ "jsonpointer": ["jsonpointer@5.0.1", "", {}, "sha512-p/nXbhSEcu3pZRdkW1OfJhpsVtW1gd4Wa1fnQc9YLiTfAjn0312eMKimbdIQzuZl9aa9xUGaRlP9T/CJE/ditQ=="], - "jsonv-ts": ["jsonv-ts@0.8.6", "", { "optionalDependencies": { "hono": "*" }, "peerDependencies": { "typescript": "^5.0.0" } }, "sha512-z5jJ017LFOvAFFVodAIiCY024yW72RWc/K0Sct+OtuiLN+lKy+g0pI0jaz5JmuXaMIePc6HyopeeYHi8ffbYhw=="], + "jsonv-ts": ["jsonv-ts@0.9.1", "", { "optionalDependencies": { "hono": "*" }, "peerDependencies": { "typescript": "^5.0.0" } }, "sha512-sQZn7kdSMK9m3hLWvTLyNk2zCUmte2lVWIcK02633EwMosk/VAdRgpMyfMDMV6/ZzSMI0/SwevkUbkxdGQrWtg=="], "jsonwebtoken": ["jsonwebtoken@9.0.2", "", { "dependencies": { "jws": "^3.2.2", "lodash.includes": "^4.3.0", "lodash.isboolean": "^3.0.3", "lodash.isinteger": "^4.0.4", "lodash.isnumber": "^3.0.3", "lodash.isplainobject": "^4.0.6", "lodash.isstring": "^4.0.1", "lodash.once": "^4.0.0", "ms": "^2.1.1", "semver": "^7.5.4" } }, "sha512-PRp66vJ865SSqOlgqS8hujT5U4AOgMfhrwYIuIhfKaoSCZcirrmASQr8CX7cUg+RMih+hgznrjp99o+W4pJLHQ=="], @@ -4095,7 +4095,7 @@ "@testing-library/jest-dom/chalk": ["chalk@3.0.0", "", { "dependencies": { "ansi-styles": "^4.1.0", "supports-color": "^7.1.0" } }, "sha512-4D3B6Wf41KOYRFdszmDqMCGq5VV/uMAB273JILmO+3jAlh8X4qDtdtgCR3fxtbLEMzSx22QdhnDcJvu2u1fVwg=="], - "@types/bun/bun-types": ["bun-types@1.3.0", "", { "dependencies": { "@types/node": "*" }, "peerDependencies": { "@types/react": "^19" } }, "sha512-u8X0thhx+yJ0KmkxuEo9HAtdfgCBaM/aI9K90VQcQioAmkVp3SG3FkwWGibUFz3WdXAdcsqOcbU40lK7tbHdkQ=="], + "@types/bun/bun-types": ["bun-types@1.3.1", "", { "dependencies": { "@types/node": "*" }, "peerDependencies": { "@types/react": "^19" } }, "sha512-NMrcy7smratanWJ2mMXdpatalovtxVggkj11bScuWuiOoXTiKIu2eVS1/7qbyI/4yHedtsn175n4Sm4JcdHLXw=="], "@typescript-eslint/experimental-utils/eslint-utils": ["eslint-utils@2.1.0", "", { "dependencies": { "eslint-visitor-keys": "^1.1.0" } }, "sha512-w94dQYoauyvlDc43XnGB8lU3Zt713vNChgt4EWwhXAP2XkBvndfxF0AgIqKOOasjPIPzj9JqgwkwbCYD0/V3Zg=="], From 5d4a77fb10ddfdcc08d83318f968250ef7e7f17a Mon Sep 17 00:00:00 2001 From: dswbx Date: Fri, 24 Oct 2025 09:20:59 +0200 Subject: [PATCH 09/14] Update permission context handling and improve JSON field component - Enhanced `MediaController` to include context in the `entityCreate` permission for better access control. - Refactored permission checks in `useBkndAuth` to ensure correct validation of role permissions. - Modified `JsonField` component to directly use `formData` in `JsonEditor`, simplifying data handling and improving user experience. --- app/src/media/api/MediaController.ts | 4 +++- app/src/ui/client/schema/auth/use-bknd-auth.ts | 2 +- .../components/form/json-schema/fields/JsonField.tsx | 12 +----------- 3 files changed, 5 insertions(+), 13 deletions(-) diff --git a/app/src/media/api/MediaController.ts b/app/src/media/api/MediaController.ts index e1f795b..0523b6a 100644 --- a/app/src/media/api/MediaController.ts +++ b/app/src/media/api/MediaController.ts @@ -189,7 +189,9 @@ export class MediaController extends Controller { }), ), jsc("query", s.object({ overwrite: s.boolean().optional() })), - permission(DataPermissions.entityCreate, {}), + permission(DataPermissions.entityCreate, { + context: (c) => ({ entity: c.req.param("entity") }), + }), permission(MediaPermissions.uploadFile, {}), async (c) => { const { entity: entity_name, id: entity_id, field: field_name } = c.req.valid("param"); diff --git a/app/src/ui/client/schema/auth/use-bknd-auth.ts b/app/src/ui/client/schema/auth/use-bknd-auth.ts index b48d1e1..7f83358 100644 --- a/app/src/ui/client/schema/auth/use-bknd-auth.ts +++ b/app/src/ui/client/schema/auth/use-bknd-auth.ts @@ -49,7 +49,7 @@ export function useBkndAuth() { has_admin: Object.entries(config.auth.roles ?? {}).some( ([name, role]) => role.implicit_allow || - minimum_permissions.every((p) => role.permissions?.includes(p)), + minimum_permissions.every((p) => role.permissions?.some((p) => p.permission === p)), ), }, routes: { diff --git a/app/src/ui/components/form/json-schema/fields/JsonField.tsx b/app/src/ui/components/form/json-schema/fields/JsonField.tsx index 1517a29..9fd2d5a 100644 --- a/app/src/ui/components/form/json-schema/fields/JsonField.tsx +++ b/app/src/ui/components/form/json-schema/fields/JsonField.tsx @@ -10,23 +10,13 @@ export default function JsonField({ readonly, ...props }: FieldProps) { - const value = JSON.stringify(formData, null, 2); - - function handleChange(data) { - try { - onChange(JSON.parse(data)); - } catch (err) { - console.error(err); - } - } - const isDisabled = disabled || readonly; const id = props.idSchema.$id; return (
); } From 2d56b54e0c1423b5a323017b750c747cacbc28c4 Mon Sep 17 00:00:00 2001 From: dswbx Date: Fri, 24 Oct 2025 09:40:02 +0200 Subject: [PATCH 10/14] Enhance Guard and Form components with improved error handling and debugging - Added debug logging in the `Guard` class to track policy evaluations and conditions. - Updated error logging in the `Form` component to provide more context on invalid submissions. - Introduced state management for form errors in the `AuthRolesEdit` component, displaying alerts for invalid data submissions. --- app/src/auth/authorize/Guard.ts | 5 +++++ app/src/ui/components/form/json-schema-form/Form.tsx | 2 +- app/src/ui/routes/auth/auth.roles.edit.$role.tsx | 10 +++++++++- 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/app/src/auth/authorize/Guard.ts b/app/src/auth/authorize/Guard.ts index e66dba1..6336a59 100644 --- a/app/src/auth/authorize/Guard.ts +++ b/app/src/auth/authorize/Guard.ts @@ -203,14 +203,17 @@ export class Guard { // set the default effect of the role permission let allowed = rolePermission.effect === "allow"; for (const policy of rolePermission.policies) { + $console.debug("guard: checking policy", { policy: policy.toJSON(), ctx }); // skip filter policies if (policy.content.effect === "filter") continue; // if condition is met, check the effect const meets = policy.meetsCondition(ctx); if (meets) { + $console.debug("guard: policy meets condition"); // if deny, then break early if (policy.content.effect === "deny") { + $console.debug("guard: policy is deny, setting allowed to false"); allowed = false; break; @@ -218,6 +221,8 @@ export class Guard { } else if (policy.content.effect === "allow") { allowed = true; } + } else { + $console.debug("guard: policy does not meet condition"); } } diff --git a/app/src/ui/components/form/json-schema-form/Form.tsx b/app/src/ui/components/form/json-schema-form/Form.tsx index acfa25b..5608796 100644 --- a/app/src/ui/components/form/json-schema-form/Form.tsx +++ b/app/src/ui/components/form/json-schema-form/Form.tsx @@ -130,7 +130,7 @@ export function Form< if (errors.length === 0) { await onSubmit(data as Data); } else { - console.log("invalid", errors); + console.error("form: invalid", { data, errors }); onInvalidSubmit?.(errors, data); } } catch (e) { diff --git a/app/src/ui/routes/auth/auth.roles.edit.$role.tsx b/app/src/ui/routes/auth/auth.roles.edit.$role.tsx index 9637ed1..e1be5dd 100644 --- a/app/src/ui/routes/auth/auth.roles.edit.$role.tsx +++ b/app/src/ui/routes/auth/auth.roles.edit.$role.tsx @@ -37,6 +37,8 @@ import { cn } from "ui/lib/utils"; import { JsonViewer } from "ui/components/code/JsonViewer"; import { mountOnce, useApiQuery } from "ui/client"; import { CodePreview } from "ui/components/code/CodePreview"; +import type { JsonError } from "json-schema-library"; +import { Alert } from "ui/components/display/Alert"; export function AuthRolesEdit(props) { useBrowserTitle(["Auth", "Roles", props.params.role]); @@ -72,6 +74,7 @@ const formConfig = { function AuthRolesEditInternal({ params }: { params: { role: string } }) { const [navigate] = useNavigate(); const { config, schema: authSchema, actions } = useBkndAuth(); + const [error, setError] = useState(); const roleName = params.role; const role = config.roles?.[roleName]; const { readonly, permissions } = useBknd(); @@ -91,6 +94,7 @@ function AuthRolesEditInternal({ params }: { params: { role: string } }) { } } async function handleUpdate(data: any) { + setError(undefined); await actions.roles.patch(roleName, data); } @@ -102,10 +106,13 @@ function AuthRolesEditInternal({ params }: { params: { role: string } }) { beforeSubmit={(data) => { return { ...data, - permissions: [...Object.values(data.permissions)], + permissions: [...Object.values(data.permissions).filter(Boolean)], }; }} onSubmit={handleUpdate} + onInvalidSubmit={(errors) => { + setError(errors); + }} > + {error && }
From cfb4b0e336944a90e48077c9b009e8a80873b889 Mon Sep 17 00:00:00 2001 From: dswbx Date: Fri, 24 Oct 2025 09:59:00 +0200 Subject: [PATCH 11/14] Refactor JsonEditor and Permission components for improved state management and performance - Implemented debounced input handling in `JsonEditor` to enhance user experience and reduce unnecessary updates. - Updated `Permission` component to streamline permission state management and improve clarity in policy handling. - Refactored `Policies` component to utilize derived field context for better data handling and rendering efficiency. --- app/src/ui/components/code/JsonEditor.tsx | 5 +- .../ui/routes/auth/auth.roles.edit.$role.tsx | 66 ++++++++++++------- 2 files changed, 46 insertions(+), 25 deletions(-) diff --git a/app/src/ui/components/code/JsonEditor.tsx b/app/src/ui/components/code/JsonEditor.tsx index c65e59a..93bb8d1 100644 --- a/app/src/ui/components/code/JsonEditor.tsx +++ b/app/src/ui/components/code/JsonEditor.tsx @@ -1,6 +1,7 @@ import { Suspense, lazy, useState } from "react"; import { twMerge } from "tailwind-merge"; import type { CodeEditorProps } from "./CodeEditor"; +import { useDebouncedCallback } from "@mantine/hooks"; const CodeEditor = lazy(() => import("./CodeEditor")); export type JsonEditorProps = Omit & { @@ -21,13 +22,13 @@ export function JsonEditor({ const [editorValue, setEditorValue] = useState( JSON.stringify(value, null, 2), ); - const handleChange = (given: string) => { + const handleChange = useDebouncedCallback((given: string) => { const value = given === "" ? (emptyAs === "null" ? null : undefined) : given; try { setEditorValue(value); onChange?.(value ? JSON.parse(value) : value); } catch (e) {} - }; + }, 500); const handleBlur = (e) => { setEditorValue(JSON.stringify(value, null, 2)); onBlur?.(e); diff --git a/app/src/ui/routes/auth/auth.roles.edit.$role.tsx b/app/src/ui/routes/auth/auth.roles.edit.$role.tsx index e1be5dd..51d909d 100644 --- a/app/src/ui/routes/auth/auth.roles.edit.$role.tsx +++ b/app/src/ui/routes/auth/auth.roles.edit.$role.tsx @@ -233,15 +233,19 @@ const Permission = ({ permission, index }: { permission: TPermission; index?: nu const { value } = useDerivedFieldContext("permissions", (ctx) => { const v = ctx.value; if (!Array.isArray(v)) return undefined; - return v.find((v) => v && v.permission === permission.name); + const v2 = v.find((v) => v && v.permission === permission.name); + return { + set: !!v2, + policies: (v2?.policies?.length ?? 0) as number, + }; }); const { setValue } = useFormContext(); const [open, setOpen] = useState(false); - const data = value as PermissionData | undefined; - const policiesCount = data?.policies?.length ?? 0; + const policiesCount = value?.policies ?? 0; + const isSet = value?.set ?? false; async function handleSwitch() { - if (data) { + if (isSet) { setValue(path, undefined); setOpen(false); } else { @@ -253,6 +257,10 @@ const Permission = ({ permission, index }: { permission: TPermission; index?: nu } } + function toggleOpen() { + setOpen((o) => !o); + } + return ( <>
setOpen((o) => !o)} + onClick={toggleOpen} />
- +
{open && ( @@ -299,13 +307,22 @@ const Permission = ({ permission, index }: { permission: TPermission; index?: nu }; const Policies = ({ path, permission }: { path: string; permission: TPermission }) => { - const { value: _value } = useFormValue(path); - const { setValue, schema: policySchema, lib, deleteValue } = useDerivedFieldContext(path); - const value = _value ?? []; + const { + setValue, + schema: policySchema, + lib, + deleteValue, + value, + } = useDerivedFieldContext(path, ({ value }) => { + return { + policies: (value?.length ?? 0) as number, + }; + }); + const policiesCount = value?.policies ?? 0; function handleAdd() { setValue( - `${path}.${value.length}`, + `${path}.${policiesCount}`, lib.getTemplate(undefined, policySchema!.items, { addOptionalProps: true, }), @@ -317,19 +334,20 @@ const Policies = ({ path, permission }: { path: string; permission: TPermission } return ( -
0 && "gap-8")}> +
0 && "gap-8")}>
- {value.map((policy, i) => ( - - {i > 0 &&
} -
-
- + {policiesCount > 0 && + Array.from({ length: policiesCount }).map((_, i) => ( + + {i > 0 &&
} +
+
+ +
+ handleDelete(i)} size="sm" />
- handleDelete(i)} size="sm" /> -
-
- ))} + + ))}
@@ -366,7 +384,9 @@ const Policy = ({ }: { permission: TPermission; }) => { - const { value } = useFormValue(""); + const { value } = useDerivedFieldContext("", ({ value }) => ({ + effect: (value?.effect ?? "allow") as "allow" | "deny" | "filter", + })); const $bknd = useBknd(); const $permissions = useApiQuery((api) => api.system.permissions(), { use: [mountOnce], From 88e5c06e9d284a68ff6b0493f34b12f0e04b90c5 Mon Sep 17 00:00:00 2001 From: dswbx Date: Fri, 24 Oct 2025 10:37:52 +0200 Subject: [PATCH 12/14] Enhance SystemController to improve config modification checks Updated the `SystemController` to include additional checks for read-only status and user permissions when modifying configurations. --- app/src/modules/server/SystemController.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/app/src/modules/server/SystemController.ts b/app/src/modules/server/SystemController.ts index 1190d55..9cdbd99 100644 --- a/app/src/modules/server/SystemController.ts +++ b/app/src/modules/server/SystemController.ts @@ -17,6 +17,7 @@ import { mcp as mcpMiddleware, isNode, type McpServer, + threw, } from "bknd/utils"; import type { Context, Hono } from "hono"; import { Controller } from "modules/Controller"; @@ -380,7 +381,11 @@ export class SystemController extends Controller { async (c) => { const module = c.req.param("module") as ModuleKey | undefined; const { config, secrets, fresh } = c.req.valid("query"); - const readonly = this.app.isReadOnly(); + const readonly = + // either if app is read only in general + this.app.isReadOnly() || + // or if user is not allowed to modify the config + threw(() => this.ctx.guard.granted(SystemPermissions.configWrite, c, { module })); if (config) { this.ctx.guard.granted(SystemPermissions.configRead, c, { From 869031bbfa4b8b5015c91afa9a727bf0231640fe Mon Sep 17 00:00:00 2001 From: dswbx Date: Fri, 24 Oct 2025 12:43:32 +0200 Subject: [PATCH 13/14] Refactor CustomFieldWrapper and enhance schema handling in Policy component - Updated `CustomFieldWrapper` to accept a more structured schema object, improving clarity and type safety. - Modified schema handling in the `Policy` component to ensure proper context and variable naming, enhancing the overall user experience. - Introduced `autoFormatString` utility for dynamic button labeling based on schema name. --- .../ui/routes/auth/auth.roles.edit.$role.tsx | 45 ++++++++++++++----- 1 file changed, 35 insertions(+), 10 deletions(-) diff --git a/app/src/ui/routes/auth/auth.roles.edit.$role.tsx b/app/src/ui/routes/auth/auth.roles.edit.$role.tsx index 51d909d..d24ac9b 100644 --- a/app/src/ui/routes/auth/auth.roles.edit.$role.tsx +++ b/app/src/ui/routes/auth/auth.roles.edit.$role.tsx @@ -13,7 +13,7 @@ import { Breadcrumbs2 } from "ui/layouts/AppShell/Breadcrumbs2"; import { routes } from "ui/lib/routes"; import * as AppShell from "ui/layouts/AppShell/AppShell"; import * as Formy from "ui/components/form/Formy"; -import { ucFirst, s, transformObject, isObject } from "bknd/utils"; +import { ucFirst, s, transformObject, isObject, autoFormatString } from "bknd/utils"; import type { ModuleSchemas } from "bknd"; import { CustomField, @@ -357,8 +357,7 @@ const Policies = ({ path, permission }: { path: string; permission: TPermission }; const mergeSchemas = (...schemas: object[]) => { - const schema = s.allOf(schemas.filter(Boolean).map(s.fromSchema)); - return s.toTypes(schema, "Context"); + return s.allOf(schemas.filter(Boolean).map(s.fromSchema)); }; function replaceEntitiesEnum(schema: Record, entities: string[]) { @@ -407,7 +406,12 @@ const Policy = ({ name="condition" label="Condition" description="The condition that must be met for the policy to be applied." - schema={ctx} + schema={ + ctx && { + name: "Context", + content: s.toTypes(ctx, "Context"), + } + } > @@ -442,7 +446,12 @@ const Policy = ({ name="filter" label="Filter" description="Filter to apply to all queries on met condition." - schema={ctx} + schema={ + ctx && { + name: "Variables", + content: s.toTypes(ctx, "Variables"), + } + } > @@ -451,7 +460,22 @@ const Policy = ({ ); }; -const CustomFieldWrapper = ({ children, name, label, description, schema }: any) => { +const CustomFieldWrapper = ({ + children, + name, + label, + description, + schema, +}: { + children: React.ReactNode; + name: string; + label: string; + description: string; + schema?: { + name: string; + content: string | object; + }; +}) => { const errors = useFormError(name, { strict: true }); const Errors = errors.length > 0 && ( {errors.map((e) => e.message).join(", ")} @@ -480,15 +504,16 @@ const CustomFieldWrapper = ({ children, name, label, description, schema }: any) }} position="bottom-end" target={() => - typeof schema === "object" ? ( + typeof schema.content === "object" ? ( ) : ( @@ -496,7 +521,7 @@ const CustomFieldWrapper = ({ children, name, label, description, schema }: any) } >
From f3c6cd7620bbe5d6f943acb4012082234d1c3b8d Mon Sep 17 00:00:00 2001 From: dswbx Date: Fri, 24 Oct 2025 15:12:47 +0200 Subject: [PATCH 14/14] fix typo on AdminController flash message --- app/src/modules/server/AdminController.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/modules/server/AdminController.tsx b/app/src/modules/server/AdminController.tsx index 454ad40..e098101 100644 --- a/app/src/modules/server/AdminController.tsx +++ b/app/src/modules/server/AdminController.tsx @@ -114,7 +114,7 @@ export class AdminController extends Controller { }), permission(SystemPermissions.schemaRead, { onDenied: async (c) => { - addFlashMessage(c, "You not allowed to read the schema", "warning"); + addFlashMessage(c, "You are not allowed to read the schema", "warning"); }, context: (c) => ({}), }),