[Django] #27807: Overriding username validators does not seem to work
Django
-------------------------------------+-------------------------------------
Reporter: johnrork | Owner: nobody
Type: Bug | Status: new
Component: | Version: 1.10
contrib.auth | Keywords: validation auth
Severity: Normal | login forms username
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
The docs say you can change the default username validator by overriding
the new `username_validator` attribute on a model that subclasses
`auth.User`.
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
--
Ticket URL: <https://code.djangoproject.com/ticket/27807>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: johnrork | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: validation auth | Triage Stage:
login forms username | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* Attachment "myproject.zip" added.
Demonstration of possible Model.username_validator bug
Django
-------------------------------------+-------------------------------------
Reporter: johnrork | Owner: nobody
Type: Bug | Status: new
Severity: Release blocker | Resolution:
Keywords: validation auth | Triage Stage: Accepted
login forms username |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* severity: Normal => Release blocker
* stage: Unreviewed => Accepted
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/27807#comment:1>
Django
Reporter: johnrork | Owner: nobody
Type: Bug | Status: new
Severity: Release blocker | Resolution:
Keywords: validation auth | Triage Stage: Accepted
login forms username |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (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?
--
Ticket URL: <https://code.djangoproject.com/ticket/27807#comment:2>
Django
Reporter: johnrork | Owner: nobody
Type: Bug | Status: new
Severity: Release blocker | Resolution:
Keywords: validation auth | Triage Stage: Accepted
login forms username |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by johnrork):
Here is an example a working `AUTH_USERNAME_VALIDATORS` setting:
https://github.com/johnrork/django/commit/8afbb76f3aca306ec7b678cb394fcedc8b2edf12
--
Ticket URL: <https://code.djangoproject.com/ticket/27807#comment:3>
Django
Reporter: johnrork | Owner: nobody
Type: Bug | Status: new
Severity: Release blocker | Resolution:
Keywords: validation auth | Triage Stage: Accepted
login forms username |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Claude Paroz):
I'm very sorry for this mess, as I'm the culprit here.
I tried to play with metaclasses and obtained
[https://github.com/claudep/django/commit/3f434e5b9f7176245e9fd1195556e09dfe65e1bf
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/27807#comment:4>
Django
Reporter: johnrork | Owner: nobody
Type: Bug | Status: new
Severity: Release blocker | Resolution:
Keywords: validation auth | Triage Stage: Accepted
login forms username |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (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.
--
Ticket URL: <https://code.djangoproject.com/ticket/27807#comment:5>
Django
Reporter: johnrork | Owner: nobody
Type: Bug | Status: new
Severity: Release blocker | Resolution:
Keywords: validation auth | Triage Stage: Accepted
login forms username |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (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.
--
Ticket URL: <https://code.djangoproject.com/ticket/27807#comment:6>
Django
Reporter: johnrork | Owner: nobody
Type: Bug | Status: new
Severity: Release blocker | Resolution:
Keywords: validation auth | Triage Stage: Accepted
login forms username |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"714fdbaa7048c2321f6238d9421137c33d9af7cc" 714fdba]:
{{{
#!CommitTicketReference repository=""
revision="714fdbaa7048c2321f6238d9421137c33d9af7cc"
[1.10.x] Refs #27807 -- Removed docs for User.username_validator.
The new override functionality claimed in refs #21379 doesn't work.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27807#comment:7>
Django
Reporter: johnrork | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: validation auth | Triage Stage: Accepted
login forms username |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* severity: Release blocker => Normal
Comment:
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).
--
Ticket URL: <https://code.djangoproject.com/ticket/27807#comment:8>
Django
Reporter: johnrork | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: validation auth | Triage Stage: Accepted
login forms username |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (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...
--
Ticket URL: <https://code.djangoproject.com/ticket/27807#comment:9>
Django
Reporter: johnrork | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: validation auth | Triage Stage: Accepted
login forms username |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Tobias Wiese (added)
Comment:
The Documentation since 1.11 until now
([https://docs.djangoproject.com/en/2.2/ref/contrib/auth/#django.contrib.auth.models.User.username_validator
here] and also referenced
[https://docs.djangoproject.com/en/2.2/ref/contrib/auth/#django.contrib.auth.models.User.username
here]) say that this feature exists since 1.10.
As it feature still doesn't work I just guess the removal form the docs
somehow did not make it into master.
--
Ticket URL: <https://code.djangoproject.com/ticket/27807#comment:10>
Django
Reporter: johnrork | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: validation auth | Triage Stage: Accepted
login forms username |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"c84b91b7603e488f7171fdff8f08368ef3d6b856" c84b91b7]:
{{{
#!CommitTicketReference repository=""
revision="c84b91b7603e488f7171fdff8f08368ef3d6b856"
Refs #27807 -- Removed docs for User.username_validator.
The new override functionality claimed in refs #21379 doesn't work.
Forwardport of 714fdbaa7048c2321f6238d9421137c33d9af7cc from
stable/1.10.x.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27807#comment:11>
Django
Reporter: johnrork | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: validation auth | Triage Stage: Accepted
login forms username |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"53c83387cfb840d3b222a23a804bf9cb458bb3d0" 53c83387]:
{{{
#!CommitTicketReference repository=""
revision="53c83387cfb840d3b222a23a804bf9cb458bb3d0"
[2.2.x] Refs #27807 -- Removed docs for User.username_validator.
The new override functionality claimed in refs #21379 doesn't work.
Forwardport of 714fdbaa7048c2321f6238d9421137c33d9af7cc from
stable/1.10.x.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27807#comment:12>
Django
Reporter: johnrork | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: validation auth | Triage Stage: Accepted
login forms username |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"fb2b4253f93a85e21ee6bac4ecdac52929faeb2f" fb2b425]:
{{{
#!CommitTicketReference repository=""
revision="fb2b4253f93a85e21ee6bac4ecdac52929faeb2f"
[2.1.x] Refs #27807 -- Removed docs for User.username_validator.
The new override functionality claimed in refs #21379 doesn't work.
Forwardport of 714fdbaa7048c2321f6238d9421137c33d9af7cc from
stable/1.10.x.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27807#comment:13>
Django
Reporter: johnrork | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: validation auth | Triage Stage: Accepted
login forms username |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"331d76528154407927f88759c9e520494e29b914" 331d7652]:
{{{
#!CommitTicketReference repository=""
revision="331d76528154407927f88759c9e520494e29b914"
[1.11.x] Refs #27807 -- Removed docs for User.username_validator.
The new override functionality claimed in refs #21379 doesn't work.
Forwardport of 714fdbaa7048c2321f6238d9421137c33d9af7cc from
stable/1.10.x.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27807#comment:14>
Django
Reporter: johnrork | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: validation auth | Triage Stage: Accepted
login forms username |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Jarek Glowacki):
Just dropping a diff here for failing tests if someone picks this up in
future.
Though I'd also support just closing this as a wontfix.
{{{
diff --git a/tests/validation/models.py b/tests/validation/models.py
index e8e18cfbce..1069f5719f 100644
--- a/tests/validation/models.py
+++ b/tests/validation/models.py
@@ -1,5 +1,6 @@
from datetime import datetime
+from django.contrib.auth.models import User
from django.core.exceptions import ValidationError
from django.db import models
@@ -134,3 +135,7 @@ assert str(assertion_error) == (
"Model validation.MultipleAutoFields can't have more than one "
"auto-generated field."
)
+
+
+class ShadowUser(User):
+ username_validator = validate_answer_to_universe
diff --git a/tests/validation/test_override.py
b/tests/validation/test_override.py
new file mode 100644
index 0000000000..2c0da854bd
--- /dev/null
+++ b/tests/validation/test_override.py
@@ -0,0 +1,13 @@
+from django.test import SimpleTestCase
+
+from .models import ShadowUser
+
+
+class TestUsernameValidatorOverride(SimpleTestCase):
+ def test_username_validator_override(self):
+ self.assertEqual(ShadowUser.username_validator.__name__,
'validate_answer_to_universe')
+
+ self.assertCountEqual(
+ ['validate_answer_to_universe'],
+ [getattr(v, '__name__', v.__class__.__name__) for v in
ShadowUser._meta.get_field("username").validators]
+ )
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27807#comment:15>
Django
-------------------------------------+-------------------------------------
| Gyanwali
Type: Bug | Status: assigned
Component: contrib.auth | Version: 1.10
Severity: Normal | Resolution:
Keywords: validation auth | Triage Stage: Accepted
login forms username |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: nobody => Shekhar Gyanwali
* status: new => assigned
Comment:
Hi, I am working on it. This is my first django ticket.
--
Ticket URL: <https://code.djangoproject.com/ticket/27807#comment:16>
Django
-------------------------------------+-------------------------------------
Reporter: johnrork | Owner: Shekhar
| Gyanwali
Type: Bug | Status: assigned
Component: contrib.auth | Version: 1.10
Severity: Normal | Resolution:
Keywords: validation auth | Triage Stage: Accepted
login forms username |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Shekhar Gyanwali):
Submitted a pull request. Please see pull request
[https://github.com/django/django/pull/13114] for more detail.
--
Ticket URL: <https://code.djangoproject.com/ticket/27807#comment:17>
Django
-------------------------------------+-------------------------------------
Reporter: johnrork | Owner: Shekhar
| Gyanwali
Type: Bug | Status: assigned
Component: contrib.auth | Version: 1.10
Severity: Normal | Resolution:
Keywords: validation auth | Triage Stage: Accepted
login forms username |
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/27807#comment:18>
Django
-------------------------------------+-------------------------------------
Reporter: johnrork | Owner: Shekhar
Type: Bug | Status: assigned
Component: contrib.auth | Version: 1.10
Severity: Normal | Resolution:
Keywords: validation auth | Triage Stage: Accepted
login forms username |
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/27807#comment:19>
Django
-------------------------------------+-------------------------------------
Reporter: johnrork | Owner: Shekhar
| Nath Gyanwali
Type: Bug | Status: assigned
Component: contrib.auth | Version: 1.10
Severity: Normal | Resolution:
Keywords: validation auth | Triage Stage: Accepted
login forms username |
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Shekhar Nath Gyanwali):
Submitted pull request with the changes as per suggestion.
--
Ticket URL: <https://code.djangoproject.com/ticket/27807#comment:20>
Django
-------------------------------------+-------------------------------------
Reporter: johnrork | Owner: Shekhar
| Nath Gyanwali
Type: Bug | Status: assigned
Component: contrib.auth | Version: 1.10
Severity: Normal | Resolution:
Keywords: validation auth | Triage Stage: Accepted
login forms username |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/27807#comment:21>
Django
-------------------------------------+-------------------------------------
Reporter: johnrork | Owner: Shekhar
| Nath Gyanwali
Type: Bug | Status: assigned
Component: contrib.auth | Version: 1.10
Severity: Normal | Resolution:
Keywords: validation auth | Triage Stage: Accepted
login forms username |
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/27807#comment:22>
Django
-------------------------------------+-------------------------------------
Reporter: johnrork | Owner: Shekhar
| Nath Gyanwali
Type: Bug | Status: assigned
Component: contrib.auth | Version: 1.10
Severity: Normal | Resolution:
Keywords: validation auth | Triage Stage: Accepted
login forms username |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 1 => 0
Comment:
Made all the changes based on feedback on PR.
I am not sure if release note is 100%, might take few iteration(my
apologies).
--
Ticket URL: <https://code.djangoproject.com/ticket/27807#comment:23>
Django
-------------------------------------+-------------------------------------
Reporter: johnrork | Owner: Shekhar
| Nath Gyanwali
Type: Bug | Status: assigned
Component: contrib.auth | Version: 1.10
Severity: Normal | Resolution:
Keywords: validation auth | Triage Stage: Accepted
login forms username |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Shekhar Nath Gyanwali):
Fixed typo, apologies completely missed that one.
--
Ticket URL: <https://code.djangoproject.com/ticket/27807#comment:24>
Django
-------------------------------------+-------------------------------------
Reporter: johnrork | Owner: Shekhar
| Nath Gyanwali
Type: Bug | Status: assigned
Component: contrib.auth | Version: 1.10
Severity: Normal | Resolution:
Keywords: validation auth | Triage Stage: Accepted
login forms username |
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/27807#comment:25>
Django
-------------------------------------+-------------------------------------
Reporter: johnrork | Owner: Shekhar
| Nath Gyanwali
Type: Bug | Status: assigned
Component: contrib.auth | Version: 1.10
Severity: Normal | Resolution:
Keywords: validation auth | Triage Stage: Accepted
login forms username |
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/27807#comment:26>
Django
-------------------------------------+-------------------------------------
Reporter: johnrork | Owner: Shekhar
| Nath Gyanwali
Type: Bug | Status: assigned
Component: contrib.auth | Version: 1.10
Severity: Normal | Resolution:
Keywords: validation auth | Triage Stage: Accepted
login forms username |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Shekhar Nath Gyanwali):
Added more test and fixed typo/unwanted spaces etc.
--
Ticket URL: <https://code.djangoproject.com/ticket/27807#comment:27>
Django
-------------------------------------+-------------------------------------
Reporter: johnrork | Owner: Shekhar
| Nath Gyanwali
Type: Bug | Status: assigned
Component: contrib.auth | Version: 1.10
Severity: Normal | Resolution:
Keywords: validation auth | Triage Stage: Accepted
login forms username |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Shekhar Nath Gyanwali):
Fixed documentation typo and removed username_validator variable.
--
Ticket URL: <https://code.djangoproject.com/ticket/27807#comment:28>
Django
-------------------------------------+-------------------------------------
Reporter: johnrork | Owner: Shekhar
| Nath Gyanwali
Component: contrib.auth | Version: 1.10
Severity: Normal | Resolution: wontfix
Keywords: validation auth | Triage Stage: Accepted
login forms username |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => wontfix
Comment:
I'm going to close this as wontfix. The issue doesn't justify new settings
or signals or extensions to the validator API or and so on.
`UnicodeUsernameValidator` is a good default, and ''Use a custom user
model'' is not unreasonable. People who can't do that can add validation
at the form level, in `clean()`, or even monkey-patch the field if they
absolutely must, but flexibility for it's own sake is not worth
significant complication here.
Happy to look at a patch that is small and self-contained, but generally
the whole issue seems moot.
--
Ticket URL: <https://code.djangoproject.com/ticket/27807#comment:29>