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_URL
s.
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 i see that tests have been made in https://github.com/django/django/pull/12692.
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