dbkaplan / dry-rest-permissions

Rules based permissions for the Django Rest Framework
ISC License
374 stars 59 forks source link

Decorators don't work with class methods #21

Open clintonb opened 8 years ago

clintonb commented 8 years ago

The documentation states that global permissions can be implemented as either @staticmethod or @classmethod on the model. The decorators (e.g. @unauthenticated_users) assume the first argument of the global permissions methods is a Request. This is true if the methods are static methods. However, when implemented as class methods, the first argument is the class itself.

# Good
@staticmethod
@authenticated_users
def has_read_permission(request):
    return True

# Bad
@classmethod
@authenticated_users  # This will fail because it expects the first argument to be a Request.
def has_read_permission(cls, request):
    return True

Either the decorators should be updated, or the documentation should be modified to note this issue and/or remove the mention of @classmethod.

dbkaplan commented 8 years ago

You're right, the decorators don't work with classmethods. However, you can still use classmethods for global permissions, you just can't also use the decorators on top of that. That being said, it would be nice to be able to do both. I will look into a solution.

timstaley commented 8 years ago

Hi all, not familiar with dry-rest-permissions yet but just browsing and saw this issue. Would this library help? It provides a workaround to the inherent Python 2 classmethod decorator issue: https://wrapt.readthedocs.io/en/latest/ as discussed here: https://hynek.me/articles/decorators/

HTH.

jjlorenzo commented 8 years ago

Sorry if I don't understand deeply the problem, it is not possible to fix this problem?

Looking at the decorators I see that the is_object_permission value is used to determine if the request it is on args[1] instead of args[0]

There any problem if it gets generalized to ?

from rest_framework.request import Request
...
request = args[0] if isinstance(args[0], Request) else args[1]

Probably we can even use duck-typing to identify the Request instead of isinstance