Fixed #27807 -- Made username validation configurable through AUTH_USERNAME_VALIDATORS setting by saykharng · Pull Request #13114 · django/django

saykharng · 2020-06-26T12:31:18Z

Solution allows Username Validation to be configured through AUTH_USERNAME_VALIDATORS setting.
The setting follows similar approach to password validators with slight difference. Username Validator must be callable like core validators and requires a help_text method (returns a string explaining the requirement of validation).

Added USERNAME_VALIDATORS config in settings.
Username Validator should be callable, like core validators.
Username Validator should have help_text() method, which is to display the requirement to the user.
Added help_text method on UnicodeUsernameValidator and ASCIIUsernameValidator
Added help_text method on UnicodeUsernameValidator and ASCIIUsernameValidator
Added Username Validation page in docs/topics
Added test in auth_tests

jacobtylerwalls · 2020-06-28T00:05:50Z

Hi Shekhar, another (prospective) first time Django contributor here. Thought I would help out a fellow first timer by commenting on your documentation.

docs/topics/auth/customizing.txt

tests/auth_tests/test_validators.py

docs/ref/settings.txt

django/contrib/auth/validators.py

django/conf/global_settings.py

docs/topics/auth/customizing.txt

tests/auth_tests/test_validators.py

jacobtylerwalls

Also, this would need a test that UnicodeUsernameValidator is correctly defaulted when settings are not provided, since this is what nearly all users will experience if this approach is implemented.

saykharng · 2020-06-28T04:15:59Z

Hi Shekhar, another (prospective) first time Django contributor here. Thought I would help out a fellow first timer by commenting on your documentation.
Hi Jacob, Thank you! I will go through each comment and make changes.

saykharng · 2020-07-29T11:45:22Z

Hi Shekhar, another (prospective) first time Django contributor here. Thought I would help out a fellow first timer by commenting on your documentation.

Hi Jacob, my sincere apologies on taking long time on this.

Currently, I am following the django core validators pattern where the validator should be callable. Therefore implementing validators with call method. Is this ok? or should I make it exactly like a password validators with validate() function and make necessary changes?

Please let me know.

jacobtylerwalls · 2020-07-29T15:31:29Z

Hi Shekhar!
Since this is a new feature, I think the next step is for you to post on django-developers to find consensus on the approach. See more here: https://docs.djangoproject.com/en/dev/internals/contributing/writing-code/submitting-patches/
Cheers, Jacob

saykharng · 2020-07-30T11:09:04Z

Hi Shekhar!
Since this is a new feature, I think the next step is for you to post on django-developers to find consensus on the approach. See more here: https://docs.djangoproject.com/en/dev/internals/contributing/writing-code/submitting-patches/
Cheers, Jacob

Thank you!

Wrote a feature to allow users to customize username validators.

saykharng · 2020-08-02T08:21:25Z

Hi Shekhar!
Since this is a new feature, I think the next step is for you to post on django-developers to find consensus on the approach. See more here: https://docs.djangoproject.com/en/dev/internals/contributing/writing-code/submitting-patches/
Cheers, Jacob

Thank you!

I have opened a discussion but waiting on feedback.

jacobtylerwalls

heya @saykharng I have a few more suggestions on tests to include and some documentation fixes. I'll let a more experienced contributor mark this as ready for checkin once they've given it their own review 👍

docs/topics/auth/username.txt

django/contrib/auth/username_validation.py

tests/auth_tests/test_validators.py

jacobtylerwalls

Sorry this feedback has been so piecemeal, but I noticed a few more things when I looked again at this today. Lmk if you have any questions!

docs/releases/3.2.txt

django/contrib/auth/models.py

docs/topics/auth/username.txt

tests/auth_tests/test_validators.py

docs/ref/settings.txt

Added test for Forms changes, fixed typos and spelling.

saykharng · 2020-08-14T05:18:03Z

heya @saykharng I have a few more suggestions on tests to include and some documentation fixes. I'll let a more experienced contributor mark this as ready for checkin once they've given it their own review 👍

Hi Jacob, thanks a lot for your feedback and sorry it's taking a while.

I have learned a lot about development and django by working on this feature. I have added few more test and made all the changes as suggested. Appreciate your time. :)

jacobtylerwalls

Happy to do it, @saykharng, I left a few more comments that might say "outdated" because they were made in between your force-pushes, but I gave them a quick scan and I think they're all still relevant.

Also, be careful about the files you check into the repo, I noticed you checked in a .swp file.

docs/topics/auth/username.txt

django/contrib/auth/models.py

docs/ref/settings.txt

docs/releases/3.2.txt

fixed incorrect line break

saykharng · 2020-08-15T11:48:34Z

Happy to do it, @saykharng, I left a few more comments that might say "outdated" because they were made in between your force-pushes, but I gave them a quick scan and I think they're all still relevant.

Also, be careful about the files you check into the repo, I noticed you checked in a .swp file.

Thank you!

Fixed the docs based on comments (looking at a html page was much easier to find issue.)
Removed username_validator variable
Removed word 'English' from 'English letters' in UnicodeUsernameValidator

smithdc1 · 2020-08-09T20:31:03Z

django/contrib/auth/validators.py

Not sure about the phrase 'English letters'. (As an English speaker this phrase makes me cringe. French, German, Spanish etc all use these characters as well, do folk who speak these languages really call this set of letters 'English'?)

Could be 'letters from the Latin alphabet'?

https://en.m.wikipedia.org/wiki/Latin_alphabet

From my experience speaking those languages and as a native speaker of English, I also vote for 'letters from the Latin alphabet'. Some languages (like Chinese) tend to use "English" for everything to do with Europe, but none of the European languages I speak do in my experience!

Thank you! I have made the changes as suggested.

matthiask

I really like replacing "English" with "Latin alphabet"
Maybe you could move the existing username validators to the username_validation module too since you have been inspired by the password_validation module (and of course leave backwards compatibility shims in django.contrib.auth.validators.) (Not important, just a suggestion.)

matthiask · 2020-09-20T13:50:06Z

django/contrib/auth/validators.py

+
+
+@deconstructible
+class UsernameMinimumLengthValidator:


