Code

Opened 7 years ago

Closed 4 years ago

Last modified 3 years ago

#5605 closed (fixed)

Username email address part is case sensitive

Reported by: m.gajda@… Owned by: Leo
Component: Contrib apps Version: master
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: UI/UX:

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 7 years ago.
Patch for lowercasing domain, but leaving user unchanged.
5605_new.diff (2.9 KB) - added by Leo 6 years ago.
more pythonic fix, along with tests and doc changes
lowercase-email.diff (2.3 KB) - added by brodie 6 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 4 years ago.
revised patch against r12623

Download all attachments as: .zip

Change History (19)

Changed 7 years ago by drowe

Patch for lowercasing domain, but leaving user unchanged.

comment:1 Changed 7 years ago by drowe

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

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 Changed 7 years ago by drowe

  • Has patch set
  • Triage Stage changed from Unreviewed to Design 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 Changed 6 years ago by mtredinnick

  • Triage Stage changed from Design decision needed to Accepted

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 Changed 6 years ago by SmileyChris

  • Keywords easy-pickings added

comment:5 Changed 6 years ago by Leo

  • Owner changed from nobody to Leo
  • Status changed from new to assigned

Changed 6 years ago by Leo

more pythonic fix, along with tests and doc changes

comment:6 Changed 6 years ago by Leo

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

comment:7 Changed 6 years ago by SmileyChris

  • 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

Changed 6 years ago by brodie

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

comment:8 Changed 6 years ago by brodie

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 Changed 5 years ago by Leo

  • Patch needs improvement unset

Thanks brodie, removing needs better patch flag.

Changed 4 years ago by Leo

revised patch against r12623

comment:10 Changed 4 years ago by Leo

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 Changed 4 years ago by Leo

  • milestone set to 1.2

comment:12 Changed 4 years ago by jacob

  • Resolution set to fixed
  • Status changed from assigned to closed

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

Thanks, Leo.

comment:13 Changed 4 years ago by jacob

(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 Changed 4 years ago by erikwright

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 Changed 3 years ago by jacob

  • milestone 1.2 deleted

Milestone 1.2 deleted

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.