Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

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

Closed
johnsonjo4531 opened this issue Mar 10, 2023 · 4 comments · Fixed by #439
Closed

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

johnsonjo4531 opened this issue Mar 10, 2023 · 4 comments · Fixed by #439
Assignees
Labels
bug Something isn't working released

Comments

@johnsonjo4531
Copy link
Contributor

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: casbin/casbin#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
Copy link
Member

@casbin-bot casbin-bot added the question Further information is requested label Mar 10, 2023
@johnsonjo4531
Copy link
Contributor Author

johnsonjo4531 commented Mar 11, 2023

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
Copy link
Contributor Author

johnsonjo4531 commented Mar 11, 2023

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

johnsonjo4531 added a commit to johnsonjo4531/node-casbin that referenced this issue Mar 11, 2023
…cording to issue casbin#438

Fixed unwanted replacement of r. in evals according to issue casbin#438 (e.g. r.obj.owner.id wrongly
becoming r_obj.owner_id instead of the correct r_obj.owner.id)

fix casbin#438
@hsluoyz hsluoyz added bug Something isn't working and removed question Further information is requested labels Mar 12, 2023
hsluoyz pushed a commit that referenced this issue Mar 12, 2023
…cording to issue #438 (#439)

Fixed unwanted replacement of r. in evals according to issue #438 (e.g. r.obj.owner.id wrongly
becoming r_obj.owner_id instead of the correct r_obj.owner.id)

fix #438
github-actions bot pushed a commit that referenced this issue Mar 12, 2023
## [5.24.4](v5.24.3...v5.24.4) (2023-03-12)

### Bug Fixes

* **./src/util/util.ts:** fixed unwanted replacement of .r in evals according to issue [#438](#438) ([#439](#439)) ([39878be](39878be))
@github-actions
Copy link

🎉 This issue has been resolved in version 5.24.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working released
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants