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

CasbinJsGetPermissionForUser is too limited #31

Closed
dschmidt opened this issue Sep 17, 2020 · 14 comments
Closed

CasbinJsGetPermissionForUser is too limited #31

dschmidt opened this issue Sep 17, 2020 · 14 comments
Assignees
Labels
enhancement New feature or request

Comments

@dschmidt
Copy link

Hey everyone,

I'm checking out casbin because I'm considering to use it for a project. What especially caught my interest, is the new casbin.js because no other go authorization framework seems to provide anything similar. While I was playing around with it, I noticed limitations of CasbinJsGetPermissionForUser, which I would like to discuss:

(I hope this is the right repository to discuss, otherwise feel free to move the issue around)

deny permissions

CasbinJsGetPermissionForUser(...) cannot be used with any allow/deny models because the .eft information is completely stripped, i.e. any deny permission becomes an allow permission.

Standard rbac_with_deny_model.conf and rbac_with_deny_policy.csv make CasbinJsGetPermissionForUser(e, "alice") return

{"read":["data1","data2"],"write":["data2","data2"]}

Not only is data2 duplicated, but the second time it's actually a deny rule and here it shows up as a allow rule.

Idea

This might be solvable by more or less directly serializing the permissions as they are returned from e.GetImplicitPermissionsForUser(user). Otherwise I'm afraid with whatever we come up, will miss information for other model configurations because of the generic nature of casbin.

This would probably mean that casbin.js would need to have access to the model definition to know how to interpret the permissions?
If that wasn't the case, we wouldn't need the model configuration on the server side either ...

role inheritance

CasbinJsGetPermissionForUser(...) does not handle permissions for subroles.

Standard rbac_with_resource_roles_model.conf and rbac_with_resource_roles_policy.csv make CasbinJsGetPermissionForUser(e, "alice") return

{"read":["data1"],"write":["data_group"]}

So e.Enforce("alice", "data2", "write") will be true on the backend, but false on the frontend because because subroles are missing from the response and so unknown to the frontend.

Idea

For this very specific use case I added this to the loop:

                subRoles, err := e.GetUsersForRole(role)
		if err != nil {
			continue
		}
		for j := 0; j < len(subRoles); j++ {
			permission[action] = append(permission[action], subRoles[j])
		}

and it works. But it only handles one level of subroles (does casbin on the server side handle nested resource role permission deeper than one level?) and it seems to be very specific to this use case.

To make this a bit more abstract:
all roles of the user and the roles of resources that the user has permissions for should be known to the frontend.

Can this be done in a generic way at all and especially without leaking possibly sensitive information to the frontend?

Conclusion

CasbinJsGetPermissionForUser (and the format it returns) is in my humble opinion too limited. The structure of the response should be adjusted to be more generic and casbin.js should be adjusted to reflect a new structure.
If possible the data returned should be extended with resource roles.

I think it's rather easy to come up with a solution that works for a specific use case, but doing it in a generic way that's "upstreamable" might be rather hard. I'd be happy to hear your thoughts what can be done with reasonable effort as someone who is new to the project and then I'd be happy to work on that and upstream it.

Best regards,
Dominik

P.S.: I'm also in your Gitter channel and happy to answer questions or discuss there too

@hsluoyz
Copy link
Member

hsluoyz commented Sep 17, 2020

@kingiw can you take a look?

@hsluoyz hsluoyz self-assigned this Sep 17, 2020
@kingiw
Copy link
Collaborator

kingiw commented Sep 18, 2020

Glad to see your valuable comments!

Current implementation of CasbinJsGetPermissionForUser only support for conventional ACL and RBAC models. What we currently doing is rebuilding the design of CasbinJS and looking for a more generic solution is to let CasbinJS support all models, like RBAC w/ domains and complicated ABAC models.

To minimize the the information passed to the frontend while maintaining any necessary features is not easy. For fully support for the features of Casbin, passing the model configuration and some required policies (related to a specified subject) will be a generic solution. Temporarily we are trying to anonymize the request subject and generate the minimum information that enough for the frontend enforcement.

It could be something as follows.

Imagine we are using an RBAC models, with the following model configuration and policies:

# Model configuration
request_definition, policy_definition, policy_effect...
# Matcher Definition
m = g(r.sub, p.sub) && r.obj == p.obj && r.act == p.act

# policy
g, bob, group1
g, alice, group1
g, group1, group2,
p, group2, write, data
... other policies not related to alice

When a frontend querying the permission of alice, the backend Casbin.Js core will produce the minimum info:

# Model Configuration
request_definition, policy_definition, policy_effect...
# Matcher Definition
m = r.obj == p.obj && r.act == p.act  # g(r.sub, p.sub) is removed because the condition can be determined at the backend.

# policy
p, _, write, data # subject is anonimized

As you can see, only very few policies and matcher are used at the frontend. Frontend visitors have no idea about the backend access-control model. Meanwhile, with such permission, CasbinJs can do the enforcement at the frontend (with a lightweight enforcer similar implementation as the Casbin enforcer)

Such methodologies support for other models including rbac_with_not_deny.

This solution is still experimental. Currently I attempt introduing protobuf to help maintain, serialize and transmit the data.

The mentioned solution could still leak some sensitive information. For that I am considering some encryption methods like RSA.

Feel free to leave your comments!

@dschmidt
Copy link
Author

You're welcome and thanks for your comment! :)

Here are my thoughts on what you wrote:

# g(r.sub, p.sub) is removed because the condition can be determined at the backend.

I'm not sure this is true, if you strip sub from the matcher definition and the response to the frontend, how is the frontend supposed to prioritize conflicting rules? I'm not sure all conflicting rules can be resolved at server side, because the resource they will be used against is not known yet.

