From fb7994875974465b7473ba68d6c5f1ffb29d8a0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C4=93teris=20Caune?= Date: Mon, 16 Nov 2020 15:33:29 +0200 Subject: [PATCH] Update the "Change Email" function to use confirmation codes --- hc/accounts/models.py | 6 --- hc/accounts/tests/test_change_email.py | 33 ++++++------ hc/accounts/tests/test_profile.py | 17 ------ hc/accounts/tests/test_set_password.py | 2 +- hc/accounts/urls.py | 3 +- hc/accounts/views.py | 32 ++++------- hc/lib/emails.py | 4 -- templates/accounts/change_email.html | 57 +++++++++++--------- templates/accounts/link_sent.html | 18 ------- templates/accounts/profile.html | 7 ++- templates/emails/change-email-body-html.html | 13 ----- templates/emails/change-email-body-text.html | 11 ---- templates/emails/change-email-subject.html | 2 - 13 files changed, 65 insertions(+), 140 deletions(-) delete mode 100644 templates/accounts/link_sent.html delete mode 100644 templates/emails/change-email-body-html.html delete mode 100644 templates/emails/change-email-body-text.html delete mode 100644 templates/emails/change-email-subject.html diff --git a/hc/accounts/models.py b/hc/accounts/models.py index cc239e30..a67e9396 100644 --- a/hc/accounts/models.py +++ b/hc/accounts/models.py @@ -118,12 +118,6 @@ class Profile(models.Model): } emails.transfer_request(self.user.email, ctx) - def send_change_email_link(self): - token = self.prepare_token("change-email") - path = reverse("hc-change-email", args=[token]) - ctx = {"button_text": "Change Email", "button_url": settings.SITE_ROOT + path} - emails.change_email(self.user.email, ctx) - def send_sms_limit_notice(self, transport): ctx = {"transport": transport, "limit": self.sms_limit} if self.sms_limit != 500 and settings.USE_PAYMENTS: diff --git a/hc/accounts/tests/test_change_email.py b/hc/accounts/tests/test_change_email.py index a4aee609..79f1fd16 100644 --- a/hc/accounts/tests/test_change_email.py +++ b/hc/accounts/tests/test_change_email.py @@ -1,39 +1,42 @@ -from django.contrib.auth.hashers import make_password - from hc.test import BaseTestCase class ChangeEmailTestCase(BaseTestCase): - def test_it_shows_form(self): - self.profile.token = make_password("foo", "change-email") - self.profile.save() + def test_it_requires_sudo_mode(self): + self.client.login(username="alice@example.org", password="password") + + r = self.client.get("/accounts/change_email/") + self.assertContains(r, "We have sent a confirmation code") + def test_it_shows_form(self): self.client.login(username="alice@example.org", password="password") + self.set_sudo_flag() - r = self.client.get("/accounts/change_email/foo/") + r = self.client.get("/accounts/change_email/") self.assertContains(r, "Change Account's Email Address") - def test_it_changes_password(self): - self.profile.token = make_password("foo", "change-email") - self.profile.save() - + def test_it_updates_email(self): self.client.login(username="alice@example.org", password="password") + self.set_sudo_flag() payload = {"email": "alice2@example.org"} - self.client.post("/accounts/change_email/foo/", payload) + r = self.client.post("/accounts/change_email/", payload, follow=True) + self.assertRedirects(r, "/accounts/change_email/done/") + self.assertContains(r, "Email Address Updated") self.alice.refresh_from_db() self.assertEqual(self.alice.email, "alice2@example.org") self.assertFalse(self.alice.has_usable_password()) - def test_it_requires_unique_email(self): - self.profile.token = make_password("foo", "change-email") - self.profile.save() + # The user should have been logged out: + self.assertNotIn("_auth_user_id", self.client.session) + def test_it_requires_unique_email(self): self.client.login(username="alice@example.org", password="password") + self.set_sudo_flag() payload = {"email": "bob@example.org"} - r = self.client.post("/accounts/change_email/foo/", payload) + r = self.client.post("/accounts/change_email/", payload) self.assertContains(r, "bob@example.org is already registered") self.alice.refresh_from_db() diff --git a/hc/accounts/tests/test_profile.py b/hc/accounts/tests/test_profile.py index f9c8071c..c7d664af 100644 --- a/hc/accounts/tests/test_profile.py +++ b/hc/accounts/tests/test_profile.py @@ -75,23 +75,6 @@ class ProfileTestCase(BaseTestCase): self.assertEqual(len(mail.outbox), 0) - def test_it_sends_change_email_link(self): - self.client.login(username="alice@example.org", password="password") - - form = {"change_email": "1"} - r = self.client.post("/accounts/profile/", form) - assert r.status_code == 302 - - # profile.token should be set now - self.profile.refresh_from_db() - token = self.profile.token - self.assertTrue(len(token) > 10) - - # And an email should have been sent - self.assertEqual(len(mail.outbox), 1) - expected_subject = "Change email address on %s" % settings.SITE_NAME - self.assertEqual(mail.outbox[0].subject, expected_subject) - def test_leaving_works(self): self.client.login(username="bob@example.org", password="password") diff --git a/hc/accounts/tests/test_set_password.py b/hc/accounts/tests/test_set_password.py index 54895493..15685a1c 100644 --- a/hc/accounts/tests/test_set_password.py +++ b/hc/accounts/tests/test_set_password.py @@ -2,7 +2,7 @@ from hc.test import BaseTestCase class SetPasswordTestCase(BaseTestCase): - def test_it_requires_sudo_mod(self): + def test_it_requires_sudo_mode(self): self.client.login(username="alice@example.org", password="password") r = self.client.get("/accounts/set_password/") diff --git a/hc/accounts/urls.py b/hc/accounts/urls.py index 7d9f72ab..15341527 100644 --- a/hc/accounts/urls.py +++ b/hc/accounts/urls.py @@ -7,7 +7,6 @@ urlpatterns = [ path("logout/", views.logout, name="hc-logout"), path("signup/", views.signup, name="hc-signup"), path("login_link_sent/", views.login_link_sent, name="hc-login-link-sent"), - path("link_sent/", views.link_sent, name="hc-link-sent"), path( "check_token///", views.check_token, @@ -23,7 +22,7 @@ urlpatterns = [ ), path("set_password/", views.set_password, name="hc-set-password"), path("change_email/done/", views.change_email_done, name="hc-change-email-done"), - path("change_email//", views.change_email, name="hc-change-email"), + path("change_email/", views.change_email, name="hc-change-email"), path("two_factor/add/", views.add_credential, name="hc-add-credential"), path( "two_factor//remove/", diff --git a/hc/accounts/views.py b/hc/accounts/views.py index 97625a8c..2cedd00e 100644 --- a/hc/accounts/views.py +++ b/hc/accounts/views.py @@ -186,10 +186,6 @@ def login_link_sent(request): return render(request, "accounts/login_link_sent.html") -def link_sent(request): - return render(request, "accounts/link_sent.html") - - def check_token(request, username, token): if request.user.is_authenticated and request.user.username == username: # User is already logged in @@ -235,21 +231,17 @@ def profile(request): if ctx["removed_credential_name"]: ctx["2fa_status"] = "info" - if request.method == "POST": - if "change_email" in request.POST: - profile.send_change_email_link() - return redirect("hc-link-sent") - elif "leave_project" in request.POST: - code = request.POST["code"] - try: - project = Project.objects.get(code=code, member__user=request.user) - except Project.DoesNotExist: - return HttpResponseBadRequest() + if request.method == "POST" and "leave_project" in request.POST: + code = request.POST["code"] + try: + project = Project.objects.get(code=code, member__user=request.user) + except Project.DoesNotExist: + return HttpResponseBadRequest() - Member.objects.filter(project=project, user=request.user).delete() + Member.objects.filter(project=project, user=request.user).delete() - ctx["left_project"] = project - ctx["my_projects_status"] = "info" + ctx["left_project"] = project + ctx["my_projects_status"] = "info" return render(request, "accounts/profile.html", ctx) @@ -486,10 +478,8 @@ def set_password(request): @login_required -def change_email(request, token): - if not request.profile.check_token(token, "change-email"): - return HttpResponseBadRequest() - +@require_sudo_mode +def change_email(request): if request.method == "POST": form = forms.ChangeEmailForm(request.POST) if form.is_valid(): diff --git a/hc/lib/emails.py b/hc/lib/emails.py index 41459ca2..fd517b45 100644 --- a/hc/lib/emails.py +++ b/hc/lib/emails.py @@ -62,10 +62,6 @@ def transfer_request(to, ctx): send("transfer-request", to, ctx) -def change_email(to, ctx): - send("change-email", to, ctx) - - def alert(to, ctx, headers={}): send("alert", to, ctx, headers) diff --git a/templates/accounts/change_email.html b/templates/accounts/change_email.html index 4d7cb427..4c45112d 100644 --- a/templates/accounts/change_email.html +++ b/templates/accounts/change_email.html @@ -20,44 +20,49 @@

