Opened 4 years ago

Closed 4 years ago

#23409 closed New feature (fixed)

PasswordResetForm should not exclude users with unusable passwords

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

Description

Currently django.contrib.auth.PasswordResetForm will (silently) not send a password reset email to any user who has an unusable password set. Additionally, due to the structure of the code, its not possible to subclass PasswordResetForm to change this behavior without copying the entire 40-line save() method.

This behavior was introduced in #14674, on the theory that a user with an unusable password probably comes from some external authentication source (e.g. LDAP), and should not be allowed to reset their password and then bypass the external authentication source.

That's a reasonable policy for some situations, but there are many other reasons why one might set an unusable password (e.g. when creating a user account for someone else), and it's not at all obvious that "unusable password" should always imply "unable to reset password."

If I could go back in time, I would argue that #14674 should never have been committed, but since it was (and there have been several Django releases since), I think the default behavior should probably be left as-is for backwards-compatibility reasons.

However, I think it should be easy to subclass PasswordResetForm and change this policy. I will submit a pull request that extracts a def get_users(self, email): method of PasswordResetForm, whose responsibility it is, given an email address, to return the matching users who should receive a password-reset link.

Change History (3)

comment:1 Changed 4 years ago by Carl Meyer

Has patch: set

https://github.com/django/django/pull/3157

No new tests because this is a pure refactoring, not a behavior change. Existing tests pass.

comment:2 Changed 4 years ago by Aymeric Augustin

Looks good. Before committing this change, can you extend the docstring a bit to explain why this method was extracted?

comment:3 Changed 4 years ago by Carl Meyer <carl@…>

Resolution: fixed
Status: newclosed

In 89559bcfb096ccc625e0e9ab41e2136fcb32a514:

Fixed #23409 -- Extract PasswordResetForm.get_users method.

Allows easier customization of policies regarding which users are allowed to
reset their password.

Thanks Aymeric for review.

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