4
\$\begingroup\$

I wrote a custom decorator to check whether user is in particular group of not. Please provide the feedback on optimizing code. I currently moved from Java to Django-Python so I wasn't able to optimize the logic.

Check whether student is in labsheet_collector, assignment_collector or fund_collector.

def labsheet_collector(view_func):
    def _wrapped_view_func(request, *args, **kwargs):
        print request.user
        if request.user.is_anonymous:
            return redirect('/admin/') #?next=%s' % request.path)
        if not (request.user.groups.filter(name='labsheet_collector').exists()) or request.user.is_superuser:
            raise Http404
        return view_func(request, *args, **kwargs)

    return _wrapped_view_func


def assignment_collector(view_func):
    def _wrapped_view_func(request, *args, **kwargs):
        if request.user.is_anonymous:
            return redirect('/admin/')
        if not (request.user.groups.filter(name='assignment_collector').exists()) \
                or request.user.is_superuser:
            raise Http404
        return view_func(request, *args, **kwargs)

    return _wrapped_view_func


def fund_collector(view_func):
    def _wrapped_view_func(request, *args, **kwargs):
        if request.user.is_anonymous:
            return redirect('/admin/')
        if not (request.user.groups.filter(name='fund_collector').exists()) \
                or request.user.is_superuser:
            raise Http404
        return view_func(request, *args, **kwargs)

    return _wrapped_view_func
\$\endgroup\$

2 Answers 2

2
\$\begingroup\$

I'm not a django expert, but from what I see, you could add one more wrapping level to avoid repeating the same code in multiple decorators that are the same except for the group name:

from functools import wraps

def check_group(group_name):
    def _check_group(view_func):
        @wraps(view_func)
        def wrapper(request, *args, **kwargs):
            print request.user
            if request.user.is_anonymous:
                return redirect('/admin/')
            if (not (request.user.groups.filter(name=group_name).exists())
                or request.user.is_superuser):
                raise Http404
            return view_func(request, *args, **kwargs)
        return wrapper
    return _check_group

Then you can wrap your view functions using the group name explicitly:

@check_group('labsheet_collector')
def view_func(...

or, define aliases that work as the decorators in the original question:

labsheet_collector = checkgroup('labsheet_collector')
assignment_collector = checkgroup('assignment_collector')
fund_collector = check_group('fund_collector')

Note the use of functools.wraps which is useful to preserve information from the wrapped function. More information here.

\$\endgroup\$
10
\$\begingroup\$

As an alternative to the answer given by jcollado, Django has some built in decorators to help.

From the docs:

from django.contrib.auth.decorators import user_passes_test

def email_check(user):
    return '@example.com' in user.email

@user_passes_test(email_check)
def my_view(request):

https://docs.djangoproject.com/en/1.6/topics/auth/default/#limiting-access-to-logged-in-users-that-pass-a-test

This could be modified to something like:

from django.contrib.auth.decorators import user_passes_test

def group_check(user):
    return user.groups.filter(name__in=['assignment_collector',
                                        'fund_collector' 
                                        'labsheet_collector'])

@user_passes_test(group_check)
def my_view(request):
    ...

If they are not in these groups it will redirect by default to the login page.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.