Update the "Set Password" function to use confirmation codes

CHANGELOG.md

@@ -13,6 +13,8 @@ All notable changes to this project will be documented in this file.
 - Improve phone number sanitization: remove spaces and hyphens
 - Change the "Test Integration" behavior for webhooks: don't retry failed requests
 - Add retries to the the email sending logic
+- Require confirmation codes (sent to email) before sensitive actions
+- Implement Webauthn two-factor authentication
 
 ## v1.17.0 - 2020-10-14
 

hc/accounts/models.py

@@ -118,12 +118,6 @@ class Profile(models.Model):
         }
         emails.transfer_request(self.user.email, ctx)
 
-    def send_set_password_link(self):
-        token = self.prepare_token("set-password")
-        path = reverse("hc-set-password", args=[token])
-        ctx = {"button_text": "Set Password", "button_url": settings.SITE_ROOT + path}
-        emails.set_password(self.user.email, ctx)
-
     def send_change_email_link(self):
         token = self.prepare_token("change-email")
         path = reverse("hc-change-email", args=[token])

hc/accounts/tests/test_add_credential.py

@@ -1,6 +1,5 @@
 from unittest.mock import patch
 
-from django.core.signing import TimestampSigner
 from hc.test import BaseTestCase
 from hc.accounts.models import Credential
 
@@ -11,11 +10,6 @@ class AddCredentialTestCase(BaseTestCase):
 
         self.url = "/accounts/two_factor/add/"
 
-    def _set_sudo_flag(self):
-        session = self.client.session
-        session["sudo"] = TimestampSigner().sign("active")
-        session.save()
-
     def test_it_requires_sudo_mode(self):
         self.client.login(username="alice@example.org", password="password")
 
@@ -24,7 +18,7 @@ class AddCredentialTestCase(BaseTestCase):
 
     def test_it_shows_form(self):
         self.client.login(username="alice@example.org", password="password")
-        self._set_sudo_flag()
+        self.set_sudo_flag()
 
         r = self.client.get(self.url)
         self.assertContains(r, "Add Security Key")
@@ -37,7 +31,7 @@ class AddCredentialTestCase(BaseTestCase):
         mock_get_credential_data.return_value = b"dummy-credential-data"
 
         self.client.login(username="alice@example.org", password="password")