As you can see, only very few policies and matcher are used at the frontend. Frontend visitors have no idea about the backend access-control model. Meanwhile, with such permission, CasbinJs can do the enforcement at the frontend (with a lightweight enforcer similar implementation as the Casbin enforcer)

I think it sounds like a great idea to limit the visibility of the background access control details to the backend and hide it from the frontend, but I don't think you can do that without limiting the power of the solution.
I think it's necessary to make all roles related to the user (obviously other users and their roles can be stripped) available to the frontend - or at least make the frontend support it. The backend could still be customised to restrict visibility of certain fields etc. in individual projects

This solution is still experimental. Currently I attempt introduing protobuf to help maintain, serialize and transmit the data.

Is there any branch to look at? Does protobuf really help so much with the size and is that really such a big issue?
I assume that the computational complexity of determining what to transfer and how to interpret it on the frontend exceed the complexity of determining how to transfer the data by far :)

The mentioned solution could still leak some sensitive information. For that I am considering some encryption methods like RSA.

Uhm I wouldn't even call this encryption because the key must be known to the frontend, at some point it's decrypted there and could then be introspected in the browser - you could call it obfuscation at best IMHO. Personally I wouldn't prioritize this, if you want that kind of obfuscation you can add it on top in your project. My project uses a frontend that is decoupled from the backend service so much it can run on a completely different (sub-) domain and so I won't be able to use cookies ever. So I'm doing the transport myself and I am using the manual mode anyway. I could add this obfuscation layer myself if really desired one day.

@kingiw
Copy link
Collaborator

kingiw commented Sep 20, 2020

how is the frontend supposed to prioritize conflicting rules?

In my mind, the priority of the policies can be determined at the backend in some sort of other forms like the orders (The more ahead, the higher priority)

I think it's necessary to make all roles related to the user (obviously other users and their roles can be stripped) available to the frontend - or at least make the frontend support it.

I believe the roles are not necessary to the frontend. The frontend doesn't need to know which group the current subject belongs to. All the policies passed to the frontend are dedicatedly filtered out according to the subject and are anonymized. For example, if alice belongs to group1, the backend side will select all the policies about alice and group1, and anonymize the subject part of the policy before passing them to the frontend. From the frontend perspective, it doesn't have any info about the groups of the user.

Is there any branch to look at?

Actually only I are focusing on the development of Casbin.js currently, so I put all the codes at local temporarily. And the policy selection stratagies at the backend side are extremely naive with lots of bugs. Once I believe it's okay I will publish it and leave a link here.

Does protobuf really help so much with the size and is that really such a big issue?

This is not the key. Why I trying to use protobuf is just simply wishing to serializing an abstract class object (w/ some metadata) and passing them in between the frontend and backend. Actually maybe JSON is enough. Most importantly, as you mentioned, how to reduce the computational complexity should be the key challenge.

You could call it obfuscation at best IMHO.

Yep I agree with that. The obfuscation features are just an add-on feature for CasbinJS.

@dschmidt
Copy link
Author

Okay, sounds good. It's nothing I need urgently for now, so I'm happy to wait and see what you come up with.

Good luck on your mission! \m/

If there's anything you want feedback on or help with, let me know here.

@fgblomqvist
Copy link

Is there any update on this? It seems like the solution proposed by @kingiw is ideal. Wouldn't mind helping out if needed!

@kingiw
Copy link
Collaborator

kingiw commented Jan 12, 2021

@fgblomqvist Hi, thanks for your attention. I am so sorry that I am busy with my jobs these days. 😭
I will update the implementation when available. If you have any good ideas or requirements, plz let us know. Any PR and comments are welcome!

@fgblomqvist
Copy link

No worries. It sounds like sending the full model + a filtered policy would do it. I.e. what @dschmidt said, just serialize the output of GetImplicitPermissionsForUser and send that, along with the full actual model (that shouldn't be sensitive info).
Am I missing something?

@JayWestH
Copy link

Hi,
Would appreciate if someone could provide a working example on how to use CasbinJsGetPermissionForUser ?
With AWS get this error
{"message":"Missing Authentication Token"}
How do I handle this?
Thanks
J

@hsluoyz
Copy link
Member

hsluoyz commented Feb 12, 2021

@kingiw

@hsluoyz hsluoyz transferred this issue from casbin/casbin Feb 20, 2021
@hsluoyz hsluoyz added the enhancement New feature or request label Feb 20, 2021
@Zxilly
Copy link
Contributor

Zxilly commented Mar 4, 2021

No worries. It sounds like sending the full model + a filtered policy would do it. I.e. what @dschmidt said, just serialize the output of GetImplicitPermissionsForUser and send that, along with the full actual model (that shouldn't be sensitive info).
Am I missing something?

@fgblomqvist The frontend should be always considered as unsafe, maybe send the whole model not a good idea. This provides additional information and may cause potential security vulnerability.

@fahussain
Copy link

Current implementation of CasbinJsGetPermissionForUser only support for conventional ACL and RBAC models. What we currently doing is rebuilding the design of CasbinJS and looking for a more generic solution is to let CasbinJS support all models, like RBAC w/ domains and complicated ABAC models.

It would be best if you added this to the documentation on casbin.org. I wasted two days trying to get casbin.js working in my Angular project only to finally see this comment.

@hsluoyz
Copy link
Member

hsluoyz commented Aug 27, 2021

@Zxilly @nodece

@hsluoyz
Copy link
Member

hsluoyz commented Feb 3, 2023

@dschmidt Casbin.js now bases on casbin-core and contains all Casbin functionalities (keymatch, RBAC, ABAC) now: #271

@hsluoyz hsluoyz closed this as completed Feb 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Development

No branches or pull requests

7 participants