I'm unsure whether there's a good reason for this validator, since django.core.validators.MinLengthValidator (https://docs.djangoproject.com/en/3.1/ref/validators/#minlengthvalidator) exists already.

I know there's a MinimumLengthValidator in the password_validation.py module but maybe that one is a bit redundant too.

Thank you for the feedback. Apologies for extreme delay in response.

I have made suggested changes other than moving validators to username_validation.

matthiask · 2020-09-20T13:51:25Z

docs/ref/settings.txt

+
+The list of validators that are used to check username requirement.
+See :ref:`username-validation` for more details. By default, 
+UnicodeUsernameValidator is used to perform the validation.


Use :class:`~django.contrib.auth.validators.UnicodeUsernameValidator` here

matthiask · 2020-09-20T13:51:33Z

docs/ref/settings.txt

+
+    [
+        {
+            'NAME': 'contrib.auth.validators.UnicodeUsernameValidator'


"django." is missing

matthiask · 2020-09-20T13:57:04Z

docs/topics/auth/username.txt

+their behavior.
+
+Validation is controlled by the :setting:`AUTH_USERNAME_VALIDATORS` setting.
+The default for the setting is a ``UnicodeUsernameValidator`` which


":class:`~...`" again

matthiask · 2020-09-20T13:57:44Z

docs/topics/auth/username.txt

+        {
+            'NAME': 'django.contrib.auth.validators.UsernameMinimumLengthValidator',
+            'OPTIONS':{
+                'min_length':3,


Spaces after : missing

matthiask · 2020-09-20T13:58:45Z

docs/topics/auth/username.txt

+                'Your username must contain at least %(min_length)d 
+                character.',
+                'Your username must contain at least %(min_length)d
+                characters.',


Strange line breaks

…hanges to test and docs, fixed docs as per PR suggestion.

jacobtylerwalls

glad to see this moving along. some minor fixes after your last push.

jacobtylerwalls · 2020-10-12T13:28:47Z

tests/auth_tests/test_validators.py

                'OPTIONS': {
-                    'min_length': 3,
+                    'limit_value': 3,
+                    'message': 'This password is too short',


username, not password. :-)

glad to see this moving along. some minor fixes after your last push.

Thank you! that is so embarrassing. I actually read everything in the docs this time in html.

-- Fixed all the typos and the line break.
-- Added the new method help_text for Unicode and ASCII username validator in the docs
-- Added the UsernameMinimumLength validator in list of Validators as well.

saykharng · 2020-10-13T10:56:58Z

docs/topics/auth/username.txt

-                self.min_length
-            ) % {'min_length': self.min_length}
+                'Your username must contain at least %(limit_value)d
+                character.','Your username must contain at least 


saykharng · 2020-10-13T10:57:01Z

docs/topics/auth/username.txt

-    The minimum length can be customized with the ``min_length`` parameter.
+    Validates whether the username meets a minimum length using 
+    ``limit_value``. This validator is extended from 
+    ``django.core.validators.MinLengthValidator``. The text given in ``message``


saykharng · 2020-10-13T10:57:03Z

docs/topics/auth/username.txt


-Here's a basic example of a validator, with one optional setting::
+Here's a basic example of a validator. This validator extends 
+``django.core.validators.MinLengthValidator``::


saykharng · 2020-10-13T10:57:06Z

docs/topics/auth/username.txt

            'OPTIONS':{
-                'min_length':3,
+                'input_value': 3,
+                'message': 'Password is too short.'


…lude the new method help_text.

jacobtylerwalls · 2020-10-24T19:23:33Z

Hi again. I think to move this forward what you likely need is proof that this has support to get into Django. I was glad to see you post on the mailing list when I suggested it in July, but perhaps the question you asked was so narrow it didn't introduce the subject properly.

I think you need to ask more broadly whether this feature should be fashioned after password validation with a dict of validators in global_settings.py, and if so, whether your implementation is on track, and further, whether it should be documented with an entire new page almost identical to the password validation page, or whether they should be integrated. One respondent on the ticket modeled an approach similar to yours, so that's promising.

Minor things to make this easier to review: please update the body of the description of the PR with the ticket number (ticket-27807), and make the PR title describe the approach you used, something like: "Made username validation configurable through AUTH_USERNAME_VALIDATORS setting"

Even if people end up suggesting a different approach, you will have still made a significant contribution in terms of putting forth a concrete approach for folks to look at. 👍

saykharng · 2020-11-08T09:15:28Z

Hi again. I think to move this forward what you likely need is proof that this has support to get into Django. I was glad to see you post on the mailing list when I suggested it in July, but perhaps the question you asked was so narrow it didn't introduce the subject properly.

I think you need to ask more broadly whether this feature should be fashioned after password validation with a dict of validators in global_settings.py, and if so, whether your implementation is on track, and further, whether it should be documented with an entire new page almost identical to the password validation page, or whether they should be integrated. One respondent on the ticket modeled an approach similar to yours, so that's promising.

Minor things to make this easier to review: please update the body of the description of the PR with the ticket number (ticket-27807), and make the PR title describe the approach you used, something like: "Made username validation configurable through AUTH_USERNAME_VALIDATORS setting"

Even if people end up suggesting a different approach, you will have still made a significant contribution in terms of putting forth a concrete approach for folks to look at. 👍

Thank you again. I have started a group conversation and updated the detail in PR as well. :)

carltongibson

Hi @saykharng. Thanks for the input here, but I think we have to close this as wontfix.

The suggested fix here adds a setting, as signal, and sub-api to validators — all of which are significant changes, and I think none of which are justified by the issues.

How do I customize username validation? First and foremost, it's not clear you should: UnicodeUsernameValidator is a good default. But if you want to, use a custom user model. If you can't do that, add the validation you want at the form level, or in clean(), or if you really must monkey patch the field in a ready() handler, and so on.

There are lots of option that don't need so much structure.

I hope that makes sense. I will update the ticket accordingly.

saykharng · 2020-11-11T09:28:08Z

Hi @saykharng. Thanks for the input here, but I think we have to close this as wontfix.

The suggested fix here adds a setting, as signal, and sub-api to validators — all of which are significant changes, and I think none of which are justified by the issues.

How do I customize username validation? First and foremost, it's not clear you should: UnicodeUsernameValidator is a good default. But if you want to, use a custom user model. If you can't do that, add the validation you want at the form level, or in clean(), or if you really must monkey patch the field in a ready() handler, and so on.

There are lots of option that don't need so much structure.

I hope that makes sense. I will update the ticket accordingly.

@carltongibson
Appreciate your time and thank you for the feedback.
I did learn a lot about Django, tests and documentation while working on it.

In future, I will open up discussion/share ideas earlier.

Regards
Shekhar

saykharng changed the title ~~Ticket 27807~~ Fixed #27807 -- Added ValidatorList and UsernameValidatorList to allow username validator cutomization Jun 27, 2020