New proposal for `user_passes_test` decorator logic

[Iago González Rodríguez]

Hi everyone, this is my first proposal so please don't be too hard on me :)

Context:

I was implementing one of my first websites using Django, and when I tried to add the logic for restricting access of certain views to some users, I discovered this `user_passes_test` decorator, which is a pretty awesome decorator. But then in my test function I needed to use the Django messages framework to send a message when the user cannot access the view, but I realized that `user_passes_test` only sends the user object, and there is no option to pass the request object.

I searched my problem on the Internet and I found a few related issues:

1) How to pass Django request object in user_passes_test... - Stackoverflow

2) Show error message when decorator fails - Stackoverflow

And I didn't search more because I understood that there is only one solution right now: overwrite the `user_passes_test` function.

Proposal

This is a pretty old problem (first issue is 9 years old) and it seems that every so often a new user runs into this problem, the solution of which is inelegant (overwrite the code of the Django function).

I know that the workaround is very simple, and that seems to be the reason why no one has decided to bother to modify the logic of that function. But if the solution is so simple (and also does not change the current behavior of Django), why not implement it in the Django source code instead of forcing developers to implement a workaround in their project?

Then I opened a ticket in the Django issue tracker, but sadly it was rejected (maybe I didn't explain correctly the problem, or it seemed like it wasn't worth the effort).

Final solution

Since I was rejected, I did some simple tests (link to online-python.com) to try out if my solution would work (it was my first time working with this kind of advanced decorators). Then I improved the solution sent and the final result is this (contrib/auth/decorators.py):

def request_passes_test(test_func, login_url=None, redirect_field_name=REDIRECT_FIELD_NAME, attrname=None):
    """
    Decorator for views that checks that the request passes the given test,
    redirecting to the log-in page if necessary. The test should be a callable
    that takes the request object and returns True if the request passes.
    Optionally you can pass an attribute name in order to test an attribute
    from request.
    """
    def decorator(view_func):
        @wraps(view_func)
        def _wrapped_view(request, *args, **kwargs):
            attr = request if attrname is None else getattr(request, attrname)
            if test_func(attr):
                return view_func(request, *args, **kwargs)
            path = request.build_absolute_uri()
            resolved_login_url = resolve_url(login_url or settings.LOGIN_URL)
            # If the login url is the same scheme and net location then just
            # use the path as the "next" url.
            login_scheme, login_netloc = urlparse(resolved_login_url)[:2]
            current_scheme, current_netloc = urlparse(path)[:2]
            if ((not login_scheme or login_scheme == current_scheme) and
                    (not login_netloc or login_netloc == current_netloc)):
                path = request.get_full_path()
            from django.contrib.auth.views import redirect_to_login
            return redirect_to_login(
                path, resolved_login_url, redirect_field_name)
        return _wrapped_view
    return decorator


def user_passes_test(test_func, login_url=None, redirect_field_name=REDIRECT_FIELD_NAME):
    """
    Decorator for views that checks that the user passes the given test,
    redirecting to the log-in page if necessary. The test should be a callable
    that takes the user object and returns True if the user passes.
    """
    return request_passes_test(test_func, login_url, redirect_field_name, attrname="user")

With this solution the `user_passes_test` decorator would be working as it is working right now, but we could use `request_passes_test` in order to do more advanced stuff in our test functions, such as using the Django messages framework in the test function, or checking other parameters of the `request` object.

Thank you very much for your time, and please ask me for more information if you have questions. I am willing to open the Pull Request myself, and add modifications to the solution if needed. It would be awesome to join the team of Django contributors :D

Best regards,

Iago.

[Adam Johnson]

Hi Iago

Thanks for writing to the list.

The proposal feels like complicating the decorator too much to me. If you simplify the logic for the redirect for your particular use case, you can implement your own decorator in a handful of lines. I think that’s sufficient.

Adam

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/564e0f9c-886d-4c73-9397-28adb02b5d5dn%40googlegroups.com.

[Iago González Rodríguez]

Hi Adam,

Thanks for your response.

The only thing that makes the solution more complicated is the fact that I created a user_passes_test function to not harm anyone who is already using that function. In order to simplify this, we could replace  user_passes_test with the request_passes_test function.

def request_passes_test(test_func, login_url=None, redirect_field_name=REDIRECT_FIELD_NAME):

    """
    Decorator for views that checks that the request passes the given test,
    redirecting to the log-in page if necessary. The test should be a callable
    that takes the request object and returns True if the request passes.
    """

    def decorator(view_func):
        @wraps(view_func)
        def _wrapped_view(request, *args, **kwargs):

            if test_func(request):

                return view_func(request, *args, **kwargs)
            path = request.build_absolute_uri()
            resolved_login_url = resolve_url(login_url or settings.LOGIN_URL)
            # If the login url is the same scheme and net location then just
            # use the path as the "next" url.
            login_scheme, login_netloc = urlparse(resolved_login_url)[:2]
            current_scheme, current_netloc = urlparse(path)[:2]
            if ((not login_scheme or login_scheme == current_scheme) and
                    (not login_netloc or login_netloc == current_netloc)):
                path = request.get_full_path()
            from django.contrib.auth.views import redirect_to_login
            return redirect_to_login(
                path, resolved_login_url, redirect_field_name)
        return _wrapped_view
    return decorator

This function would allow to write the exact same test functions, but changing user to request.user. For example:

def login_required(function=None, redirect_field_name=REDIRECT_FIELD_NAME, login_url=None):
     """
    Decorator for views that checks that the user is logged in, redirecting

    to the log-in page if necessary.

    """

    actual_decorator = request_passes_test(
        lambda r: r.user.is_authenticated,
        login_url=login_url,
        redirect_field_name=redirect_field_name
    )
    if function:
        return actual_decorator(function)
    return actual_decorator

It is much simpler to change user to request.user rather than writing a custom request_passes_test function in every project that anyone wants to use it, and also it is more generic while user_passes_test is much more specific for certain use cases. There are some use cases where having the request variable instead of the user can be much more useful, for example if you want to use the Django message framework to send an error message when certain user does not pass a test. Does it make sense that there is no option to show a message to the user when they fail a test and redirect them to login without showing them any information?

In my current project I already have created my own request_passes_test decorator, but honestly I think more people would benefit from having this decorator in the source code (at least +30 people according to this post).

I hope more people can participate on this topic and give their opinion.

Thanks again for your time.

Best regards,

Iago.