ABAC eval not working with empty dict

[killswitch-GUI]

Model:

[request_definition]
r = sub, obj, type, act 

[policy_definition]
p = rule, type, act

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

[matchers]
m = eval(p.rule) && r.type == p.type && r.act == p.act

Policy:

p, r.sub.admin == True, user, delete
p, r.sub.identifier == r.obj.identifier, user, read

When attempting to eval the following:

abac_enforcer = casbin.Enforcer(
    str(file_path(...)), str(file_path(...))

if abac_enforcer.enforce({"admin": False}, {}, "user", "delete"):
        return True

Raises the following:

/simpleeval.py", line 481, in _eval_attribute
    raise AttributeDoesNotExist(node.attr, self.expr)
simpleeval.AttributeDoesNotExist: ('identifier', '(r_sub.identifier == r_obj.identifier) and r_type == p_type and r_act == p_act')

I attempted this within the online editor and it works fine. Sorry if I'm missing something. New PyCasbin user here!

[hsluoyz]

@Zxilly @ffyuanda

[ffyuanda]

I'm checking the issue...

[ffyuanda]

@killswitch-GUI

Hi, I've been checking the issue, and I think I have found the problem.

It has nothing to do with the empty dict or malfunction of PyCasbin, only your usage might be inappropriate.

In your case, the first line of the policy is evaluated to True, since a dict has the __getitem__ method and the SimpleEval (what we use to evaluate expressions) can evaluate the dict in this way. After that (the first line is evaluated to False), it starts to evaluate the second line of the policy, and the problem pops up: an empty dict has nothing in it that can be accessed by SimpleEval, so AttributeDoesNotExist occured.

Ideally you need to build a class to use ABAC model, and you should make sure that the arguments in your enforce() have the necessary attributes to collaborate with your policy. You can find its correct usage here. dict is fine to pass in as arguments, and you also want to make sure it has the required attributes.

The AttributeDoesNotExist error is a hint that your r_sub does not have the necessary attribute which is identifier. And in this case you want to build yourself a new class contains the attribute identifier.

I've drafted some examples on my branch, you can refer to that about the usage of ABAC.

Happy using PyCasbin!

[killswitch-GUI]

@ffyuanda thanks for getting back with me! I'll take a look now.

[ffyuanda]

@killswitch-GUI Sure, I've just updated my comment above, you can check it now.

[killswitch-GUI]

@ffyuanda we are actually implementing this within FastAPI and using primary Pydantic to model the data. we have been doing model.dict and passing it along. Do you think there is a better way of doing this?

EDIT: I know our use case may be wired since we are passing various models into Enforce. Maybe we need to apply a filter?

[killswitch-GUI]

Ok so putting in a Pedantic Class model does work. But like you said it still requires the "object" that is going to evaluated and the field present.

from uuid import UUID, uuid4
import casbin
from pydantic import BaseModel, Field

class User(BaseModel):
    admin: bool = Field(
        ..., description="Designates that this user has all permissions without explicitly assigning them."
    )
    identifier: UUID = Field(default_factory=uuid4, description="A unique identifier of the user.")


abac_enforcer = casbin.Enforcer("model.conf", "policy.csv")

user_1 = User(admin=True)

if abac_enforcer.enforce(user_1, user_1, "user", "read"):
    print("You can move forward")


if abac_enforcer.enforce(user_1, user_1, "user", "delete"):
    print("You can move forward")

[ffyuanda]

@killswitch-GUI yeah the object is needed to use ABAC. If you insist using a dict in this case, might you wanna use a defaultdict and pass in all the possible attributes? At least the attribute error can be resolved?

[ffyuanda]

Ok so putting in a Pedantic Class model does work. But like you said it still requires the "object" that is going to evaluated and the field present.

from uuid import UUID, uuid4
import casbin
from pydantic import BaseModel, Field

class User(BaseModel):
    admin: bool = Field(
        ..., description="Designates that this user has all permissions without explicitly assigning them."
    )
    identifier: UUID = Field(default_factory=uuid4, description="A unique identifier of the user.")


abac_enforcer = casbin.Enforcer("model.conf", "policy.csv")

user_1 = User(admin=True)

if abac_enforcer.enforce(user_1, user_1, "user", "read"):
    print("You can move forward")


if abac_enforcer.enforce(user_1, user_1, "user", "delete"):
    print("You can move forward")

This can work I think. Just make sure object all the way down :)

