Default Authorization BackEnd Denying Permissions if Object Provided

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
37 messages Options
12
Reply | Threaded
Open this post in threaded view
|

Default Authorization BackEnd Denying Permissions if Object Provided

Mehmet Dogan
Hello all,

I had opened a ticket re issue noted in subject, which happened to be a duplicate, anyways, the text is here:

Tim Graham told that it needs to be discussed here. Seems this is a long going issue, with several related issues (see also this one). I propose a solution in the first link:

If receives public support, I can also start working on a patch. 

Regards,

Mehmet

--
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 [hidden email].
To post to this group, send email to [hidden email].
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/a597ce64-4d0c-43d5-a2c6-eb7813dca244%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Default Authorization BackEnd Denying Permissions if Object Provided

Mehmet Dogan
Here is the text of linked stuff for convenience:

For authorization backends checking object level permissions (like guardian) usually requires calling the django's default authorization backend as a fallback to the more general set of permissions:

if user.has_perm('foo.change_bar', obj=bar) or user.has_perm('foo.change_bar'):
    ...

However, this not only looks ugly, but also requires polling of all the backends twice, and thus, is a performance loss. 


First, and possibly the best, solution to this is that, django does not deny permission if obj argument is provided, but just ignores it. This is also very logical, one who has a permission for the entire model/table, would also have it for an instance/row. This way by properly ordering backends in the settings, it could be a fallback solution for the lower level checkers. This might be the move in the right direction, although it is backwards incompatible. 


A second solution is a keyword argument, such as fallback_to_model=None, that will allow lower-level checkers mimic the model level permissions that django does. Obviously, this is not DRY. But is needed if the first solution is not accepted to get the necessary permissions with one round of polling, and without cluttering the code. If it was accepted, it would still be a useful addition since it would allow backends to prefer to handle the fallback by themselves. Or, it would allow users who fallback by default override that behavior and not fallback (via a value of False), i.e., when object level permissions are definitive. 

--
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 [hidden email].
To post to this group, send email to [hidden email].
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/6dec9e1b-e017-47ad-98bb-83a9e9d9b975%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Default Authorization BackEnd Denying Permissions if Object Provided

Mehmet Dogan
In reply to this post by Mehmet Dogan
And the other:

Here is what I propose in terms of working around the backward compatibility that seems to have kept it from being solved for so long. 


1) define a global setting, say: OBJECT_PERMISSION_FALLBACK_TO_MODEL=False. This is to help maintain the default behavior (unless the setting is changed of course). 


2) (as mentioned in the above comment) define a keyword argument at the method level for occasional override, say: fallback_to_model=None. Default value of None means it will be ignored in favor of the global setting, otherwise, it will take precedence. 


I can work on a patch if found reasonable. 

--
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 [hidden email].
To post to this group, send email to [hidden email].
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/2786c98d-cb01-4a22-b483-e6dd07b061fc%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Default Authorization BackEnd Denying Permissions if Object Provided

Mehmet Dogan
In reply to this post by Mehmet Dogan
Here is a sample patch:

https://github.com/doganmeh/django/commit/d85cd3a530984ab5e4cb42f93629a64eb0b65b07

--
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 [hidden email].
To post to this group, send email to [hidden email].
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/612b26a2-4d13-4d9b-a52a-ed36276a51aa%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Default Authorization BackEnd Denying Permissions if Object Provided

Mehmet Dogan
In reply to this post by Mehmet Dogan

Based on this patch: the following 3 methods in the custom authorization backends will have to admit a fallback_to_model keyword argument:

def has_perm(self, user_obj, perm, obj=None, fallback_to_model=None)
def get_group_permissions(self, user_obj, obj=None, fallback_to_model=None)
def get_all_permissions(self, user_obj, obj=None, fallback_to_model=None)

this is the backward incompatible part

--
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 [hidden email].
To post to this group, send email to [hidden email].
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/42c61480-fa7d-4031-bede-f54126fa3966%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Default Authorization BackEnd Denying Permissions if Object Provided

Mehmet Dogan
In reply to this post by Mehmet Dogan
Created a pull request: https://github.com/django/django/pull/9581

Mehmet

--
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 [hidden email].
To post to this group, send email to [hidden email].
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/e617c213-a5c7-4b84-a12f-58a4154d1482%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Default Authorization BackEnd Denying Permissions if Object Provided

