From 52ac21ca3ef6fb660c477d2f7ed97c3f36f34840 Mon Sep 17 00:00:00 2001 From: Camille Harang Date: Thu, 2 Feb 2012 02:54:25 +0100 Subject: [PATCH 1/9] Moved ModelMixin's model instance retreival from .get() .put() .delete() to a .model_instance lazy property --- djangorestframework/mixins.py | 49 +++++++++++++++++++++-------------- 1 file changed, 30 insertions(+), 19 deletions(-) diff --git a/djangorestframework/mixins.py b/djangorestframework/mixins.py index f4a9c998a..e4943affd 100644 --- a/djangorestframework/mixins.py +++ b/djangorestframework/mixins.py @@ -527,6 +527,28 @@ class ModelMixin(object): """ return self.get_queryset().get(**kwargs) + @property + def model_instance(self): + """ + Returns the model instance for read/update/delete, or None if does not exit. + """ + + if hasattr(self, '_model_instance'): return self._model_instance + + model = self.resource.model + request = self.request + args = self.args + kwargs = self.kwargs + + query_kwargs = self.get_query_kwargs(request, *args, **kwargs) + + try: + self._model_instance = self.get_instance(**kwargs) + except model.DoesNotExist: + pass + + return self._model_instance + def get_queryset(self): """ Return the queryset for this view. @@ -546,12 +568,8 @@ class ReadModelMixin(ModelMixin): Behavior to read a `model` instance on GET requests """ def get(self, request, *args, **kwargs): - model = self.resource.model - query_kwargs = self.get_query_kwargs(request, *args, **kwargs) - try: - self.model_instance = self.get_instance(**query_kwargs) - except model.DoesNotExist: + if self.model_instance == None: raise ErrorResponse(status.HTTP_404_NOT_FOUND) return self.model_instance @@ -602,18 +620,17 @@ class UpdateModelMixin(ModelMixin): Behavior to update a `model` instance on PUT requests """ def put(self, request, *args, **kwargs): + model = self.resource.model - query_kwargs = self.get_query_kwargs(request, *args, **kwargs) # TODO: update on the url of a non-existing resource url doesn't work # correctly at the moment - will end up with a new url - try: - self.model_instance = self.get_instance(**query_kwargs) - + if self.model_instance == None: + self.model_instance = model(**self.get_instance_data(model, self.CONTENT, *args, **kwargs)) + else: for (key, val) in self.CONTENT.items(): setattr(self.model_instance, key, val) - except model.DoesNotExist: - self.model_instance = model(**self.get_instance_data(model, self.CONTENT, *args, **kwargs)) + self.model_instance.save() return self.model_instance @@ -623,15 +640,9 @@ class DeleteModelMixin(ModelMixin): Behavior to delete a `model` instance on DELETE requests """ def delete(self, request, *args, **kwargs): - model = self.resource.model - query_kwargs = self.get_query_kwargs(request, *args, **kwargs) - try: - instance = self.get_instance(**query_kwargs) - except model.DoesNotExist: - raise ErrorResponse(status.HTTP_404_NOT_FOUND, None, {}) - - instance.delete() + if self.model_instance == None: raise ErrorResponse(status.HTTP_404_NOT_FOUND, None, {}) + self.model_instance.delete() return From d44a6c5a69b786dc0f920894df2bc9ff3272cdaf Mon Sep 17 00:00:00 2001 From: Camille Harang Date: Thu, 2 Feb 2012 03:06:40 +0100 Subject: [PATCH 2/9] Adding permission IsModelInstanceOwnerOrIsAnonReadOnly --- djangorestframework/mixins.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/djangorestframework/mixins.py b/djangorestframework/mixins.py index e4943affd..6f646b685 100644 --- a/djangorestframework/mixins.py +++ b/djangorestframework/mixins.py @@ -530,7 +530,7 @@ class ModelMixin(object): @property def model_instance(self): """ - Returns the model instance for read/update/delete, or None if does not exit. + Returns the model instance for read/update/delete, or None if does not exist. """ if hasattr(self, '_model_instance'): return self._model_instance From 6bd2205833e463073e271c0d408dfa69b23c1a97 Mon Sep 17 00:00:00 2001 From: Camille Harang Date: Thu, 2 Feb 2012 03:06:55 +0100 Subject: [PATCH 3/9] Adding permission IsModelInstanceOwnerOrIsAnonReadOnly --- djangorestframework/permissions.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/djangorestframework/permissions.py b/djangorestframework/permissions.py index dfe55ce94..0de1d16d8 100644 --- a/djangorestframework/permissions.py +++ b/djangorestframework/permissions.py @@ -77,6 +77,27 @@ class IsAdminUser(BasePermission): raise _403_FORBIDDEN_RESPONSE +class IsModelInstanceOwnerOrIsAnonReadOnly(BasePermission): + """ + The request is authenticated as the owner of the model instance, or is a read-only request. + """ + + def check_permission(self, user): + + if self.view.method in('GET', 'HEAD',): + return + + if not user.is_authenticated(): + raise _403_FORBIDDEN_RESPONSE + + try: + if self.view.model_instance.get_owner() == user: + return + except: pass + + raise _403_FORBIDDEN_RESPONSE + + class IsUserOrIsAnonReadOnly(BasePermission): """ The request is authenticated as a user, or is a read-only request. From ada676123e9d22e6962885aaaf2c992119701edf Mon Sep 17 00:00:00 2001 From: Camille Harang Date: Thu, 2 Feb 2012 03:25:33 +0100 Subject: [PATCH 4/9] Doc for the permission IsModelInstanceOwnerOrIsAnonReadOnly --- djangorestframework/permissions.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/djangorestframework/permissions.py b/djangorestframework/permissions.py index 0de1d16d8..cd28438d1 100644 --- a/djangorestframework/permissions.py +++ b/djangorestframework/permissions.py @@ -80,6 +80,9 @@ class IsAdminUser(BasePermission): class IsModelInstanceOwnerOrIsAnonReadOnly(BasePermission): """ The request is authenticated as the owner of the model instance, or is a read-only request. + + In order to determine the owner the model has to provide a .get_owner() function, + otherwise the permission will be denied. """ def check_permission(self, user): From fcd026f50627d7a542a385ca7404aecca8cb89c5 Mon Sep 17 00:00:00 2001 From: Camille Harang Date: Thu, 2 Feb 2012 03:27:20 +0100 Subject: [PATCH 5/9] Doc review for the permission IsModelInstanceOwnerOrIsAnonReadOnly --- djangorestframework/permissions.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/djangorestframework/permissions.py b/djangorestframework/permissions.py index cd28438d1..aad013b25 100644 --- a/djangorestframework/permissions.py +++ b/djangorestframework/permissions.py @@ -81,8 +81,8 @@ class IsModelInstanceOwnerOrIsAnonReadOnly(BasePermission): """ The request is authenticated as the owner of the model instance, or is a read-only request. - In order to determine the owner the model has to provide a .get_owner() function, - otherwise the permission will be denied. + In order to determine the owner, the model has to provide a .get_owner() function that + returns the owner, otherwise the permission will be denied. """ def check_permission(self, user): From c42a1112996e45caec4ab6d0f74d0bb77907268e Mon Sep 17 00:00:00 2001 From: Camille Harang Date: Thu, 2 Feb 2012 16:12:07 +0100 Subject: [PATCH 6/9] IsModelInstanceOwnerOrIsAnonReadOnly in __all__ --- djangorestframework/permissions.py | 1 + djangorestframework/serializer.py | 1 + 2 files changed, 2 insertions(+) diff --git a/djangorestframework/permissions.py b/djangorestframework/permissions.py index aad013b25..b350d29bf 100644 --- a/djangorestframework/permissions.py +++ b/djangorestframework/permissions.py @@ -13,6 +13,7 @@ __all__ = ( 'BasePermission', 'FullAnonAccess', 'IsAuthenticated', + 'IsModelInstanceOwnerOrIsAnonReadOnly', 'IsAdminUser', 'IsUserOrIsAnonReadOnly', 'PerUserThrottling', diff --git a/djangorestframework/serializer.py b/djangorestframework/serializer.py index 71c0d93ae..e45e583b6 100644 --- a/djangorestframework/serializer.py +++ b/djangorestframework/serializer.py @@ -211,6 +211,7 @@ class Serializer(object): raise _SkipField def serialize_model(self, instance): + print 'sdfff',self, instance """ Given a model instance or dict, serialize it to a dict.. """ From bab7b6b32bda69413b97e33b373d2010f8963b49 Mon Sep 17 00:00:00 2001 From: Camille Harang Date: Thu, 2 Feb 2012 16:35:43 +0100 Subject: [PATCH 7/9] ModelMixin's .model_instance uses .get_query_kwargs() --- djangorestframework/mixins.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/djangorestframework/mixins.py b/djangorestframework/mixins.py index 6f646b685..106b56ef0 100644 --- a/djangorestframework/mixins.py +++ b/djangorestframework/mixins.py @@ -543,7 +543,7 @@ class ModelMixin(object): query_kwargs = self.get_query_kwargs(request, *args, **kwargs) try: - self._model_instance = self.get_instance(**kwargs) + self._model_instance = self.get_instance(**query_kwargs) except model.DoesNotExist: pass From 92da63775219685d33ab9f8188be445909fb9d1f Mon Sep 17 00:00:00 2001 From: Camille Harang Date: Thu, 2 Feb 2012 16:39:24 +0100 Subject: [PATCH 8/9] remove debug print --- djangorestframework/serializer.py | 1 - 1 file changed, 1 deletion(-) diff --git a/djangorestframework/serializer.py b/djangorestframework/serializer.py index e45e583b6..71c0d93ae 100644 --- a/djangorestframework/serializer.py +++ b/djangorestframework/serializer.py @@ -211,7 +211,6 @@ class Serializer(object): raise _SkipField def serialize_model(self, instance): - print 'sdfff',self, instance """ Given a model instance or dict, serialize it to a dict.. """ From f0cc46861b1da8c5928f63da2935423803b631bc Mon Sep 17 00:00:00 2001 From: Camille Harang Date: Fri, 3 Feb 2012 03:12:20 +0100 Subject: [PATCH 9/9] Moved ownership at the BaseResource level, as any resource can provide a Django User, not only a model instance --- djangorestframework/mixins.py | 10 ++++++++++ djangorestframework/permissions.py | 15 +++++---------- djangorestframework/resources.py | 6 ++++++ 3 files changed, 21 insertions(+), 10 deletions(-) diff --git a/djangorestframework/mixins.py b/djangorestframework/mixins.py index 106b56ef0..d5eeac818 100644 --- a/djangorestframework/mixins.py +++ b/djangorestframework/mixins.py @@ -527,6 +527,16 @@ class ModelMixin(object): """ return self.get_queryset().get(**kwargs) + def get_owner(self): + """ + Returns the model instance's owner, if any. + + The owner is retrieved by calling the .get_owner() function on the model instance, if implemented. + """ + try: + return self.model_instance.get_owner() + except: pass + @property def model_instance(self): """ diff --git a/djangorestframework/permissions.py b/djangorestframework/permissions.py index b350d29bf..8d02c4e5a 100644 --- a/djangorestframework/permissions.py +++ b/djangorestframework/permissions.py @@ -13,7 +13,7 @@ __all__ = ( 'BasePermission', 'FullAnonAccess', 'IsAuthenticated', - 'IsModelInstanceOwnerOrIsAnonReadOnly', + 'IsResourceOwnerOrIsAnonReadOnly', 'IsAdminUser', 'IsUserOrIsAnonReadOnly', 'PerUserThrottling', @@ -78,12 +78,9 @@ class IsAdminUser(BasePermission): raise _403_FORBIDDEN_RESPONSE -class IsModelInstanceOwnerOrIsAnonReadOnly(BasePermission): +class IsResourceOwnerOrIsAnonReadOnly(BasePermission): """ - The request is authenticated as the owner of the model instance, or is a read-only request. - - In order to determine the owner, the model has to provide a .get_owner() function that - returns the owner, otherwise the permission will be denied. + The request is authenticated as the owner of the resource, or is a read-only request. """ def check_permission(self, user): @@ -94,10 +91,8 @@ class IsModelInstanceOwnerOrIsAnonReadOnly(BasePermission): if not user.is_authenticated(): raise _403_FORBIDDEN_RESPONSE - try: - if self.view.model_instance.get_owner() == user: - return - except: pass + if self.view.get_owner() == user: + return raise _403_FORBIDDEN_RESPONSE diff --git a/djangorestframework/resources.py b/djangorestframework/resources.py index cc338cc05..7d0d1d5ff 100644 --- a/djangorestframework/resources.py +++ b/djangorestframework/resources.py @@ -32,6 +32,12 @@ class BaseResource(Serializer): """ return self.serialize(obj) + def get_owner(self): + """ + Returns a Django User instance as the owner of the resource, if any. + """ + return None + class Resource(BaseResource): """