From 088a4f1a7b9c50331b8ef4d1a03d85294eec0426 Mon Sep 17 00:00:00 2001 From: Raymond Penners Date: Fri, 15 Dec 2023 16:00:08 +0100 Subject: [PATCH] feat(account): get_password_change_redirect_url() --- ChangeLog.rst | 8 ++ allauth/account/adapter.py | 8 ++ allauth/account/tests/test_change_password.py | 87 +++++++++++++++++++ allauth/account/tests/test_reset_password.py | 28 ------ allauth/account/views.py | 46 ++++------ test_settings.py | 1 + 6 files changed, 123 insertions(+), 55 deletions(-) create mode 100644 allauth/account/tests/test_change_password.py diff --git a/ChangeLog.rst b/ChangeLog.rst index fa68267d22..b112dafcee 100644 --- a/ChangeLog.rst +++ b/ChangeLog.rst @@ -1,6 +1,14 @@ 0.60.0 (unreleased) ******************* +Note worthy changes +------------------- + +- You can now more easily change the URL to redirect to after a successful password + change/set via the newly introduced ``get_password_change_redirect_url()`` + adapter method. + + Backwards incompatible changes ------------------------------ diff --git a/allauth/account/adapter.py b/allauth/account/adapter.py index 7c82300393..5e305f940c 100644 --- a/allauth/account/adapter.py +++ b/allauth/account/adapter.py @@ -221,6 +221,14 @@ def get_email_confirmation_redirect_url(self, request): else: return app_settings.EMAIL_CONFIRMATION_ANONYMOUS_REDIRECT_URL + def get_password_change_redirect_url(self, request): + """ + The URL to redirect to after a successful password change/set. + + NOTE: Not called during the password reset flow. + """ + return reverse("account_change_password") + def is_open_for_signup(self, request): """ Checks whether or not the site is open for signups. diff --git a/allauth/account/tests/test_change_password.py b/allauth/account/tests/test_change_password.py new file mode 100644 index 0000000000..64bb56471b --- /dev/null +++ b/allauth/account/tests/test_change_password.py @@ -0,0 +1,87 @@ +from django.urls import reverse + +import pytest + + +def test_change_unusable_password_redirects_to_set(client, user, user_password): + user.set_unusable_password() + user.save() + client.force_login(user) + resp = client.get(reverse("account_change_password")) + assert resp.status_code == 302 + assert resp["location"] == reverse("account_set_password") + + +def test_set_usable_password_redirects_to_change(auth_client, user): + resp = auth_client.get(reverse("account_set_password")) + assert resp.status_code == 302 + assert resp["location"] == reverse("account_change_password") + + +@pytest.mark.parametrize( + "logout,redirect_chain", + [ + ( + False, + [ + (reverse("account_change_password"), 302), + ], + ), + ( + True, + [ + (reverse("account_change_password"), 302), + ( + f'{reverse("account_login")}?next={reverse("account_change_password")}', + 302, + ), + ], + ), + ], +) +def test_set_password(client, user, password_factory, logout, settings, redirect_chain): + settings.ACCOUNT_LOGOUT_ON_PASSWORD_CHANGE = logout + user.set_unusable_password() + user.save() + client.force_login(user) + password = password_factory() + resp = client.post( + reverse("account_set_password"), + {"password1": password, "password2": password}, + follow=True, + ) + assert resp.redirect_chain == redirect_chain + + +@pytest.mark.parametrize( + "logout,redirect_chain", + [ + ( + False, + [ + (reverse("account_change_password"), 302), + ], + ), + ( + True, + [ + (reverse("account_change_password"), 302), + ( + f'{reverse("account_login")}?next={reverse("account_change_password")}', + 302, + ), + ], + ), + ], +) +def test_change_password( + auth_client, user, user_password, password_factory, logout, settings, redirect_chain +): + settings.ACCOUNT_LOGOUT_ON_PASSWORD_CHANGE = logout + password = password_factory() + resp = auth_client.post( + reverse("account_change_password"), + {"oldpassword": user_password, "password1": password, "password2": password}, + follow=True, + ) + assert resp.redirect_chain == redirect_chain diff --git a/allauth/account/tests/test_reset_password.py b/allauth/account/tests/test_reset_password.py index 85652a5d15..8a851da218 100644 --- a/allauth/account/tests/test_reset_password.py +++ b/allauth/account/tests/test_reset_password.py @@ -62,14 +62,6 @@ def test_password_reset_get(self): resp = self.client.get(reverse("account_reset_password")) self.assertTemplateUsed(resp, "account/password_reset.html") - def test_password_set_redirect(self): - resp = self._password_set_or_change_redirect("account_set_password", True) - self.assertRedirects( - resp, - reverse("account_change_password"), - fetch_redirect_response=False, - ) - def test_set_password_not_allowed(self): user = self._create_user_and_login(True) pwd = "!*123i1uwn12W23" @@ -83,22 +75,6 @@ def test_set_password_not_allowed(self): self.assertTrue(user.has_usable_password()) self.assertEqual(resp.status_code, 302) - def test_password_change_no_redirect(self): - resp = self._password_set_or_change_redirect("account_change_password", True) - self.assertEqual(resp.status_code, 200) - - def test_password_set_no_redirect(self): - resp = self._password_set_or_change_redirect("account_set_password", False) - self.assertEqual(resp.status_code, 200) - - def test_password_change_redirect(self): - resp = self._password_set_or_change_redirect("account_change_password", False) - self.assertRedirects( - resp, - reverse("account_set_password"), - fetch_redirect_response=False, - ) - def test_password_forgotten_username_hint(self): user = self._request_new_password() body = mail.outbox[0].body @@ -318,7 +294,3 @@ def _create_user_and_login(self, usable_password=True): user = self._create_user(password=password) self.client.force_login(user) return user - - def _password_set_or_change_redirect(self, urlname, usable_password): - self._create_user_and_login(usable_password) - return self.client.get(reverse(urlname)) diff --git a/allauth/account/views.py b/allauth/account/views.py index df59171384..f6cfe51808 100644 --- a/allauth/account/views.py +++ b/allauth/account/views.py @@ -668,33 +668,26 @@ def get_ajax_data(self): class PasswordChangeView(AjaxCapableProcessFormViewMixin, FormView): template_name = "account/password_change." + app_settings.TEMPLATE_EXTENSION form_class = ChangePasswordForm - success_url = reverse_lazy("account_change_password") def get_form_class(self): return get_form_class(app_settings.FORMS, "change_password", self.form_class) @sensitive_post_parameters_m def dispatch(self, request, *args, **kwargs): - return super(PasswordChangeView, self).dispatch(request, *args, **kwargs) - - def render_to_response(self, context, **response_kwargs): - if self.request.user.is_anonymous: - # We end up here when `ACCOUNT_LOGOUT_ON_PASSWORD_CHANGE = True`. - redirect_url = get_adapter(self.request).get_logout_redirect_url( - self.request - ) - return HttpResponseRedirect(redirect_url) - elif not self.request.user.has_usable_password(): + if not self.request.user.has_usable_password(): return HttpResponseRedirect(reverse("account_set_password")) - return super(PasswordChangeView, self).render_to_response( - context, **response_kwargs - ) + return super().dispatch(request, *args, **kwargs) def get_form_kwargs(self): - kwargs = super(PasswordChangeView, self).get_form_kwargs() + kwargs = super().get_form_kwargs() kwargs["user"] = self.request.user return kwargs + def get_success_url(self): + if self.success_url: + return self.success_url + return get_adapter().get_password_change_redirect_url(self.request) + def form_valid(self, form): form.save() logout_on_password_change(self.request, form.user) @@ -708,10 +701,10 @@ def form_valid(self, form): request=self.request, user=self.request.user, ) - return super(PasswordChangeView, self).form_valid(form) + return super().form_valid(form) def get_context_data(self, **kwargs): - ret = super(PasswordChangeView, self).get_context_data(**kwargs) + ret = super().get_context_data(**kwargs) # NOTE: For backwards compatibility ret["password_change_form"] = ret.get("form") # (end NOTE) @@ -730,7 +723,6 @@ def get_context_data(self, **kwargs): class PasswordSetView(AjaxCapableProcessFormViewMixin, FormView): template_name = "account/password_set." + app_settings.TEMPLATE_EXTENSION form_class = SetPasswordForm - success_url = reverse_lazy("account_set_password") def get_form_class(self): return get_form_class(app_settings.FORMS, "set_password", self.form_class) @@ -739,18 +731,18 @@ def get_form_class(self): def dispatch(self, request, *args, **kwargs): if self.request.user.has_usable_password(): return HttpResponseRedirect(reverse("account_change_password")) - return super(PasswordSetView, self).dispatch(request, *args, **kwargs) - - def render_to_response(self, context, **response_kwargs): - return super(PasswordSetView, self).render_to_response( - context, **response_kwargs - ) + return super().dispatch(request, *args, **kwargs) def get_form_kwargs(self): - kwargs = super(PasswordSetView, self).get_form_kwargs() + kwargs = super().get_form_kwargs() kwargs["user"] = self.request.user return kwargs + def get_success_url(self): + if self.success_url: + return self.success_url + return get_adapter().get_password_change_redirect_url(self.request) + def form_valid(self, form): form.save() logout_on_password_change(self.request, form.user) @@ -762,10 +754,10 @@ def form_valid(self, form): request=self.request, user=self.request.user, ) - return super(PasswordSetView, self).form_valid(form) + return super().form_valid(form) def get_context_data(self, **kwargs): - ret = super(PasswordSetView, self).get_context_data(**kwargs) + ret = super().get_context_data(**kwargs) # NOTE: For backwards compatibility ret["password_set_form"] = ret.get("form") # (end NOTE) diff --git a/test_settings.py b/test_settings.py index b6aa3a499f..1a36cd1a6f 100644 --- a/test_settings.py +++ b/test_settings.py @@ -85,6 +85,7 @@ "allauth.socialaccount.providers.bitly", "allauth.socialaccount.providers.box", "allauth.socialaccount.providers.cilogon", + "allauth.socialaccount.providers.clever", "allauth.socialaccount.providers.coinbase", "allauth.socialaccount.providers.dataporten", "allauth.socialaccount.providers.daum",