-        self._set_sudo_flag()
+        self.set_sudo_flag()
 
         payload = {
             "name": "My New Key",
@@ -54,7 +48,7 @@ class AddCredentialTestCase(BaseTestCase):
 
     def test_it_rejects_bad_base64(self):
         self.client.login(username="alice@example.org", password="password")
-        self._set_sudo_flag()
+        self.set_sudo_flag()
 
         payload = {
             "name": "My New Key",
@@ -67,7 +61,7 @@ class AddCredentialTestCase(BaseTestCase):
 
     def test_it_requires_client_data_json(self):
         self.client.login(username="alice@example.org", password="password")
-        self._set_sudo_flag()
+        self.set_sudo_flag()
 
         payload = {
             "name": "My New Key",

hc/accounts/tests/test_profile.py

@@ -9,23 +9,6 @@ from hc.api.models import Check
 
 
 class ProfileTestCase(BaseTestCase):
-    def test_it_sends_set_password_link(self):
-        self.client.login(username="alice@example.org", password="password")
-
-        form = {"set_password": "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 = "Set password on %s" % settings.SITE_NAME
-        self.assertEqual(mail.outbox[0].subject, expected_subject)
-
     def test_it_sends_report(self):
         check = Check(project=self.project, name="Test Check")
         check.last_ping = now()

hc/accounts/tests/test_remove_credential.py

@@ -1,5 +1,3 @@
-from django.core.signing import TimestampSigner
-
 from hc.test import BaseTestCase
 from hc.accounts.models import Credential
 
@@ -11,14 +9,15 @@ class RemoveCredentialTestCase(BaseTestCase):
         self.c = Credential.objects.create(user=self.alice, name="Alices Key")
         self.url = f"/accounts/two_factor/{self.c.code}/remove/"
 
-    def _set_sudo_flag(self):
+    def test_it_requires_sudo_mode(self):
-        session = self.client.session
+        self.client.login(username="alice@example.org", password="password")
-        session["sudo"] = TimestampSigner().sign("active")
+
-        session.save()
+        r = self.client.get(self.url)
+        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()
+        self.set_sudo_flag()
 
         r = self.client.get(self.url)
         self.assertContains(r, "Remove Security Key")
@@ -26,7 +25,7 @@ class RemoveCredentialTestCase(BaseTestCase):
 
     def test_it_removes_credential(self):
         self.client.login(username="alice@example.org", password="password")
-        self._set_sudo_flag()
+        self.set_sudo_flag()
 
         r = self.client.post(self.url, {"remove_credential": ""}, follow=True)
         self.assertRedirects(r, "/accounts/profile/")
@@ -36,7 +35,7 @@ class RemoveCredentialTestCase(BaseTestCase):
 
     def test_it_checks_owner(self):
         self.client.login(username="charlie@example.org", password="password")
-        self._set_sudo_flag()
+        self.set_sudo_flag()
 
         r = self.client.post(self.url, {"remove_credential": ""})
         self.assertEqual(r.status_code, 400)

hc/accounts/tests/test_set_password.py

@@ -2,45 +2,37 @@ from hc.test import BaseTestCase
 
 
 class SetPasswordTestCase(BaseTestCase):
-    def test_it_shows_form(self):
+    def test_it_requires_sudo_mod(self):
-        token = self.profile.prepare_token("set-password")
-
         self.client.login(username="alice@example.org", password="password")
-        r = self.client.get("/accounts/set_password/%s/" % token)
-        self.assertEqual(r.status_code, 200)
 
+        r = self.client.get("/accounts/set_password/")
+        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/set_password/")
         self.assertContains(r, "Please pick a password")
 
-    def test_it_checks_token(self):
-        self.profile.prepare_token("set-password")
-        self.client.login(username="alice@example.org", password="password")
-
-        # GET
-        r = self.client.get("/accounts/set_password/invalid-token/")
-        self.assertEqual(r.status_code, 400)
-
-        # POST
-        r = self.client.post("/accounts/set_password/invalid-token/")
-        self.assertEqual(r.status_code, 400)
-
     def test_it_sets_password(self):
-        token = self.profile.prepare_token("set-password")
-
         self.client.login(username="alice@example.org", password="password")
+        self.set_sudo_flag()
+
         payload = {"password": "correct horse battery staple"}
-        r = self.client.post("/accounts/set_password/%s/" % token, payload)
+        r = self.client.post("/accounts/set_password/", payload)
-        self.assertEqual(r.status_code, 302)
+        self.assertRedirects(r, "/accounts/profile/")
 
         old_password = self.alice.password
         self.alice.refresh_from_db()
         self.assertNotEqual(self.alice.password, old_password)
 
     def test_post_checks_length(self):
-        token = self.profile.prepare_token("set-password")
-
         self.client.login(username="alice@example.org", password="password")
+        self.set_sudo_flag()
+
         payload = {"password": "abc"}
-        r = self.client.post("/accounts/set_password/%s/" % token, payload)
+        r = self.client.post("/accounts/set_password/", payload)
         self.assertEqual(r.status_code, 200)
 
         old_password = self.alice.password

hc/accounts/urls.py

@@ -21,7 +21,7 @@ urlpatterns = [
         views.unsubscribe_reports,
         name="hc-unsubscribe-reports",
     ),
-    path("set_password/<slug:token>/", views.set_password, name="hc-set-password"),
+    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/<slug:token>/", views.change_email, name="hc-change-email"),
     path("two_factor/add/", views.add_credential, name="hc-add-credential"),

hc/accounts/views.py

@@ -239,9 +239,6 @@ def profile(request):
         if "change_email" in request.POST:
             profile.send_change_email_link()
             return redirect("hc-link-sent")
-        elif "set_password" in request.POST:
-            profile.send_set_password_link()
-            return redirect("hc-link-sent")
         elif "leave_project" in request.POST:
             code = request.POST["code"]
             try:
@@ -466,10 +463,8 @@ def notifications(request):
 
 
 @login_required
-def set_password(request, token):
+@require_sudo_mode
-    if not request.profile.check_token(token, "set-password"):
+def set_password(request):
-        return HttpResponseBadRequest()
-
     if request.method == "POST":
         form = forms.SetPasswordForm(request.POST)
         if form.is_valid():

hc/lib/emails.py

@@ -62,10 +62,6 @@ def transfer_request(to, ctx):
     send("transfer-request", to, ctx)
 
 
-def set_password(to, ctx):
-    send("set-password", to, ctx)
-
-
 def change_email(to, ctx):
     send("change-email", to, ctx)
 

hc/test.py

@@ -1,4 +1,5 @@
 from django.contrib.auth.models import User
+from django.core.signing import TimestampSigner
 from django.test import TestCase
 
 from hc.accounts.models import Member, Profile, Project
@@ -51,3 +52,8 @@ class BaseTestCase(TestCase):
         self.charlies_profile.save()
 
         self.channels_url = "/projects/%s/integrations/" % self.project.code
+
+    def set_sudo_flag(self):
+        session = self.client.session
+        session["sudo"] = TimestampSigner().sign("active")
+        session.save()

templates/accounts/profile.html

@@ -50,10 +50,9 @@
                     <p class="clearfix"></p>
                     <p>
                         Attach a password to your {{ site_name }} account
-                        <button
+                        <a
-                            type="submit"
+                            href="{% url 'hc-set-password' %}"
-                            name="set_password"
+                            class="btn btn-default pull-right">Set Password</a>
-                            class="btn btn-default pull-right">Set Password</button>
                     </p>
                 </form>
             </div>

templates/accounts/sudo.html

@@ -20,7 +20,8 @@
                         type="text"
                         class="form-control input-lg"
                         maxlength="6"
-                        name="sudo_code" />
+                        name="sudo_code"
+                        autofocus />
 
                     {% if wrong_code %}
                     <div class="help-block">

templates/emails/set-password-body-html.html

@@ -1,13 +0,0 @@
-{% extends "emails/base.html" %}
-{% load hc_extras %}
-
-{% block content %}
-Hello,<br />
-To set up a password for your account on {% site_name %}, please press the
-button below:</p>
-{% endblock %}
-
-{% block content_more %}
-Regards,<br />
-The {% site_name %} Team
-{% endblock %}

templates/emails/set-password-body-text.html

@@ -1,11 +0,0 @@
-{% load hc_extras %}
-Hello,
-
-Here's a link to set a password for your account on {% site_name %}:
-
-{{ button_url }}
-
-
---
-Regards,
-{% site_name %}

templates/emails/set-password-subject.html

@@ -1,2 +0,0 @@
-{% load hc_extras %}
-Set password on {% site_name %}