Include basic permission mixins into Django

[Raphael Michel]

Django does provide very convenient decorators in django.contrib.auth, most notably the login_required decorator. At the same time, Django encourages the use of class-based views, where this cannot be used directly. The de-facto standard to do this is a mixin, there even is an example on this mixin in the documentation: ​https://docs.djangoproject.com/en/1.8/topics/class-based-views/intro/#mixins-that-wrap-as-view

I'll now be working on a proposed pull request during djangocon.eu 2015 sprint.

It is to discuss whether the same is to be done with permission_required.

[Raphael Michel]

Development of the pull request is happening on ​https://github.com/raphaelm/django/tree/ticket_24914

[Joachim Jablon]

It's worth noting that it's a feature that exists in ​http://django-braces.readthedocs.org/en/latest/index.html

[Ana Balica]

It is a good idea to provide a LoginRequiredMixin, which basically will act the same way as the login_required decorator. It might be implemented in a cleaner way by decorating dispatch method (in contrast with decorating as_view classmethod).

I would opt for getting started with LoginRequiredMixin and afterwards providing mixins for user_passes_test and permission_required from django.contrib.auth.decorators.

​https://docs.djangoproject.com/en/dev/_modules/django/contrib/auth/decorators/

[Raphael Michel]

I created a pull request for this ticket. An initial version of implementation and tests is in the pull request, documentation is yet to be done: ​https://github.com/django/django/pull/4749

This proposal provides all the and only the functionality of all three authentication decorators from contrib.auth.

While it is slightly differently implemented, it tries to stay compatible with the widely-used 3rd-party package django-braces in terms of parameter and method names. django-braces does have more functionality than the decorators have, but implementing this functionality in django itself is outside the scope of this ticket.

[Raphael Michel]

I added documentation. As I'm not a native speaker, I'd love someone to review the language.

I decieded to drop a section in the documentation on class-based view on wrapping as_view with decorators, as there no longer seems to be any decorator in django core where this would make any sense.

[Markus Holtermann]

Updated PR: ​https://github.com/django/django/pull/4836

[Markus Holtermann]

The current implementation has some drawbacks as pointed out on the PR. I'll fix the in aforementioned PR.

[Thomas Randle]

What would be the recommend approach for stacking different permission mixins on a CBV?

e.g. using LoginRequiredMixin and a PermissionRequiredMixin on the same view.

[Markus Holtermann]

Thanks for mentioning that, Rundll. The current implementation does not support stacking them (braces' LoginRequired and PermissionRequired mixins do, though). However, neither our UserPassesTestMixin nor braces' allows stacking inherited mixins, either.

That makes me wonder how one is supposed to combine different permission checks that are used separately and together:

class CheckOneMixin(UserPassesTestMixin):
    def test_func(self, user):
        return something_is_true_or_false

class CheckTwoMixin(UserPassesTestMixin):
    def test_func(self, user):
        return something_else_is_true_or_false

class MyView1(CheckOneMixin, View):
    pass

class MyView2(CheckTwoMixin, View):
    pass

class MyView3(CheckOneMixin, CheckTwoMixin, View):
    pass

Effectively, MyView3 will never bother about the value of something_else_is_true_or_false as that function is never called.

[Raphael Michel]

I don't really see an easy and obvious way to support stacking of UserPassesTestMixin. You could either do something like

class CheckOneMixin(UserPassesTestMixin):
    def test_func(self, user):
        return something_is_true_or_false and super().test_func()

or make the API more complex by allowing each subclass to set/append to a list of test functions in some way.

[Markus Holtermann]

I just added some tests to check for the mixin stacking.

[Markus Holtermann]

Here's a PR that adopts django-braces' mixins: ​https://github.com/django/django/pull/4844

[Jayson Reis]

Replying to MarkusH:

Thanks for mentioning that, Rundll. The current implementation does not support stacking them (braces' LoginRequired and PermissionRequired mixins do, though). However, neither our UserPassesTestMixin nor braces' allows stacking inherited mixins, either.

That makes me wonder how one is supposed to combine different permission checks that are used separately and together:

class CheckOneMixin(UserPassesTestMixin):
    def test_func(self, user):
        return something_is_true_or_false

class CheckTwoMixin(UserPassesTestMixin):
    def test_func(self, user):
        return something_else_is_true_or_false

class MyView1(CheckOneMixin, View):
    pass

class MyView2(CheckTwoMixin, View):
    pass

class MyView3(CheckOneMixin, CheckTwoMixin, View):
    pass

Effectively, MyView3 will never bother about the value of something_else_is_true_or_false as that function is never called.

About ensuring at least one valid response, I thought something like this:

class CheckOneMixin:
    def test_func(self):
        print(self)
        return True


class CheckTwoMixin:
    def test_func(self):
        print(self)
        return False


class Dispatcher:
    def dispatch(self):
        validators = [b.test_func for b in self.__class__.__bases__ if hasattr(b, 'test_func')]
        for validator in validators:
            if validator(self):
                return 'Dispatch for real'

        return 'Invalid permissions'



class MyView1(CheckOneMixin, Dispatcher):
    pass


class MyView2(CheckTwoMixin, Dispatcher):
    pass


class MyView3(CheckOneMixin, CheckTwoMixin, Dispatcher):
    pass


for klass in (MyView1, MyView2, MyView3):
    print(klass().dispatch(), '\n')

In this case the class that act like the real dispatcher could call every function on View's bases.

[Jayson Reis]

Never mind I just saw that there is a pull request already :)

[Markus Holtermann]

Thanks jaysonsantos, I had something like that in an experimental version, but dropped it because I disliked iterating over __bases__ or mro().

[Markus Holtermann <info@…>]

In e5cb4e1:

Fixed #24914 -- Added authentication mixins for CBVs

Added the mixins LoginRequiredMixin, PermissionRequiredMixin and
UserPassesTestMixin to contrib.auth as counterparts to the respective
view decorators.

The authentication mixins UserPassesTestMixin, LoginRequiredMixin and
PermissionRequiredMixin have been inspired by django-braces
<​https://github.com/brack3t/django-braces/>

Thanks Raphael Michel for the initial patch, tests and docs on the PR
and Ana Balica, Kenneth Love, Marc Tamlyn, and Tim Graham for the
review.

[Tim Graham]

Reopening to add documentation for the new methods like get_redirect_field_name(), get_permission_denied_message(), and get_permission_required(). Anything in the changeset above that says " Override this method ..." should be documented.

[Tim Graham]

​PR for additional documentation.

[Tim Graham <timograham@…>]

In 6c6eb8a6:

Refs #24914 -- Added docs for more auth mixin methods.