[Django] #21379: class AbstractUser: validators should compile re with Unicode
Django
------------------------------+--------------------
Reporter: anonymous | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: 1.5
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
------------------------------+--------------------
{{{
class AbstractUser(AbstractBaseUser, PermissionsMixin):
"""
An abstract base class implementing a fully featured User model with
admin-compliant permissions.
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 '
'@/./+/-/_ characters'),
validators=[
validators.RegexValidator(re.compile('^[\w.@+-]+$'), _('Enter
a valid username.'), 'invalid')
])
}}}
re.compile should use re.U
--
Ticket URL: <https://code.djangoproject.com/ticket/21379>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: anonymous | Owner: nobody
Type: Bug | Status: closed
Component: contrib.auth | Version: 1.5
Severity: Normal | Resolution: needsinfo
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by chrismedrela):
* status: new => closed
* needs_better_patch: => 0
* resolution: => needsinfo
* needs_tests: => 0
* needs_docs: => 0
Comment:
Could you elaborate on the issue? Non-ascii characters are not allowed by
design and if you want to allow unicode in usernames, you need to use a
custom user model (see https://code.djangoproject.com/ticket/20694).
--
Ticket URL: <https://code.djangoproject.com/ticket/21379#comment:1>
Django
Reporter: anonymous | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: 1.5
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by xelnor):
* status: closed => new
* resolution: needsinfo =>
Comment:
The above regexp doesn't do what it seems to be doing: on Python 2,
`[\w.@+-]` is equivalent to `[a-zA-Z0-9.@+-]`.
In Python 3, this will also match all "accented" characters.
The regexp should be updated for consistency between versions:
* If the goal is to allow any letter-like char, add `re.UNICODE` to the
`re.compile` call
* If it should instead only allow ascii letters, the simplest way would be
to use the explicit regexp, since the `re.ASCII` flag exists only in Py3.
Exemple (Python 3):
{{{
#!python
>>> import re
>>> from django.core import validators
>>> v = validators.RegexValidator(re.compile('^[\w.@+-]+$'), "Enter a
valid username.", 'invalid')
>>> v('foo.bar')
>>> v('foo bar')
Traceback (most recent call last):
File "<console>", line 1, in <module>
File "/home/xelnor/dev/venvs/bluesys-tools-py3/lib/python3.3/site-
packages/django/core/validators.py", line 39, in __call__
raise ValidationError(self.message, code=self.code)
django.core.exceptions.ValidationError: ['Enter a valid username.']
>>> v('jean-rené')
}}}
And on Python 2:
{{{
#!python
>>> import re
>>> from django.core import validators
>>> v = validators.RegexValidator(re.compile('^[\w.@+-]+$'), "Enter a
valid username.", 'invalid')
>>> v('foo.bar')
>>> v('foo bar')
Traceback (most recent call last):
File "<console>", line 1, in <module>
File "/home/xelnor/dev/venvs/bluesys-tools/lib/python2.7/site-
packages/django/core/validators.py", line 39, in __call__
raise ValidationError(self.message, code=self.code)
ValidationError: [u'Enter a valid username.']
>>> v('jean-rené')
Traceback (most recent call last):
File "<console>", line 1, in <module>
File "/home/xelnor/dev/venvs/bluesys-tools/lib/python2.7/site-
packages/django/core/validators.py", line 39, in __call__
raise ValidationError(self.message, code=self.code)
ValidationError: [u'Enter a valid username.']
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/21379#comment:2>
Django
Reporter: anonymous | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: 1.5
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Alex):
* stage: Unreviewed => Accepted
Comment:
The behavior difference between py2 and py3 is clearly a bug, not sure
which fix is correct.
--
Ticket URL: <https://code.djangoproject.com/ticket/21379#comment:3>
Django
Reporter: anonymous | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: 1.5
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by jorgecarleitao):
I digged a bit and the validation is defined in two places:
(1)- UserCreationForm and UserChangeForm uses `'^[\w.@+-]+$'`, which is
compiled to `re.compile('^[\w.@+-]+$', re.UNICODE)`
(https://github.com/django/django/blob/master/django/forms/fields.py#L542)
(2)- AbstractUser ORM field validation uses `'^[\w.@+-]+$'` but it does
not pass flags to the RegexValidator, so it compiles as
`re.compile('^[\w.@+-]+$')`.
In #20694 was pointing out that AbstractUser is rejecting non-ascii in
python2, which is consistent with (2). @aaugustin pointed out that we
cannot change AbstractUser because it is backward incompatible and
proposed to use UserCreationForm. However, I'm not sure this can be fixed
using a custom UserCreationForm. My concern is that the validation is
performed by both the validator constructed from
UserCreationForm.username, and by the validator of the
AbstractUser.username. Because both are tested and both must validate,
this gives different results whether we use python2 or python3 because of
(2).
To support this, I attach a diff to be run using both versions:
{{{ PYTHONPATH=..:$PYTHONPATH python2.7 runtests.py --settings=test_sqlite
django.contrib.auth.tests.test_forms.UserCreationFormTest.test_invalid_non_ascii_username
}}}
{{{ PYTHONPATH=..:$PYTHONPATH python3.3 runtests.py --settings=test_sqlite
django.contrib.auth.tests.test_forms.UserCreationFormTest.test_invalid_non_ascii_username
}}}
This diff has a test for this ticket and also prints which regex was used
on `validador.RegexValidator.__call__` (for the sake of this discussion)
{{{
print(self.regex.pattern, self.regex.flags)
}}}
In Python 2, this prints
{{{
^[\w.@+-]+$ 32 # 32 = re.UNICODE
^[\w.@+-]+$ 0 # 0 = default flag of RegexValidator
}}}
and `username=u'jsmithé'` is correctly invalidated
In Python 3, this prints
{{{
^[\w.@+-]+$ 32
^[\w.@+-]+$ 32
}}}
and `username=u'jsmithé'` is not invalidated, failing the test.
In summary, I believe we have two issues here:
* non-correspondence python2<>python3, confirmed by the fail of this test.
* double validation of the same field, which seems to violate DRY (and
thus harder to maintain, as #20694 and this ticket shows).
Notice that this double verification happens on all ModelForms that
validate a field that is also validated by a validator defined on a
model.Field.
--
Ticket URL: <https://code.djangoproject.com/ticket/21379#comment:4>
Django
Reporter: anonymous | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: 1.5
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by jorgecarleitao):
* cc: jorgecarleitao (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/21379#comment:5>
Django
Reporter: anonymous | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: 1.5
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by tovmeod):
I think we should allow non latin characters, users should be able to have
usernames in their native language (hebrew, chinese etc)
I propose we use unicode
[https://docs.python.org/2/library/unicodedata.html#unicodedata.normalize
normalization]
unicodedata.normalize(input, 'NFKD')
this could be used for usernames, passwords and other kind of input and
should allow non latin.
--
Ticket URL: <https://code.djangoproject.com/ticket/21379#comment:6>
Django
Reporter: anonymous | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: 1.5
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by claudep):
* needs_better_patch: 0 => 1
* has_patch: 0 => 1
Comment:
Here's a [https://github.com/django/django/pull/6494 tentative PR].
--
Ticket URL: <https://code.djangoproject.com/ticket/21379#comment:7>
Django
Reporter: anonymous | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by claudep):
* needs_better_patch: 1 => 0
* version: 1.5 => master
Comment:
Should be ready for review.
--
Ticket URL: <https://code.djangoproject.com/ticket/21379#comment:8>
Django
Reporter: anonymous | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by claudep):
The [https://groups.google.com/forum/#!topic/django-developers/MBSWXcQBP3k
django-developers] discussion.
--
Ticket URL: <https://code.djangoproject.com/ticket/21379#comment:9>
Django
Reporter: anonymous | Owner: nobody
Type: Bug | Status: new
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by claudep):
* severity: Normal => Release blocker
Comment:
Marking as blocker, just as we don't forget to make a decision before
releasing 1.10.
--
Ticket URL: <https://code.djangoproject.com/ticket/21379#comment:10>
Django
Reporter: anonymous | Owner: nobody
Type: Bug | Status: new
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by claudep):
So beside the [https://github.com/django/django/pull/6494 original PR]
which also added Unicode username on Python 2, we have now the
[https://github.com/django/django/pull/6600 alternative PR] to keep ASCII-
only usernames on Python 2 and Unicode usernames on Python 3. This is more
or less the same situation as Django 1.9, but with the ability to change
the default behavior, and with improvements regarding Unicode
normalization.
--
Ticket URL: <https://code.djangoproject.com/ticket/21379#comment:11>
Django
Reporter: anonymous | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* keywords: => 1.10
* severity: Release blocker => Normal
--
Ticket URL: <https://code.djangoproject.com/ticket/21379#comment:12>
Django
-------------------------------------+-------------------------------------
Reporter: anonymous | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/21379#comment:13>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: closed
Component: contrib.auth | Version: master
Severity: Normal | Resolution: fixed
Keywords: 1.10 | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Claude Paroz <claude@…>):
In [changeset:"9935f97cd203bdcc722bc3d4e96858e221d96ff8" 9935f97]:
{{{
#!CommitTicketReference repository=""
revision="9935f97cd203bdcc722bc3d4e96858e221d96ff8"
Refs #21379 -- Normalized unicode username inputs
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/21379#comment:15>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: closed
Component: contrib.auth | Version: master
Severity: Normal | Resolution: fixed
Keywords: 1.10 | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Claude Paroz <claude@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"526575c64150e10dd8666d1ed3f86eedd00df2ed" 526575c]:
{{{
#!CommitTicketReference repository=""
revision="526575c64150e10dd8666d1ed3f86eedd00df2ed"
Fixed #21379 -- Created auth-specific username validators
Thanks Tim Graham for the review.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/21379#comment:14>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: closed
Component: contrib.auth | Version: master
Severity: Normal | Resolution: fixed
Keywords: 1.10 | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"39805686b364358af725b695924a5a6dfa7f5302" 39805686]:
{{{
#!CommitTicketReference repository=""
revision="39805686b364358af725b695924a5a6dfa7f5302"
Refs #21379, #26719 -- Moved username normalization to AbstractBaseUser.
Thanks Huynh Thanh Tam for the initial patch and Claude Paroz for review.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/21379#comment:16>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: closed
Component: contrib.auth | Version: master
Severity: Normal | Resolution: fixed
Keywords: 1.10 | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"1b0b6f0342e5ac9e3e789ca522ad64a532602c3f" 1b0b6f03]:
{{{
#!CommitTicketReference repository=""
revision="1b0b6f0342e5ac9e3e789ca522ad64a532602c3f"
[1.10.x] Refs #21379, #26719 -- Moved username normalization to
AbstractBaseUser.
Thanks Huynh Thanh Tam for the initial patch and Claude Paroz for review.
Backport of 39805686b364358af725b695924a5a6dfa7f5302 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/21379#comment:17>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: closed
Component: contrib.auth | Version: master
Severity: Normal | Resolution: fixed
Keywords: 1.10 | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 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/21379#comment:18>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: closed
Component: contrib.auth | Version: master
Severity: Normal | Resolution: fixed
Keywords: 1.10 | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 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/21379#comment:19>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: closed
Component: contrib.auth | Version: master
Severity: Normal | Resolution: fixed
Keywords: 1.10 | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 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/21379#comment:20>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: closed
Component: contrib.auth | Version: master
Severity: Normal | Resolution: fixed
Keywords: 1.10 | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 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/21379#comment:21>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: closed
Component: contrib.auth | Version: master
Severity: Normal | Resolution: fixed
Keywords: 1.10 | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 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/21379#comment:22>