AuthenticationForm's username field doesn't set maxlength HTML attribute.

[Mariusz Felisiak]

AuthenticationForm's username field doesn't render with maxlength HTML attribute anymore.

Regression introduced in #27515 and 5ceaf14686ce626404afb6a5fbd3d8286410bf13.

​https://groups.google.com/forum/?utm_source=digest&utm_medium=email#!topic/django-developers/qnfSqro0DlA
​https://forum.djangoproject.com/t/possible-authenticationform-max-length-regression-in-django-2-1/241

[Mariusz Felisiak]

Regression test.

[Sam Reynolds]

​PR

[gopackgo90]

Is it a good idea to change a bunch of the field properties in the form's init? Wouldn't it make more sense to set these as part of the UsernameField's init like ​https://github.com/gopackgo90/django/commit/dd9be672ec079e90b8dfa4a8e91f31ec8185438f? In Django 2.0, the username field also had a MaxLengthValidator when it was created, which wouldn't exist if the proposed PR is accepted.

[Mariusz Felisiak]

Good point about MaxLengthValidator we should restore it. I don't think that changing UsernameField.__init__() is a good idea because it is not necessary for other forms which inherit from forms.ModelForm, also UsernameField can be used without max_length.

[Mariusz Felisiak]

@gopackgo90 After reconsideration I don't think that MaxLengthValidator is really important here since a browser truncates username when we set maxlength HTML attribute. This is not a ModelForm so we don't risk IntegrityError etc. even if someone send manipulated POST data.

Nevermind, let's bring it back, it doesn't hurt.

[Carlton Gibson]

OK 2¢ here... :)

Given this:

class UsernameField(forms.CharField)

... I think I prefer setting max_length in UsernameField.__init__(), if not passed as a kwarg, and so leveraging CharField's auto-adding the validator.
(Validating the POST data seems the best chance to error early, even if we're trying an INSERT...)

Update: Bit too quick there. This issue only affects AuthenticationForm so it looks like we must pass the kwarg, or adjust in the form __init__() method, otherwise we're making a logic change to UsernameField. (Such may or may not be worth doing, but not on this ticket...)

[Carlton Gibson]

Is it a good idea to change a bunch of the field properties in the form's init?

Grrr. This is horrible... :) — It seems necessary, at least for the tests, because UserModel is swappable, so when AuthenticationForm (and others) are declared, UserModel is auth.User, so we can't use UserModel to pass max_length in the class definition.

Nor, though can we add the user name field in the form __init__(), in order to avoid this kind of logic, and duplicating CharField.__init__():

This kind of thing, in AuthenticationForm.__init__():

        self.fields['username'] = UsernameField(
            max_length=username_max_length,
            widget=forms.TextInput(attrs={'autofocus': True, 'max_length': username_max_length})
        )

• Other logic, e.g. correct label setting assumes the current setup, i.e. that the field was declared with auth.User, and...
• e.g. We swap out for IntegerFields, depending on the type of the username field.

And then, given that last, we'd need to type check before manually re-adding the validator, at which point I come back to Mariusz' suggestion that it's probably not worth it.

Sorry for the back and forth. This is significantly more complex than it at first appears.

[Mariusz Felisiak <felisiak.mariusz@…>]

In 6c9778a:

Fixed #30776 -- Restored max length validation on AuthenticationForm.UsernameField.

Regression in 5ceaf14686ce626404afb6a5fbd3d8286410bf13.

Thanks gopackgo90 for the report and Mariusz Felisiak for tests.

[Mariusz Felisiak <felisiak.mariusz@…>]

In e74ca022:

[3.0.x] Fixed #30776 -- Restored max length validation on AuthenticationForm.UsernameField.

Regression in 5ceaf14686ce626404afb6a5fbd3d8286410bf13.

Thanks gopackgo90 for the report and Mariusz Felisiak for tests.

Backport of 6c9778a58e4f680db180d4cc9dc5639d2ec1b40c from master

[Sam Reynolds]

Sorry Carlton. Had I been paying attention to the ticket, I might have saved you some pain. I saw the issue with swapping out the field/widget when making the change.

My rationale for sticking with the simpler option was that we can rely on the authentication process to ensure the max length of a username. After all, if the entered username is longer than the max_length of the user model, it can't be a validusername!

[Carlton Gibson]

No problem Sam! Thank you for your efforts. If nothing else, going through it refreshed my Swapable Models: Here be Dragons chops. 👍