Inconsistency between doc and code: authenticate() when user.is_active == False

[Shen Li]

In the code:
​https://github.com/django/django/blob/stable/1.10.x/django/contrib/auth/backends.py#L12-L32

    def authenticate(self, username=None, password=None, **kwargs):
        UserModel = get_user_model()
        if username is None:
            username = kwargs.get(UserModel.USERNAME_FIELD)
        try:
            user = UserModel._default_manager.get_by_natural_key(username)
        except UserModel.DoesNotExist:
            # Run the default password hasher once to reduce the timing
            # difference between an existing and a non-existing user (#20760).
            UserModel().set_password(password)
        else:
            if user.check_password(password) and self.user_can_authenticate(user):
                return user

    def user_can_authenticate(self, user):
        """
        Reject users with is_active=False. Custom user models that don't have
        that attribute are allowed.
        """
        is_active = getattr(user, 'is_active', None)
        return is_active or is_active is None

authenticate() returns None when user.is_active == False.

However, the documentation suggests that when user.is_active == False, the authenticate() method returns the user normally.

​https://docs.djangoproject.com/en/1.10/topics/auth/default/#auth-web-requests

from django.contrib.auth import authenticate, login

def my_view(request):
    username = request.POST['username']
    password = request.POST['password']
    user = authenticate(username=username, password=password)
    if user is not None:
        if user.is_active:
            login(request, user)
            # Redirect to a success page.
        else:
            # Return a 'disabled account' error message
            ...
    else:
        # Return an 'invalid login' error message.
        ...

[Tim Graham]

Yes, it would be good to adjust or clarify that example in light of #25232.

[Damian]

I understand that the user.is_active() check is redundant after we run authenticate method in the example code. However I would like to ask for some advice, before I do anything(as a newbie I would like to make it all right). I would add some information in the box that the authenticate method returns None if the user is inactive, even if the user entered correct login detail(thus checking if user is active after that is redundant)? And then I would mention to use separately user.is_active() and user.check_password() to display separate error messages, for each case, as in the example code above ? Would that clear things up?

[Shen Li]

Running user.is_active() and user.check_password() separately is a good idea, but I think it would result in redundant SQL queries, right?

[Damian]

You are right. So, how about authenticate user using authenticate() method (as in the example provided in the ticket description), then if user is not None login user without performing user.is_active() check. Else, if user is None create an if statement, which will allow us to display custom error messages for 'disabled account' and 'invalid login'

[Damian]

Pull request created at ​https://github.com/django/django/pull/7059.

[Tim Graham <timograham@…>]

In c52350b:

[1.10.x] Fixed #26957 -- Corrected authenticate() docs regarding User.is_active.

Backport of c412aaca735c7cc1c766b85c1512f9ff434ce63a from master

[Tim Graham <timograham@…>]

In c412aac:

Fixed #26957 -- Corrected authenticate() docs regarding User.is_active.