#8342 closed (fixed)
The admin wrongly assumes you can't login with your email
Reported by: | Julien Phalip | Owned by: | nobody |
---|---|---|---|
Component: | contrib.admin | Version: | dev |
Severity: | Keywords: | ||
Cc: | ramusus@…, daniel@… | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Description
On one of my sites, I have a custom authentication backend which allows you to login with your email, and just your email (the username
attribute is basically ignored in the process).
It works fine, you can login both on the front-end site and on the admin site with your email.
However, if you mistype the email address or try to login with an email that doesn't exist, in the admin, then you get the message "Usernames cannot contain the '@' character.".
I don't know what's the best approach to fix this. I report now and will try to think of a patch.
This is something that I think is important to fix before 1.0, as that error message is quite misleading.
Attachments (1)
Change History (21)
comment:1 by , 16 years ago
Resolution: | → duplicate |
---|---|
Status: | new → closed |
comment:2 by , 16 years ago
Sure.. but this ticket was not to include email authentication in core, like in #7591, but to make customization a bit more flexible. The admin delegates authentication to custom backends but then wrongly assumes it should be a certain way, I maintain that. I believe the right approach would have been to provide some overridable hook to customize the display of error messages. Anyway...
follow-up: 6 comment:3 by , 16 years ago
Resolution: | duplicate |
---|---|
Status: | closed → reopened |
I don't think this is a duplicate of #7591. #7591 asked to include email authentication in core, whereas this ticket is asking for more flexibility in customisation.
The admin willingly delegates authentication to whatever backend you've set up:
user = authenticate(username=username, password=password)
That's great, but then why assume that you can't authenticate with an email? I understand that this is the default behaviour, but it should be made easy to override that behaviour.
If you decide to wontfix this, it would be appreciated if you could give some pointers to other clean ways to achieve this.
comment:4 by , 16 years ago
Has patch: | set |
---|
The attached patch adds a hook for displaying a custom error message when login fails. You can then simply override it to customise it to your heart's content:
class CustomAdminSite(AdminSite): def get_login_error_message(self, request, is_authenticated, username, password): return 'blah' admin.site = CustomAdminSite()
by , 16 years ago
Attachment: | 8342.login.admin.error.diff added |
---|
comment:5 by , 16 years ago
Just another thought on the patch. Would it be best to send a boolean is_authenticated
(as in current patch) or to send the actual user. I suspect there might be cases where you'd like to get some information from the user to display a custom message:
class CustomAdminSite(AdminSite): def get_login_error_message(self, request, user, username, password): if user is not None: return 'blah: %s' % (user.whatever) else: return 'blah'
comment:6 by , 16 years ago
Resolution: | → wontfix |
---|---|
Status: | reopened → closed |
Replying to julien:
That's great, but then why assume that you can't authenticate with an email? I understand that this is the default behaviour, but it should be made easy to override that behaviour.
It is easy to override that behavior. Just override AdminSite.login()
, which you'll probably want to do anyway if you're using a non-default authentication scheme.
comment:7 by , 16 years ago
Fair enough for the wontfix. Overriding AdminSite.login()
is not what I call a clean way though. That's still 56 lines of code to hack... On a final note, that view does already allow non-default authentication schemes. What it does wrong is the error message it displays when login fails...
comment:8 by , 16 years ago
I see that #8878 was closed as duplicate.
Hmmm... the more I think about this, the more I really think this should be fixed. The current behaviour is just wrong.
Your suggestion of overriding AdminSite.login()
defeats the purpose of custom authentication backends altogether.
The fact is: email authentication is becoming increasingly popular and inevitably a growing number of users are going to get hit by this. Again, I'm not saying email authentication should be in core, I'm saying the admin should not make any assumption about the way you're supposed to login. The admin should make it easy and clean to override the behaviour.
comment:9 by , 15 years ago
Cc: | added |
---|
comment:10 by , 14 years ago
Cc: | added |
---|---|
Resolution: | wontfix |
Status: | closed → reopened |
Is there any way to upvote tickets? If I could I would upvote this one. There are so many websites using the email address as username. The '@' symbol should be valid by default.
What's wrong with julien's code?
comment:11 by , 14 years ago
Resolution: | → wontfix |
---|---|
Status: | reopened → closed |
Per the docs, when you disagree with a wontfix the thing to do is take it to the django-developers mailing list to try to build consensus for your view, not to just reopen the ticket.
comment:12 by , 14 years ago
milestone: | 1.0 → 1.3 |
---|---|
Resolution: | wontfix |
Status: | closed → reopened |
Triage Stage: | Unreviewed → Accepted |
Actually this needs to be fixed, since r12634 has changed the rules for usernames to allow '@'. That message, which is still present in current trunk, is flat-out wrong now, even if you are not doing anything funky to allow login with email address instead of username.
comment:13 by , 14 years ago
Patch needs improvement: | set |
---|
See #13928 and note that same code/message is currently present in at least two places. Both needs to be fixed.
comment:14 by , 14 years ago
Honestly, I don't understand why there's any validation on the log-in form other than to authenticate the given credentials. That kind of validation should only be done when creating or modifying a user, not when logging in. In fact, best security practices dictate that the only error message you should show when logging in is "the username and/or password is incorrect".
comment:15 by , 14 years ago
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
Duplicate of #7591, which was marked wontfix by one of Django's lead developers.