From 0d2c6217d33cdbfd3fa326910deb166ec1d17f2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C4=93teris=20Caune?= Date: Wed, 18 Dec 2019 16:10:30 +0200 Subject: [PATCH] Auto-submit the unsubscribe confirmation form only if signature is more than 5 minutes old. Idea from https://stackoverflow.com/questions/59281750/strategies-to-prevent-email-scanners-from-activating-unsubscribe-links/59381066#59381066 --- hc/accounts/tests/test_unsubscribe_reports.py | 15 +++++ hc/accounts/urls.py | 2 +- hc/accounts/views.py | 21 +++++-- hc/api/models.py | 5 +- hc/front/tests/test_unsubscribe_email.py | 55 ++++++++++++++++--- hc/front/urls.py | 2 +- hc/front/views.py | 33 ++++++++--- templates/accounts/unsubscribe_submit.html | 5 ++ 8 files changed, 113 insertions(+), 25 deletions(-) diff --git a/hc/accounts/tests/test_unsubscribe_reports.py b/hc/accounts/tests/test_unsubscribe_reports.py index 6dd86db2..1213e4f5 100644 --- a/hc/accounts/tests/test_unsubscribe_reports.py +++ b/hc/accounts/tests/test_unsubscribe_reports.py @@ -1,8 +1,10 @@ from datetime import timedelta as td +import time from django.core import signing from django.utils.timezone import now from hc.test import BaseTestCase +from mock import patch class UnsubscribeReportsTestCase(BaseTestCase): @@ -36,3 +38,16 @@ class UnsubscribeReportsTestCase(BaseTestCase): r = self.client.get(url) self.assertContains(r, "Please press the button below") + self.assertNotContains(r, "submit()") + + def test_aged_signature_autosubmits(self): + with patch("django.core.signing.time") as mock_time: + mock_time.time.return_value = time.time() - 301 + signer = signing.TimestampSigner(salt="reports") + sig = signer.sign("alice") + + url = "/accounts/unsubscribe_reports/%s/" % sig + + r = self.client.get(url) + self.assertContains(r, "Please press the button below") + self.assertContains(r, "submit()") diff --git a/hc/accounts/urls.py b/hc/accounts/urls.py index f5e417aa..4e6643c4 100644 --- a/hc/accounts/urls.py +++ b/hc/accounts/urls.py @@ -16,7 +16,7 @@ urlpatterns = [ path("profile/notifications/", views.notifications, name="hc-notifications"), path("close/", views.close, name="hc-close"), path( - "unsubscribe_reports//", + "unsubscribe_reports//", views.unsubscribe_reports, name="hc-unsubscribe-reports", ), diff --git a/hc/accounts/views.py b/hc/accounts/views.py index b45dafb8..14dacf7c 100644 --- a/hc/accounts/views.py +++ b/hc/accounts/views.py @@ -433,17 +433,28 @@ def change_email_done(request): @csrf_exempt -def unsubscribe_reports(request, username): +def unsubscribe_reports(request, signed_username): + # Some email servers open links in emails to check for malicious content. + # To work around this, for GET requests we serve a confirmation form. + # If the signature is more than 5 minutes old, we also include JS code to + # auto-submit the form. + + ctx = {} signer = signing.TimestampSigner(salt="reports") + # First, check the signature without looking at the timestamp: try: - username = signer.unsign(username) + username = signer.unsign(signed_username) except signing.BadSignature: return render(request, "bad_link.html") - # Some email servers open links in emails to check for malicious content. - # To work around this, we serve a form that auto-submits with JS. + # Check if timestamp is older than 5 minutes: + try: + username = signer.unsign(signed_username, max_age=300) + except signing.SignatureExpired: + ctx["autosubmit"] = True + if request.method != "POST": - return render(request, "accounts/unsubscribe_submit.html") + return render(request, "accounts/unsubscribe_submit.html", ctx) user = User.objects.get(username=username) profile = Profile.objects.for_user(user) diff --git a/hc/api/models.py b/hc/api/models.py index dfb57129..c09ab233 100644 --- a/hc/api/models.py +++ b/hc/api/models.py @@ -7,6 +7,7 @@ from datetime import datetime, timedelta as td from croniter import croniter from django.conf import settings +from django.core.signing import TimestampSigner from django.db import models from django.urls import reverse from django.utils import timezone @@ -372,7 +373,9 @@ class Channel(models.Model): emails.verify_email(self.email_value, {"verify_link": verify_link}) def get_unsub_link(self): - args = [self.code, self.make_token()] + signer = TimestampSigner(salt="alerts") + signed_token = signer.sign(self.make_token()) + args = [self.code, signed_token] verify_link = reverse("hc-unsubscribe-alerts", args=args) return settings.SITE_ROOT + verify_link diff --git a/hc/front/tests/test_unsubscribe_email.py b/hc/front/tests/test_unsubscribe_email.py index d3fd1f3d..c9c2866b 100644 --- a/hc/front/tests/test_unsubscribe_email.py +++ b/hc/front/tests/test_unsubscribe_email.py @@ -1,5 +1,8 @@ +import time +from django.core.signing import TimestampSigner from hc.api.models import Channel from hc.test import BaseTestCase +from mock import patch class UnsubscribeEmailTestCase(BaseTestCase): @@ -9,7 +12,15 @@ class UnsubscribeEmailTestCase(BaseTestCase): self.channel.value = "alice@example.org" self.channel.save() - def test_it_works(self): + def test_it_serves_confirmation_form(self): + token = self.channel.make_token() + url = "/integrations/%s/unsub/%s/" % (self.channel.code, token) + + r = self.client.get(url) + self.assertContains(r, "Please press the button below") + self.assertNotContains(r, "submit()") + + def test_post_unsubscribes(self): token = self.channel.make_token() url = "/integrations/%s/unsub/%s/" % (self.channel.code, token) @@ -19,6 +30,39 @@ class UnsubscribeEmailTestCase(BaseTestCase): q = Channel.objects.filter(code=self.channel.code) self.assertEqual(q.count(), 0) + def test_fresh_signature_does_not_autosubmit(self): + signer = TimestampSigner(salt="alerts") + signed_token = signer.sign(self.channel.make_token()) + + url = "/integrations/%s/unsub/%s/" % (self.channel.code, signed_token) + + r = self.client.get(url) + self.assertContains( + r, "Please press the button below to unsubscribe", status_code=200 + ) + self.assertNotContains(r, "submit()", status_code=200) + + def test_aged_signature_does_autosubmit(self): + with patch("django.core.signing.time") as mock_time: + mock_time.time.return_value = time.time() - 301 + signer = TimestampSigner(salt="alerts") + signed_token = signer.sign(self.channel.make_token()) + + url = "/integrations/%s/unsub/%s/" % (self.channel.code, signed_token) + + r = self.client.get(url) + self.assertContains( + r, "Please press the button below to unsubscribe", status_code=200 + ) + self.assertContains(r, "submit()", status_code=200) + + def test_it_checks_signature(self): + signed_token = self.channel.make_token() + ":bad:signature" + url = "/integrations/%s/unsub/%s/" % (self.channel.code, signed_token) + + r = self.client.get(url) + self.assertContains(r, "link you just used is incorrect", status_code=200) + def test_it_checks_token(self): url = "/integrations/%s/unsub/faketoken/" % self.channel.code @@ -33,11 +77,4 @@ class UnsubscribeEmailTestCase(BaseTestCase): url = "/integrations/%s/unsub/%s/" % (self.channel.code, token) r = self.client.get(url) - self.assertEqual(r.status_code, 400) - - def test_it_serves_confirmation_form(self): - token = self.channel.make_token() - url = "/integrations/%s/unsub/%s/" % (self.channel.code, token) - - r = self.client.get(url) - self.assertContains(r, "Please press the button below") + self.assertEqual(r.status_code, 404) diff --git a/hc/front/urls.py b/hc/front/urls.py index 97ed384a..68df1844 100644 --- a/hc/front/urls.py +++ b/hc/front/urls.py @@ -57,7 +57,7 @@ channel_urls = [ "/verify//", views.verify_email, name="hc-verify-email" ), path( - "/unsub//", + "/unsub//", views.unsubscribe_email, name="hc-unsubscribe-alerts", ), diff --git a/hc/front/views.py b/hc/front/views.py index 484c4044..03b036e7 100644 --- a/hc/front/views.py +++ b/hc/front/views.py @@ -702,18 +702,35 @@ def verify_email(request, code, token): @csrf_exempt -def unsubscribe_email(request, code, token): - channel = get_object_or_404(Channel, code=code) +def unsubscribe_email(request, code, signed_token): + # Some email servers open links in emails to check for malicious content. + # To work around this, on GET requests we serve a confirmation form. + # If the signature is at least 5 minutes old, we also include JS code to + # auto-submit the form. + ctx = {} + if ":" in signed_token: + signer = signing.TimestampSigner(salt="alerts") + # First, check the signature without looking at the timestamp: + try: + token = signer.unsign(signed_token) + except signing.BadSignature: + return render(request, "bad_link.html") + + # Check if timestamp is older than 5 minutes: + try: + signer.unsign(signed_token, max_age=300) + except signing.SignatureExpired: + ctx["autosubmit"] = True + + else: + token = signed_token + + channel = get_object_or_404(Channel, code=code, kind="email") if channel.make_token() != token: return render(request, "bad_link.html") - if channel.kind != "email": - return HttpResponseBadRequest() - - # Some email servers open links in emails to check for malicious content. - # To work around this, we serve a form that auto-submits with JS. if request.method != "POST": - return render(request, "accounts/unsubscribe_submit.html") + return render(request, "accounts/unsubscribe_submit.html", ctx) channel.delete() return render(request, "front/unsubscribe_success.html") diff --git a/templates/accounts/unsubscribe_submit.html b/templates/accounts/unsubscribe_submit.html index 2f345e60..d730dc94 100644 --- a/templates/accounts/unsubscribe_submit.html +++ b/templates/accounts/unsubscribe_submit.html @@ -12,4 +12,9 @@ class="btn btn-lg btn-primary" value="Unsubscribe"> + +{% if autosubmit %} + +{% endif %} + {% endblock %}