More secure unsubscribe links for monthly reports.

hc/accounts/models.py

@@ -6,7 +6,7 @@ from datetime import timedelta
 from django.conf import settings
 from django.contrib.auth.hashers import check_password, make_password
 from django.contrib.auth.models import User
-from django.core import signing
+from django.core.signing import TimestampSigner
 from django.db import models
 from django.urls import reverse
 from django.utils import timezone
@@ -68,6 +68,12 @@ class Profile(models.Model):
     def notifications_url(self):
         return settings.SITE_ROOT + reverse("hc-notifications")
 
+    def reports_unsub_url(self):
+        signer = TimestampSigner(salt="reports")
+        signed_username = signer.sign(self.user.username)
+        path = reverse("hc-unsubscribe-reports", args=[signed_username])
+        return settings.SITE_ROOT + path
+
     def team(self):
         # compare ids to avoid SQL queries
         if self.current_team_id and self.current_team_id != self.id:
@@ -137,10 +143,6 @@ class Profile(models.Model):
         if nag and num_down == 0:
             return False
 
-        token = signing.Signer().sign(uuid.uuid4())
-        path = reverse("hc-unsubscribe-reports", args=[self.user.username])
-        unsub_link = "%s%s?token=%s" % (settings.SITE_ROOT, path, token)
-
         # Sort checks by owner. Need this because will group by owner in
         # template.
         checks = checks.order_by("user_id")
@@ -149,8 +151,8 @@ class Profile(models.Model):
             "checks": checks,
             "sort": self.sort,
             "now": timezone.now(),
-            "unsub_link": unsub_link,
+            "unsub_link": self.reports_unsub_url(),
-            "notifications_url": self.notifications_url,
+            "notifications_url": self.notifications_url(),
             "nag": nag,
             "nag_period": self.nag_period.total_seconds(),
             "num_down": num_down

hc/accounts/tests/test_unsubscribe_reports.py

@@ -6,7 +6,7 @@ from hc.test import BaseTestCase
 
 class UnsubscribeReportsTestCase(BaseTestCase):
 
-    def test_it_works(self):
+    def test_token_works(self):
         self.profile.nag_period = td(hours=1)
         self.profile.save()
 
@@ -18,3 +18,22 @@ class UnsubscribeReportsTestCase(BaseTestCase):
         self.profile.refresh_from_db()
         self.assertFalse(self.profile.reports_allowed)
         self.assertEqual(self.profile.nag_period.total_seconds(), 0)
+
+    def test_bad_token_gets_rejected(self):
+        url = "/accounts/unsubscribe_reports/alice/?token=invalid"
+        r = self.client.get(url)
+        self.assertContains(r, "Incorrect Link")
+
+    def test_signed_username_works(self):
+        sig = signing.TimestampSigner(salt="reports").sign("alice")
+        url = "/accounts/unsubscribe_reports/%s/" % sig
+        r = self.client.get(url)
+        self.assertContains(r, "You have been unsubscribed")
+
+        self.profile.refresh_from_db()
+        self.assertFalse(self.profile.reports_allowed)
+
+    def test_bad_signature_gets_rejected(self):
+        url = "/accounts/unsubscribe_reports/invalid/"
+        r = self.client.get(url)
+        self.assertContains(r, "Incorrect Link")

hc/accounts/urls.py

@@ -18,7 +18,7 @@ urlpatterns = [
     url(r'^profile/badges/$', views.badges, name="hc-badges"),
     url(r'^close/$', views.close, name="hc-close"),
 
-    url(r'^unsubscribe_reports/([\w-]+)/$',
+    url(r'^unsubscribe_reports/([\w\:-]+)/$',
         views.unsubscribe_reports, name="hc-unsubscribe-reports"),
 
     url(r'^set_password/([\w-]+)/$',

hc/accounts/views.py

@@ -348,10 +348,20 @@ def change_email_done(request):
 
 
 def unsubscribe_reports(request, username):
-    try:
+    if ":" in username:
-        signing.Signer().unsign(request.GET.get("token"))
+        signer = signing.TimestampSigner(salt="reports")
-    except signing.BadSignature:
+        try:
-        return HttpResponseBadRequest()
+            username = signer.unsign(username)
+        except signing.BadSignature:
+            return render(request, "bad_link.html")
+    else:
+        # Username is not signed but there should be a ?token=... parameter
+        # This is here for backwards compatibility and will be removed
+        # at some point.
+        try:
+            signing.Signer().unsign(request.GET.get("token"))
+        except signing.BadSignature:
+            return render(request, "bad_link.html")
 
     user = User.objects.get(username=username)
     profile = Profile.objects.for_user(user)

templates/bad_link.html

@@ -4,10 +4,10 @@
 <div class="row">
     <div class="col-sm-6 col-sm-offset-3">
         <div class="hc-dialog">
-            <h1>Incorrect Confirmation Link</h1>
+            <h1>Incorrect Link</h1>
             <div class="dialog-body">
                 <p>
-                    The confirmation link you just used is incorrect.
+                    The link you just used is incorrect.
                     If you copy-pasted the link, please make sure you did not
                     miss any characters.
                 </p>