Opened 14 years ago
Closed 9 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: | dev |
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 |
Description
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 by , 14 years ago
comment:2 by , 14 years ago
Opened up also a thread to django-devs for this if anyone is interested.
comment:3 by , 14 years ago
Triage Stage: | Unreviewed → Design 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 by , 14 years ago
Cc: | added |
---|
comment:5 by , 14 years ago
Severity: | → Normal |
---|---|
Type: | → New feature |
comment:8 by , 13 years ago
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 :)
comment:9 by , 13 years ago
Cc: | added |
---|
comment:10 by , 12 years ago
Cc: | added |
---|
comment:11 by , 12 years ago
Cc: | removed |
---|
comment:12 by , 12 years ago
Cc: | added |
---|
comment:13 by , 12 years ago
Triage Stage: | Design decision needed → Accepted |
---|
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 by , 11 years ago
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
comment:15 by , 11 years ago
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?
comment:16 by , 11 years ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
Talked to Jacob, the solution here is to bring mixins into Django directly.
comment:17 by , 11 years ago
Resolution: | wontfix |
---|---|
Status: | closed → new |
One of these should be kept open to track the issue.
comment:18 by , 9 years ago
Resolution: | → duplicate |
---|---|
Status: | new → closed |
This could be implemented as Mixin too, could be more pythonic, kind of like:
I have not tested this, but it this ForbiddenMixin should be part of Django it is so general and useful.