Opened 5 years ago
Closed 5 years ago
#31443 closed Bug (fixed)
LoginRequiredMixin/AccessMixin bug with off-site LOGIN_URL
| Reported by: | minusf | Owned by: | minusf |
|---|---|---|---|
| Component: | contrib.auth | Version: | dev |
| Severity: | Normal | Keywords: | login required |
| Cc: | Triage Stage: | Ready for checkin | |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | yes | UI/UX: | no |
Description
The class based "login required" mechanism lost a bit of functionality compared to its decorator sister: it always sends request.get_full_path(), even for off-site LOGIN_URLs.
https://github.com/django/django/blob/master/django/contrib/auth/mixins.py#L44
vs
https://github.com/django/django/blob/master/django/contrib/auth/decorators.py#L22-L33
The patch is trivial and i will make the PR after i'll know the ticket number.
Change History (10)
comment:1 by , 5 years ago
comment:2 by , 5 years ago
| Needs tests: | set |
|---|---|
| Triage Stage: | Unreviewed → Accepted |
OK, thanks. Looks right at a first glance, but the patch needs a test case showing the exact issue.
(Is user_passes_test covered for this case? Could add any missing tests in a separate commit...)
👍
comment:3 by , 5 years ago
i have added a test case. PermissionRequiredMixin is tested "indirectly" as it also inherits from AccessMixin.
comment:5 by , 5 years ago
comment:6 by , 5 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → assigned |
comment:7 by , 5 years ago
I am bit confused it says patch has been applied but it is not present in django master.
comment:8 by , 5 years ago
| Needs tests: | unset |
|---|---|
| Owner: | changed from to |
28Rahul28, this ticket already has a patch that is waiting for another review.
comment:9 by , 5 years ago
| Triage Stage: | Accepted → Ready for checkin |
|---|---|
| Version: | 3.0 → master |
https://github.com/django/django/pull/12692