Opened 15 years ago
Closed 13 years ago
#13125 closed Bug (wontfix)
login_required does not check User.is_active
Reported by: | Owned by: | nobody | |
---|---|---|---|
Component: | contrib.auth | Version: | 1.1 |
Severity: | Normal | Keywords: | login_required, authorization, users |
Cc: | Gonzalo Saavedra, Boris Savelev | Triage Stage: | Design decision needed |
Has patch: | yes | Needs documentation: | no |
Needs tests: | yes | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Description
The login_required
decorator is not checking User.is_active
, as staff_member_required
does. If an authenticated user is deactivated (via setting is_active
to False
), the user is still able to browse login_required
-protected views.
Attachments (2)
Change History (11)
by , 15 years ago
Attachment: | login_required_active.diff added |
---|
comment:1 by , 15 years ago
Needs tests: | set |
---|---|
Patch needs improvement: | set |
Triage Stage: | Unreviewed → Accepted |
It's an edge case, but one worth catching.
There is an edge case that the proposed patch doesn't catch - the case of a custom user object (from a custom auth backend) that doesn't provide is_active (since auth backends aren't required to provide it). Changing the simple 'user.is_active' check to 'getattr(user, 'is_active', True)' should provide a backwards compatible solution.
Patch also requires tests.
follow-up: 9 comment:2 by , 15 years ago
comment:3 by , 15 years ago
Triage Stage: | Accepted → Design decision needed |
---|
Thanks for the historical pointers ramiro. Thinking about this a bit more, there are some edge cases that require some additional consideration. Discussion on django-dev
comment:4 by , 15 years ago
Cc: | added |
---|
comment:5 by , 14 years ago
Cc: | added |
---|
comment:6 by , 14 years ago
Type: | → Bug |
---|
comment:7 by , 14 years ago
Severity: | → Normal |
---|
comment:8 by , 13 years ago
Easy pickings: | unset |
---|
This should be changed, fast! It's a silly design decision!
comment:9 by , 13 years ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
UI/UX: | unset |
See ramiro's comments above. is_active
is a hook for custom auth sources and custom auth logic; the built-in stuff doesn't check it. Yes it's a bit counter-intuitive, but changing it is going to break a number of expectations in users' code, so it's going to need to stay as is.
Add "is_active" check to login_required