Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tutorial - turning on PAGINATE_BY breaks custom permissions #2205

Closed
detectedstealth opened this issue Dec 4, 2014 · 15 comments
Closed

Tutorial - turning on PAGINATE_BY breaks custom permissions #2205

detectedstealth opened this issue Dec 4, 2014 · 15 comments
Labels
Milestone

Comments

@detectedstealth
Copy link

When turning on PAGINATE_BY in settings Adding Pagination

REST_FRAMEWORK = {
    'PAGINATE_BY': 10
}

The custom permissions Object Level Permissions

from rest_framework import permissions

class IsOwnerOrReadOnly(permissions.BasePermission):
    """
    Custom permission to only allow owners of an object to edit it.
    """
    def has_object_permission(self, request, view, obj):
        # Read permissions are allowed to any request,
        # so we'll always allow GET, HEAD, or OPTIONS requests.
        if request.method in permissions.SAFE_METHODS:
            return True
        print(obj)
        # Write permissions are only allowed to the owner of the snippet.
        return obj.owner == request.user

break when trying to view the list of Snippets with the following error:

AttributeError at /snippets/
'Page' object has no attribute 'owner'
Request Method: GET
Request URL:    http://127.0.0.1:8000/snippets/
Django Version: 1.7.1
Exception Type: AttributeError
Exception Value:    
'Page' object has no attribute 'owner'
Exception Location: /Development/Python/django/snippets_tutorial/snippets/permissions.py in has_object_permission, line 14

When disabling the PAGINATE_BY setting you are able to view the Snippets list without any error.

@tomchristie
Copy link
Member

Can you double check that this is the 3.0 installed from PyPI, and not the 3.0-beta installed from GitHub?

Same as #2073, which was closed by 084354d

@detectedstealth
Copy link
Author

This is version 3.0.0 installed with pip

Bruces-MacBook-Pro:snippets_tutorial brucewade$ workon snippets_tutorial
(snippets_tutorial)Bruces-MacBook-Pro:snippets_tutorial brucewade$ ls
db.sqlite3  manage.py   snippets    tutorial
(snippets_tutorial)Bruces-MacBook-Pro:snippets_tutorial brucewade$ pip freeze > requirements.txt
(snippets_tutorial)Bruces-MacBook-Pro:snippets_tutorial brucewade$ cat requirements.txt 
Django==1.7.1
Pygments==2.0.1
djangorestframework==3.0.0
(snippets_tutorial)Bruces-MacBook-Pro:snippets_tutorial brucewade$

@tomchristie
Copy link
Member

Any chance you could include the full traceback?

@detectedstealth
Copy link
Author

Also note when not authenticated and viewing the list of snippets there is no errors. Once logging in and trying to view the list is when there is an error.

Environment:
Request Method: GET
Request URL: http://127.0.0.1:8000/snippets/

Django Version: 1.7.1
Python Version: 3.4.2
Installed Applications:
('django.contrib.admin',
 'django.contrib.auth',
 'django.contrib.contenttypes',
 'django.contrib.sessions',
 'django.contrib.messages',
 'django.contrib.staticfiles',
 'rest_framework',
 'snippets')
Installed Middleware:
('django.contrib.sessions.middleware.SessionMiddleware',
 'django.middleware.common.CommonMiddleware',
 'django.middleware.csrf.CsrfViewMiddleware',
 'django.contrib.auth.middleware.AuthenticationMiddleware',
 'django.contrib.auth.middleware.SessionAuthenticationMiddleware',
 'django.contrib.messages.middleware.MessageMiddleware',
 'django.middleware.clickjacking.XFrameOptionsMiddleware')


Traceback:
File "/Users/brucewade/.virtualenvs/snippets_tutorial/lib/python3.4/site-packages/django/core/handlers/base.py" in get_response
  137.                 response = response.render()
File "/Users/brucewade/.virtualenvs/snippets_tutorial/lib/python3.4/site-packages/django/template/response.py" in render
  103.             self.content = self.rendered_content
File "/Users/brucewade/.virtualenvs/snippets_tutorial/lib/python3.4/site-packages/rest_framework/response.py" in rendered_content
  59.         ret = renderer.render(self.data, media_type, context)
File "/Users/brucewade/.virtualenvs/snippets_tutorial/lib/python3.4/site-packages/rest_framework/renderers.py" in render
  715.         context = self.get_context(data, accepted_media_type, renderer_context)
File "/Users/brucewade/.virtualenvs/snippets_tutorial/lib/python3.4/site-packages/rest_framework/renderers.py" in get_context
  665.         raw_data_post_form = self.get_raw_data_form(data, view, 'POST', request)
File "/Users/brucewade/.virtualenvs/snippets_tutorial/lib/python3.4/site-packages/rest_framework/renderers.py" in get_raw_data_form
  595.             if not self.show_form_for_method(view, method, request, instance):
File "/Users/brucewade/.virtualenvs/snippets_tutorial/lib/python3.4/site-packages/rest_framework/renderers.py" in show_form_for_method
  509.                 view.check_object_permissions(request, obj)
File "/Users/brucewade/.virtualenvs/snippets_tutorial/lib/python3.4/site-packages/rest_framework/views.py" in check_object_permissions
  290.             if not permission.has_object_permission(request, self, obj):
File "/Users/brucewade/Development/Python/django/snippets_tutorial/snippets/permissions.py" in has_object_permission
  14.         return obj.owner == request.user

Exception Type: AttributeError at /snippets/
Exception Value: 'Page' object has no attribute 'owner'

@xordoquy
Copy link
Collaborator

xordoquy commented Dec 6, 2014

@detectedstealth I just ran the tutorial and did not had this issue.
This might be linked with how you fixed:

owner = serializers.Field(source='owner.username')

which should have caused an exception: "to_representation() must be implemented."

@detectedstealth
Copy link
Author

@xordoquy You would be correct with that error if you were supposed to use:

owner = serializers.Field(source='owner.username')

However the tutorials Updating our Serializer uses this:

owner = serializers.ReadOnlyField(source='owner.username')

Notice the ReadOnlyField was supposed to be used not a plain Field for the owner.

@tomchristie
Copy link
Member

Sorry I'm a bit lost how these two things are related. If .Field is still being used directly then yes that needs to be fixed in the tutorial. How does that affect this PAGINATE_BY issue that the reporting is seeing?

@xordoquy
Copy link
Collaborator

xordoquy commented Dec 7, 2014

@tomchristie my bad, I copy/pasted the wrong version.

@xordoquy
Copy link
Collaborator

xordoquy commented Dec 7, 2014

@detectedstealth forget my comment anyway, the tutorial has issues with the current master, not the latest release.

@detectedstealth
Copy link
Author

I think this is an even bigger issue. When turning off PAGINATE_BY as stated there is no error, which is because

def has_object_permission(self, request, view, obj):

does not get called when PAGINATE_BY is turned off. Then when PAGINATE_BY is turned on the obj param is passed a Page object which causes the error submitted in this reporting.

@xordoquy
Copy link
Collaborator

xordoquy commented Dec 7, 2014

@detectedstealth Please note that the tutorial says to add IsOwnerOrReadOnly to the SnippetDetail while you seem to have it applied to SnippetList. Thing that tip me is in your trace, it shows "Request URL: http://127.0.0.1:8000/snippets/" while IsOwnerOrReadOnly is called.

@detectedstealth
Copy link
Author

@xordoquy I think you need to read to the end of the tutorial Refactoring to Use Viewsets where you combined both List and Detail views.

from rest_framework.decorators import detail_route

class SnippetViewSet(viewsets.ModelViewSet):
    """
    This viewset automatically provides `list`, `create`, `retrieve`,
    `update` and `destroy` actions.

    Additionally we also provide an extra `highlight` action.
    """
    queryset = Snippet.objects.all()
    serializer_class = SnippetSerializer
    permission_classes = (permissions.IsAuthenticatedOrReadOnly,
                          IsOwnerOrReadOnly,)

    @detail_route(renderer_classes=[renderers.StaticHTMLRenderer])
    def highlight(self, request, *args, **kwargs):
        snippet = self.get_object()
        return Response(snippet.highlighted)

    def pre_save(self, obj):
        obj.owner = self.request.user

@xordoquy
Copy link
Collaborator

xordoquy commented Dec 8, 2014

Right, this is reproduced here. Using the pagination makes DRF use the pagination serializer which result is sent to the permission object. This leads to inconsistent data sent to the permission class because it'll be either a Page instance in list mode either a Snippet in detail mode.
I was tempted to update the tutorial code but I'm not sure whether we should fixed the tutorial or in DRF itself. @tomchristie, thoughts ?

@tomchristie
Copy link
Member

in DRF itself.

This.

@tomchristie
Copy link
Member

Well pretty ugly, plus no tests at the moment, but this will do for right now. #2089 remains open to improve on it.

carltongibson pushed a commit to carltongibson/rest-framework-tutorial that referenced this issue Dec 15, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants