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

@@ -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 { Role, type RoleSchema } from "auth/authorize/Role";
-import { objectTransform } from "bknd/utils";
+import { objectTransform, s } from "bknd/utils";
 
 function createGuard(
    permissionNames: string[],
-   roles?: Record<
+   roles?: Record<string, Omit<RoleSchema, "name">>,
-      string,
-      {
-         permissions?: string[];
-         is_default?: boolean;
-         implicit_allow?: boolean;
-      }
-   >,
    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();
+      });
+   });
 });

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 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", () => {

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);
+   });
 });

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,34 +193,39 @@ 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
+            // if condition is met, check the effect
-            const meets = policy.meetsCondition(ctx2);
+            const meets = policy.meetsCondition(ctx);
-            if (!meets || (meets && policy.content.effect === "deny")) {
+            if (meets) {
-               throw new GuardPermissionsException(
+               // if deny, then break early
-                  permission,
+               if (policy.content.effect === "deny") {
-                  policy,
+                  allowed = false;
-                  "Policy does not meet condition",
+                  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", {
          permission: permission.name,
          role: role.name,
@@ -235,20 +246,24 @@ export class Guard {
       c: GuardContext,
       context?: PermissionContext<P>,
    ): 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;
             }
          }
       }

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<Schema extends PolicySchema = PolicySchema> {
    }
 
    meetsCondition(context: object, vars?: Record<string, any>) {
+      if (!this.content.condition) return true;
       return query.validate(this.replace(this.content.condition!, vars), context);
    }
 
    meetsFilter(subject: object, vars?: Record<string, any>) {
+      if (!this.content.filter) return true;
       return query.validate(this.replace(this.content.filter!, vars), subject);
    }
 

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<typeof rolePermissionSchema>;
@@ -20,12 +24,14 @@ export class RolePermission {
    constructor(
       public permission: Permission<any, any, any, any>,
       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);
    }

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<P extends Permission<any, any, any, any>>(
 
          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);

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<Exps extends Expressions>(
    expressions: Exps,
    path: string[] = [],
 ): FilterQuery<Exps> {
+   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<Exps> = {};
 
@@ -157,7 +159,7 @@ function _build<Exps extends Expressions>(
    // 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<Exps extends Expressions>(
 
    // 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]));

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;
    }
 }