DjangoObjectPermissions and has_object_permission

[Juergen Schackmann]

Hi all,

I have been trying to use DjangoObjectPermissions. However, now normal never access to any method. While I was investigating the issue I had a look at the source code and the following questions popped up:

DjangoObjectPermissions have a has_object_permission method. This method would be called in APIView from the check_object_permissions method. However that method is never called from anywhere. Consequently, DjangoObjectPermissions does not seem to have any effect.

To make it worse, instead in APIView, check_permissions calls the has_permissiosn of all permission classes, even the DjangoObjectPermissions class, which fails.

Do I miss something completely, or is there a bug

Regards,

schacki

[Tom Christie]

Howdy,

> However that method is never called from anywhere.

From here: http://www.django-rest-framework.org/api-guide/permissions#object-level-permissions

"Object level permissions are run by REST framework's generic views when .get_object() is called."

and

"If you're writing your own views and want to enforce object level permissions, or if you override the get_object method on a generic view, then you'll need to explicitly call the .check_object_permissions(request, obj) method on the view at the point at which you've retrieved the object."

Hope that helps!

  Tom :)

[Juergen Schackmann]

Hi Tom,

thanks a lot for the hints - I somehow only stared at the section  "DjangoObjectPermissions".

Now I ran into the next problem. I have been using DjangoObjectPermissions which inherits from DjangoModelPermissions. DjangoModelPermissions has a method has_permission which checks for Model level permissions. However, if you are working with object level permissions, usually the model level permissions evaluate to false. Consequently DjangoObjectPermissions raises a permission denied. 

I have easily fixed that issue be overwriting the has_permission method of DjangoObjectPermissions, but again I am wondering, have I missed anything or should that not be the default behavior of DjangoObjectPermissions.

Regards,

schacki

[Juergen Schackmann]

any feedback would be appreciated :-)

[Tom Christie]

There will be some tests for DjangoObjectPermissions. If you want to do some digging on what's going wrong in your application it might be worth comparing REST framework's integration tests for it against your own code. That would at least give you a starting point in figuring out if you believe this to be an application-side issue, or a bug. I believe there's plenty of people using it without issue, so perhaps there's something underdocumented or non-obvious that's causing the confusion.
Hope that helps some.

[Juergen Schackmann]

ok, wil do so

[Juergen Schackmann]

I had a look at test_permissions.py and I am wondering about this comment at line 203:

# give everyone model level permissions, as we are not testing those

This is exactly the part that confuses me. Or frankly, I think something is going wrong here.

Let me come up with an example. Maybe that will make it easier.

- I do have a model BlogPost.

- For this model BlogPost I do have a permission called change_blogpost. To make things easier (and I think this is best practice as well), the permission change_blogpost can be used as model permission and as object permission.

- I do have a user blog_admin that has the change_permission for the model BlogPost; this user does not have change_permission for any specific objects.

- I do have a user author that has the change_permission for the BlogPost object a and b; this user does not have change_permission for the model BlogPost

For the permission checks, I do expect the following results:

- User blog_admin should have the change_permission on all BlogPost objects

- User author should only have the change_permission for objects a and b

SO THE FIRST CRITICAL QUESTION IS, DO WE AGREE ON THIS!

If we do agree, now my problem is, that with the current implementation of DjangoObjectPermissions, the check of user author for object a will fail, because the has_permission method of DjangoObjectPermission is always called and user author does not have the change_permission for the model BlogPost.

And by doing this: "# give everyone model level permissions, as we are not testing those", you do not catch the problem, because your are giving the user model level permissions.

[Tom Christie]

Hi Juergen,

I'll start by noting that I've never used django-guardian or the object level permissions myself, so I'm not necessarily the best person to move this along, but I'll do what I can.

Yes, certainly from my reading of this right now, your argument looks like it *might* be correct to me.  It seems like the right behaviour for DjangoObjectPermissions would be:

* Always return True for `has_permission`- we're not in a position to deny the user access until we've checked if they have a per-object permission.

* `has_object_permission` should return `True` if the user has *either* of the model level permission or the object level permission.

I'd be interested to head from anyone else currently using this permission class and if they're having any issues or not.

The interaction between Django's object-level and model-level permission has always seemed a little woolly to me.

For example if a user does have model level permissions, should an implementation always also return True when querying a given object permission in that class.  (Or should it only do so if they *additionally* also have *explicitly assigned* object level permissions to it?)

You next best steps might be to:

* Look into getting together a failing test and fix, and submit a pull request for that.

* See if anyone else raises a voice on this thread or against the issue raised, and follow that up if so.

All the best,

  Tom

[Tim Watts]

I'm on my phone so sorry for being brief. We currently use django-guardian and DRF.

has_permission is if the user can do something in the general sense. Perhaps an abusive user can no longer comment. Or a manager can edit users.

has_object_permission is if in addition, the user can do something on that specific object.

In practice we use django guadian as a last resort and check for properties on an object or its parent and return True early if possible.

I can go into more details if yall want :)

--
You received this message because you are subscribed to the Google Groups "Django REST framework" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-rest-fram...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Juergen Schackmann]

the critical question is, can a user commont on a specific object without having general permission on the model?

I personally would say: absolutely yes. What do you guys think

To unsubscribe from this group and stop receiving emails from it, send an email to django-rest-framework+unsub...@googlegroups.com.

[Tim Watts]

That is where it become interesting and the answer depends on your needs.

Let's say a Comment can be on Post and some Posts are private.

In order to comment "has_permission" should return True for both of the following:

>> request.user.has_perm('comments.add_comment') # The minimum requirement

True

>> current_post.is_public or request.user.has_perm('posts.view_post', current_post)

True

And "has_object_permission" never gets called because this is going to be a new comment, nothing exists yet.

So the user can add_comments in general (is authenticated and not banned) and if the post is not public, then the user must have view permission on this specific post. Anyways, all this is up to you and what your project needs. Try it out, and post some specific code and we can work from there.

I'd be interested in how other solve this, currently we have some gnarly conditionals based on the model/object of the current view our permissions.py

To unsubscribe from this group and stop receiving emails from it, send an email to django-rest-fram...@googlegroups.com.

[Ihor Kaharlichenko]

the critical question is, can a user commont on a specific object without having general permission on the model?
I personally would say: absolutely yes. What do you guys think

I second this.

The Django's permission model is designed to specify what to permit a user to do, instead of specifying what to forbid him to do. Here are the proofs: https://github.com/django/django/blob/1.6.5/django/contrib/auth/models.py#L322, https://github.com/django/django/blob/1.6.5/django/contrib/auth/models.py#L270. The bottom line is: "it is sufficient for any of the backends to allow you to do something". So you don't have to have a model level 'comment_post' permission in order to comment a particular BlogPost that you have an object-level 'comment_post' permission.

So, now the question is: should we consider the current behavior as a bug and act appropriately? For me, the code taken from DjangoObjectPermissionsFilter documentation doesn't work out of the box exactly because of this problem.

[Ben Reilly]

Are DRF Permission backends expected to conform to inclusive behavior of the the contrib auth backends? Because that could be unexpected when you consider other viewset components - like the Filter backends and, I believe, Throttles -  are more logically conjunctive and exclusive (an item needs to make it through every filter to be part of the response).

If it is changed to something more inclusive, like model permission implies object permission, we should be very careful as existing consumers could have private data exposed.

It might be worth considering getting DjangoObjectPermissions out of the model permission business entirely (inherit from BasePermission rather than DjangoModelPermissions). Consumers that want both can include both in permission_classes.

[Nabeel]

Has there been any progress on this? I'm also facing the same issue...