Opened 17 years ago

Closed 14 years ago

Last modified 13 years ago

#5605 closed (fixed)

Username email address part is case sensitive

Reported by: m.gajda@… Owned by: Leo Shklovskii
Component: Contrib apps Version: dev
Severity: Keywords: auth email lowerase easy-pickings
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I've discovered that create_user from User model manager (auth contrib module) lowercases newly created user's email address. The RFC document says, that only domain part of email address is case insensitive, so I think that lowercasing the username part is a bug. See RFC 821 and RFC 1035.

Please correct it, by lowercasing only domain name part of email address.

Attachments (4)

5605_EmailCase.diff (986 bytes ) - added by drowe 17 years ago.
Patch for lowercasing domain, but leaving user unchanged.
5605_new.diff (2.9 KB ) - added by Leo Shklovskii 16 years ago.
more pythonic fix, along with tests and doc changes
lowercase-email.diff (2.3 KB ) - added by Brodie Rao 16 years ago.
Updated to work with email addresses without at signs, including a test for that case
5605_new.patch (2.5 KB ) - added by Leo Shklovskii 14 years ago.
revised patch against r12623

Download all attachments as: .zip

Change History (19)

by drowe, 17 years ago

Attachment: 5605_EmailCase.diff added

Patch for lowercasing domain, but leaving user unchanged.

comment:1 by drowe, 17 years ago

Please see the attached patch. I'm new to Django / Python development, but it does work. Hopefully it doesn't cause too much heartburn. The patch lowercases the domain section, but leaves the user / account information unchanged.

comment:2 by drowe, 17 years ago

Has patch: set
Triage Stage: UnreviewedDesign decision needed

Changing status. Is this something the core devs want in the code? How many mail clients / MTAs adhere to the RFC on case-sensitivity?

comment:3 by Malcolm Tredinnick, 17 years ago

Triage Stage: Design decision neededAccepted

Yes, we should apply this. There are plenty of MTA's out there that still match in a case-sensitive fashion (I've even inadvertently set up a few in that fashion -- cyrus-imap works like that out of the box, for example).

comment:4 by Chris Beaven, 16 years ago

Keywords: easy-pickings added

comment:5 by Leo Shklovskii, 16 years ago

Owner: changed from nobody to Leo Shklovskii
Status: newassigned

by Leo Shklovskii, 16 years ago

Attachment: 5605_new.diff added

more pythonic fix, along with tests and doc changes

comment:6 by Leo Shklovskii, 16 years ago

I've added a more pythonic patch along with a unit test and a documentation change to note the behavior.

comment:7 by Chris Beaven, 16 years ago

Patch needs improvement: set

Reviewing the patch, I see that it's going to fall over if the user doesn't pass in an email address with an '@' (e.g. an empty string).

>>> a, b = ''.split('@', 1)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: need more than 1 value to unpack

by Brodie Rao, 16 years ago

Attachment: lowercase-email.diff added

Updated to work with email addresses without at signs, including a test for that case

comment:8 by Brodie Rao, 16 years ago

Also, I should note that the '@'.join() call happily works with unicode objects. I removed the Unicode test in the patch thinking it wasn't relevant, but I can see how it might have been an issue.

comment:9 by Leo Shklovskii, 15 years ago

Patch needs improvement: unset

Thanks brodie, removing needs better patch flag.

by Leo Shklovskii, 14 years ago

Attachment: 5605_new.patch added

revised patch against r12623

comment:10 by Leo Shklovskii, 14 years ago

I've just updated this patch to work against the current trunk. It would be great to get it merged in before 1.2.

comment:11 by Leo Shklovskii, 14 years ago

milestone: 1.2

comment:12 by Jacob, 14 years ago

Resolution: fixed
Status: assignedclosed

(In [12641]) Fixed #5605: only lowercase the domain portion of an email address in UserManager.create_user.

Thanks, Leo.

comment:13 by Jacob, 14 years ago

(In [12642]) [1.1.X] Fixed #5605: only lowercase the domain portion of an email address in UserManager.create_user.

Thanks, Leo.

Backport of [12641] from trunk.

comment:14 by erikwright, 14 years ago

Out of curiosity, why is it important to lowercase even the domain? Users are not aware of or interested in these case-sensitivity rules but probably expect to see the case that they entered preserved.

In fact, this causes an issue for a Digest authentication package that I maintain (http://bitbucket.org/akoha/django-digest/). We wish to allow users to authenticate via Digest auth using their username or a verified email address. We need to pre-calculate partial digests for all possible (case-sensitive) logins. If a user signs up with the email address 'SomeUser@Example.com' we want to allow them to authenticate with either the user supplied case OR the lower case version.

If the user-supplied case is discarded by Django we must keep our own copy in order to support this behaviour.

At the moment the only Digest client we support is in-house, so that client forcibly lowercases logins, avoiding this problem. But as we begin supporting other clients, this may be an issue. And I don't understand why it would ever be important to store the lower case domain name.

comment:15 by Jacob, 13 years ago

milestone: 1.2

Milestone 1.2 deleted

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