casbin / node-casbin

An authorization library that supports access control models like ACL, RBAC, ABAC in Node.js and Browser
https://casbin.org
Apache License 2.0
2.61k stars 218 forks source link

Chaining on nested struct / object fields in ABAC policy #438

Closed johnsonjo4531 closed 1 year ago

johnsonjo4531 commented 1 year ago

Hi, I'm new to casbin and am using ABAC as outlined here: https://casbin.org/docs/abac

Overall I really like Casbin and I am currently writing a little helper library to help with writing policies easier with type safety.

Anyways, I'm running into an issue with chaining on nested structs and object fields in a policy sub_rule using ABAC.

I know policy rules support nested lookups atleast in Go's casbin, because of this issue: https://github.com/casbin/casbin/issues/677

The following example is what I'm trying to get to work :D. Help would be appreciated :D

Code Sandbox Link

import { Enforcer, newModelFromString } from "casbin";

describe("casbin's nested properties should work", () => {
  // TODO: Get this to pass, because currently THIS DOES NOT PASS!!!
  it("should not error on nested property access", async () => {
    const model = newModelFromString(`
[request_definition]
r = sub_type, sub, obj_type, obj, act

[policy_definition]
p = sub_rule, sub_type, obj_type, act

[policy_effect]
e = some(where (p.eft == allow))

[matchers]
m = eval(p.sub_rule) && r.sub_type == p.sub_type && r.obj_type === p.obj_type && r.act == p.act
`);
    const enforcer = new Enforcer();
    enforcer.setModel(model);
    const sub = { id: "foo" };
    const obj = { owner: { id: "foo" } };
    const result = ["r.sub.id == r.obj.owner.id", "User", "Book", "read"];

    await enforcer.addPolicy(...result);

    expect(await enforcer.enforce("User", sub, "Book", obj, "read")).toBe(true);
  });

  // THIS PASSES
  it("already does work without nesting", async () => {
    const model = newModelFromString(`
[request_definition]
r = sub_type, sub, obj_type, obj, act

[policy_definition]
p = sub_rule, sub_type, obj_type, act

[policy_effect]
e = some(where (p.eft == allow))

[matchers]
m = eval(p.sub_rule) && r.sub_type == p.sub_type && r.obj_type === p.obj_type && r.act == p.act
`);
    const enforcer = new Enforcer();
    enforcer.setModel(model);
    const sub = { id: "foo" };
    const obj = { id: "foo" };
    const result = ["r.sub.id == r.obj.id", "User", "Book", "read"];

    await enforcer.addPolicy(...result);

    expect(await enforcer.enforce("User", sub, "Book", obj, "read")).toBe(true);
  });
});
casbin-bot commented 1 year ago

@nodece @Zxilly @Shivansh-yadav13

johnsonjo4531 commented 1 year ago

After a little digging I found the solution after finding the online editor finds the above code valid with setup like so:

Model

[request_definition]
r = sub_type, sub, obj_type, obj, act

[policy_definition]
p = sub_rule, sub_type, obj_type, act

[policy_effect]
e = some(where (p.eft == allow))

[matchers]
m = eval(p.sub_rule) && r.sub_type == p.sub_type && r.obj_type === p.obj_type && r.act == p.act

Policy

p, r.sub.id == r.obj.owner.id, User, Book, "read"

Request

User, { id: "foo"}, Book, {owner: {id: "foo"}}, read

Enforcement Result

true

I needed to JSON.stringify my subject and object like so:

expect(
  await enforcer.enforce(
    "User",
    JSON.stringify(sub),
    "Book",
    JSON.stringify(obj),
    "read"
  )
).toBe(true);

Working CodeSandbox Link

This was a little unexpected to me, because on the type hints for enforcer.enforce it says (my own emphasis added):

enforce decides whether a "subject" can access a "object" with the operation "action", input parameters are usually: (sub, obj, act).

@param rvals the request needs to be mediated, usually an array of strings, can be class instances if ABAC is used.

@return — whether to allow the request.

So, just wondering what good is making it the rvals an any[] in TypeScript and adding the phrase can be class instances if it doesn't handle nested structs properly in the js version unless they are strinigified? Seems strinigifying it would possibly be slower especially if it just becomes unstrinigified in JS? I'm mainly just wondering if I'm missing something here, or if there is actually a bug in using nested objects, or if I should just be strinigfying my objects.

johnsonjo4531 commented 1 year ago

I think I found the bug :D woohoo!!!

In your utils in casbin you have the following escape assertions function:

function escapeAssertion(s) {
    s = s.replace(/r\./g, 'r_');
    s = s.replace(/p\./g, 'p_');
    return s;
}

The problem is owner ends with 'r' and has a dot after it like so in the policy line r.obj.owner.id. So this function turns "r.obj.owner.id".replace(/r./g, 'r_'); and turns it into "r_obj.owner_id".

I propose the following fix this adds a negative look behind that will ensure's that the characters will only be replaced if not preceded by another word character :D

function escapeAssertion(s) {
    s = s.replace(/(?<!\w)r\./g, 'r_');
    s = s.replace(/(?<!\w)p\./g, 'p_');
    return s;
}
github-actions[bot] commented 1 year ago

:tada: This issue has been resolved in version 5.24.4 :tada:

The release is available on:

Your semantic-release bot :package::rocket: