#26957 closed Cleanup/optimization (fixed)
Inconsistency between doc and code: authenticate() when user.is_active == False
Reported by: | Shen Li | Owned by: | Damian |
---|---|---|---|
Component: | contrib.auth | Version: | 1.10 |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Accepted | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | yes | UI/UX: | no |
Description (last modified by )
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. ...
Change History (11)
comment:1 by , 8 years ago
Description: | modified (diff) |
---|---|
Version: | 1.9 → 1.10 |
comment:2 by , 8 years ago
Easy pickings: | set |
---|---|
Triage Stage: | Unreviewed → Accepted |
comment:3 by , 8 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:4 by , 8 years ago
Owner: | removed |
---|---|
Status: | assigned → new |
comment:5 by , 8 years ago
Owner: | set to |
---|---|
Status: | new → assigned |
comment:6 by , 8 years ago
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?
comment:7 by , 8 years ago
Running user.is_active()
and user.check_password()
separately is a good idea, but I think it would result in redundant SQL queries, right?
Yes, it would be good to adjust or clarify that example in light of #25232.