Opened 7 years ago

Closed 3 years ago

#15215 closed New feature (duplicate)

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

Reported by: Jari Pennanen Owned by: nobody
Component: Generic views Version: master
Severity: Normal Keywords:
Cc: Tom Christie, Selwin Ong, amirouche.boubekki@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no


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.

Change History (18)

comment:1 Changed 7 years ago by 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.

comment:2 Changed 7 years ago by Jari Pennanen

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

comment:3 Changed 7 years ago by Russell Keith-Magee

Triage Stage: UnreviewedDesign decision needed

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).

comment:4 Changed 7 years ago by Tom Christie

Cc: Tom Christie added

comment:5 Changed 7 years ago by Łukasz Rekucki

Severity: Normal
Type: New feature

comment:6 Changed 6 years ago by Aymeric Augustin

UI/UX: unset

Change UI/UX from NULL to False.

comment:7 Changed 6 years ago by Aymeric Augustin

Easy pickings: unset

Change Easy pickings from NULL to False.

comment:8 Changed 6 years ago by 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']


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 can simply check for login_required boolean attribute on the view:

class MyView(View):
    login_required = True

*Edit: added login required check API

Last edited 6 years ago by Selwin Ong (previous) (diff)

comment:9 Changed 6 years ago by Selwin Ong

Cc: Selwin Ong added

comment:10 Changed 5 years ago by Amirouche

Cc: amirouche.boubekki@… added

comment:11 Changed 5 years ago by Amirouche

Cc: amirouche.boubekki@… removed

comment:12 Changed 5 years ago by Amirouche

Cc: amirouche.boubekki@… added

comment:13 Changed 5 years ago by Florian Apolloner

Triage Stage: Design decision neededAccepted

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

comment:14 Changed 5 years ago by Garry Polley

I think this ticket and are pretty much the same ticket.

Another good spot that has been talked about is here:!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:

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.

comment:15 Changed 5 years ago by 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

should both be closed, in favor of mixins.


comment:16 Changed 5 years ago by Garry Polley

Resolution: wontfix
Status: newclosed

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

comment:17 Changed 5 years ago by Marc Tamlyn

Resolution: wontfix
Status: closednew

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

comment:18 Changed 3 years ago by Markus Holtermann

Resolution: duplicate
Status: newclosed

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.

Note: See TracTickets for help on using tickets.
Back to Top