Opened 9 years ago

Closed 7 years ago

Last modified 5 years ago

#5786 closed (fixed)

Relax character restrictions on auth usernames

Reported by: Armin Ronacher Owned by: nobody
Component: contrib.auth Version: 1.1
Severity: Keywords: username, character, restriction
Cc: gaobo@…, eric@…, bronger@…, leon.dignon@…, alex-django@…, lbruno@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

We have to use the auth module with legacy data and need to support more characters. Additionally the regexp there disallows german umlauts. There should be a way to configure the validator used for the username without monkeypatching.

Attachments (8)

form.diff (3.1 KB) - added by Bob Gao 9 years ago.
a patch for this ticket
form.1.patch (3.6 KB) - added by Bob Gao 9 years ago.
an improved version
auth.1.path (2.3 KB) - added by Bob Gao 9 years ago.
5786.django.contrib.auth.diff (3.3 KB) - added by Pete Crosier 9 years ago.
A more liberal approach to relaxing username restrictions
django-auth-username-characters.patch (1.8 KB) - added by alextreme 7 years ago.
Allows common punctuation characters in username
overrides.py (1.5 KB) - added by leond 7 years ago.
django-auth-username-characters-round2.patch (2.5 KB) - added by Luis Bruno 7 years ago.
add to alextreme's patch: change models.py help_text
django-auth-username-relax-character-restrictions.diff (4.2 KB) - added by clayg 7 years ago.
diff from the top-level trunk directory including tests

Download all attachments as: .zip

Change History (45)

comment:1 Changed 9 years ago by Simon G <dev@…>

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Triage Stage: UnreviewedAccepted

comment:2 Changed 9 years ago by Wang Chun <wangchun@…>

Cc: wangchun@… added

comment:3 Changed 9 years ago by Bob Gao

Cc: gaobo@… added; wangchun@… removed

comment:4 Changed 9 years ago by munhitsu

What about adding sth like this to settings.py

AUTH_USER_VALIDATORS = (validators.myValidator)

Default settings.py and Django level settings will contain a line like:

AUTH_USER_VALIDATORS = (isAlphaNumeric)

comment:5 Changed 9 years ago by Jacob

I'm not at all a fan of a list of validators in the settings module, but yes, we should relax characters allowed in usernames -- another common request is to allow the use of email addresses as usernames.

Custom registration/signup forms can deal with further restrictions.

Changed 9 years ago by Bob Gao

Attachment: form.diff added

a patch for this ticket

comment:6 Changed 9 years ago by Bob Gao

Has patch: set
Owner: changed from nobody to Bob Gao
Status: newassigned

comment:7 Changed 9 years ago by Marc Fargas

Patch needs improvement: set

Reviewing the patch,

  • the setting should be r'\w+$' (the 'r' to denote it's a regular expression).
  • the setting should be documented in docs/settings.txt and (maybe) docs/authentication.txt not only on the tutorial.

Anyway, this will solve that on oldforms, contrib.auth still uses them!

Maybe when contrib.auth moves to newforms we could think about making it easy to subclass contrib.auth forms ;)

Changed 9 years ago by Bob Gao

Attachment: form.1.patch added

an improved version

comment:8 Changed 9 years ago by Bob Gao

yes, i just slove the problem on oldforms.

comment:9 Changed 9 years ago by Bob Gao

Triage Stage: AcceptedReady for checkin

comment:10 Changed 9 years ago by Jacob

Summary: contrib.auth.models.User username regexp should be configurableRelax character restrictions on auth usernames
Triage Stage: Ready for checkinAccepted

I'm still not a big fan of this patch. We don't need Yet Another Setting; we just need to relax the current restrictions.

Also, luli, please don't mark your own tickets as "Ready for checkin"; a triager or contributer needs to do that.

Changed 9 years ago by Bob Gao

Attachment: auth.1.path added

comment:11 Changed 9 years ago by Bob Gao

i have committed another patch

comment:12 Changed 9 years ago by Pete Crosier

Think there's an issue with the old django/contrib/auth/tests.py which is breaking the pretty diff display unfortunately - the patch I've attached goes really liberal on usernames. At the moment, User.objects.create_user doesn't actually enforce the current restrictions, hence it not needing to be changed in the patch.

