Opened 16 years ago
Closed 14 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 , 16 years ago
| Attachment: | login_required_active.diff added |
|---|
comment:1 by , 16 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 , 16 years ago
comment:3 by , 16 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 , 16 years ago
| Cc: | added |
|---|
comment:5 by , 15 years ago
| Cc: | added |
|---|
comment:6 by , 15 years ago
| Type: | → Bug |
|---|
comment:7 by , 15 years ago
| Severity: | → Normal |
|---|
comment:8 by , 14 years ago
| Easy pickings: | unset |
|---|
This should be changed, fast! It's a silly design decision!
comment:9 by , 14 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