Change password form fixes and clean up (#1111)

* Add set password form (fixes #1110)
* Simplify and minimize django-allauth customization
* Use standard Django password validator for min length

open_humans/account_views.py

@@ -7,21 +7,19 @@
 from django.shortcuts import redirect
 from django.urls import reverse, reverse_lazy
 from django.views.generic import View
-from django.views.generic.edit import DeleteView, FormView
+from django.views.generic.edit import DeleteView
 
 import allauth.account.app_settings as allauth_settings
-from allauth.account.forms import (
-    AddEmailForm as AllauthAddEmailForm,
-    default_token_generator,
-)
-from allauth.account.utils import perform_login, url_str_to_user_pk
+from allauth.account.forms import AddEmailForm as AllauthAddEmailForm
 from allauth.account.models import EmailAddress
 from allauth.account.views import (
     ConfirmEmailView as AllauthConfirmEmailView,
     LoginView as AllauthLoginView,
     EmailView as AllauthEmailView,
     PasswordChangeView as AllauthPasswordChangeView,
     PasswordResetView as AllauthPasswordResetView,
+    PasswordResetFromKeyView as AllauthPasswordResetFromKeyView,
+    PasswordSetView as AllauthPasswordSetView,
     SignupView as AllauthSignupView,
 )
 from allauth.socialaccount.helpers import complete_social_login
@@ -32,14 +30,14 @@
 from common.mixins import PrivateMixin
 
 from .forms import (
-    ChangePasswordForm,
     MemberLoginForm,
     MemberSignupForm,
-    PasswordResetForm,
     ResetPasswordForm,
     SocialSignupForm,
 )
-from .models import User, Member
+from .models import Member
+
+User = get_user_model()
 
 
 class MemberLoginView(AllauthLoginView):
@@ -134,90 +132,41 @@ def get_object(self, queryset=None):
 
 class ResetPasswordView(AllauthPasswordResetView):
     """
-    Ooops, we've done lost our password, Martha!  Subclasses Allauth's
-    view to use our template and use our own form which preserves the
+    Subclasses Allauth's to use our own form which preserves the
     next url for when the password reset process is complete.
     """
 
-    template_name = "account/password_reset.html"
     form_class = ResetPasswordForm
-    success_url = reverse_lazy("account_reset_password_done")
 
 
-class PasswordResetFromKeyView(FormView):
+class PasswordResetFromKeyView(AllauthPasswordResetFromKeyView):
     """
-    Let's get a new password!  Allauth tries to be fancy with ajax,
-    but we don't really use ajax ourselves, so this view does the work
-    of getting the key and calling the check functions directly.
+    Override form_valid to use stored redirect on login.
     """
 
-    template_name = "account/password_reset_token.html"
-    form_class = PasswordResetForm
+    def form_valid(self, form):
+        default_return = super().form_valid(form)
+        next_url = self.reset_user.member.password_reset_redirect
+        if next_url and allauth_settings.LOGIN_ON_PASSWORD_RESET:
+            self.reset_user.member.password_reset_redirect = ""
+            self.reset_user.member.save()
+            return redirect(next_url)
+        return default_return
 
-    def _get_user(self, uidb36):
-        User = get_user_model()
-        try:
-            pk = url_str_to_user_pk(uidb36)
-            return User.objects.get(pk=pk)
-        except (ValueError, User.DoesNotExist):
-            return None
-
-    def dispatch(self, request, uidb36, key, **kwargs):
-        self.request = request
-        self.key = key
-        self.reset_user = self._get_user(uidb36)
-        if self.reset_user is None:
-            return redirect("account-password-reset-fail")
-        token = default_token_generator.check_token(self.reset_user, key)
-        if not token:
-            return redirect("account-password-reset-fail")
-        ret = super().dispatch(request, uidb36, key, **kwargs)
-        return ret
 
-    def get_context_data(self, **kwargs):
-        ret = super().get_context_data(**kwargs)
-        ret["action_url"] = reverse(
-            "account_reset_password_from_key",
-            kwargs={"uidb36": self.kwargs["uidb36"], "key": self.kwargs["key"]},
-        )
-        return ret
-
-    def change_password(self, password):
-        user = self.reset_user
-        user.set_password(password)
-        user.save()
-        return user
+class PasswordSetView(AllauthPasswordSetView):
+    """
+    Change the success_url for password set.
+    """
 
-    def form_valid(self, form):
-        if form.is_valid():
-            self.change_password(form.clean_password())
-
-            if allauth_settings.LOGIN_ON_PASSWORD_RESET:
-                perform_login(
-                    self.request,
-                    self.reset_user,
-                    email_verification=allauth_settings.EMAIL_VERIFICATION,
-                )
-            member = Member.objects.get(user=self.reset_user)
-            next_url = member.password_reset_redirect
-            member.password_reset_redirect = ""  # Clear redirect from db
-            member.save()
-            messages = {
-                "settings_updated": {
-                    "level": django_messages.SUCCESS,
-                    "text": "Password successfully reset.",
-                }
-            }
-            return redirect(next_url)
+    success_url = reverse_lazy("my-member-settings")
 
 
 class PasswordChangeView(AllauthPasswordChangeView):
     """
-    Change the password, subclass allauth to use our own template
+    Change success_url for password change.
     """
 
-    template_name = "account/password_change.html"
-    form_class = ChangePasswordForm
     success_url = reverse_lazy("my-member-settings")
 
 

open_humans/forms.py

@@ -13,6 +13,7 @@
     LoginForm as AllauthLoginForm,
     ResetPasswordForm as AllauthResetPasswordForm,
     SignupForm as AllauthSignupForm,
+    SetPasswordForm as AllauthSetPasswordForm,
 )
 from allauth.account.models import EmailAddress
 from allauth.account.utils import filter_users_by_email
@@ -69,9 +70,8 @@ class MemberSignupForm(AllauthSignupForm):
     """
     A subclass of django-allauth's SignupForm with additions.
 
-    A `terms` field is added for the Terms of Use checkbox, a `name` field
-    is added to store a Member's username, and additional validation is
-    added for passwords to impose a minimum length.
+    A `terms` field is added for the Terms of Use checkbox, and a `name` field
+    is added to store a Member's username.
     """
 
     name = forms.CharField(max_length=30)
@@ -80,48 +80,6 @@ class MemberSignupForm(AllauthSignupForm):
     class Meta:  # noqa: D101
         fields = "__all__"
 
-    def clean_password(self):
-        return _clean_password(AllauthSignupForm, self, "password")
-
-
-class ChangePasswordForm(AllauthChangePasswordForm):
-    """
-    A subclass of account's ChangePasswordForm that checks password length.
-    """
-
-    def clean_password_new(self):
-        return _clean_password(ChangePasswordForm, self, "password_new")
-
-
-class PasswordResetForm(forms.Form):
-    """
-    Change the user's password, matches our template better than the form class
-    shipped by allauth.
-    """
-
-    password = forms.CharField(
-        label="New Password", widget=forms.PasswordInput(render_value=False)
-    )
-    password_confirm = forms.CharField(
-        label="New Password (again)", widget=forms.PasswordInput(render_value=False)
-    )
-
-    def clean(self):
-        super().clean()
-        if self._errors:
-            return
-
-        if "password" in self.cleaned_data and "password_confirm" in self.cleaned_data:
-            if self.cleaned_data["password"] != self.cleaned_data["password_confirm"]:
-                self.add_error(
-                    "password_confirm", "You must type the same password each time."
-                )
-
-        return self.cleaned_data
-
-    def clean_password(self):
-        return _clean_password(PasswordResetForm, self, "password")
-
 
 class MemberProfileEditForm(forms.ModelForm):
     """

open_humans/settings.py

@@ -303,6 +303,12 @@ def to_bool(env, default="false"):
 LOGIN_REDIRECT_URL = "home"
 
 AUTH_USER_MODEL = "open_humans.User"
+AUTH_PASSWORD_VALIDATORS = [
+    {
+        "NAME": "django.contrib.auth.password_validation.MinimumLengthValidator",
+        "OPTIONS": {"min_length": 8},
+    }
+]
 
 ACCOUNT_ADAPTER = "common.adapters.MyAccountAdapter"
 ACCOUNT_AUTHENTICATED_LOGIN_REDIRECTS = True

open_humans/templates/account/password_change.html

@@ -8,7 +8,7 @@
 {% block panel_content %}
 <div class="row pad-all-sides">
   <form class="form-horizontal" role="form" method="POST"
-    action="{% url 'account_password' %}" id="password-change-form">
+    action="{% url 'account_change_password' %}" id="password-change-form">
     {% csrf_token %}
 
     <input type="hidden" name="next" value="{% next_page %}">

open_humans/templates/account/password_set.html

@@ -0,0 +1,22 @@
+{% extends 'panel.html' %}
+
+{% load bootstrap_tags %}
+{% load utilities %}
+
+{% block head_title %}Set Password{% endblock %}
+
+{% block panel_content %}
+<div class="row pad-all-sides">
+  <form class="form-horizontal" role="form" method="POST"
+    action="{% url 'account_set_password' %}" id="password-set-form">
+    {% csrf_token %}
+
+    <input type="hidden" name="next" value="{% next_page %}">
+
+    {{ form|as_bootstrap_horizontal:'col-md-4' }}
+
+    <input id="password-set" type="submit" value="Set password"
+      form="password-set-form" class="btn btn-primary col-md-offset-4">
+  </form>
+</div>
+{% endblock %}

open_humans/templates/member/my-member-settings.html

@@ -79,7 +79,7 @@ <h5>Allow other members to contact you?</h5>
     </div>
 
     <div class="col-md-6">
-      <a href="{% url 'account_password' %}?next={% url 'my-member-settings' %}"
+      <a href="{% url 'account_change_password' %}?next={% url 'my-member-settings' %}"
           class="btn btn-sm btn-primary">
         Change password
       </a>

open_humans/tests.py

@@ -89,7 +89,8 @@ class SmokeTests(SmokeTestCase):
     ]
 
     authenticated_urls = redirect_urls + [
-        "/account/password/",
+        "/account/password/change/",
+        "/account/password/set/",
         (
             "/oauth2/authorize/?origin=external&response_type=code"
             "&scope=go-viral%20read%20write&client_id=example-id-15"
@@ -168,12 +169,13 @@ def test_password_reset(self):
         key = reset_url.split("/")[7]
         # Go ahead and reset the mailbox
         mail.outbox = []
-        do_reset_response = self.client.get(reset_url)
+        do_reset_response = self.client.get(reset_url, follow=True)
         self.assertEqual(do_reset_response.status_code, 200)
         self.assertContains(do_reset_response, "Set your new password")
 
         do_reset_post_response = self.client.post(
-            reset_url, {"password": "asdfqwerty", "password_confirm": "asdfqwerty"}
+            do_reset_response.redirect_chain[-1][0],
+            {"password1": "asdfqwerty", "password2": "asdfqwerty"},
         )
         self.assertEqual(do_reset_post_response.status_code, 302)
         self.assertEqual(do_reset_post_response.url, redirect)

open_humans/urls.py

@@ -102,22 +102,22 @@
     path(
         "account/login/", account_views.MemberLoginView.as_view(), name="account_login"
     ),
-    # More overrides - custom forms to enforce password length minimum.
+    # Override to change success_url.
     path(
-        "account/password/",
+        "account/password/set/",
+        account_views.PasswordSetView.as_view(),
+        name="account_set_password",
+    ),
+    path(
+        "account/password/change/",
         account_views.PasswordChangeView.as_view(),
-        name="account_password",
+        name="account_change_password",
     ),
     re_path(
         r"^account/confirm-email/(?P<key>[-:\w]+)/$",
         account_views.ConfirmEmailView.as_view(),
         name="account_confirm_email",
     ),
-    path(
-        "account/password/change/",
-        account_views.PasswordChangeView.as_view(),
-        name="account_change_password",
-    ),
     path(
         "account/password/reset/done/",
         TemplateView.as_view(template_name="account/password_reset_sent.html"),