Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed #35693 -- Made password validators callable. #18481

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

iamkorniichuk
Copy link

@iamkorniichuk iamkorniichuk commented Aug 14, 2024

Trac ticket number

#35693

Branch description

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.

Checklist

  • This PR targets the main branch.
  • The commit message is written in past tense and ends with a period.
  • I have checked the "Has patch" ticket flag in the Trac system.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello! Thank you for your contribution 💪

As it's your first contribution be sure to check out the patch review checklist.

If you're fixing a ticket from Trac make sure to set the "Has patch" flag and include a link to this PR in the ticket!

If you have any design or process questions then you can ask in the Django forum.

Welcome aboard ⛵️!

@RealOrangeOne
Copy link
Contributor

Given validator is a class, I'd expect to call a .validate method off it, rather than calling the class directly.

You can also call validate_password('hunter2', password_validators=[validator]), which whilst more verbose might be clearer.

@iamkorniichuk
Copy link
Author

Given validator is a class, I'd expect to call a .validate method off it, rather than calling the class directly.

I expect class-based validators (including password validators) to have the same logic because they do the exact same thing and follow the exact same naming "convention".
For example, Django default class RegexValidator implement .__call__() method for validating usage and don't even have .validate() method.

@nessita
Copy link
Contributor

nessita commented Aug 15, 2024

I think the overall goal of this PR makes sense, and I agree that it feels weird that auth's validators do not implement __call__ like core validators do.

@iamkorniichuk Since this is not a trivial PR, could you please create a ticket and follow the guidelines for contributing to Django? Specifically, besides the accepted ticket, this work would need tests and a small release note for 5.2.

Thank you both for your contributions! 🌟

@iamkorniichuk iamkorniichuk changed the title Made password validators callable. Fixed #35693 -- Made password validators callable. Aug 20, 2024
@iamkorniichuk
Copy link
Author

@iamkorniichuk Since this is not a trivial PR, could you please create a ticket

I've done it.
Thanks for validating the idea. Hope it will be implemented!

@nessita
Copy link
Contributor

nessita commented Aug 20, 2024

@iamkorniichuk Since this is not a trivial PR, could you please create a ticket

I've done it. Thanks for validating the idea. Hope it will be implemented!

Thank you! Please note that this work needs tests and docs updates. Once those are completed, please remember to set the proper Trac flags in the ticket as described in the PR checklist when this is ready for re-review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants