Object ownership validation in Django UpdateView

Question

EDIT:

The better solution for me was just using a permissions system, especially since I needed other types of controlled access to objects. I now use Django-guardian to help with object level permissions like this.

Original:

I'm expanding a bit on the standard django book guide by letting users upload stories, as well as having author, publisher, etc. I'm attempting to only let authors (creators) of a story use the updateview, with other users being redirected away.

Modifying get_object in the UpdateStory view set it off, but the traceback goes through my StoryForm init for some reason. The error is 'HttpResponseRedirect' object has no attribute '_meta'

views.py

class UpdateStory(LoginRequiredMixin, UpdateView):
    model = Story
    template_name = 'stories/story_update.html'
    form_class = StoryForm

    def get_object(self, queryset=None):
        obj = super(UpdateStory, self).get_object()
        if not obj.author == self.request.user:
            return redirect(obj)
        return obj

forms.py

class StoryForm(forms.ModelForm):
    def __init__(self, *args, **kwargs):
        super(StoryForm,self).__init__(*args, **kwargs)

I'm still new, so it might be obvious, but I've been looking for a couple hours and I'm stumped.

Nathalienathan

I do not understand what it is that you want. Would you mind explaining what exactly your problem is, to me, it seems that you are getting an error for using class based views, and seemingly this is because you have not put in a meta attribute inside. – Styrax

See ragsagar's answer on why the error came up. I didn't define any aspects of UpdateView's meta, and didn't need to. I just wanted to add in a method of verifying only users that created an object (its "author") could edit that object. – Nathalienathan

Answer 1

http://ccbv.co.uk/projects/Django/1.5/django.views.generic.edit/UpdateView/

Go through the above link to understand how UpdateView works. get_object is supposed to return the model instance, It is not supposed to return HttpResponseRedirect object, that's why you are getting that error.

Try doing the check in dispatch method like the following.

def dispatch(self, request, *args, **kwargs):
    """ Making sure that only authors can update stories """
    obj = self.get_object()
    if obj.author != self.request.user:
        return redirect(obj)
    return super(UpdateStory, self).dispatch(request, *args, **kwargs)

PS: I guess it is not recommended to override dispatch. But as you have to do the check on both get and post methods, overriding dispatch will be easier.

Gangrene

Dispatch does seem like a good way to add in the functionality. Can you elaborate on why it's not recommended? Google-ing turns up plenty of overrides for it, and I can't find any specific recommendation on why not to. – Nathalienathan

Marking this as correct since it also explains the error as well as giving code (thanks!), but I think Berislav Lopac is right on this being most effective as a mixin, since several views will use it. – Nathalienathan

I believe overriding dispatch and calling self.get_object() will result in a redundant call of self.get_object(). Overriding get and post separately avoids this redundant call. – Joachim

Answer 2

The best approach would be to use another mixin, something like this:

class AuthorRequiredMixin(object):
    def dispatch(self, request, *args, **kwargs):
        if self.object.author != self.request.user:
            return HttpResponseForbidden()
        return super(AuthorRequiredMixin, self).dispatch(request, *args, **kwargs)

Of course you can return another HttpResponse, but keep in mind what is the proper use here.

Regelation

How do you access the self.object from within AuthorRequiredMixin? – Autoionization

You don't -- the point of mixins is that they are not instantiated directly. Rather, you need to create a custom view by inheriting from both a base view class and the mixin. – Regelation

Ok, thanks! But in that case, I don't understand why overriding dispatch directly in the View is not recommended – Autoionization

A mixin can be applied to all the views that require a same functionality. You can of course override dispatch on a single class, but usually there are better places for that (such as get_queryset and similar methods). Dispatch is a lower level method and should be modified when a behaviour is needed on a wider scale. – Regelation

Uh, you are absolutely right, @knbk, I followed the standard pattern without thinking. Will update the answer. – Regelation

Answer 3

This specific issue is considered in Django anti-patterns.

We're encouraged to filter the QuerySet to only retrieve objects where the user is the author, as opposed to UserPassesTestMixin.

In OP's case it would actually be quite similar to what they have there

from django.contrib.auth.mixins import LoginRequiredMixin

class UpdateStory(LoginRequiredMixin, UpdateView):
    model = Story
    # …

    def get_queryset(self, *args, **kwargs):
        return super().get_queryset(*args, **kwargs).filter(
            author=self.request.user
        )

Grizel