Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#34002 closed New feature (wontfix)

PasswordResetView should work with AbstractBaseUser subclasses without is_active field.

Reported by: Brylie Christopher Oxley Owned by: nobody
Component: contrib.auth Version: 4.1
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

I have a user model that extends AbstractBaseUser. When trying to reset a user password via the django.contrib.auth.views.PasswordResetView, I get the following error:

FieldError at /accounts/password_reset/
Cannot resolve keyword 'is_active' into field.

While the AbstractBaseUser class does have an is_active property set to True, it seems that the PasswordResetView is somehow excluding this property.

The issue is resolved by adding an is_active BooleanField to my User model, but seems like a bug in PasswordResetView.

Change History (3)

comment:1 by Mariusz Felisiak, 2 years ago

Resolution: invalid
Status: newclosed

This is a documented limitation:

"PasswordResetForm: Assumes that the user model has a field that stores the user’s email address with the name returned by get_email_field_name() (email by default) that can be used to identify the user and a boolean field named is_active to prevent password resets for inactive users."

comment:2 by Brylie Christopher Oxley, 2 years ago

Thanks for pointing me to the docs Mariusz.

I still believe this is a bug for a couple of reasons. First, Django is inconsistent since the AbstractBaseUser can't be used in the standard password reset views, despite having the is_active boolean property. Likewise, the default Django password reset flow is not resilient to User models that don't have is_active defined as a field, even when the downstream project may not need an activation flow (or use the same model field/property).

I believe there could actually be a middle ground here if we could give this issue a bit more time for consideration, rather than immediately dismissing it as invalid. One solution could be to check for the existence of the is_active property/field and run the relevant checks only if the field/property has a value.

From what I understand, some contributor has likely added the is_active boolean property to the AbstractBaseUser model to harmonize it with other parts of the authentication framework. Conversely, the different parts of the authentication framework, namely the password reset flow, could be made more resilient to the absence of an is_active field/property.

comment:3 by Mariusz Felisiak, 2 years ago

Resolution: invalidwontfix
Summary: PasswordResetView cannot find is_active property of User model extending AbstractBaseUserPasswordResetView should work with AbstractBaseUser subclasses without is_active field.
Type: UncategorizedNew feature

I still believe this is a bug for a couple of reasons.

Documented behavior cannot be called a bug (it's documented since f0425c72601f466c6a71518749c6d15b94945514). Password reset views/forms make various assumptions about the user model and all of them are documented.

As far as I'm aware you're proposing a new feature. You can always define your own subclass of PasswordResetForm and override the get_users() method.

As it's a long standing behavior in a security-related area, I'm closing this as "wontfix" with a request for a discussion on the mailing list, so please first start a discussion on the DevelopersMailingList, where you'll reach a wider audience and see what other think. Also, follow the guidelines with regards to requesting features. Thanks.

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