API for simpler (permission or any) checks for generic view classes

[Jari Pennanen]

The generic class based views are difficult and cumbersome to decorate, why even trying to reuse the decorator functions? Why not implement simple functions that would do the checking.

I propose following API for checking the class based views (in this case just permissions):

from django.contrib.auth.viewchecks import login_required, has_perms

class ProtectedView(TemplateView):
    template_name = 'secret.html'
    dispatch_checks = (login_required, has_perms('auth.change_user'),)

(Notice that I used hypothetical django.contrib.auth.viewchecks)

dispatch_checks is list of functions (request, *args, **kwargs) -> bool if allowed to dispatch. Thus the has_perms('auth.change_user') should return function.

Above would also allow overriding the dispatch_checks tuple in subclassed views which I think is very important.

I'm working on a simple patch for django.contrib.auth and to View class to allow this, these checker functions should be reusable in decorators.

[Jari Pennanen]

This could be implemented as Mixin too, could be more pythonic, kind of like:

class ProtectedView(TemplateView, ForbiddenMixin):
    template_name = 'secret.html'
    forbidden_checks = (login_required, has_perms('auth.change_user'),

I have not tested this, but it this ForbiddenMixin should be part of Django it is so general and useful.

[Jari Pennanen]

Opened up also a thread to ​django-devs for this if anyone is interested.

[Russell Keith-Magee]

I'm not convinced that this is exactly the right approach, but after using CBVs in action for a bit, I will grant the premise that it's unnecessarily difficult to add a login_required decorator to a CBV on the class itself.

#14512 was an original swing at this with a specific solution aimed at making decorators work for class based views -- it turns out to be inherently complex to do so. The discussions related to that thread provide more detail (​thread 1 ​thread 2).

[Aymeric Augustin]

Change UI/UX from NULL to False.

[Aymeric Augustin]

Change Easy pickings from NULL to False.

[Selwin Ong]

This is (roughly) how I implemented permission checks in one of my projects:

def dispatch(self, request, *args, **kwargs):
    if hasattr(self, 'check_permissions'):
        self.check_permissions(request, *args, **kwargs)
    elif self.require_permissions:
        if not request.user.has_perms(self.require_permissions):
            raise PermissionDenied
    return super(TableView, self).dispatch(request, *args, **kwargs)

You can use the API in two different ways:

class MyView(View):
    require_permissions = ['app.view_model', 'app2.view_model2']

or:

class MyView(View):

   def check_permissions(self, request, *args, **kwargs):
       # Custom code

If this approach is acceptable, I'm more than happy to try writing a proper patch and tests for inclusion into django. Comments welcome :)

If we simply require people to be logged in, we could also check for login_required boolean attribute on the view:

class MyView(View):
    login_required = True

*Edit: added login required check API

[Florian Apolloner]

Accepting the ticket based on the fact that this stuff should be easier. Not sure that the suggested approach is the best way though.

[Garry Polley]

I think this ticket and https://code.djangoproject.com/ticket/14512 are pretty much the same ticket.

Another good spot that has been talked about is here:

​https://groups.google.com/forum/#!topic/django-developers/jrfbenCJYYU

With all the current comments on the issue I think there are 2 ideas getting mixed with "mixins".

1) A mixin is getting used to only apply decorators, I think that is the case for:

@my_decorator
class MyView(View):
    def get:
        # my code

2) A mixin is being used to define a clean API. (e.g. form_valid, form_invalid, get_context_data)

I think point 1 is the one that Django should support, and it should only get applied to dispatch.

Looking into a comment by Alex Gaynor I believe this is possible. ​https://groups.google.com/d/msg/django-developers/jrfbenCJYYU/thffw4vO1D8J

[Garry Polley]

After spending some time on this, I don't think this is worth the effort at. I think this ticket and the related one

https://code.djangoproject.com/ticket/14512

should both be closed, in favor of mixins.

Thoughts?

[Garry Polley]

Talked to Jacob, the solution here is to bring mixins into Django directly.

[Marc Tamlyn]

One of these should be kept open to track the issue.

[Markus Holtermann]

There has been some discussion among core developers at DjangoCon Europe 2015 where we basically agreed on the implementation as suggested in #24914. I'm therefore closing this ticket as duplicate.