Changed 9 years ago by Pete Crosier

A more liberal approach to relaxing username restrictions

comment:13 Changed 9 years ago by Pete Crosier

OK, the pretty diff is working after removing the "\ No newline at end of file" which was causing the breakage!

comment:14 Changed 9 years ago by railk

The patch does not apply properly without the "\ No newline at end of file". Sounds like the pretty diff is broken.

comment:15 Changed 8 years ago by Michael Richardson <michael@…>

See also: #3011

comment:16 Changed 8 years ago by Éric St-Jean

Cc: eric@… added

comment:17 Changed 8 years ago by dc

Also, see #6588 for a patches.

comment:18 Changed 8 years ago by Torsten Bronger

Cc: bronger@… added

comment:19 Changed 7 years ago by Torsten Bronger

I think it's time to merge the ideas on this one.

According to this issue, we want to have all alphanumeric characters from Unicode allowed in usernames. According to #6588, also dots should be allowed, and according to <http://permalink.gmane.org/gmane.comp.python.django.user/88509>, dashes might be highly sensible, too.

So what about re.compile(r'^[-@.\w]+$', re.U)? Or do we need even more? (E.g. :#*+~'$%&()[]{}!^)

comment:20 Changed 7 years ago by anonymous

The basic problem involves having a username/restrictive user model in general... see #3011

comment:21 Changed 7 years ago by Torsten Bronger

So we should close this one, and post links to #6588 and this one to #3011?

comment:22 Changed 7 years ago by Torsten Bronger

Actually, I think we should consider them separate issues. #3011 is a large beast, and this one is pretty light-weight, so I'm afraid of waiting for a solution much longer than necessary. Additionally, we discuss about changing Django's default behaviour here, whereas #3011 talks about extensibility. I think that Django should support more usernames out-of-the-box.

The demand that occurs again and again (on the mainglist and in the tickets) is that you have to include existing user databases. In my case, we must authenticate against an external Windows Active Directory with pre-existing usernames. I think that this is an important use case, and I think that characters like dot, dash, and plus are pretty common outside Django.

comment:23 Changed 7 years ago by leond

Cc: leon.dignon@… added
Component: Contrib appsAuthentication
Has patch: unset
Keywords: username character restriction added; auth removed
Owner: changed from Bob Gao to leond
Patch needs improvement: unset
Status: assignednew
Triage Stage: AcceptedUnreviewed
Version: SVN

I really don't understand the ticket system very well. That's why I am always asking myself, after finding some tickets discussing a problem I have at the moment, why this takes so long? This ticket was opened two years ago and I see that this is a common problem which is demanded again and again. So for the sake of the principles of DRY this ticket should be considered of.

There should be implemented something like a setting to customize the regex strings used in auth/forms.py.

I will change some properties, because they are out-of-date.

  • lulin's last comment was in 2007, I will reassign this ticket to anonymous.
  • Further I change the triage stage to unreviewed.
  • The Component is clearly authentication.

comment:24 Changed 7 years ago by leond

Owner: changed from leond to nobody

comment:25 in reply to:  23 Changed 7 years ago by Karen Tracey

Replying to leond:

Understanding the ticket system does not have a lot to do with understanding why something may take a long time. It isn't the ticket system or any process associated with it that holds things up. In this particular case the original patches apparently added yet another setting, which Jacob is opposed to (see http://code.djangoproject.com/ticket/5786#comment:10). So calling for another setting to fix this is not likely to move things forward. Nor is changing the ticket state from Accepted to Unreviewed -- why did you feel Accepted was "out of date"? Jacob, in that same comment, indicated he agrees that the character restrictions need to be relaxed. So near as I can see this ticket should still be Accepted. The triage states are described here:

http://docs.djangoproject.com/en/dev/internals/contributing/#ticket-triage

Another reason this ticket got slowed down is likely that the fixes proposed involved changing the validators for the field, and with newforms in progress validators were on their way out (to be replaced by model validation, which is still not yet in trunk). Changing code that is about to be removed/replaced is generally not high priority. I don't see that anyone has posted what changes are actually required now that newforms have replaced oldforms and validators are gone.

Changed 7 years ago by alextreme

Allows common punctuation characters in username

comment:26 Changed 7 years ago by alextreme

Cc: alex-django@… added
Has patch: set
Triage Stage: UnreviewedDesign decision needed
Version: 1.1

I've included a very simple patch (django-auth-username-characters.patch, 6 lines in 1 file) against Django 1.1 that eases the username restrictions. It allows the following characters (next to alphanumerics): @.+-_

The auth system actually works perfectly fine with these characters, it's only the admin forms that restrict input. This probably was different in the past.

I'm not sure if there was a rationale on restricting usernames (perhaps their use in URLs?) but this patch only allows a few more characters, those that probably cause the most annoyances.

No major changes, no extra settings and good enough until #3011 sees some progress. Tests and a review are welcome however.

Changed 7 years ago by leond

Attachment: overrides.py added

comment:27 Changed 7 years ago by leond

I did it this way! I just overwrote these classes and now it's working and it's really easy!

Changed 7 years ago by Luis Bruno

add to alextreme's patch: change models.py help_text

comment:28 Changed 7 years ago by Luis Bruno

Cc: lbruno@… added
milestone: 1.2

This is what I'm using in my in-house Django fork. Can this find its way into the 1.2 series?

comment:29 Changed 7 years ago by Russell Keith-Magee

milestone: 1.2

comment:30 Changed 7 years ago by clayg

Triage Stage: Design decision needed

"we just need to relax the current restrictions"

lbruno's patch looks good to me...

Changed 7 years ago by clayg

diff from the top-level trunk directory including tests

comment:31 Changed 7 years ago by clayg

patch -p1 < django-auth-username-relax-character-restrictions.diff
applies cleanly from the root directory

./tests/runtests.py --settings=settings auth


Ran 44 tests in 1.649s

OK

Will the documentation get updated from the doc strings?
e.g.
http://docs.djangoproject.com/en/1.1/topics/auth/#topics-auth

comment:32 Changed 7 years ago by leond

First I was convinced that this had to be changed. Later I discovered a good workaround because of the following: If you change this in the module code, there will be developers who may be dissatisfied with the relaxed restrictions. Or there may be developers who still are not satisfied. It may be that there are alway unsatisfied developers, no matter how you rewrite the module code. So the best solution (in my opinion) is to simply put a piece of code into your own admin.py and your own forms.py. You can find it in the attachments. I uploaded them several months ago and it works really fine.

I am completely against changing the module code as that never satisfies every single developer! Please use the workaround I posted or take it as an idead to code a better one.

comment:33 Changed 7 years ago by Torsten Bronger

Please change the character restrictions to the best common practice. It is highly unusual to forbid dash or dot in login names. Moreover, email addresses as logins are very common.

It is not about satisfying all; that's never the case for default values. Instead, it's about satisfying almost all. Given that almost all logins that I know allow more characters, I see no real use case for the very limited set being the default. From this it also follows that only very few Django installation are expected to break due to relaxed restrictions.

comment:34 Changed 7 years ago by Torsten Bronger

Triage Stage: Design decision neededAccepted

I restore the "Accepted" tag of Jacob as in <http://code.djangoproject.com/ticket/5786#comment:10>. I don't see why this was removed. As far as I can see, we have patches and tests now that use newforms, so the issues mentioned in http://code.djangoproject.com/ticket/5786#comment:25 are addressed.

Could some ticket triager come over and check the latest patch please? So that it can be marked "Ready for checkin". Thank you.

comment:35 Changed 7 years ago by Jacob

milestone: 1.2

comment:36 Changed 7 years ago by Jacob

Resolution: fixed
Status: newclosed

(In [12634]) Fixed #5786: relaxed the validation for usernames to allow more common characters '@', etc.

This is really just a stop-gap until we come up with a improved way of handling
disparate auth data, but it should help us stretch a bit more milage out of the
current system.

Thanks to alextreme, lbruno, and clayg.

comment:37 Changed 5 years ago by Jacob

milestone: 1.2

Milestone 1.2 deleted

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