Mehmet Dogan
In reply to this post by Mehmet Dogan

Seems like I found a better keyword argument than fallback_to_model. For the following backends setting:

AUTHENTICATION_BACKENDS = (
    'django.contrib.auth.backends.ModelBackend',  
    'guardian.backends.ObjectBackend',
    'roles.backends.RoleBackend',
)

And the ways to check:

user.has_perm('foo.change_bar', obj) 
# default: check all, model checker expected to NOT disown her child

user.has_perm('foo.change_bar', obj, backends=('object', 'role', ))  
# just check backends specified; 'role' for *.RoleBackend

Advantages:

  • One will not have to use package specific API for checking just the object permissions, for example.
  • Cleaner and generic: allows backends re-use each other. For example, I am writing a RoleBackend and can easily make calls to model or object permission backends, in order not to reinvent the wheel.

--
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 [hidden email].
To post to this group, send email to [hidden email].
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/f637e9ca-b345-4c0f-92e7-b80464e9b6da%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Default Authorization BackEnd Denying Permissions if Object Provided

Mehmet Dogan
In reply to this post by Mehmet Dogan
And I forgot; 3rd advantage:

  • The 3 backend methods mentioned above won't have to take an extra kwarg such as fallback_to_model; thus backward compatible there.

--
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 [hidden email].
To post to this group, send email to [hidden email].
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/7b494b07-bef1-4824-87f8-d010f9b5432a%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Default Authorization BackEnd Denying Permissions if Object Provided

Mehmet Dogan
In reply to this post by Mehmet Dogan
I updated my patch:

https://github.com/django/django/pull/9581

--
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 [hidden email].
To post to this group, send email to [hidden email].
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/b309e677-816b-4c5f-a2d1-e642d6c3551a%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Default Authorization BackEnd Denying Permissions if Object Provided

Carlton Gibson-3
In reply to this post by Mehmet Dogan
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 [hidden email].
To post to this group, send email to [hidden email].
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/cab981b8-7dc7-4e9d-9dcc-442b36820cdf%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Default Authorization BackEnd Denying Permissions if Object Provided

Florian Apolloner


On Wednesday, January 17, 2018 at 11:45:05 AM UTC+1, Carlton Gibson wrote:
    The objection is to this kind of check:

        if user.has_perm('foo.change_bar', obj=bar) or user.has_perm('foo.change_bar'):
            ...

FWIW  I would never write code like this. `user.has_perm('foo.change_bar', obj=bar)` would be enough, in the worst case the user would have to change the permission backend which is easy enough…

--
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 [hidden email].
To post to this group, send email to [hidden email].
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/242593f2-b218-43b7-bb17-bd5ca7e2634f%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Default Authorization BackEnd Denying Permissions if Object Provided

Mehmet Dogan
Florian,

Can you clarify this part, I am not sure what you meant:

in the worst case the user would have to change the permission backend which is easy enough…

On Wed, Jan 17, 2018 at 10:31 AM Florian Apolloner <[hidden email]> wrote:


On Wednesday, January 17, 2018 at 11:45:05 AM UTC+1, Carlton Gibson wrote:
    The objection is to this kind of check:

        if user.has_perm('foo.change_bar', obj=bar) or user.has_perm('foo.change_bar'):
            ...

FWIW  I would never write code like this. `user.has_perm('foo.change_bar', obj=bar)` would be enough, in the worst case the user would have to change the permission backend which is easy enough…

--
You received this message because you are subscribed to a topic in the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/django-developers/MLWfvPPVwDk/unsubscribe.
To unsubscribe from this group and all its topics, send an email to [hidden email].
To post to this group, send email to [hidden email].
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/242593f2-b218-43b7-bb17-bd5ca7e2634f%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

--
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 [hidden email].
To post to this group, send email to [hidden email].
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/CAFdefwNXkA8c2SD2kRsMBuzKk2Xwt4SJMtpWyro3q%2BtXzYEHpQ%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Default Authorization BackEnd Denying Permissions if Object Provided

Florian Apolloner
On Wednesday, January 17, 2018 at 5:48:03 PM UTC+1, Mehmet Dogan wrote:
Florian,

Can you clarify this part, I am not sure what you meant:


