Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#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)

minimal_example.zip (8.1 KB ) - added by Oskar Haller 6 years ago.
Have a look at settings.py line 57

Download all attachments as: .zip

Change History (13)

comment:1 by Carlton Gibson, 6 years ago

Resolution: needsinfo
Status: newclosed

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 fetches settings.LOGIN_URL, so doesn't explain your report. (You're not hardcoding the de I presume...)

Is this all @login_required views or just contrib.auth?

Can you upload a minimal example project demonstrating your issue? (Or otherwise give an actual reproduce.)

Thanks!

by Oskar Haller, 6 years ago

Attachment: minimal_example.zip added

Have a look at settings.py line 57

comment:2 by Oskar Haller, 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

Last edited 6 years ago by Oskar Haller (previous) (diff)

comment:3 by Carlton Gibson, 6 years ago

Resolution: needsinfo
Status: closednew

Super. Thanks Oskar! I will have a look this afternoon.

comment:4 by Carlton Gibson, 6 years ago

Component: InternationalizationDocumentation
Has patch: set
Triage Stage: UnreviewedAccepted
Type: BugCleanup/optimization
Version: 2.1master

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 Oskar Haller, 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

Last edited 6 years ago by Oskar Haller (previous) (diff)

comment:6 by Carlton Gibson, 6 years ago

Resolution: wontfix
Status: newclosed

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.

Last edited 6 years ago by Carlton Gibson (previous) (diff)

comment:7 by Oskar Haller, 6 years ago

Ah, I see.

comment:8 by Oskar Haller, 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.

Last edited 6 years ago by Oskar Haller (previous) (diff)

comment:9 by Carlton Gibson, 6 years ago

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!

Last edited 6 years ago by Carlton Gibson (previous) (diff)

comment:10 by Oskar Haller, 6 years ago

Sure, might take a while - but I'll do it.

in reply to:  9 comment:12 by Oskar Haller, 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?

Note: See TracTickets for help on using tickets.
Back to Top