Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#32553 closed New feature (wontfix)

Allow UserAttributeSimilarityValidator to validate against fields of related models

Reported by: Meiyer Owned by: nobody
Component: contrib.auth Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Meiyer)

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)

Change History (4)

comment:1 by Meiyer, 3 years ago

Description: modified (diff)

comment:2 by Mariusz Felisiak, 3 years ago

Resolution: wontfix
Status: newclosed

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.

comment:3 by Meiyer, 3 years ago

  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
  def get_user_attribute(self, user, attribute_name):
      return getattr(user, attribute_name, None)
  1. 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.

in reply to:  3 comment:4 by Mariusz Felisiak, 3 years ago

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.

Note: See TracTickets for help on using tickets.
Back to Top