login_required doesn't work with bound methods

[steven.bethard@…]

The django.contrib.auth.decorators.login_required decorator doesn't work correctly with bound methods. If I have code like:

class ViewManager(object):
    @login_required
    def get_main_view(self, request):
        return HttpResponse('hello')

view_manager = ViewManager()
urlpatterns = patterns('',
    (r'^$', view_manager.get_main_view),
)

I'll get an error saying:

AttributeError at /
'ViewManager' object has no attribute 'user'

I believe this is because __get__ is being invoked on the _checklogin function returned by login_required and so the ViewManager instance is being bound as the request argument of _checklogin instead of being bound as the self argument of get_main_view as it should be. To fix this, I had to replace the _dec function in django.contrib.auth.decorators.user_passes_test like so::

def user_passes_test(test_func, login_url=None):
    if not login_url:
        from django.conf import settings
        login_url = settings.LOGIN_URL
    class CheckLogin(object):
        def __init__(self, func):
            self.func = func
        def __get__(self, obj, cls=None):
            return CheckLogin(self.func.__get__(obj, cls))
        def __call__(self, request, *args, **kwargs):
            if test_func(request.user):
                return view_func(request, *args, **kwargs)
            return HttpResponseRedirect('%s?%s=%s' % (login_url, REDIRECT_FIELD_NAME, quote(request.get_full_path())))
    return CheckLogin

This simply redirects the __get__ call to the wrapped function, instead of allowing it to be called on the _checklogin function.

[steven.bethard@…]

rewrite of django.contrib.auth.decorators to handle bound methods properly

[steven.bethard@…]

bugfix for last patch

[steven.bethard@…]

I don't know how to write unit tests for Django, but if someone can point me in the right direction, I can make some tests for this. I need to know know how to programatically visit, say, '/' and verify that it returns the appropriate output.

[Gary Wilson <gary.wilson@…>]

have you seen the ​testing documentation?

[anonymous]

updated version with tests

[steven.bethard@…]

Thanks for the pointer. I think I figured out how to modify modeltests.test_client where the only existing django.contrib.auth.decorators test I could find was. The patch I just uploaded rewrites the decorators module to handle methods properly, and adds three new tests: test_view_with_method_login, test_view_with_permissions and test_view_with_method_permissions.

Note that I didn't see the @permission_required decorator tested anywhere, so I added both a regular function test and a method test for that one. Both of these tests have TODO comments on them - I was able to test that when the permissions are wrong, they redirect to a login page, but I wasn't sure how to get a user with the right permissions to complete the tests. Of course, if @permission_required really wasn't tested at all before, half-way there is better than nothing, right? ;-)

[Malcolm Tredinnick]

I like this patch. Code looks code and everything.

Unfortunately, due to the changes during the sprint over the last few days, in particular, it no longer cleanly applies and at least one of the changes (django.contrib.auth.decorators) requires a bit of reworking of the code.

If somebody could update the patch, it's then ready to go in.

[steven.bethard@…]

Patch against revision 6364. This should now apply cleanly.

[scompt@…]

I think this patch may have fallen through the cracks. I've just applied it against revision 6652 and it still applies cleanly, so I'm going to tweak this ticket so somebody sees it. I hope that's cool with the powers that be.

[Luke Plant]

(In [6658]) Fixed #4376 -- login_required now works with bound methods. Thanks, Steven Bethard.