From 8d81d27af3223bcf41a982aaf90b88c92870a2b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C4=93teris=20Caune?= Date: Tue, 10 Dec 2019 09:14:54 +0200 Subject: [PATCH] Unsubscribe links serve a form, and require HTTP POST to actually unsubscribe --- CHANGELOG.md | 1 + hc/accounts/tests/test_unsubscribe_reports.py | 11 ++--------- hc/accounts/views.py | 2 +- hc/api/tests/test_bounce.py | 5 +++++ hc/api/views.py | 2 ++ hc/front/tests/test_unsubscribe_email.py | 11 ++--------- hc/front/views.py | 2 +- 7 files changed, 14 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1f6ac963..e5ec6950 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ All notable changes to this project will be documented in this file. ### Bug Fixes - Don't set CSRF cookie on first visit. Signup is exempt from CSRF protection - Fix List-Unsubscribe email header value: add angle brackets +- Unsubscribe links serve a form, and require HTTP POST to actually unsubscribe ## v1.11.0 - 2019-11-22 diff --git a/hc/accounts/tests/test_unsubscribe_reports.py b/hc/accounts/tests/test_unsubscribe_reports.py index 2cba8964..6dd86db2 100644 --- a/hc/accounts/tests/test_unsubscribe_reports.py +++ b/hc/accounts/tests/test_unsubscribe_reports.py @@ -15,7 +15,7 @@ class UnsubscribeReportsTestCase(BaseTestCase): sig = signing.TimestampSigner(salt="reports").sign("alice") url = "/accounts/unsubscribe_reports/%s/" % sig - r = self.client.get(url) + r = self.client.post(url) self.assertContains(r, "Unsubscribed") self.profile.refresh_from_db() @@ -30,16 +30,9 @@ class UnsubscribeReportsTestCase(BaseTestCase): r = self.client.get(url) self.assertContains(r, "Incorrect Link") - def test_post_works(self): - sig = signing.TimestampSigner(salt="reports").sign("alice") - url = "/accounts/unsubscribe_reports/%s/" % sig - - r = self.client.post(url) - self.assertContains(r, "Unsubscribed") - def test_it_serves_confirmation_form(self): sig = signing.TimestampSigner(salt="reports").sign("alice") - url = "/accounts/unsubscribe_reports/%s/?ask=1" % sig + url = "/accounts/unsubscribe_reports/%s/" % sig r = self.client.get(url) self.assertContains(r, "Please press the button below") diff --git a/hc/accounts/views.py b/hc/accounts/views.py index e7b7b614..b45dafb8 100644 --- a/hc/accounts/views.py +++ b/hc/accounts/views.py @@ -442,7 +442,7 @@ def unsubscribe_reports(request, username): # 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 "ask" in request.GET and request.method != "POST": + if request.method != "POST": return render(request, "accounts/unsubscribe_submit.html") user = User.objects.get(username=username) diff --git a/hc/api/tests/test_bounce.py b/hc/api/tests/test_bounce.py index 617383c3..46562345 100644 --- a/hc/api/tests/test_bounce.py +++ b/hc/api/tests/test_bounce.py @@ -49,3 +49,8 @@ class BounceTestCase(BaseTestCase): url = "/api/v1/notifications/%s/bounce" % fake_code r = self.client.post(url, "", content_type="text/plain") self.assertEqual(r.status_code, 404) + + def test_it_requires_post(self): + url = "/api/v1/notifications/%s/bounce" % self.n.code + r = self.client.get(url) + self.assertEqual(r.status_code, 405) diff --git a/hc/api/views.py b/hc/api/views.py index b6f07a98..6a9b2cbb 100644 --- a/hc/api/views.py +++ b/hc/api/views.py @@ -14,6 +14,7 @@ from django.shortcuts import get_object_or_404 from django.utils import timezone from django.views.decorators.cache import never_cache from django.views.decorators.csrf import csrf_exempt +from django.views.decorators.http import require_POST from hc.api import schemas from hc.api.decorators import authorize, authorize_read, cors, validate_json @@ -256,6 +257,7 @@ def badge(request, badge_key, signature, tag, fmt="svg"): @csrf_exempt +@require_POST def bounce(request, code): notification = get_object_or_404(Notification, code=code) diff --git a/hc/front/tests/test_unsubscribe_email.py b/hc/front/tests/test_unsubscribe_email.py index 034985cd..d3fd1f3d 100644 --- a/hc/front/tests/test_unsubscribe_email.py +++ b/hc/front/tests/test_unsubscribe_email.py @@ -13,7 +13,7 @@ class UnsubscribeEmailTestCase(BaseTestCase): token = self.channel.make_token() url = "/integrations/%s/unsub/%s/" % (self.channel.code, token) - r = self.client.get(url) + r = self.client.post(url) self.assertContains(r, "has been unsubscribed", status_code=200) q = Channel.objects.filter(code=self.channel.code) @@ -35,16 +35,9 @@ class UnsubscribeEmailTestCase(BaseTestCase): r = self.client.get(url) self.assertEqual(r.status_code, 400) - def test_post_works(self): - token = self.channel.make_token() - url = "/integrations/%s/unsub/%s/" % (self.channel.code, token) - - r = self.client.post(url) - self.assertContains(r, "has been unsubscribed", status_code=200) - def test_it_serves_confirmation_form(self): token = self.channel.make_token() - url = "/integrations/%s/unsub/%s/?ask=1" % (self.channel.code, token) + url = "/integrations/%s/unsub/%s/" % (self.channel.code, token) r = self.client.get(url) self.assertContains(r, "Please press the button below") diff --git a/hc/front/views.py b/hc/front/views.py index 70b9f29d..484c4044 100644 --- a/hc/front/views.py +++ b/hc/front/views.py @@ -712,7 +712,7 @@ def unsubscribe_email(request, code, token): # 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 "ask" in request.GET and request.method != "POST": + if request.method != "POST": return render(request, "accounts/unsubscribe_submit.html") channel.delete()