#30154 closed Cleanup/optimization (wontfix)
i18n: redirects to default login page if LOGIN_URL = 'login' not specified
Reported by: | Oskar Haller | Owned by: | nobody |
---|---|---|---|
Component: | Documentation | Version: | dev |
Severity: | Normal | Keywords: | i18n LOGIN_URL |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Hi,
Setup: a multilanguage page that uses django.contrib.auth.
Bug: A request to a view that is only visible for authorized.users is redirected to the default login page and not the language specific login page. Unless LOGIN_URL is specified in settings.py.
Example:
Wrong behavior: request /de/accounts/password_change/ -> /accounts/login/?next=/de/accounts/password_change/
Correct behavior (if LOGIN_URL is set): request /de/accounts/password_change/ -> /de/accounts/login/?next=/de/accounts/password_change/
Attachments (1)
Change History (13)
comment:1 by , 6 years ago
Resolution: | → needsinfo |
---|---|
Status: | new → closed |
comment:2 by , 6 years ago
Hi Carlton,
I added a minimal example that includes the bug.
Have a look at settings.py line 57. There you can turn off the behavior by setting the LOGIN_URL.
Thanks
comment:3 by , 6 years ago
Resolution: | needsinfo |
---|---|
Status: | closed → new |
Super. Thanks Oskar! I will have a look this afternoon.
comment:4 by , 6 years ago
Component: | Internationalization → Documentation |
---|---|
Has patch: | set |
Triage Stage: | Unreviewed → Accepted |
Type: | Bug → Cleanup/optimization |
Version: | 2.1 → master |
OK, I see.
The issue is that /accounts/login/
, not being a named URL pattern doesn't go through the whole i18n_patterns()
dance, so django.shortcuts.resolve_url()
doesn't get a i18n'd version.
There's not much we can do about this at the behaviour level. I've opened a PR on the documentation to hopefully clarify it.
Thanks for the report, and the sample project!
comment:5 by , 6 years ago
Wouldn't changing the default LOGIN_URL from LOGIN_URL=/accounts/login/
to LOGIN_URL='login'
be a superior fix?
I created a PR https://github.com/django/django/pull/10929
comment:6 by , 6 years ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
Hi Oskar.
The trouble with that is it would create a dependency on contrib.auth
, or at least having the views registered with those names.
It would also be a breaking change, one not worth the disruption.
On review we don't think the PR is worth it. So we're going to opt for wontfix
here.
Thanks again.
comment:8 by , 6 years ago
Hi again,
one last thing since also your PR did not make it.
What do you think about some text below here https://docs.djangoproject.com/en/2.1/topics/i18n/translation/#module-django.conf.urls.i18n, here https://docs.djangoproject.com/en/2.1/topics/i18n/translation/#implementation-notes, or here https://docs.djangoproject.com/en/2.1/topics/auth/default/#using-the-views ?
I think of something like 'internationaliziation and authentication'.
The thing with the current situation is that the wrong behavior is difficult to detect as everything seems to be working fine out of the box.
follow-up: 12 comment:9 by , 6 years ago
Hi Oskar. Yes, I was half
comment:12 by , 6 years ago
Replying to Carlton Gibson:
Hi Oskar. Yes, I was half-thinking that. If you want to draft up a PR with an addition in an appropriate place, that would be great!
What do you think of the proposed PR?
Hi Oskar.
Can I ask for more info here please?
Off the top, I can't quite see how any issue is arriving.
LOGIN_URL
is always set; if you don't set it it comes from the global settings (and is/accounts/login/
)Then,
django.contrib.auth.views.redirect_to_login()
which is the function in play here just fetchessettings.LOGIN_URL
, so doesn't explain your report. (You're not hardcoding thede
I presume...)Is this all
@login_required
views or justcontrib.auth
?Can you upload a minimal example project demonstrating your issue? (Or otherwise give an actual reproduce.)
Thanks!