{% if request.user.has_usable_password %} -

- Note: Changing the email address will also - reset your current password +

+ Your password will be reset. + For security purposes, after updating your email address, + {% site_name %} will also reset your current password and log you out.

{% endif %} + + {% if request.user.credentials.exists %} +

+ Two-factor authentication is active. + If you are handing this account over to somebody else, + consider disabling two-factor authentication first. +

+ {% endif %} -
+ {% csrf_token %}
- -
- -
+ +
- -
- - {% if form.email.errors %} -
- {{ form.email.errors|join:"" }} -
- {% endif %} + + + {% if form.email.errors %} +
+ {{ form.email.errors|join:"" }} +
+ {% endif %} -
diff --git a/templates/accounts/link_sent.html b/templates/accounts/link_sent.html deleted file mode 100644 index 450d0ad5..00000000 --- a/templates/accounts/link_sent.html +++ /dev/null @@ -1,18 +0,0 @@ -{% extends "base.html" %} - -{% block content %} -
-
-
-

Email with Instructions Sent!

-
-

- We've sent you an email with further instructions. - Please check your inbox! -

- -
-
-
- -{% endblock %} diff --git a/templates/accounts/profile.html b/templates/accounts/profile.html index fa52129b..756f2fe1 100644 --- a/templates/accounts/profile.html +++ b/templates/accounts/profile.html @@ -42,10 +42,9 @@ Your account's email address is {{ request.user.email }} - + Change Email

diff --git a/templates/emails/change-email-body-html.html b/templates/emails/change-email-body-html.html deleted file mode 100644 index 235a98a5..00000000 --- a/templates/emails/change-email-body-html.html +++ /dev/null @@ -1,13 +0,0 @@ -{% extends "emails/base.html" %} -{% load hc_extras %} - -{% block content %} -Hello,
-To change the email address for your account on {% site_name %}, please press -the button below:

-{% endblock %} - -{% block content_more %} -Regards,
-The {% site_name %} Team -{% endblock %} diff --git a/templates/emails/change-email-body-text.html b/templates/emails/change-email-body-text.html deleted file mode 100644 index c483c2ab..00000000 --- a/templates/emails/change-email-body-text.html +++ /dev/null @@ -1,11 +0,0 @@ -{% load hc_extras %} -Hello, - -Here's a link to change the email address for your account on {% site_name %}: - -{{ button_url }} - - --- -Regards, -{% site_name %} diff --git a/templates/emails/change-email-subject.html b/templates/emails/change-email-subject.html deleted file mode 100644 index c7164418..00000000 --- a/templates/emails/change-email-subject.html +++ /dev/null @@ -1,2 +0,0 @@ -{% load hc_extras %} -Change email address on {% site_name %}