-
Notifications
You must be signed in to change notification settings - Fork 144
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
Request for Comments: Abstracting permissions framework #10067
Comments
Thanks for submitting this issue @philtweir. The initial need for resource permissions was to restrict a relatively limited number of sensitive instances - Guardian was well suited for that solution and we were already using it for nodegroup permissions, so it made sense at the time. However, it turns out that the default-deny scenario is a relatively common need for Arches implementations, and as you point out, for implementations with a large number of resources, Guardian is quite slow. For me this raises the question of whether Casbin would be better as the default permission framework, or whether Casbin should simply replace Guardian altogether. It's very possible that everyone might really like Casbin, and if that's the case, I think we should make it as easy as possible for them to use it. Also, if we have to maintain object permission code, I think we'd rather maintain code that leverages the better framework. I'm not that familiar with Casbin, so I'm wondering:
|
Thanks @chiatt and @dwuthrich, that sounds good - yes, of course, definitely logical how the defaults came about! As to Guardian vs Casbin -- I'm not too sure myself, Guardian seems decent in general, Casbin is quite a bit more complex so may be overkill for the general case (although definitely useful for us) - on the other hand, most users won't touch the permissions framework internals, so Casbin might be the most flexible if only picking one. However, the PR I have linked is actually using Guardian for default-deny, so if that would be handy, it is a relatively short bit of code itself - the key addition is the abstraction that enables swapping between two different sets of logic (i.e. arches/app/permissions/arches_default_deny.py) Also, since it removes most of the need for
I think it should be the same, as it would flow down through any rules and then take the default at the end.
Good question - without in-memory storage, any benefit is more likely to be from perhaps the rules being more general (e.g. that we can apply group logic without having to tag individual instances). That said, and keen to understand any relevant counterexamples, it seems in most situations there would be a small number of instance permissions that do not follow a general group-based rule (i.e. only this instance has this permission) -- as such, if the majority are "Instances for Foo County are accessible to Bar Team" then we can happily write that rule without expanding it to (instances for Foo County)x(Bar Team members) permission table entries, and the memory footprint is still pretty low. In that case, then you can do something like:
without the DB getting hit on the second line, or all the permissions being loaded from DB on the first, so it's very fast. The two main caveats there are:
I would certainly say only one at a time (so picking one for a project means not using the other, at least without DB reset), but if the suggested abstraction looks OK conceptually, then being able to run the project with either should be easy enough. But the management is a good question - codewise, that's essentially what is in the linked PR, so the existing UI components and plugin calls should be a change of import, but UI pages for management interfaces outside the Arches tree would need implemented. For example, we like the idea of using Casbin for hierarchical permissions, which is not (AFAIU) an existing use-case, so we will have to code up a widget for managing that. |
Just as an update on this (and in case this is of interest more generally) -- we are trying an approach (in Arches HER) where:
Coming back to the points above in discussion with @chiatt - that last step would make the permissions logic specific to Arches HER, so being able to "plug in" the permissions from the Arches project itself has been handy -- however, that's just an example, there may be a better recommended approach here! |
Hi @philtweir, I think that sounds really interesting. I feel that being able to implement a custom permission backed could be a nice improvement to Arches and would welcome a PR for it. I'm wondering what others think (@robgaston, @apeters, @bferguso)? |
@philtweir After reviewing the thread and discussing with @chiatt I too am interested in seeing what a pluggable permissions backend might add to Arches in general and also very curious about some of the performance advantages to using Casbin (although I realize that's not part of the proposed PR). |
Following up on ideas and challenges that have come up, where the established permissions approach is not optimal for some of our use-cases, this issue aims to open dialogue on how permissions rules could be more internally abstracted.
TL;DR
Could we make permission frameworks swappable? A first attempt is in flaxandteal#1 for feedback.
What is being suggested?
Centralizing (and abstracting) any
django-guardian
calls such that alternative permission logic can be swapped in, in a similar way to how storage backends, search components, datatypes, etc. can be currently.Why is this necessary?
At present, to manage resource visibility users or groups must be red-listed rather than green-listed, which works well when most data is open, but for instances with substantial amounts of non-public data, it can slow indexing for large numbers of users and falls back to allowing access if any coding/role-assignment issues occur. Using custom Django groups helps, but some of the (cascading sequences of) checks make group logic unintuitive, and require hooking extra logic on save to do, for instance, per-group visibility of resources.
What has been tried?
Originally, we did not want to end up with a custom Arches base, so monkey patching
check_resource_instance_permissions
andsearch_results
from our project, got us the basics. Our project that relied on group-based visibility needed some further tweaks and hooks. However, a cleaner solution would be to provide aPermissionFramework
object of some sort that can plug in and encapsulates and overwrites this via an agreed interface.To try and get the ball rolling, flaxandteal#1 shows a working example of pulling all the permissions logic into individual (subclassable)
PermissionFramework
classes.Note that this is not intended to be the nicest or succinct way, but aiming for a less-invasive, reasonable, low-risk, familiar approach, to get feedback and prep a PR for the functionality (if agreeable), so we could do something more ambitious as a refactor once it is proven working and useful. More testing and tidying would have to happen, but keen to get feedback before going too far.
What opportunities does this open up?
As well as giving us an easy way to set default-deny for resources via
settings.py
(see video in linked PR), or cleanly change how group permissions work via an in-project module, this would allow us to use other authorization libraries.In particular, (but not in this PR), we are hoping to use https://casbin.org/ for holding the full group permission table in-memory, since it provides efficient ways to recache, reload and trigger updates without involving the database for perms during (most) web requests. It also works across languages, so a NodeJS, C# or Java application using Casbin could read the same live authorization table to ensure permissions are consistent (handy for legacy systems or tight integrations). Additionally, we have done previous work on transitive permissions that we would like to incorporate [1]. This abstraction would not only make future Casbin-based RBAC much easier to integrate (and swap in/out), but makes it easy to provide a standalone Casbin plugin for Arches, instead of either copying logic into each new project, or PR-ing Casbin logic to Arches core, which may not be desirable dependency/scope-expansion (unless everyone really likes Casbin!).
Based on previous conversations, this discussion may be of particular interest to Arches UK forum members from the last meet, as well as @dwuthrich and @SDScandrettKint .
[1] - Not critical to this issue, but e.g. resource A is in resource group X under group Y (under Z, ...), to which usergroup V under usergroup W has access, so all users in W can see A -- for instance, to give a non-trivial concrete example, suppose the resources for heritage sites at Mahabalipuram sit within the resource group for (the surrounding Indian state) Tamil Nadu, which is under the resource group for India. Suppose also that the India Working Group sits under the project's Asia management team. Then linking India to the India Working Group will mean the Asia team lead can immediately see Mahabalipuram data, but the e.g. Europe lead still cannot. Of course, there is no comment intended on the general desirability of hiding/showing data by default.
The text was updated successfully, but these errors were encountered: