From 117160ea875dd6a601f9d9f792438ffc35d86c17 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sascha=20I=C3=9Fbr=C3=BCcker?=
 <sascha.issbruecker@gmail.com>
Date: Fri, 20 May 2022 16:51:50 +0200
Subject: [PATCH] Enforce CSRF check for acknowledging toasts

---
 bookmarks/templates/bookmarks/layout.html |  5 +++-
 bookmarks/tests/test_toasts_view.py       | 28 ++++++++++++++++++-----
 bookmarks/urls.py                         |  2 +-
 bookmarks/views/toasts.py                 |  3 ++-
 4 files changed, 29 insertions(+), 9 deletions(-)

diff --git a/bookmarks/templates/bookmarks/layout.html b/bookmarks/templates/bookmarks/layout.html
index e6ce7a7b..9724b2b3 100644
--- a/bookmarks/templates/bookmarks/layout.html
+++ b/bookmarks/templates/bookmarks/layout.html
@@ -30,12 +30,15 @@
 <header>
     {% if has_toasts %}
     <div class="toasts container grid-lg">
+      <form action="{% url 'bookmarks:toasts.acknowledge' %}?return_url={{ request.path | urlencode }}" method="post">
+        {% csrf_token %}
         {% for toast in toast_messages %}
             <div class="toast">
                 {{ toast.message }}
-                <a href="{% url 'bookmarks:toasts.acknowledge' toast.id %}?return_url={{ request.path | urlencode }}" class="btn btn-clear float-right"></a>
+                <button type="submit" name="toast" value="{{ toast.id }}" class="btn btn-clear float-right"></button>
             </div>
         {% endfor %}
+      </form>
     </div>
     {% endif %}
     <div class="navbar container grid-lg">
diff --git a/bookmarks/tests/test_toasts_view.py b/bookmarks/tests/test_toasts_view.py
index bad35308..48b4ae3c 100644
--- a/bookmarks/tests/test_toasts_view.py
+++ b/bookmarks/tests/test_toasts_view.py
@@ -60,12 +60,20 @@ def test_should_not_render_toasts_of_other_users(self):
         # Should not render toasts
         self.assertContains(response, '<div class="toast">', count=0)
 
+    def test_form_tag(self):
+        self.create_toast()
+        expected_form_tag = f'<form action="{reverse("bookmarks:toasts.acknowledge")}?return_url={reverse("bookmarks:index")}" method="post">'
+
+        response = self.client.get(reverse('bookmarks:index'))
+
+        self.assertContains(response, expected_form_tag)
+
     def test_toast_content(self):
         toast = self.create_toast()
         expected_toast = f'''
             <div class="toast">
                 {toast.message}
-                <a href="{reverse('bookmarks:toasts.acknowledge', args=[toast.id])}?return_url={reverse('bookmarks:index')}" class="btn btn-clear float-right"></a>
+                <button type="submit" name="toast" value="{toast.id}" class="btn btn-clear float-right"></button>
             </div>        
         '''
 
@@ -77,7 +85,9 @@ def test_toast_content(self):
     def test_acknowledge_toast(self):
         toast = self.create_toast()
 
-        self.client.get(reverse('bookmarks:toasts.acknowledge', args=[toast.id]))
+        self.client.post(reverse('bookmarks:toasts.acknowledge'), {
+            'toast': [toast.id],
+        })
 
         toast.refresh_from_db()
         self.assertTrue(toast.acknowledged)
@@ -85,17 +95,21 @@ def test_acknowledge_toast(self):
     def test_acknowledge_toast_should_redirect_to_return_url(self):
         toast = self.create_toast()
         return_url = reverse('bookmarks:settings.general')
-        acknowledge_url = reverse('bookmarks:toasts.acknowledge', args=[toast.id])
+        acknowledge_url = reverse('bookmarks:toasts.acknowledge')
         acknowledge_url = acknowledge_url + '?return_url=' + return_url
 
-        response = self.client.get(acknowledge_url)
+        response = self.client.post(acknowledge_url, {
+            'toast': [toast.id],
+        })
 
         self.assertRedirects(response, return_url)
 
     def test_acknowledge_toast_should_redirect_to_index_by_default(self):
         toast = self.create_toast()
 
-        response = self.client.get(reverse('bookmarks:toasts.acknowledge', args=[toast.id]))
+        response = self.client.post(reverse('bookmarks:toasts.acknowledge'), {
+            'toast': [toast.id],
+        })
 
         self.assertRedirects(response, reverse('bookmarks:index'))
 
@@ -104,5 +118,7 @@ def test_acknowledge_toast_should_not_acknowledge_other_users_toast(self):
         other_user = User.objects.create_user('otheruser', 'otheruser@example.com', 'password123')
         toast = self.create_toast(user=other_user)
 
-        response = self.client.get(reverse('bookmarks:toasts.acknowledge', args=[toast.id]))
+        response = self.client.post(reverse('bookmarks:toasts.acknowledge'), {
+            'toast': [toast.id],
+        })
         self.assertEqual(response.status_code, 404)
diff --git a/bookmarks/urls.py b/bookmarks/urls.py
index 241473b0..c6fc4530 100644
--- a/bookmarks/urls.py
+++ b/bookmarks/urls.py
@@ -23,7 +23,7 @@
     path('settings/import', views.settings.bookmark_import, name='settings.import'),
     path('settings/export', views.settings.bookmark_export, name='settings.export'),
     # Toasts
-    path('toasts/<int:toast_id>/acknowledge', views.toasts.acknowledge, name='toasts.acknowledge'),
+    path('toasts/acknowledge', views.toasts.acknowledge, name='toasts.acknowledge'),
     # API
     path('api/', include(router.urls), name='api')
 ]
diff --git a/bookmarks/views/toasts.py b/bookmarks/views/toasts.py
index 2af5bd34..328e3278 100644
--- a/bookmarks/views/toasts.py
+++ b/bookmarks/views/toasts.py
@@ -7,7 +7,8 @@
 
 
 @login_required
-def acknowledge(request, toast_id: int):
+def acknowledge(request):
+    toast_id = request.POST['toast']
     try:
         toast = Toast.objects.get(pk=toast_id, owner=request.user)
     except Toast.DoesNotExist: