Opened 8 months ago

Last modified 8 months ago

#27807 new Bug

Overriding username validators doesn't work as documented

Reported by: johnrork Owned by: nobody
Component: contrib.auth Version: 1.10
Severity: Normal Keywords: validation auth login forms username
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The docs say you can change the default username validator by overriding the new username_validator attribute on a model that subclasses auth.User.

https://docs.djangoproject.com/en/1.10/ref/contrib/auth/#django.contrib.auth.models.User.username_validator

In practice, it seems that this value is always overwritten by the defaults, causing difficult-to-debug form validation errors.

class CustomUser(User):
    username_validator = validate_username
    
    class Meta:
        proxy = True 

>>> from myapp.models import CustomUser
>>> CustomUser.username_validator
<function validate_username at 0x105a5eea0>
>>> CustomUser._meta.get_field("username").validators
[<django.contrib.auth.validators.UnicodeUsernameValidator object at 0x105a5df28>, <django
.core.validators.MaxLengthValidator object at 0x105a622b0>]

I've attached a sample project in which you can replicate the issue:

  1. Log in at http://localhost:8000/admin with username admin and password password123
  2. Attempt to create a new Custom User with an apostrophe (single quote) in the username
  3. Get validation error despite both the form validator and CustomUser.username_validator allowing apostrophes
  4. Optionally uncomment the CustomUser.__init__ method to see expected behavior, even without a custom admin form

Attachments (1)

myproject.zip (14.7 KB) - added by johnrork 8 months ago.
Demonstration of possible Model.username_validator bug

Download all attachments as: .zip

Change History (10)

Changed 8 months ago by johnrork

Attachment: myproject.zip added

Demonstration of possible Model.username_validator bug

comment:1 Changed 8 months ago by Tim Graham

Severity: NormalRelease blocker
Summary: Overriding username validators does not seem to workOverriding username validators doesn't work as documented
Triage Stage: UnreviewedAccepted

In retrospect, it's a bit obvious that overriding won't work since the username field refers to username_validator directly. If it used AbstractUser.username_validator would that fix the issue?

class AbstractUser(model.Model):
    username_validator = UnicodeUsernameValidator() if six.PY3 else ASCIIUsernameValidator()

    username = models.CharField(...validators=[username_validator])

Marking as release blocker since it's a new feature in 1.10 (526575c64150e10dd8666d1ed3f86eedd00df2ed). Depending on how invasive a fix is required, it could be acceptable to remove the inaccurate documentation and revisit the ability to customize the validator in a sublcass in some future release.

comment:2 Changed 8 months ago by johnrork

Depending on how invasive a fix is required, it could be acceptable to remove the inaccurate documentation and revisit the ability to customize the validator in a sublcass in some future release.

The docs seem to indicate that subclassing is the only way to customize the username validator.

It would be nice if there was an easier way that worked better with existing codebases.

In my case, it would be enough if the existing validators accepted customizable arguments.

UnicodeUsernameValidator already accepts a regex, are settings.USERNAME_PATTERN and settings.USERNAME_LENGTH unreasonable pony requests?

An AUTH_USERNAME_VALIDATORS dict (à la AUTH_PASSWORD_VALIDATORS) would also work.

Last edited 8 months ago by johnrork (previous) (diff)

comment:3 Changed 8 months ago by johnrork

Here is an example a working AUTH_USERNAME_VALIDATORS setting:

https://github.com/johnrork/django/commit/8afbb76f3aca306ec7b678cb394fcedc8b2edf12

comment:4 Changed 8 months ago by Claude Paroz

I'm very sorry for this mess, as I'm the culprit here.

I tried to play with metaclasses and obtained that patch. However, other auth tests are horribly failing. I don't know if I'm simply doing something absolutely hacky and forbidden, or if it's just some isolation issue in tests.

comment:5 Changed 8 months ago by Tim Graham

Claude, after some brief debugging, the issue with your patch might be limited to the test suite where multiple user models are present. It looks like CustomValidatorUser initialization modifies the validators for the auth.User.username field and this change persists regardless of the user model in use. I'm not sure about the best way to proceed, offhand.

comment:6 Changed 8 months ago by Claude Paroz

Clearly I'm not confident to introduce such metaclass hacking in stable. So for 1.10 at least, I think the docs should be updated. For 1.11, I'm still undecided. Maybe the technical team might bring its expertise here.

comment:7 Changed 8 months ago by Tim Graham <timograham@…>

In 714fdba:

[1.10.x] Refs #27807 -- Removed docs for User.username_validator.

The new override functionality claimed in refs #21379 doesn't work.

comment:8 Changed 8 months ago by Tim Graham

Severity: Release blockerNormal

I'm not against solving this, but considering it's recommended to start with a custom user model (#24370), I wouldn't invest a lot of effort into this functionality myself. We can forwardport that documentation change as necessary (at least the 1.10 releases notes portion must be ported).

comment:9 Changed 8 months ago by Claude Paroz

Yes, recommending a custom user model is a good thing. However, for those using the default contrib.auth with existing projects, and considering the migration to a custom user model is far from straightforward, it is still a bit annoying...

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