Opened 3 years ago

Closed 11 months ago

Last modified 10 months ago

#20846 closed New feature (fixed)

Increase contrib.auth's User.username length

Reported by: ivoras@… Owned by: nobody
Component: contrib.auth Version: master
Severity: Normal Keywords:
Cc: cmawebsite@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Currently, contrib.auth's User model sets the username field's max_length at 30, the same as for first_name and last_name. This is reasonable but a bit conservative (http://www.kalzumeus.com/2010/06/17/falsehoods-programmers-believe-about-names/). Nowadays it is common to use e-mail addresses as usernames (especially if using external auth sources such as OAuth), but while the email field is reasonably long (75), the 30 characters for the username seems too short for modern sites.

I'd actually like to propose increasing all the naming fields (username, email, first_name, last_name) to 150, but I'll settle for making the username field as long as the e-mail field.

Patches:

--- models.py   2013-06-28 15:08:30.000000000 +0200
+++ /home/ivoras/temp/Django-1.6b1/django/contrib/auth/models.py        2013-08-01 18:13:01.280591492 +0200
@@ -366,8 +366,8 @@
 
     Username, password and email are required. Other fields are optional.
     """
-    username = models.CharField(_('username'), max_length=30, unique=True,
-        help_text=_('Required. 30 characters or fewer. Letters, numbers and '
+    username = models.CharField(_('username'), max_length=75, unique=True,
+        help_text=_('Required. 75 characters or fewer. Letters, numbers and '
                     '@/./+/-/_ characters'),
         validators=[
             validators.RegexValidator(re.compile('^[\w.@+-]+$'), _('Enter a valid username.'), 'invalid')

and

--- forms.py    2013-06-28 15:08:30.000000000 +0200
+++ /home/ivoras/temp/Django-1.6b1/django/contrib/auth/forms.py 2013-08-01 18:14:52.760588650 +0200
@@ -73,9 +73,9 @@
         'duplicate_username': _("A user with that username already exists."),
         'password_mismatch': _("The two password fields didn't match."),
     }
-    username = forms.RegexField(label=_("Username"), max_length=30,
+    username = forms.RegexField(label=_("Username"), max_length=75,
         regex=r'^[\w.@+-]+$',
-        help_text=_("Required. 30 characters or fewer. Letters, digits and "
+        help_text=_("Required. 75 characters or fewer. Letters, digits and "
                       "@/./+/-/_ only."),
         error_messages={
             'invalid': _("This value may contain only letters, numbers and "
@@ -123,8 +123,8 @@
 
 class UserChangeForm(forms.ModelForm):
     username = forms.RegexField(
-        label=_("Username"), max_length=30, regex=r"^[\w.@+-]+$",
-        help_text=_("Required. 30 characters or fewer. Letters, digits and "
+        label=_("Username"), max_length=75, regex=r"^[\w.@+-]+$",
+        help_text=_("Required. 75 characters or fewer. Letters, digits and "
                       "@/./+/-/_ only."),
         error_messages={
             'invalid': _("This value may contain only letters, numbers and "

Change History (34)

comment:1 Changed 3 years ago by Russell Keith-Magee

Resolution: wontfix
Status: newclosed

This *isn't* as simple as the change you suggest. If we applied the diff you describe, every Django user on the planet would need to issue a migration on their database, which is a *massive* backwards incompatibility.

You're also then into the territory where you are discussing whether 150 characters is enough for a username. Email addresses, for example, can be up to 254 characters and still be RFC compliant.

This is why Django 1.5 introduced the ability to define a custom User model -- this puts the control in the hands of the developer. See the docs and release notes for more details.

comment:2 Changed 3 years ago by ivoras@…

This *isn't* as simple as the change you suggest. If we applied the diff you describe, every Django user on the planet would need to issue a migration on their database, which is a *massive* backwards incompatibility.

Surely now that Django has a kind of built-in migration facility something can be done about it?

You're also then into the territory where you are discussing whether 150 characters is enough for a username. Email addresses, for example, can be up to 254 characters and still be RFC compliant.

Great, I'd happy with 254 characters limit, and it would be future proof as well.

I hope you realize that this change will have to be done sooner or later, so better do it sooner?

comment:3 in reply to:  2 Changed 3 years ago by Russell Keith-Magee

Replying to ivoras@…:

This *isn't* as simple as the change you suggest. If we applied the diff you describe, every Django user on the planet would need to issue a migration on their database, which is a *massive* backwards incompatibility.

Surely now that Django has a kind of built-in migration facility something can be done about it?

Possibly. There are a bunch of size limits in Django that have analogous problems; I'm certainly interested in seeing if the new built-in migrations framework can be used to address these.

I hope you realize that this change will have to be done sooner or later, so better do it sooner?

I hope you realize that this is completely untrue. As I said in my previous email -- we have a pluggable User model *specifically* to address this sort of limitation.

comment:4 Changed 3 years ago by ivoras@…

I hope you realize that this is completely untrue. As I said in my previous email -- we have a pluggable User model *specifically* to address this sort of limitation.

Requiring devs to add yet another level of indirection just to get sane behaviour instead of having sane default is, IMO, wrong. But then we will probably get into a discussion on the definition of "sane" or "wrong" so I'll let someone else fight this one out. Cheers.

comment:5 Changed 22 months ago by Daniel Hawkins

This absolutely should have been fixed in Django 1.7. With all the warnings attached here (https://docs.djangoproject.com/en/dev/topics/auth/customizing/#substituting-a-custom-user-model), why would anyone think creating a custom User model is a better idea than a sane default / running a single migration as a part of a gigantic migrations overhaul?

As far as the definition of "sane", this is a max_length property, so a sane value would be the smallest maximum allowed length for a CharField in any of Django's supported database backends. I guarantee that value is not 30.

comment:6 in reply to:  1 ; Changed 22 months ago by Daniel Hawkins

Replying to russellm:

If we applied the diff you describe, every Django user on the planet would need to issue a migration on their database

Is this even true? Wouldn't this migration only have to run if the user was already running manage.py migrate ? And if the user never ran that command, they would just keep the old limit of 30?

I'm re-opening this in hopes of getting a second opinion.

comment:7 Changed 22 months ago by Daniel Hawkins

Resolution: wontfix
Status: closednew

comment:8 Changed 22 months ago by Carl Meyer

Please don't reopen tickets closed wontfix by a core committer just "to get a second opinion"; that's against our documented policy, as it leads to too many open/close wars. The right course of action is to raise the question on the django-developers mailing list.

I'm not sure if this should have been fixed in 1.7 or not. We're still ironing out some issues with the base initial migrations shipped with the contrib apps, so I think it's advisable to be cautious in shipping out new migrations. The 30-character limit has been around for 10 years, another year plus or minus isn't the end of the world.

That said, I do think that the overly-restrictive max-lengths in the default user model should be fixed, custom-user-models notwithstanding, and that built-in migrations should make it possible to fix. Russell, it sounds like you agree, at least in theory? Could we reflect that consensus by reopening this ticket?

It occurs to me that custom user models may actually prove an obstacle to fixing this in a much more direct way, in that fixing it will likely change the abstract base models that many custom user models are inheriting from, and we can't ship a migration to change someone's custom user model. I guess if we start to allow shadowing of fields on abstract bases, as has been discussed recently on the mailing list, we could make this change only on the final concrete User model, and leave the abstract bases alone, for backwards-compatibility. But then future custom user models inheriting from those bases won't benefit from the change...

comment:9 in reply to:  6 ; Changed 22 months ago by Carl Meyer

Replying to hwkns:

Replying to russellm:

If we applied the diff you describe, every Django user on the planet would need to issue a migration on their database

Is this even true? Wouldn't this migration only have to run if the user was already running manage.py migrate ? And if the user never ran that command, they would just keep the old limit of 30?

If they never run migrate, that results in a much much worse situation, because their models no longer match their database schema, meaning that instead of getting nice Django-level form validation errors, they now get unpredictable errors depending on their database backend -- some backends will silently truncate the data, some will raise a database-error resulting in a 500 Internal Server Error.

comment:10 in reply to:  9 ; Changed 22 months ago by Daniel Hawkins

Apologies for re-opening if that was not the correct course of action. I just got burned by this issue, after migrating a very large Django project, from 1.6 using the third-party longerusername package, to 1.7 (for which that package does not yet have a migration). This is our only blocker for deploying with 1.7, and it's painful that my choices are to either create a custom User model or fork a third-party library.

Replying to carljm:

Replying to hwkns:

Replying to russellm:

If we applied the diff you describe, every Django user on the planet would need to issue a migration on their database

Is this even true? Wouldn't this migration only have to run if the user was already running manage.py migrate ? And if the user never ran that command, they would just keep the old limit of 30?

If they never run migrate, that results in a much much worse situation, because their models no longer match their database schema, meaning that instead of getting nice Django-level form validation errors, they now get unpredictable errors depending on their database backend -- some backends will silently truncate the data, some will raise a database-error resulting in a 500 Internal Server Error.

max_length can and should be specified in forms for validation. It was my impression that the model's max_length information was only for the database backend... is that inaccurate?

comment:11 in reply to:  10 Changed 22 months ago by Carl Meyer

Replying to hwkns:

Apologies for re-opening if that was not the correct course of action. I just got burned by this issue, after migrating a very large Django project, from 1.6 using the third-party longerusername package, to 1.7 (for which that package does not yet have a migration). This is our only blocker for deploying with 1.7, and it's painful that my choices are to either create a custom User model or fork a third-party library.

Or contribute a fix to the third-party library, which shouldn't take them long to accept if it's maintained (and if it's not maintained, you may be better off moving that monkeypatch into your own code base anyway).

Replying to carljm:

If they never run migrate, that results in a much much worse situation, because their models no longer match their database schema, meaning that instead of getting nice Django-level form validation errors, they now get unpredictable errors depending on their database backend -- some backends will silently truncate the data, some will raise a database-error resulting in a 500 Internal Server Error.

max_length can and should be specified in forms for validation. It was my impression that the model's max_length information was only for the database backend... is that inaccurate?

If you use a ModelForm without overriding the field, the form field's max_length will be automatically derived from the model field's max_length.

comment:12 Changed 22 months ago by Tim Graham

In fact, Collin started a thread on django-developers a few days ago to discuss the possibility of reopening this.

comment:13 in reply to:  12 Changed 22 months ago by Daniel Hawkins

Replying to carljm:

If you use a ModelForm without overriding the field, the form field's max_length will be automatically derived from the model field's max_length.

That does sound bad, then. I'm tempted to say that ModelForms for the User model are not very common, but I have no data to support that assertion.

Or contribute a fix to the third-party library, which shouldn't take them long to accept if it's maintained (and if it's not maintained, you may be better off moving that monkeypatch into your own code base anyway).

Yes, my plan is to contribute the fix to longerusername, and I'm happy to do that. My frustration is with the "wontfix" sentiment surrounding what was obviously a poor design choice in Django, as if it will magically get better on its own if we ignore it for another 10 years (usernames are only getting shorter, right?).

Replying to timgraham:

In fact, Collin started a thread on django-developers a few days ago to discuss the possibility of reopening this.

Thanks for the link -- I'll direct my comments to the mailing list thread from here on out.

comment:14 in reply to:  8 ; Changed 22 months ago by Russell Keith-Magee

Replying to carljm:

That said, I do think that the overly-restrictive max-lengths in the default user model should be fixed, custom-user-models notwithstanding, and that built-in migrations should make it possible to fix. Russell, it sounds like you agree, at least in theory? Could we reflect that consensus by reopening this ticket?

Now that we actually *have* a migration system, I agree that the discussion has moved from being a practical impossibility to at least a plausible option.

However: To me, the idea of a > 30 character *username* is pretty laughable - even *my* full name fits in 30 characters. The idea that, using the classical definition a username, a user would select a username > 30 seems like a pretty unusual case to me. IMHO, increasing to some arbitrary other value (60, 150, whatever) to support some arbitrary concept of what "big enough" will mean for usernames seems like a waste of effort. 30 is a reasonable default, and we have custom user models if you really do need something different.

Of course, that doesn't allow for the case where someone is using "username" to store something that wouldn't classically be called a "username" - the "email address as username" case, for example. I have some mild reservations about that as a design pattern - if you want to log in by email, then a custom user model with USERNAME_FIELD='email' is a much better solution to me. We added custom user models for a reason - trying to bash every possible use case into Django's default user model is just wrong-headed.

To be clear, though: I'm not morally opposed to making a change to max_length. I'm mostly bemused that people seem to have such an aversion to using CUSTOM_USER_MODEL, and that for some reason, using and/or contributing to a separate username model app third-party project is considered harder than pushing a patch into Django (a course of action that won't have practical results for 6-12 months).

If we *are* going to do this, a max_length of 254 makes the most sense to me (matching the length of the email field).

It occurs to me that custom user models may actually prove an obstacle to fixing this in a much more direct way, in that fixing it will likely change the abstract base models that many custom user models are inheriting from, and we can't ship a migration to change someone's custom user model. I guess if we start to allow shadowing of fields on abstract bases, as has been discussed recently on the mailing list, we could make this change only on the final concrete User model, and leave the abstract bases alone, for backwards-compatibility. But then future custom user models inheriting from those bases won't benefit from the change...

Agreed that this is a complication. Anyone deploying a third party custom-user app will need to have migrations. The application of those migrations will need to be Django-version dependent. If you're using the custom app on Django 1.7, you won't need to migrate the third party app - but as soon as you upgrade Django, the third party app will need to run the 1.7->1.8 migration.

comment:15 in reply to:  14 Changed 22 months ago by Daniel Hawkins

Replying to freakboy3742:

To me, the idea of a > 30 character *username* is pretty laughable - even *my* full name fits in 30 characters.

Frankly, the length of your own name is irrelevant. And being unable to imagine the need for a username longer than 30 characters is not a good reason to set that arbitrary limit.

If we *are* going to do this, a max_length of 254 makes the most sense to me (matching the length of the email field).

Yes, 254 makes sense if we're talking about using email addresses as usernames, but that's still arbitrary. Why limit the length of username in the model at all, when that can be easily accomplished in forms? Why not use the largest maximum length for a CharField that is still supported by all the database backends?

I'm mostly bemused that people seem to have such an aversion to using CUSTOM_USER_MODEL ...

Let me quote from the docs:

Warning:
Changing AUTH_USER_MODEL has a big effect on your database structure. It changes the tables that are available, and it will affect the construction of foreign keys and many-to-many relationships. If you intend to set AUTH_USER_MODEL, you should set it before creating any migrations or running manage.py migrate for the first time.
Changing this setting after you have tables created is not supported by makemigrations and will result in you having to manually fix your schema, port your data from the old user table, and possibly manually reapply some migrations.

I can't imagine why anyone would be averse to that...

... and that for some reason, using and/or contributing to a separate username model app third-party project is considered harder than pushing a patch into Django (a course of action that won't have practical results for 6-12 months).

As it turns out, the mechanism used by the longerusername package to set the max_length of User.username is not available in Django 1.7, so yes, it is going to be considerably more difficult. Even if it weren't, providing sensible defaults should be the goal of any good framework, so pushing a patch into Django is a worthwhile pursuit.

comment:16 Changed 22 months ago by Michael Parsons

I would also like to chime in here, having just run into this issue. We use a user's email address as their username and are running into a number of complaints from user's who are unable to register because their email addresses are longer than 30 characters. I realize that the proposed solution is to use a custom user model, but as other posters above have stated, that is not always a simple process and is a lot of work solely to increase the length of a field.

comment:18 Changed 22 months ago by Tim Graham

Easy pickings: unset
Has patch: unset
Triage Stage: UnreviewedAccepted
Version: 1.5master

comment:19 Changed 21 months ago by Collin Anderson

Cc: cmawebsite@… added
Has patch: set

comment:20 Changed 21 months ago by Tim Graham

Patch needs improvement: set

comment:21 Changed 21 months ago by Collin Anderson

Patch needs improvement: unset

comment:22 Changed 21 months ago by Collin Anderson

Patch needs improvement: set

comment:23 Changed 14 months ago by Tim Graham

Patch needs improvement: unset

comment:24 Changed 14 months ago by Tim Graham <timograham@…>

Resolution: fixed
Status: newclosed

In 15ef1dd:

Fixed #20846 -- Increased User.username max_length to 254 characters.

Thanks Collin Anderson and Nick Sandford for work on the patch.

comment:25 Changed 13 months ago by Adam Chainz

This patch breaks on MySQL installations using the utf8mb4 charset, because it breaks the index length limit - it comes out at a maximum of 254 * 4 = 1016 bytes whilst by default InnoDB can only index 767 bytes. I found this because I am using utf8mb4 in django-mysql's tests.

Django encourages using the utf8 charset (3 bytes per character - cannot store emojis), although there has been some discussion for moving to utf8mb4 in #18392. One can index 254 character utf8mb4 fields in MySQL by using a couple settings as described in that ticket, Django could enforce those, or the field could be changed to just 191 characters instead which is the maximum indexable (767 // 4).

Last edited 13 months ago by Adam Chainz (previous) (diff)

comment:26 Changed 13 months ago by Collin Anderson

Resolution: fixed
Severity: NormalRelease blocker
Status: closednew

Yeah, we should really fix this, ideally by resolving #18392. (hint: add a DATABASES OPTIONS on mysql for max index length that defaults to either 191 or 255.) I was under the impression that utf8mb4 _really_ didn't work with django but apparently you can get by ok besides this change.

comment:27 Changed 13 months ago by Collin Anderson

Has patch: unset

comment:28 Changed 13 months ago by Adam Chainz

We use utf8mb4 in production at YPlan just fine - we enable innodb_large_prefix, and use ROW_FORMAT=DYNAMIC or ROW_FORMAT=COMPRESSED on all tables, allowing the long indexes. However mandating this of all Django apps would be difficult - not everyone can change their DB config, and migrations don't currently allow ROW_FORMAT to be set and MySQL unfortunately doesn't let you set a default.

comment:29 Changed 13 months ago by Collin Anderson

re #18392: "not everyone can change their DB config" - Exactly. I think the best plan is to have django be cautious and limit the index to 191 by default.

If the user has changed their DB config to allow longer prefixes, they can tell django to allow creating longer indexes, and can at least manually convert ROW_FORMAT on the tables.

Last edited 13 months ago by Collin Anderson (previous) (diff)

comment:30 Changed 12 months ago by Collin Anderson

Actually, I think the shorter index solution doesn't work for UNIQUE indexes, which is what we're running into here.
https://groups.google.com/d/msg/django-developers/DuSwAy0670w/wS2ohfFVDwAJ

comment:31 Changed 12 months ago by Collin Anderson

Has patch: set

comment:32 Changed 11 months ago by Tim Graham <timograham@…>

Resolution: fixed
Status: newclosed

In 780bddf7:

Fixed #20846 -- Decreased User.username max_length to 150 characters.

comment:33 Changed 10 months ago by Samuel Bishop

Is this going to wind up in a 1.9.x release or only land in 1.10 ?

I ask because it was marked as a release blocker before 1.9 was shipped, so it makes some sense that it would appear in a 1.9.x release unless there is some other reason for it to be 7 months away in 1.10.

comment:34 Changed 10 months ago by Tim Graham

Severity: Release blockerNormal

It was marked as a release blocker for 1.10 because the first commit to master for 1.10 caused a regression that needed to be fixed or else the original fix reverted before 1.10 final.

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