Add CSRF validation to most disruptive actions

This also switches all such actions to POST requests.

Deleting the project is handled in another commit because it requires more
changes.

ihatemoney/forms.py

@@ -373,3 +373,9 @@ def validate_emails(form, field):
                 raise ValidationError(
                     _("The email %(email)s is not valid", email=email)
                 )
+
+
+class EmptyForm(FlaskForm):
+    """Used for CSRF validation"""
+
+    pass

ihatemoney/static/css/main.css

@@ -313,7 +313,7 @@ footer .footer-left {
   text-align: center;
 }
 
-.bill-actions > .delete,
+.bill-actions > form > .delete,
 .bill-actions > .edit,
 .bill-actions > .show {
   font-size: 0px;
@@ -323,9 +323,10 @@ footer .footer-left {
   margin: 2px;
   margin-left: 5px;
   float: left;
+  border: none;
 }
 
-.bill-actions > .delete {
+.bill-actions > form > .delete {
   background: url("../images/delete.png") no-repeat right;
 }
 

ihatemoney/templates/list_bills.html

@@ -131,7 +131,10 @@ <h3 class="modal-title">{{ _('Add a bill') }}</h3>
                 </td>
                 <td class="bill-actions">
                     <a class="edit" href="{{ url_for(".edit_bill", bill_id=bill.id) }}" title="{{ _("edit") }}">{{ _('edit') }}</a>
-                    <a class="delete" href="{{ url_for(".delete_bill", bill_id=bill.id) }}" title="{{ _("delete") }}">{{ _('delete') }}</a>
+                    <form action="{{ url_for(".delete_bill", bill_id=bill.id) }}" method="POST">
+                        {{ csrf_form.csrf_token }}
+                        <button class="action delete" type="submit" title="{{ _("delete") }}"></button>
+                    </form>
                     {% if bill.external_link %}
                     <a class="show" href="{{ bill.external_link }}" ref="noopener" target="_blank" title="{{ _("show") }}">{{ _('show') }} </a>
                     {% endif %}

ihatemoney/templates/sidebar_table_layout.html

@@ -22,6 +22,7 @@
           {%- if member.activated %}
             <td>
               <form class="action delete" action="{{ url_for(".remove_member", member_id=member.id) }}" method="POST">
+                {{ csrf_form.csrf_token }}
                 <button type="submit">{{ _("deactivate") }}</button>
               </form>
               <form class="action edit" action="{{ url_for(".edit_member", member_id=member.id) }}" method="GET">
@@ -31,6 +32,7 @@
           {%- else %}
             <td>
               <form class="action reactivate" action="{{ url_for(".reactivate", member_id=member.id) }}" method="POST">
+                {{ csrf_form.csrf_token }}
                 <button type="submit">{{ _("reactivate") }}</button>
               </form>
             </td>

ihatemoney/tests/budget_test.py

@@ -550,7 +550,7 @@ def test_manage_bills(self):
         self.assertEqual(bill.amount, 10, "bill edition")
 
         # delete the bill
-        self.client.get(f"/raclette/delete/{bill.id}")
+        self.client.post(f"/raclette/delete/{bill.id}")
         self.assertEqual(0, len(models.Bill.query.all()), "bill deletion")
 
         # test balance
@@ -1375,10 +1375,10 @@ def test_access_other_projects(self):
         resp = self.client.post("/tartiflette/edit/1", data=modified_bill)
         self.assertStatus(404, resp)
         # Try to delete bill
-        resp = self.client.get("/raclette/delete/1")
+        resp = self.client.post("/raclette/delete/1")
         self.assertStatus(303, resp)
         # Try to delete bill by ID
-        resp = self.client.get("/tartiflette/delete/1")
+        resp = self.client.post("/tartiflette/delete/1")
         self.assertStatus(302, resp)
 
         # Additional check that the bill was indeed not modified or deleted
@@ -1396,7 +1396,7 @@ def test_access_other_projects(self):
         bill = models.Bill.query.filter(models.Bill.id == 1).one_or_none()
         self.assertNotEqual(bill, None, "bill not found")
         self.assertEqual(bill.what, "roblochon")
-        self.client.get("/raclette/delete/1")
+        self.client.post("/raclette/delete/1")
         bill = models.Bill.query.filter(models.Bill.id == 1).one_or_none()
         self.assertEqual(bill, None)
 

ihatemoney/tests/history_test.py

@@ -222,7 +222,7 @@ def do_misc_database_operations(self, logging_mode):
         )
         self.assertEqual(resp.status_code, 200)
         # delete the bill
-        resp = self.client.get(f"/demo/delete/{bill_id}", follow_redirects=True)
+        resp = self.client.post(f"/demo/delete/{bill_id}", follow_redirects=True)
         self.assertEqual(resp.status_code, 200)
 
         # delete user using POST method
@@ -389,7 +389,7 @@ def test_logs_for_common_actions(self):
         )
 
         # delete the bill
-        resp = self.client.get("/demo/delete/1", follow_redirects=True)
+        resp = self.client.post("/demo/delete/1", follow_redirects=True)
         self.assertEqual(resp.status_code, 200)
 
         resp = self.client.get("/demo/history")
@@ -527,7 +527,7 @@ def test_bill_add_remove_add(self):
         )
 
         # delete the bill
-        self.client.get("/demo/delete/1", follow_redirects=True)
+        self.client.post("/demo/delete/1", follow_redirects=True)
 
         resp = self.client.get("/demo/history")
         self.assertEqual(resp.status_code, 200)

ihatemoney/web.py

@@ -40,6 +40,7 @@
     AdminAuthenticationForm,
     AuthenticationForm,
     EditProjectForm,
+    EmptyForm,
     InviteForm,
     MemberForm,
     PasswordReminder,
@@ -608,6 +609,7 @@ def invite():
 @main.route("/<project_id>/")
 def list_bills():
     bill_form = get_billform_for(g.project)
+    csrf_form = EmptyForm()
     # set the last selected payer as default choice if exists
     if "last_selected_payer" in session:
         bill_form.payer.data = session["last_selected_payer"]
@@ -623,6 +625,7 @@ def list_bills():
         bills=bills,
         member_form=MemberForm(g.project),
         bill_form=bill_form,
+        csrf_form=csrf_form,
         add_bill=request.values.get("add_bill", False),
         current_view="list_bills",
     )
@@ -644,6 +647,12 @@ def add_member():
 
 @main.route("/<project_id>/members/<member_id>/reactivate", methods=["POST"])
 def reactivate(member_id):
+    # Used for CSRF validation
+    form = EmptyForm()
+    if not form.validate():
+        flash(_("CSRF Token: The CSRF token is invalid."), category="danger")
+        return redirect(url_for(".list_bills"))
+
     person = (
         Person.query.filter(Person.id == member_id)
         .filter(Project.id == g.project.id)
@@ -658,6 +667,12 @@ def reactivate(member_id):
 
 @main.route("/<project_id>/members/<member_id>/delete", methods=["POST"])
 def remove_member(member_id):
+    # Used for CSRF validation
+    form = EmptyForm()
+    if not form.validate():
+        flash(_("CSRF Token: The CSRF token is invalid."), category="danger")
+        return redirect(url_for(".list_bills"))
+
     member = g.project.remove_member(member_id)
     if member:
         if not member.activated:
@@ -715,9 +730,14 @@ def add_bill():
     return render_template("add_bill.html", form=form)
 
 
-@main.route("/<project_id>/delete/<int:bill_id>")
+@main.route("/<project_id>/delete/<int:bill_id>", methods=["POST"])
 def delete_bill(bill_id):
-    # fixme: everyone is able to delete a bill
+    # Used for CSRF validation
+    form = EmptyForm()
+    if not form.validate():
+        flash(_("CSRF Token: The CSRF token is invalid."), category="danger")
+        return redirect(url_for(".list_bills"))
+
     bill = Bill.query.get(g.project, bill_id)
     if not bill:
         return redirect(url_for(".list_bills"))