Opened 8 years ago

Closed 5 years ago

Last modified 4 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 luli 7 years ago.
a patch for this ticket
form.1.patch (3.6 KB) - added by luli 7 years ago.
an improved version
auth.1.path (2.3 KB) - added by luli 7 years ago.
5786.django.contrib.auth.diff (3.3 KB) - added by PJCrosier 7 years ago.
A more liberal approach to relaxing username restrictions
django-auth-username-characters.patch (1.8 KB) - added by alextreme 6 years ago.
Allows common punctuation characters in username
overrides.py (1.5 KB) - added by leond 6 years ago.
django-auth-username-characters-round2.patch (2.5 KB) - added by lbruno 5 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 5 years ago.
diff from the top-level trunk directory including tests

Download all attachments as: .zip

Change History (45)

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

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

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

  • Cc wangchun@… added

comment:3 Changed 7 years ago by luli

  • Cc gaobo@… added; wangchun@… removed

comment:4 Changed 7 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 7 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 7 years ago by luli

a patch for this ticket

comment:6 Changed 7 years ago by luli

  • Has patch set
  • Owner changed from nobody to luli
  • Status changed from new to assigned

comment:7 Changed 7 years ago by telenieko

  • 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 7 years ago by luli

an improved version

comment:8 Changed 7 years ago by luli

yes, i just slove the problem on oldforms.

comment:9 Changed 7 years ago by luli

  • Triage Stage changed from Accepted to Ready for checkin

comment:10 Changed 7 years ago by jacob

  • Summary changed from contrib.auth.models.User username regexp should be configurable to Relax character restrictions on auth usernames
  • Triage Stage changed from Ready for checkin to Accepted

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 7 years ago by luli

comment:11 Changed 7 years ago by luli

i have committed another patch

comment:12 Changed 7 years ago by PJCrosier

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 7 years ago by PJCrosier

A more liberal approach to relaxing username restrictions

comment:13 Changed 7 years ago by PJCrosier

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

comment:14 Changed 7 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 7 years ago by Michael Richardson <michael@…>

See also: #3011

comment:16 Changed 7 years ago by wiswaud

  • Cc eric@… added

comment:17 Changed 6 years ago by dc

Also, see #6588 for a patches.

comment:18 Changed 6 years ago by bronger

  • Cc bronger@… added

comment:19 Changed 6 years ago by 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 6 years ago by anonymous

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

comment:21 Changed 6 years ago by bronger

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

comment:22 Changed 6 years ago by 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 follow-up: Changed 6 years ago by leond

  • Cc leon.dignon@… added
  • Component changed from Contrib apps to Authentication
  • Has patch unset
  • Keywords username character restriction added; auth removed
  • Owner changed from luli to leond
  • Patch needs improvement unset
  • Status changed from assigned to new
  • Triage Stage changed from Accepted to Unreviewed
  • Version SVN deleted

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

  • Owner changed from leond to nobody

comment:25 in reply to: ↑ 23 Changed 6 years ago by kmtracey

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

Allows common punctuation characters in username

comment:26 Changed 6 years ago by alextreme

  • Cc alex-django@… added
  • Has patch set
  • Triage Stage changed from Unreviewed to Design decision needed
  • Version set to 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 6 years ago by leond

comment:27 Changed 6 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 5 years ago by lbruno

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

comment:28 Changed 5 years ago by lbruno

  • Cc lbruno@… added
  • milestone set to 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 5 years ago by russellm

  • milestone 1.2 deleted

comment:30 Changed 5 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 5 years ago by clayg

diff from the top-level trunk directory including tests

comment:31 Changed 5 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 5 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 5 years ago by 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 5 years ago by bronger

  • Triage Stage changed from Design decision needed to Accepted

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

  • milestone set to 1.2

comment:36 Changed 5 years ago by jacob

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

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

  • milestone 1.2 deleted

Milestone 1.2 deleted

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