[Django] #32553: Allow UserAttributeSimilarityValidator to validate against fields of related models
Django
related models
----------------------------------------+------------------------
Reporter: Meiyer | Owner: nobody
Type: New feature | Status: new
Component: contrib.auth | Version: dev
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
----------------------------------------+------------------------
The password validator `UserAttributeSimilarityValidator` is currently too
rigid and allows comparison of the password only to the fields of the
`User` model, while ignoring the common case where there is also a profile
attached to the user (as a separate model). My suggestion is to relax the
attribute resolution so that also fields defined on related models could
be accessed; for example, by traversing a dotted path as in
`reduce(getattr, path.split('.'), obj)`.
Yet a better option could be to allow plugging into the validator by means
of an overridable method `get_user_attribute(self, attribute_name)` used
instead of a direct `getattr`, that will allow more customisation while
leaving the validation logic intact.
--
Ticket URL: <https://code.djangoproject.com/ticket/32553>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
related models
Reporter: Meiyer | Owner: nobody
Type: New feature | Status: new
Component: contrib.auth | Version: dev
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Description changed by Meiyer:
Old description:
> The password validator `UserAttributeSimilarityValidator` is currently
> too rigid and allows comparison of the password only to the fields of the
> `User` model, while ignoring the common case where there is also a
> profile attached to the user (as a separate model). My suggestion is to
> relax the attribute resolution so that also fields defined on related
> models could be accessed; for example, by traversing a dotted path as in
> `reduce(getattr, path.split('.'), obj)`.
>
> Yet a better option could be to allow plugging into the validator by
> means of an overridable method `get_user_attribute(self, attribute_name)`
> used instead of a direct `getattr`, that will allow more customisation
> while leaving the validation logic intact.
New description:
The password validator `UserAttributeSimilarityValidator` is currently too
rigid and allows comparison of the password only to the fields of the
`User` model, while ignoring the common case where there is also a profile
attached to the user (as a separate model). My suggestion is to relax the
attribute resolution so that also fields defined on related models could
be accessed; for example, by traversing a dotted path as in
`reduce(getattr, path.split('.'), obj)`.
Yet a better option could be to allow plugging into the validator by means
of an overridable method `get_user_attribute(self, attribute_name)` used
instead of a direct `getattr`, that will allow more customisation while
leaving the validation logic intact.
(Related to #26909 and #28127)
--
--
Ticket URL: <https://code.djangoproject.com/ticket/32553#comment:1>
Django
related models
Reporter: Meiyer | Owner: nobody
Type: New feature | Status: closed
Component: contrib.auth | Version: dev
Severity: Normal | Resolution: wontfix
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 Mariusz Felisiak):
* status: new => closed
* resolution: => wontfix
Comment:
Thanks for this ticket, however I don't think it's worth addition
complexity. You can always add properties based on related models and pass
them to the validator in `user_attributes`.
--
Ticket URL: <https://code.djangoproject.com/ticket/32553#comment:2>
Django
related models
Reporter: Meiyer | Owner: nobody
Type: New feature | Status: closed
Component: contrib.auth | Version: dev
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Meiyer):
1. I do not see the complexity being mentioned in Mariusz’s feedback. The
change required for the validator itself is customisable logic to obtain
the attribute’s `value` (and possibly `verbose_name`) by substitution of
the line which performs `getattr` with a call to
`self.get_user_attribute`. This pluggable helper could be defined per
default as
{{{#!python
def get_user_attribute(self, user, attribute_name):
return getattr(user, attribute_name, None)
}}}
2. It is not possible to add properties to the built-in `User` model, and
switching the `User` model for code which runs in production multiple
years already with thousands of users is an endeavour too risky.
--
Ticket URL: <https://code.djangoproject.com/ticket/32553#comment:3>
Django
related models
Reporter: Meiyer | Owner: nobody
Type: New feature | Status: closed
Component: contrib.auth | Version: dev
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak):
> I do not see the complexity being mentioned in Mariusz’s feedback.
I was talking about complexity of supporting related fields e.g. with
lookups `user_attributes=('profile__description', 'username')`. On the
other hand adding `get_user_attribute()` to
`UserAttributeSimilarityValidator` is misleading, validators are not a
place for such hooks, which are rather workarounds for a specific
situation. `get_user_attribute()` would be useful but for a quite niche
scenario i.e. when you have a user model with custom profile but you
cannot control user properties.
Finally, `UserAttributeSimilarityValidator` is a small validator, you can
always create a custom implementation in your code.
You can start a discussion on DevelopersMailingList if you don't agree.
--
Ticket URL: <https://code.djangoproject.com/ticket/32553#comment:4>