Opened 18 years ago
Closed 18 years ago
#3185 closed enhancement (fixed)
Allow django.contrib.auth's LOGIN_URL, LOGOUT_URL and ACCOUNT_URL to be set in settings.py
Reported by: | Owned by: | Adrian Holovaty | |
---|---|---|---|
Component: | Contrib apps | Version: | dev |
Severity: | normal | Keywords: | |
Cc: | Vasily, Sulatskov, <redvasily@…>, jonasvp@… | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Description
Currently it's impossible to change LOGIN_URL varaible which is used in django.contrib.auth
I found this discussion on mailing list.
Adrian Holovaty said that LOGIN_URL should be made configurable. I made tiny patch to make it possible to specify LOGIN_URL in site settings.py module. This patch works at least for me and doesn't break anything else (as far as I know).
Attachments (9)
Change History (31)
by , 18 years ago
Attachment: | login_url.diff added |
---|
comment:1 by , 18 years ago
Summary: | Possibility of overriding django.contrib.auth.LOGIN_URL in settings.py → [patch]Possibility of overriding django.contrib.auth.LOGIN_URL in settings.py |
---|
comment:2 by , 18 years ago
comment:3 by , 18 years ago
Cc: | added |
---|---|
Summary: | [patch]Possibility of overriding django.contrib.auth.LOGIN_URL in settings.py → [patch]Possibility of overriding django.contrib.auth.LOGIN_URL and account URL used in auth.views,login() in settings.py |
I noticed that I myself use custom version of django.contrib.auth.views.login() that used different account URL, so I made it configurable also.
Patch: login_url2.diff
by , 18 years ago
Attachment: | login_url2.diff added |
---|
comment:4 by , 18 years ago
Summary: | [patch]Possibility of overriding django.contrib.auth.LOGIN_URL and account URL used in auth.views,login() in settings.py → [patch]Possibility of overriding django.contrib.auth.LOGIN_URL and account URL used in auth.views.login() in settings.py |
---|
comment:5 by , 18 years ago
Keywords: | auth LOGIN_URL removed |
---|---|
Needs documentation: | set |
Summary: | [patch]Possibility of overriding django.contrib.auth.LOGIN_URL and account URL used in auth.views.login() in settings.py → Allow django.contrib.auth's LOGIN_URL and ACCOUNT_URL to be set in settings.py |
Triage Stage: | Unreviewed → Accepted |
Adrian gave the ok in the thread mentioned in the ticket description.
follow-up: 8 comment:6 by , 18 years ago
Needs documentation: | unset |
---|
Documentation added, please take one thing in account:
I set a link to @login_decorator twice (in the ACCOUNT_URL and LOGIN_URL sections) thus the line ".. _@login_decorator" is at the bottom of the file, I have no idea if this works, and this silly PC doesn't want to download docutils now to let me check... so please take this into account before appliying!
Also, I have no idea if the "@" can be there on the link... ;)
Hope I didn't miss anything to change.
comment:7 by , 18 years ago
Some more advocacy for this ticket: in this thread Why I'm giving up on Django one of the reasons for giving Django up was:
"This is a biggie for me. I can't believe that the authentication
module forces you to use hard coded urls for login/logout pages --
that's just maddening!"
Ahd that's what this patch fixes.
comment:8 by , 18 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
Replying to Marc Fargas <telenieko@telenieko.com>:
Documentation added, please take one thing in account:
I set a link to @login_decorator twice (in the ACCOUNT_URL and LOGIN_URL sections) thus the line ".. _@login_decorator" is at the bottom of the file, I have no idea if this works, and this silly PC doesn't want to download docutils now to let me check... so please take this into account before appliying!
Also, I have no idea if the "@" can be there on the link... ;)
Seems to work for me. Thanks for the patch.
comment:9 by , 18 years ago
Patch needs improvement: | set |
---|---|
Triage Stage: | Ready for checkin → Accepted |
After playing around with contrib.comments
, it appears that we should probably have a LOGOUT_URL
also since in the comment app's form.html
template there is a hardcoded /accounts/logout/
. This would be simple enough to add, that we should just add it to this ticket instead of creating a new one.
comment:10 by , 18 years ago
Needs documentation: | set |
---|---|
Summary: | Allow django.contrib.auth's LOGIN_URL and ACCOUNT_URL to be set in settings.py → Allow django.contrib.auth's LOGIN_URL, LOGOUT_URL and ACCOUNT_URL to be set in settings.py |
+1
Adding need documentation to document the settings.LOGOUT_URL when the patch comes in.
by , 18 years ago
Attachment: | 3185_3.diff added |
---|
Added LOGOUT_URL variable to auth/init.py. Modified comments app to use it.
comment:11 by , 18 years ago
I didn't test 3185_3.diff because I don't use comments app myself. But changes were obvious so perhaps patch will work fine.
comment:12 by , 18 years ago
Needs documentation: | unset |
---|---|
Patch needs improvement: | unset |
Triage Stage: | Accepted → Ready for checkin |
comment:16 by , 18 years ago
Cc: | added |
---|
comment:17 by , 18 years ago
Patch needs improvement: | set |
---|
It's not clear to me which of these patches should be applied, and in which order. Can one of the patch authors upload an über-patch that does it all in one fell swoop?
comment:18 by , 18 years ago
I think that ACCOUNT_URL variable name is misleading, because it's not necessary to redirect user after successful login to account page. So I changed it to POSTLOGIN_REDIRECT_URL in 3185_4.diff.
3185_4.diff contains all needed changes.
comment:19 by , 18 years ago
Patch needs improvement: | unset |
---|
Removing "needs better patch" as Vasily uploaded what jacob requested ;)
comment:20 by , 18 years ago
Patch needs improvement: | set |
---|---|
Triage Stage: | Ready for checkin → Accepted |
The most recent patch still has a few subtle problems:
- Anything that is configurable should always be read from settings, not shoved in as a constant into some other module (it's not a constant any longer, after all). I realise LOGIN_URL is already a constant in the auth module, but that now becomes a proxy for the value from settings for backwards compatibility reasons. Everything else comes straight out of settings (including all current internal uses of LOGIN_URL).
- We should not be reading anything from settings at import time; only at execution time. The reason for this is manually configured settings. The developer needs to be able to import the necessary modules and then call
settings.configure()
. The current code reads from settings at import time and so will raise a configuration error. This problem exists in other parts of Django as well, but that's not a reason to perpetuate the problem.
by , 18 years ago
Attachment: | login_url.patch added |
---|
comment:21 by , 18 years ago
Took a stab at fixing the patch based on comments, as I would like this as well :)
comment:22 by , 18 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
While we're at it we should go ahead and make the profile URL configurable too, which is currently hardcoded in the contrib.auth.views.login view.