Hey Mehmet,
If a backend relies on PermissionAuthorizationBackend, and another
require the ModelOnlyPermissionAuthorizationBackend
So I think this is the point that confuses me. Why you would use
the user API if you cared about a specific backend?
Using your example of the RolesBackend, either
A) You want to leave it up to the user whether a role grants object
level permissions or not.
B) You want to have consistent behavior for your backend.
Explanation:
A) In this case you would use your pseudo code example.
To return true (proposed) for an object when a user has the model level
permission (ie a role with this permission) the user would configure
their setting so
```
AUTHENTICATION_BACKENDS = [
'PermissionAuthorizationBackend',
'RolesBackend',
]
```
To return false (current) for an object when a user has the model level
permission the user would configure their setting so
```
AUTHENTICATION_BACKENDS = [
'ModelOnlyPermissionAuthorizationBackend',
'RolesBackend',
]
```
In either case the backend order does not particularly matter for this
example.
B) In this case the backend should derive or call the backend whose
behaviour it requires.
```
class RolesBackend(PermissionAuthorizationBackend):
def has_perm(self, user_obj, perm, obj=None):
...
for delegate in delegates:
if super(RolesBackend, self).has_perm(user_obj, perm, obj):
return True
return False
```
Cheers,
Andrew
On 1/18/2018 8:25 AM, Mehmet Dogan wrote:
Andrew,
Yes, I think we can safely assume *apps *would be backend agnostic. I
was actually referring to the *backends *themselves, if they use
user.has_perm(...). This might sound counter-intuitive at first but I
think it is possible, and actually I am working on one right now (see
my long answer for details).
_Long Answer:_ I am trying to write a Roles backend with, and without
reinventing the wheels for, object and model permissions. The logic is
very simple.
class Role(models.Model):
delegate = models.OneToOneField(Permission,*...*)
perms = models.ManyToManyField(Permission,*...*)
*...*
Delegate is a permission for each role that lets a user use it, e.g.,
'roles.use_role_xxxxxx'; and the `perms` are the actual permission in
the Role. And my backend (pseudo code):
def has_perm(self, user_obj, perm, obj=None):
if perm is delegate:
ignore;
delegates = get_delegates_representing(perm)
for delegatein delegates:
if user_obj.has_perm(delegate, obj):# refer to existing other backends
return True return False
1) Now, as seen aboveuser_obj.has_perm(delegate, obj)is written for
the proposed way, and would not work with the current way as model
permissions would be excluded.
2) In order to prevent infinite loops, I have to strictly separate
permissions that represent Roles (namely delegates) and
permissions in the roles. If I could specify backends as I proposed
earlier, I could even take this even one step further
and allow roles include other roles (that would be awesome).
3) I have not tested or used this. And one might see it as a hairy
design to be disregarded. But I claim it has a fair chance to be
otherwise. /*When it comes to backends we don't have plenty of
choices, and allowing them leverage each other will just make*/
/*it easier to write them. */
/Thanks for reading so far! :)/
On Wed, Jan 17, 2018 at 8:42 PM Andrew Standley
<astand...@linear-systems.com <mailto:astand...@linear-systems.com>>
wrote:
Mehmet,
Can you explain to me what the situation would be that an app
could simply not be backend agnostic?
Given that AUTHENTICATION_BACKENDS is a /user/ configuration, it
seems odd that an app would ever design around specific backends
being installed. I'm not sure I follow your logic on this.
Regarding an API that will allow picking and choosing I already
have some ideas, but I think Carlton made an excellent point that
that is another discussion all together.
Cheers,
Andrew
On 1/17/2018 5:34 PM, Mehmet Dogan wrote:
Andrew,
Thank you for the input. Having options is good. My concern about
that is, it may divide the already small backends population. If
a backend relies on PermissionAuthorizationBackend, and another
require the ModelOnlyPermissionAuthorizationBackend; then one
cannot use both. Guardian, at its core, is agnostic of the
default backend behavior, however, relies on it in some
utilities. However, I am working on a simple
RolePermissionsBackend now that leverages existing Object and
Model backends at its core. Currently, if I do
`user.has_perm('foo.change_bar')` or
`user.has_perm('foo.change_bar', obj)` I know one is the model
and the other is the object permissions. In case of plug-ins,
that will go away, and it will be even harder to leverage
existing backends. Also, the logic that polls backends is in
auth/models.py, I do not see how would it be possible to design
an API that will let picking and choosing.
On Wed, Jan 17, 2018 at 12:58 PM Andrew Standley
<astand...@linear-systems.com
<mailto:astand...@linear-systems.com>> wrote:
Hi Carlton,
Thanks for the thoughts. I just wanted to share my
opinion on your options.
1. "Won't Fix"
I have yet to find anywhere the original design decisions
were documented. Based on what I can find, it appears that
object level permissions where a bit of an after-though for
the initial auth framework. Unless we can find the motivation
for the original design decision it seems foolish to leave
/unexpected/ behaviour simply because that's how it has
always been.
I understand the motivation for wanting to only check object
level permissions, but I would argue that the design of the
API insinuates a hierarchical behaviour which is part of the
reason the current behaviour is unexpected.
Specifically why use an 'obj' kwarg instead of having two
methods?
To me if the API where:
```
has_perm('foo.change_bar')
has_obj_perm('foo.change_bar', bar_obj)
```
I would expect `has_perm` to only deal with model level
permissions and `has_obj_perm` to only deal with object level
permissions
To me the current API:
```
has_perm('foo.change_bar', obj=bar_obj)
```
Implies that the method will always check model level
permissions and can /optionally/ check object level permissions.
Having to always call `if user.has_perm('foo.change_bar',
obj=bar) or user.has_perm('foo.change_bar')` both seems ugly,
and relies on backend authors following ModelBackend's
example of caching permission look-ups. Otherwise there may
be a performance cost. The AUTH_BACKEND system's strength is
the ability to plug in third-party and custom backends, it
seems dangerous to make assumptions based on ModelBackend
unless those assumptions are clearly documented.
2. Change ModelBackend
Initially this would have been my preferred option. However
having given the issue significant thought, I think that it
is important that users are still given the option of the
current behaviour.
Having an API return only object level permissions when
called with obj, while returning only model level permissions
when called without obj is a legitimate use case.
Which leads me to
3. Break out permissions aspects.
I agree this would require some planning for migration. It
also requires discussion on whether the desire would be to
separate authentication from authorization or if the auth
framework should just offer more default options.
However overall I think this is the best option. It leverages
the flexibility of the backend system and offers the option
of maintaining BC if desired.
For example it would make sense to me to plan on
restructuring the auth.backends to something like:
```
ModelAuthenticationBackend(object):
# defines get_user and authenticate
PermissionAuthorizationBackend(object):
# defines get_user_permissions, get_group_permissions,
get_all_permissions, has_perm, has_module_perms
# 'New' behaviour: checks model level permissions /even/ when
obj is None
ModelOnlyPermissionAuthorizationBackend(object):
# defines get_user_permissions, get_group_permissions,
get_all_permissions, has_perm, has_module_perms
# Current behaviour: checks model level permissions /only/
when obj is Non
ModelPermissionBackend(ModelAuthenticationBackend,
PermissionAuthorizationBackend):
pass
ModelBackend(ModelAuthenticationBackend,
ModelOnlyPermissionAuthorizationBackend):
pass
```
This offers users more options for their
authorization/authentication needs, and should make migration
easier as you could gradually depreciate the name
'ModelBackend' in favour of something like
'ModelPermissionOnlyBackend'
Additionally if Mehmet wanted to finalize an API that allowed
users to specify a subset of backends to check against, this
approach would support that.
Cheers,
Andrew
On 1/17/2018 2:45 AM, Carlton Gibson wrote:
Hi Mehmet,
Due to the BC issues, this is fairly in-depth.
Having looked at the history, here are my initial thoughts.
The initial issue here is this behaviour from `ModelBackend`:
```
user.has_perm('foo.change_bar', obj)
False
user.has_perm('for.change_bar')
True
```
Although the long-standing behaviour, this is considered
_unexpected_.
Ticket is: https://code.djangoproject.com/ticket/20218
There are two related tickets regarding permission-checking
in the Admin:
* https://code.djangoproject.com/ticket/13539
* https://code.djangoproject.com/ticket/11383
Currently, I see three options:
1. Close as "Won't Fix", perhaps with a review of the
documentation to see if we
can't clarify/emphasise the behaviour somewhere.
This is the path of least resistance. It conforms to the
original design
decision. It preserves the long-standing behaviour.
(Whilst, yes, some find the
behaviour unexpected, it has a sense; it just depends
how you look at it.)
The objection is to this kind of check:
if user.has_perm('foo.change_bar', obj=bar) or
user.has_perm('foo.change_bar'):
...
* Whilst, granted, it's a little clumsy, there's no
reason this couldn't be
wrapped in a utility function. (There's a suggestion
to that effect on the
Django-Guardian issue tracker[^1]. This seems like a
good idea, simple enough
to live in user code.)
* `ModelBackend` permission lookups are cached[^2] so
the performance worry
here should be negligible.
2. Implement the (straight) backwards incompatible change.
The difficulty here is (we guess) why this ticket has
been open so long.
If we are convinced this is the right way to go — i.e.
that the current
behaviour is in fact **wrong** — then we should go ahead
despite the
difficulty.
Working out what that entails is non-trivial. That's why
it needs a decent
discussion and consensus here.
Which leads to...
3. Break out the permissions aspect of `ModelBackend` in
order to make it
pluggable in some way. Then allow users to opt-in to a
new version with the
adjusted behaviour.
There is some discussion of this on one of the related
tickets[^3].
Again, exactly what the migration path is needs some
planning.
I'm not sure what the correct answer is.
[^1]:
https://github.com/django-guardian/django-guardian/issues/459
[^2]:
https://docs.djangoproject.com/en/2.0/topics/auth/default/#permission-caching
[^3]: f.f.
https://code.djangoproject.com/ticket/13539#comment:16
Kind Regards,
Carlton
--
--
You received this message because you are subscribed to the Google Groups "Django
developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-developers/9dae74de-2bf9-2784-e53d-2a65f736263d%40linear-systems.com.
For more options, visit https://groups.google.com/d/optout.
