[Django] #28645: The inactive user error never is raised in login form
Django
-----------------------------------------------+------------------------
Reporter: Guilherme Junqueira | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: 1.11
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-----------------------------------------------+------------------------
Looking in file django.contrib.auth.forms
The class AuthenticationForm and clean method.
The inactive user never is raised, this happens because after Django 1.10
all users that is not active cannot authenticate, so self.user_chache is
always be None for inactive users, even if has a correct user and pass.
So the code needed to be changed to raise the correct error for a user
that is not active.
My stackoverflow thread about this:
https://stackoverflow.com/questions/46459258/how-to-inform-a-user-that-he-
is-not-active-in-django-login-view/46459998#46459998
--
Ticket URL: <https://code.djangoproject.com/ticket/28645>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
ModelBackend
-------------------------------------+------------------------------------
Reporter: Guilherme Junqueira | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: 1.11
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* stage: Unreviewed => Accepted
Comment:
Regression in e0a3d937309a82b8beea8f41b17d8b6298da2a86 (#25232).
--
Ticket URL: <https://code.djangoproject.com/ticket/28645#comment:1>
Django
ModelBackend
Reporter: Guilherme Junqueira | Owner: hui shang
Type: Bug | Status: assigned
Component: contrib.auth | Version: 1.11
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by hui shang):
* owner: nobody => hui shang
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/28645#comment:2>
Django
ModelBackend
-------------------------------------+-------------------------------------
Reporter: Guilherme Junqueira | Owner: hui shang
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by hui shang):
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/9308 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/28645#comment:3>
Django
ModelBackend
-------------------------------------+-------------------------------------
Reporter: Guilherme Junqueira | Owner: hui shang
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/28645#comment:4>
Django
ModelBackend
-------------------------------------+-------------------------------------
Reporter: Guilherme Junqueira | Owner: hui shang
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by hui shang):
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/28645#comment:5>
Django
ModelBackend
-------------------------------------+-------------------------------------
Reporter: Guilherme Junqueira | Owner: hui shang
Component: contrib.auth | Version: 1.11
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"359370a8b8ca0efe99b1d4630b291ec060b69225" 359370a]:
{{{
#!CommitTicketReference repository=""
revision="359370a8b8ca0efe99b1d4630b291ec060b69225"
Fixed #28645 -- Reallowed AuthenticationForm to raise the inactive user
error when using ModelBackend.
Regression in e0a3d937309a82b8beea8f41b17d8b6298da2a86.
Thanks Guilherme Junqueira for the report and Tim Graham for the review.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28645#comment:6>
Django
ModelBackend
-------------------------------------+-------------------------------------
Reporter: Guilherme Junqueira | Owner: hui shang
Type: Bug | Status: closed
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"36dd0126a5a9688530f099ab6cc4f8621d1494b3" 36dd0126]:
{{{
#!CommitTicketReference repository=""
revision="36dd0126a5a9688530f099ab6cc4f8621d1494b3"
[2.0.x] Fixed #28645 -- Reallowed AuthenticationForm to raise the inactive
user error when using ModelBackend.
Regression in e0a3d937309a82b8beea8f41b17d8b6298da2a86.
Thanks Guilherme Junqueira for the report and Tim Graham for the review.
Backport of 359370a8b8ca0efe99b1d4630b291ec060b69225 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28645#comment:7>
Django
ModelBackend
-------------------------------------+-------------------------------------
Reporter: Guilherme Junqueira | Owner: hui shang
Type: Bug | Status: closed
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"308f64462421b09b21ef0dcd9cc3654cc25bceba" 308f644]:
{{{
#!CommitTicketReference repository=""
revision="308f64462421b09b21ef0dcd9cc3654cc25bceba"
[1.11.x] Fixed #28645 -- Reallowed AuthenticationForm to raise the
inactive user error when using ModelBackend.
Regression in e0a3d937309a82b8beea8f41b17d8b6298da2a86.
Thanks Guilherme Junqueira for the report and Tim Graham for the review.
Backport of 359370a8b8ca0efe99b1d4630b291ec060b69225 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28645#comment:8>
Django
ModelBackend
-------------------------------------+-------------------------------------
Reporter: Guilherme Junqueira | Owner:
Type: Bug | Status: closed
Component: contrib.auth | Version: 1.11
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"af33fb250e9847f1ca8c0ba0d72671d76659704f" af33fb25]:
{{{
#!CommitTicketReference repository=""
revision="af33fb250e9847f1ca8c0ba0d72671d76659704f"
Fixed CVE-2018-6188 -- Fixed information leakage in AuthenticationForm.
Reverted 359370a8b8ca0efe99b1d4630b291ec060b69225 (refs #28645).
This is a security fix.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28645#comment:9>
Django
ModelBackend
-------------------------------------+-------------------------------------
Reporter: Guilherme Junqueira | Owner:
| shangdahao
Type: Bug | Status: closed
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"c37bb28677295f6edda61d8ac461014ef0d3aeb2" c37bb286]:
{{{
#!CommitTicketReference repository=""
revision="c37bb28677295f6edda61d8ac461014ef0d3aeb2"
[2.0.x] Fixed CVE-2018-6188 -- Fixed information leakage in
AuthenticationForm.
Reverted 359370a8b8ca0efe99b1d4630b291ec060b69225 (refs #28645).
This is a security fix.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28645#comment:10>
Django
ModelBackend
-------------------------------------+-------------------------------------
Reporter: Guilherme Junqueira | Owner:
| shangdahao
Type: Bug | Status: closed
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"57b95fedad5e0b83fc9c81466b7d1751c6427aae" 57b95fed]:
{{{
#!CommitTicketReference repository=""
revision="57b95fedad5e0b83fc9c81466b7d1751c6427aae"
[1.11.x] Fixed CVE-2018-6188 -- Fixed information leakage in
AuthenticationForm.
Reverted 359370a8b8ca0efe99b1d4630b291ec060b69225 (refs #28645).
This is a security fix.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28645#comment:11>
Django
ModelBackend
-------------------------------------+-------------------------------------
Reporter: Guilherme Junqueira | Owner:
| shangdahao
Component: contrib.auth | Version: 1.11
Keywords: 2.1 | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* keywords: => 2.1
* status: closed => new
* has_patch: 1 => 0
* resolution: fixed =>
Comment:
Reopening since the fix had to be reverted. We'll try to develop a
solution for Django 2.1. Probably the solution will be too invasive to
backport to the stable branches.
In a mail to the security mailing list, Jack Cushman suggested:
It's desirable for auth backends to enforce rules like “no inactive
users” when supplied with otherwise-correct credentials – that’s more of a
backend concern than a display concern, and forms shouldn’t be required to
enforce it. But it is desirable for auth forms to show custom error
messages when an auth backend rejects a user, if and only if the user
supplied correct credentials. The ideal way to solve both problems would
be for auth backends to return a tuple of `(user or None,
custom_error_code or None)`, but that would break backwards compatibility.
\\ \\
So, can we let auth backends return custom error codes with backwards
compatibility? \\ \\
Attached is an untested patch that hopefully does that, by adding an
authenticate_with_error_code method that backends can optionally implement
and forms can optionally consume. I think this is a good angle on the
problem – it cleans up the can of worms with displaying custom errors, and
also totally avoids dealing with attacker-submitted data after credentials
fail to validate, which is key to avoiding any subtle security issues.
I'll attach the patch, but I haven't evaluated it in much detail.
--
Ticket URL: <https://code.djangoproject.com/ticket/28645#comment:12>
Django
ModelBackend
-------------------------------------+-------------------------------------
Reporter: Guilherme Junqueira | Owner:
| shangdahao
Component: contrib.auth | Version: 1.11
Keywords: 2.1 | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* Attachment "28645-jc.diff" added.
Django
ModelBackend
-------------------------------------+-------------------------------------
Reporter: Guilherme Junqueira | Owner:
| shangdahao
Component: contrib.auth | Version: 1.11
Keywords: 2.1 | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Christoph Schwarzenberg):
* cc: Christoph Schwarzenberg (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/28645#comment:13>
Django
ModelBackend
-------------------------------------+-------------------------------------
Reporter: Guilherme Junqueira | Owner:
| shangdahao
Component: contrib.auth | Version: 1.11
Keywords: 2.1 | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Christoph Schwarzenberg):
Maybe it is enough to check the supplied password.
I've modified the code from shangdahao accordingly:
https://stackoverflow.com/a/49138231/9453030
--
Ticket URL: <https://code.djangoproject.com/ticket/28645#comment:14>
Django
ModelBackend
-------------------------------------+-------------------------------------
Reporter: Guilherme Junqueira | Owner:
| shangdahao
Component: contrib.auth | Version: 1.11
Keywords: 2.1 | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham):
That approach may leak whether or not a username exists because of the
time it takes to hash a password. For user names that exist, password
hashing will run twice compared to once for user names that don't exist.
See #20760 for a past example.
--
Ticket URL: <https://code.djangoproject.com/ticket/28645#comment:15>
Django
ModelBackend
-------------------------------------+-------------------------------------
Reporter: Guilherme Junqueira | Owner:
| shangdahao
Component: contrib.auth | Version: 1.11
Keywords: 2.1 | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Christoph Schwarzenberg):
Is there any progress regarding the issue?
An other suggestion:
In ModelBackend authentication maybe add an optional parameter, defaulting
to False to allow inactive users to pass through authenticate() and catch
them later in AuthenticationForm.confirm_login_allowed()
E.g.
{{{#!python
class ModelBackend:
def authenticate(self, request, username=None, password=None,
allow_inactive=False, **kwargs):
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 nonexistent user
(#20760).
UserModel().set_password(password)
else:
if user.check_password(password) and
(self.user_can_authenticate(user) or allow_inactive):
return user
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28645#comment:16>
Django
ModelBackend
-------------------------------------+-------------------------------------
Reporter: Guilherme Junqueira | Owner:
| shangdahao
Component: contrib.auth | Version: 1.11
Keywords: 2.1 | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by DonExo):
3 years have passed and still no fix for this?
I see it was fixed and then reverted.
Last comment from 15 months ago provides nice suggestion as well, but I
guess it was somehow "missed" and left out?
I'm using v3.0 in 2020 and this is still a bug (even though there are
custom work around(s)) - I'd appreciate when the Django team would fix
this by default.
--
Ticket URL: <https://code.djangoproject.com/ticket/28645#comment:17>
Django
ModelBackend
-------------------------------------+-------------------------------------
Reporter: Guilherme Junqueira | Owner:
| shangdahao
Component: contrib.auth | Version: 3.1
Severity: Normal | Resolution:
Keywords: 2.1 | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by alv2017):
* version: 1.11 => 3.1
Comment:
I'm using Django 3.1, and I spent some time on figuring the issue with
ModelBackend.authenticate() method. In my application I was willing to
authenticate active and inactive users, and the default ModelBackend is
not designed to do this.
In Django 3.1 there is a class (backends.py) AllowAllUsersModelBackend
that inherits all the functionality from the ModelBackend class and
permits authentication of active and inactive users.
PS: The problem seems to be solved, however it took me some time to figure
the things out.
--
Ticket URL: <https://code.djangoproject.com/ticket/28645#comment:19>
Django
ModelBackend
-------------------------------------+-------------------------------------
Reporter: Guilherme Junqueira | Owner:
| shangdahao
Component: contrib.auth | Version: 3.1
Severity: Normal | Resolution:
Keywords: 2.1 | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mohamed El-Kalioby):
Shouldn’t this be closed?
--
Ticket URL: <https://code.djangoproject.com/ticket/28645#comment:20>