class MyBackend(ModelBackend):

   def has_perm(…, obj=None,…):
     if obj: obj = None
     return super().has_perm(…)

something along the lines of this should do it. 

--
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 [hidden email].
To post to this group, send email to [hidden email].
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/ff0142a5-cca0-4d8c-8497-b988b04ba4ca%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Default Authorization BackEnd Denying Permissions if Object Provided

Andrew Standley
In reply to this post by Carlton Gibson-3

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_.


There are two related tickets regarding permission-checking in the Admin:


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. 



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 [hidden email].
To post to this group, send email to [hidden email].
Visit this group at MailScanner has detected definite fraud in the website at "groups.google.com". Do not trust this website: MailScanner has detected definite fraud in the website at "groups.google.com". Do not trust this website: https://groups.google.com/group/django-developers.
To view this discussion on the web visit MailScanner has detected definite fraud in the website at "groups.google.com". Do not trust this website: MailScanner has detected definite fraud in the website at "groups.google.com". Do not trust this website: https://groups.google.com/d/msgid/django-developers/cab981b8-7dc7-4e9d-9dcc-442b36820cdf%40googlegroups.com.
For more options, visit MailScanner has detected definite fraud in the website at "groups.google.com". Do not trust this website: MailScanner has detected definite fraud in the website at "groups.google.com". Do not trust this website: https://groups.google.com/d/optout.

--
Andrew Standley
Senior Software Engineer
Linear Systems
(909) 899-4345 *225
[hidden email]

--
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 [hidden email].
To post to this group, send email to [hidden email].
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/cfcefc08-e8a6-71d3-6269-31ccd9eaa52d%40linear-systems.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Default Authorization BackEnd Denying Permissions if Object Provided

Carlton Gibson-3
In reply to this post by Florian Apolloner
Hi Florian, 

I was hoping you'd post. You've been active on the entire (very-long) history of this issue. 

Can I ask for your take? 

Given your comment, would it be along the lines of "Close as "Won't Fix", perhaps with a review of the documentation", to point users to subclassing ModelBackend if they need the alternate behaviour?

Thanks!

Kind Regards,

Carlton


On Wednesday, 17 January 2018 19:45:06 UTC+1, Florian Apolloner wrote:
On Wednesday, January 17, 2018 at 5:48:03 PM UTC+1, Mehmet Dogan wrote:
Florian,

Can you clarify this part, I am not sure what you meant:


class MyBackend(ModelBackend):

   def has_perm(…, obj=None,…):
     if obj: obj = None
     return super().has_perm(…)

something along the lines of this should do it. 

--
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 [hidden email].
To post to this group, send email to [hidden email].
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/0f326294-a59d-41dc-bdd7-4d3ed85acf4f%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

RE: Default Authorization BackEnd Denying Permissions if ObjectProvided

Mehmet Dogan
In reply to this post by Carlton Gibson-3

Carlton,

 

First, thanks for stirring the conversation. Can you give an example of what you mean by option 3. The comment you linked did not have much detail. Thanks,

 

From: [hidden email]
Sent: Wednesday, January 17, 2018 4:45 AM
To: [hidden email]
Subject: Re: Default Authorization BackEnd Denying Permissions if ObjectProvided

 

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 a topic in the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/django-developers/MLWfvPPVwDk/unsubscribe.
To unsubscribe from this group and all its topics, send an email to [hidden email].
To post to this group, send email to [hidden email].
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/cab981b8-7dc7-4e9d-9dcc-442b36820cdf%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

 

--
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 [hidden email].
To post to this group, send email to [hidden email].
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/5a5fa805.0543ca0a.cde47.dde6%40mx.google.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

RE: Default Authorization BackEnd Denying Permissions if ObjectProvided

Mehmet Dogan
In reply to this post by Florian Apolloner

Although I found it very interesting at first, this looks dangerous since it changes how the API work for all apps installed. Example, guardian makes calls to user.has_perm(perm) in several places to check model permissions. Although periphery features, they would be broken.

 

From: [hidden email]
Sent: Wednesday, January 17, 2018 12:45 PM
To: [hidden email]
Subject: Re: Default Authorization BackEnd Denying Permissions if ObjectProvided

 

On Wednesday, January 17, 2018 at 5:48:03 PM UTC+1, Mehmet Dogan wrote:

Florian,

 

Can you clarify this part, I am not sure what you meant:

 

 

class MyBackend(ModelBackend):

 

   def has_perm(…, obj=None,…):

     if obj: obj = None

     return super().has_perm(…)

 

something along the lines of this should do it. 

--
You received this message because you are subscribed to a topic in the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/django-developers/MLWfvPPVwDk/unsubscribe.
To unsubscribe from this group and all its topics, send an email to [hidden email].
To post to this group, send email to [hidden email].
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/ff0142a5-cca0-4d8c-8497-b988b04ba4ca%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

 

--
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 [hidden email].
To post to this group, send email to [hidden email].
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/5a5fac41.1b5b9d0a.5a28c.069e%40mx.google.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Default Authorization BackEnd Denying Permissions if ObjectProvided

Carlton Gibson-3
In reply to this post by Mehmet Dogan
Hi. 

@Andrew: I'll look at your post anon, as it's longer. 

On Wednesday, 17 January 2018 20:46:27 UTC+1, Mehmet Dogan wrote:

Can you give an example of what you mean by option 3. 


Well, I don't a concrete suggestion in mind, but the general idea would be to have ModelBackend proxy to another class (a Strategy ?) that did the actual permission check. 

We'd then have (at least) two versions: one with the current implementation, and one with the alternative. 

Exactly how the user would configure which version to use (etc) would need thinking about. 

The question is whether it's worth the price of admission, vs just, say, subclassing ModelBackend. 

C.

--
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 [hidden email].
To post to this group, send email to [hidden email].
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/fc4b4544-dd15-4672-8a5b-27e9bcb7d4a9%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

RE: Default Authorization BackEnd Denying Permissions ifObjectProvided

Mehmet Dogan

Or, extend ModelBackend to override the object-denying behavior, and provided that as an option. Am I getting you right?

 

Seems like options for changing the global behavior is growing. However, I still think there is need for local override. Otherwise, re-usable apps will have to adopt either of the global defaults, and so will the app developers, since it will not be possible to mix the two exclusive groups.

 

 

From: [hidden email]
Sent: Wednesday, January 17, 2018 2:04 PM
To: [hidden email]
Subject: Re: Default Authorization BackEnd Denying Permissions ifObjectProvided

 

Hi. 

 

@Andrew: I'll look at your post anon, as it's longer. 

On Wednesday, 17 January 2018 20:46:27 UTC+1, Mehmet Dogan wrote:

Can you give an example of what you mean by option 3. 

 

Well, I don't a concrete suggestion in mind, but the general idea would be to have ModelBackend proxy to another class (a Strategy ?) that did the actual permission check. 

 

We'd then have (at least) two versions: one with the current implementation, and one with the alternative. 

 

Exactly how the user would configure which version to use (etc) would need thinking about. 

 

The question is whether it's worth the price of admission, vs just, say, subclassing ModelBackend. 

 

C.

--
You received this message because you are subscribed to a topic in the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/django-developers/MLWfvPPVwDk/unsubscribe.
To unsubscribe from this group and all its topics, send an email to [hidden email].
To post to this group, send email to [hidden email].
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/fc4b4544-dd15-4672-8a5b-27e9bcb7d4a9%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

 

--
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 [hidden email].
To post to this group, send email to [hidden email].
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/5a5fb78e.cc579d0a.c60a5.0459%40mx.google.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Default Authorization BackEnd Denying Permissions if Object Provided

Florian Apolloner
In reply to this post by Carlton Gibson-3
On Wednesday, January 17, 2018 at 7:58:17 PM UTC+1, Carlton Gibson wrote:
Given your comment, would it be along the lines of "Close as "Won't Fix", perhaps with a review of the documentation", to point users to subclassing ModelBackend if they need the alternate behaviour?

My comment https://github.com/django/django/pull/9581#pullrequestreview-88665130 sums up my thoughts. We should decide what the expected behavior for auth backends should be and then work towards that (if that means it should return True instead of False for obj!=None so be it). I am not happy with any of the proposed solutions so far (be that new arguments or settings).

Btw, it would be nice if you could be on IRC if your time permits it.

Cheers,
Florian 

--
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 [hidden email].
To post to this group, send email to [hidden email].
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/c981a74f-4dd1-418f-b1f1-a2d6d383466c%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
12