[Django] #25232: Deactivating users with is_active
Django
-------------------------------+--------------------
Reporter: OleLaursen | Owner: nobody
Type: Uncategorized | Status: new
Component: contrib.auth | Version: 1.8
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------
I just got a bug report that inactive users could still access a site I'm
maintaining. It turns out that is_active doesn't really deactivate people,
it just prevents them from logging in again.
This was discussed in 2008:
https://groups.google.com/forum/#!topic/django-developers/P0b0g0sr-b8
I think the short version is that this happened by accident (login view
checks is_active, so does permissions, but auth backend doesn't) but
discovered late enough that Malcolm Tredinnick didn't want to break
backwards compatibility.
This leaves no proper built-in way to deactivate users, a useful feature.
Hence, I humbly suggest that we add a setting ala
PREVENT_INACTIVE_USERS_FROM_BEING_AUTHENTICATED? It would default to None,
meaning leave the current semi-broken behaviour, but you could set it to
True to have the ModelBackend do a check on is_active in get_user:
https://github.com/django/django/blob/master/django/contrib/auth/backends.py#L90
Perhaps it could also be set to False to prevent the login view and
permissions from checking is_active, in case anyone finds that useful.
If people like the setting, it could perhaps in the future default to
True.
--
Ticket URL: <https://code.djangoproject.com/ticket/25232>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: OleLaursen | Owner: nobody
Type: Uncategorized | Status: new
Component: contrib.auth | Version: 1.8
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
It would be better to first write to the DevelopersMailingList to get a
consensus on the design issues. New settings are to be avoided if
possible.
--
Ticket URL: <https://code.djangoproject.com/ticket/25232#comment:1>
Django
------------------------------+--------------------------------------
Reporter: OleLaursen | Owner: nobody
Type: New feature | Status: closed
Component: contrib.auth | Version: 1.8
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* status: new => closed
* type: Uncategorized => New feature
* resolution: => wontfix
Comment:
It seems to me this could be accomplished without much difficulty by
overriding the `ModelBackend`. Something like (untested):
{{{#!python
class RejectInactiveUsersBackend(ModelBackend):
def get_user(self, user_id):
user = super(RejectInactiveUsersBackend, self).get_user(user_id)
if user and not user.is_active:
return None
return user
}}}
It's probably better to recommend that route instead of adding a setting.
--
Ticket URL: <https://code.djangoproject.com/ticket/25232#comment:2>
Django
------------------------------+--------------------------------------
Type: New feature | Status: new
Component: contrib.auth | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by aaugustin):
* status: closed => new
* resolution: wontfix =>
Comment:
The manual workaround is to change a user's password, assuming
`SessionAuthenticationMiddleware` is installed... That said I'm not
comfortable with suggesting workarounds. The current situation could
easily be described as a security issue.
I don't think adding a setting is a solution because using the default
value will leave sites vulnerable. I think we should fix the bug, document
the backwards-incompatibility and provide a way to restore the previous
behavior.
I'm going to reopen the bug in the hope to gather more feedback. If no one
thinks fixing is a good idea, we can close it again.
--
Ticket URL: <https://code.djangoproject.com/ticket/25232#comment:3>
Django
------------------------------+--------------------------------------
Type: New feature | Status: new
Component: contrib.auth | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by carljm):
I agree with Aymeric that it's just a bug if the backend behavior doesn't
match the login form behavior. Both are easily overrideable.
--
Ticket URL: <https://code.djangoproject.com/ticket/25232#comment:4>
Django
------------------------------+--------------------------------------
Reporter: OleLaursen | Owner: nobody
Type: New feature | Status: new
Component: contrib.auth | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* version: 1.8 => master
Comment:
It might also be possible to fix #24987 and remove the `user.is_active`
check in the test client `login()` method. A draft patch is attached, but
some test failures remain.
For backwards compatibility, should we provide an authentication backend
that allows inactive users:
{{{
class AllowInactiveUsersModelBackend(ModelBackend):
allow_inactive_users = True
}}}
(incorporating that flag into the patch).
--
Ticket URL: <https://code.djangoproject.com/ticket/25232#comment:5>
Django
------------------------------+--------------------------------------
Type: New feature | Status: new
Component: contrib.auth | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* Attachment "25232.diff" added.
Django
------------------------------+------------------------------------
Type: New feature | Status: new
Component: contrib.auth | Version: master
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/25232#comment:6>
Django
------------------------------+------------------------------------
Type: New feature | Status: assigned
Component: contrib.auth | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by sasha0):
* status: new => assigned
* owner: nobody => sasha0
* has_patch: 0 => 1
Comment:
I've started with the [https://github.com/django/django/pull/6090 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/25232#comment:7>
Django
------------------------------+------------------------------------
Reporter: OleLaursen | Owner: sasha0
Type: New feature | Status: assigned
Component: contrib.auth | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: 0 => 1
Comment:
Left comments for improvement.
--
Ticket URL: <https://code.djangoproject.com/ticket/25232#comment:8>
Django
------------------------------+------------------------------------
Reporter: OleLaursen | Owner: sasha0
Type: New feature | Status: assigned
Component: contrib.auth | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/25232#comment:9>
Django
------------------------------+------------------------------------
Reporter: OleLaursen | Owner: sasha0
Type: New feature | Status: assigned
Component: contrib.auth | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: 1 => 0
Comment:
[https://github.com/django/django/pull/6309 Updated PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/25232#comment:9>
Django
------------------------------+------------------------------------
Reporter: OleLaursen | Owner: sasha0
Type: New feature | Status: assigned
Component: contrib.auth | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: 0 => 1
Comment:
I think `RemoteUserBackend` should have the same behavior as
`ModelBackend` and respect the proposed `user_can_authenticate()` method.
--
Ticket URL: <https://code.djangoproject.com/ticket/25232#comment:10>
Django
users
------------------------------+------------------------------------
Reporter: OleLaursen | Owner: sasha0
Type: New feature | Status: assigned
Component: contrib.auth | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: 1 => 0
Comment:
I updated the PR for the above comment. The second commit there should be
good to go but if someone could double check the first that would be
great.
--
Ticket URL: <https://code.djangoproject.com/ticket/25232#comment:11>
Django
users
------------------------------+------------------------------------
Reporter: OleLaursen | Owner: sasha0
Component: contrib.auth | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"e0a3d937309a82b8beea8f41b17d8b6298da2a86" e0a3d937]:
{{{
#!CommitTicketReference repository=""
revision="e0a3d937309a82b8beea8f41b17d8b6298da2a86"
Fixed #25232 -- Made ModelBackend/RemoteUserBackend reject inactive users.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25232#comment:12>
Django
users
------------------------------+------------------------------------
Reporter: OleLaursen | Owner: sasha0
Type: New feature | Status: closed
Component: contrib.auth | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"e69091b34a34697fe7eac38763dd372b305e1ab4" e69091b]:
{{{
#!CommitTicketReference repository=""
revision="e69091b34a34697fe7eac38763dd372b305e1ab4"
Refs #25232 -- Documented AllowAll*Backend in "new features" section of
1.10 release notes.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25232#comment:13>
Django
users
------------------------------+------------------------------------
Reporter: OleLaursen | Owner: sasha0
Type: New feature | Status: closed
Component: contrib.auth | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"738a65a597f9a448b000ad89500db5caf3807363" 738a65a5]:
{{{
#!CommitTicketReference repository=""
revision="738a65a597f9a448b000ad89500db5caf3807363"
[1.10.x] Refs #25232 -- Documented AllowAll*Backend in "new features"
section of 1.10 release notes.
Backport of e69091b34a34697fe7eac38763dd372b305e1ab4 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25232#comment:14>
Django
users
Reporter: Ole Laursen | Owner: Sasha Gaevsky
Type: New feature | Status: closed
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by GitHub <noreply@…>):
In [changeset:"282d58e19385ee4e5c125a9d3cba820949314f3f" 282d58e]:
{{{
#!CommitTicketReference repository=""
revision="282d58e19385ee4e5c125a9d3cba820949314f3f"
Refs #25232 -- Simplified ModelBackend.user_can_authenticate().
Thanks Jay Turner for the suggestion.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25232#comment:15>