[Django] #35693: Password validators aren't callable

[Django]

#35693: Password validators aren't callable
-------------------------------------+-------------------------------------
Reporter: iamkorniichuk | Type:
| Cleanup/optimization
Status: new | Component:
| contrib.auth
Version: 5.1 | Severity: Normal
Keywords: validator,password | Triage Stage:
validator,callable | Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
By default validating fields use this syntax {{{validator(value)}}} but
password validators don't implement {{{.__call__()}}}. Therefore making
them unusable outside of {{{validate_password()}}}.
This is small change doesn't change any behavior but add support for
intuitive approach to password validators.
--
Ticket URL: <https://code.djangoproject.com/ticket/35693>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#35693: Password validators aren't callable
-------------------------------------+-------------------------------------

Reporter: iamkorniichuk | Owner: (none)
Type: | Status: new
Cleanup/optimization |
Component: contrib.auth | Version: 5.1
Severity: Normal | Resolution:

Keywords: validator,password | Triage Stage:
validator,callable | Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by iamkorniichuk):

I think it will be enough to add such a method to
{{{django.contrib.auth.password_validation}}} classes:
{{{
def __call__(self, *args, **kwargs):
return self.validate(*args, **kwargs)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/35693#comment:1>

[Django]

#35693: Password validators aren't callable
-------------------------------------+-------------------------------------
Reporter: iamkorniichuk | Owner:

Type: | iamkorniichuk
Cleanup/optimization | Status: assigned
Component: contrib.auth | Version: dev
Severity: Normal | Resolution:
Keywords: validators password | Triage Stage: Accepted
callable |
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Natalia Bidart):

* keywords: validator,password validator,callable => validators password
callable
* needs_docs: 0 => 1
* needs_tests: 0 => 1
* owner: (none) => iamkorniichuk
* stage: Unreviewed => Accepted
* status: new => assigned
* version: 5.1 => dev

Comment:

Thank you iamkorniichuk for creating this ticket.

As I mentioned in your PR, I agree that the inconsistency between the
django.core.validators providing `__call__` but password validators not
providing it feels off. Accepting on that basis, also I couldn't find a
previous conversation or request about this issue.

Please note that the PR definitely needs tests and docs updates, besides a
small release note for 5.2.

Good luck!
--
Ticket URL: <https://code.djangoproject.com/ticket/35693#comment:2>

[Django]

#35693: Password validators aren't callable
-------------------------------------+-------------------------------------
Reporter: iamkorniichuk | Owner:
Type: | iamkorniichuk
Cleanup/optimization | Status: assigned
Component: contrib.auth | Version: dev
Severity: Normal | Resolution:
Keywords: validators password | Triage Stage: Accepted
callable |
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Antoliny):

Can I work on this ticket?
--
Ticket URL: <https://code.djangoproject.com/ticket/35693#comment:3>

[Django]

#35693: Password validators aren't callable
-------------------------------------+-------------------------------------

Reporter: iamkorniichuk | Owner: Antoliny
Type: | Status: assigned
Cleanup/optimization |

Component: contrib.auth | Version: dev
Severity: Normal | Resolution:
Keywords: validators password | Triage Stage: Accepted
callable |
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Antoliny):

* owner: iamkorniichuk => Antoliny

--
Ticket URL: <https://code.djangoproject.com/ticket/35693#comment:4>

[Django]

#35693: Password validators aren't callable
-------------------------------------+-------------------------------------
Reporter: iamkorniichuk | Owner: Antoliny
Type: | Status: assigned
Cleanup/optimization |
Component: contrib.auth | Version: dev
Severity: Normal | Resolution:
Keywords: validators password | Triage Stage: Accepted
callable |
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Antoliny):

If we provide {{{.__call__}}} magic method to the password validator
class, it can be used like a regular validator class.
{{{
# password_validation.py

class CommonPasswordValidator:

def __call__(self, *args, **kwargs):
return self.validate(*args, **kwargs)

# views.py
from django import forms
from django.contrib.auth.password_validation import
CommonPasswordValidator

valid_common_password = CommonPasswordValidator()

class TestForm(forms.Form):
email = forms.EmailField()
password = forms.CharField(validators=[valid_common_password])

>>> form = TestForm(data={"email":"antoli...@gmail.com",
"password":"1234"})
>>> form.is_valid()
False
>>> form.errors
{'password': ['This password is too common.']}
}}}

However, unlike the general validation class, the password validation
requires two arguments for verification. (password, user)
{{{
def validate_password(password, user=None, password_validators=None):
...
for validator in password_validators:
try:
validator.validate(password, user)
}}}
Fortunately, the validation class validate method specifies the default
value of the user argument to None, so there is no problem calling it, but
I don't know if __call__ is required for classes such as
UserAttributeSimilarityValidator because the validate is terminated when
the user argument does not exist.
I think it's a good way to add {{{.__call__}}}magic method to the password
validation class, but there are questions as above. I'm sorry, but could
you please review it again?
--
Ticket URL: <https://code.djangoproject.com/ticket/35693#comment:5>

[Django]

#35693: Password validators aren't callable
-------------------------------------+-------------------------------------
Reporter: iamkorniichuk | Owner: Antoliny
Type: | Status: assigned
Cleanup/optimization |
Component: contrib.auth | Version: dev
Severity: Normal | Resolution:
Keywords: validators password | Triage Stage:

callable | Unreviewed

Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Antoliny):

* stage: Accepted => Unreviewed

--
Ticket URL: <https://code.djangoproject.com/ticket/35693#comment:6>

[Django]

#35693: Password validators aren't callable
-------------------------------------+-------------------------------------
Reporter: iamkorniichuk | Owner: Antoliny
Type: | Status: assigned
Cleanup/optimization |
Component: contrib.auth | Version: dev
Severity: Normal | Resolution:

Keywords: validators password | Triage Stage: Accepted
callable |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Antoliny):

* needs_docs: 1 => 0
* needs_tests: 1 => 0

[Django]

#35693: Password validators aren't callable
-------------------------------------+-------------------------------------
Reporter: iamkorniichuk | Owner: Antoliny
Type: | Status: assigned
Cleanup/optimization |
Component: contrib.auth | Version: dev
Severity: Normal | Resolution:
Keywords: validators password | Triage Stage: Accepted
callable |

Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Sarah Boyce):

* needs_better_patch: 0 => 1

* needs_docs: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/35693#comment:7>

[Django]

#35693: Password validators aren't callable
-------------------------------------+-------------------------------------
Reporter: iamkorniichuk | Owner: Antoliny
Type: | Status: assigned
Cleanup/optimization |
Component: contrib.auth | Version: dev
Severity: Normal | Resolution:
Keywords: validators password | Triage Stage: Accepted
callable |
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Sarah Boyce):

Also commented on the PR but I'm not 100% sure this is worth it just for
the consistency, but if we were to do this:

* validators also have an `__eq__` method, if we are adding the call for
consistency, I feel like we should add this for consistency. We should
also consider whether to inherit from `BaseValidator` for example.
* I would expect the docs in the
[https://docs.djangoproject.com/en/5.1/topics/auth/passwords/#writing-
your-own-validator Writing your own validator] section to be updated.
* Any custom validator not provided by Django will not have this change

I think the next steps would be to raise this on
[https://forum.djangoproject.com/c/internals/5 forum] and see if there is
a consensus that this additional is valuable and wanted.

I would suggest instead that the
[https://docs.djangoproject.com/en/5.1/topics/auth/passwords/#module-
django.contrib.auth.password_validation Password validation] docs make it
clear that these validators are not field validators and perhaps say
"password validator" instead of "validator" consistently
--
Ticket URL: <https://code.djangoproject.com/ticket/35693#comment:8>

[Django]

#35693: Password validators aren't callable
-------------------------------------+-------------------------------------
Reporter: iamkorniichuk | Owner: Antoliny
Type: | Status: assigned
Cleanup/optimization |
Component: contrib.auth | Version: dev
Severity: Normal | Resolution:
Keywords: validators password | Triage Stage: Accepted
callable |
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Antoliny):

[https://forum.djangoproject.com/t/improving-consistency-between-field-
validators-and-password-validators/37040 discussion]
--
Ticket URL: <https://code.djangoproject.com/ticket/35693#comment:9>

[Django]

#35693: Password validators aren't callable
-------------------------------------+-------------------------------------
Reporter: iamkorniichuk | Owner: Antoliny

Type: | Status: closed

Cleanup/optimization |
Component: contrib.auth | Version: dev

Severity: Normal | Resolution: wontfix

Keywords: validators password | Triage Stage: Accepted
callable |
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Sarah Boyce):

* cc: Ryan Hiebert (added)
* resolution: => wontfix
* status: assigned => closed

Comment:

By default validating fields use this syntax validator(value) but password
validators don’t implement `.__call__()`. Therefore making them unusable
outside of `validate_password()`.

As Ryan pointed out in the forum discussion, these are not “unusable
outside `validate_password()`” as they can be used with:
{{{
CharField(..., validators=[MinimumLengthValidator().validate]
}}}
Those that need a `user` will pass silently when a `user` is not passed
(adding a `.__call__()` doesn’t change that).

So I think the problem this wanted to solve “make them usable outside
`validate_password()`” is not really a problem and, as this was accepted
to solve this, I will close this ticket as `wontfix`.

In the forum I think there were good ideas and thoughts around the
direction of validators and password validators. I think these thoughts
need more time to crystalize before we jump into making a change here.
Once there is a solid proposal with support from the community, that
should probably be a new ticket (as I imagine it might be more than just
adding `__call__`).
--
Ticket URL: <https://code.djangoproject.com/ticket/35693#comment:10>