[killswitch-GUI]

@ffyuanda if this works within the Casbin editor should this issue be raised to simpleeval?

You can it working at: https://casbin.org/casbin-editor/#7MX2727D9

EDIT: After talking with the team we found one way to short-circuit the eval by placing it after the type and act but its still to seen how long that will last:

[request_definition]
r = sub, obj, type, act 

[policy_definition]
p = rule, type, act

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

[matchers]
m = r.type == p.type && r.act == p.act && eval(p.rule)

[ffyuanda]

@killswitch-GUI Since the Casbin editor is built on Javascript and Node.js (on the Node.js version of Casbin), I believe the issue may not be reproduced when you use the editor. But yeah I think it could work better for our scenario if simpleeval spits out a False instead of error.

But generally I think simpleeval performs very well and does not need to change much of its syntax parser to cater this need. You can still try to catch this error, or make a better use of ABAC to avoid the issue, I guess.

Or maybe rewrite part of the simpleeval to suit your need?

[ffyuanda]

And if you feel this issue is not needed anymore, please close it when you leave :)

[Zxilly]

@ffyuanda We can catch the possible error and return False in enforce(), but I don't know if that's necessary.

[ffyuanda]

@Zxilly I don't think it's quite necessary, since this error comes from the nature of ABAC model.

[killswitch-GUI]

@Zxilly and @ffyuanda since PyCasbin is already monkey patching we just monkey patched it to work. Before I close this, what would you suggest the right way is to use ABAC/Policy here. You seemed to hint we may be misusing it.

from uuid import UUID, uuid4
import casbin
from pydantic import BaseModel, Field

def eval(self, names=None):
    try:
        return self.__eval(names)
    except AttributeDoesNotExist as e:
        return False

casbin.util.expression.SimpleEval.__eval = casbin.util.expression.SimpleEval.eval
casbin.util.expression.SimpleEval.eval = eval

class User(BaseModel):
    admin: bool = Field(
        ..., description="Designates that this user has all permissions without explicitly assigning them."
    )
    identifier: UUID = Field(default_factory=uuid4, description="A unique identifier of the user.")


abac_enforcer = casbin.Enforcer("model.conf", "policy.csv")

user_1 = User(admin=True)

if abac_enforcer.enforce(user_1, user_1, "user", "read"):
    print("You can move forward")


if abac_enforcer.enforce(user_1, user_1, "user", "delete"):
    print("You can move forward")

if abac_enforcer.enforce(user_1, {}, "user", "delete"):
    print("This now works")

It passed our test suite as well 🎉

[ffyuanda]

@killswitch-GUI I don't quite suggest change PyCasbin or simpleeval. You should instead follow the "object all the way down" logic when you use A(ttribute)BAC.
And BTW
this is working simply because you set

user_1 = User(admin=True)

It evaluates the first line in policy and turns out to be True and ignored the second line :)

[killswitch-GUI]

@ffyuanda we used this on our production models as example (We also moved the eval to end to reduce down the policy evaluation that is triggered):

EDIT: Sorry we forgot to add the models and policies used.
policy:

p, r.sub.identifier == r.obj.identifier, user, read
p, r.sub.is_superuser == True, user, read
p, r.sub.is_superuser == True, user, delete

Model:

[request_definition]
r = sub, obj, type, act 

[policy_definition]
p = rule, type, act

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

[matchers]
m = r.type == p.type && r.act == p.act && eval(p.rule)

worked good! To be 100% honest here we are mixing RBAC and ABAC here to support a superuser context.

[ffyuanda]

Sure then that's great.

Though I didn't see any RBAC definition around here haha, it contains a g normally in the Model. You are using pure ABAC but with "RBAC mindset" here if I'm not mistaken.

Anyway, happy using PyCasbin!

[killswitch-GUI]

Sure then that's great.

Though I didn't see any RBAC definition around here haha, it contains a g normally in the Model. You are using pure ABAC but with "RBAC mindset" here if I'm not mistaken.

Anyway, happy using PyCasbin!

100% using that r.sub.is_superuser == True as more of a RBAC style evaluation rule, like you said mindset haha. Thanks so much @ffyuanda ~!!

[ffyuanda]

@killswitch-GUI Sure, no worries!