From 796c3ae318eea183fc88c87ec5a27355b0f6a99d Mon Sep 17 00:00:00 2001
From: Uzay-G <uzgirit@gmail.com>
Date: Fri, 24 Dec 2021 12:16:44 +0100
Subject: [PATCH] better CSRF protection; change delete route to POST

---
 archivy/__init__.py                           | 2 ++
 archivy/click_web/resources/cmd_exec.py       | 6 ++----
 archivy/routes.py                             | 2 +-
 archivy/templates/click_web/command_form.html | 1 +
 archivy/templates/dataobjs/show.html          | 2 +-
 tests/functional/test_routes.py               | 4 ++--
 6 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/archivy/__init__.py b/archivy/__init__.py
index b3aa3e7c..a4eaeee9 100644
--- a/archivy/__init__.py
+++ b/archivy/__init__.py
@@ -6,6 +6,7 @@
 from flask import Flask
 from flask_compress import Compress
 from flask_login import LoginManager
+from flask_wtf.csrf import CSRFProtect
 
 from archivy import helpers
 from archivy.api import api_bp
@@ -77,6 +78,7 @@
 login_manager.login_view = "login"
 login_manager.init_app(app)
 app.register_blueprint(api_bp, url_prefix="/api")
+csrf = CSRFProtect(app)
 
 # compress files
 Compress(app)
diff --git a/archivy/click_web/resources/cmd_exec.py b/archivy/click_web/resources/cmd_exec.py
index 3e31f89c..c92e9d07 100644
--- a/archivy/click_web/resources/cmd_exec.py
+++ b/archivy/click_web/resources/cmd_exec.py
@@ -137,10 +137,8 @@ def _get_download_link(field_info):
 
 class RequestToCommandArgs:
     def __init__(self):
-        field_infos = [
-            FieldInfo.factory(key)
-            for key in list(request.form.keys()) + list(request.files.keys())
-        ]
+        keys = [key for key in list(request.form.keys()) + list(request.files.keys())]
+        field_infos = [FieldInfo.factory(key) for key in keys if key != "csrf_token"]
         # important to sort them so they will be in expected order on command line
         self.field_infos = list(sorted(field_infos))
 
diff --git a/archivy/routes.py b/archivy/routes.py
index 1c623a5b..dc7a368a 100644
--- a/archivy/routes.py
+++ b/archivy/routes.py
@@ -232,7 +232,7 @@ def move_item(dataobj_id):
         return redirect(f"/dataobj/{dataobj_id}")
 
 
-@app.route("/dataobj/delete/<int:dataobj_id>", methods=["DELETE", "GET"])
+@app.route("/dataobj/delete/<int:dataobj_id>", methods=["POST"])
 def delete_data(dataobj_id):
     try:
         data.delete_item(dataobj_id)
diff --git a/archivy/templates/click_web/command_form.html b/archivy/templates/click_web/command_form.html
index cdffd0cf..3c68052a 100644
--- a/archivy/templates/click_web/command_form.html
+++ b/archivy/templates/click_web/command_form.html
@@ -29,6 +29,7 @@ <h3 class="command-title">{{ command.name|title }}</h3>
             </div>
         {% endfor %}
     {% endfor %}
+	<input type="hidden" name="csrf_token" value="{{ csrf_token() }}"/>
     <button type="submit" id="submit_btn" class="btn btn-primary m-2">Run</button>
 </form>
 
diff --git a/archivy/templates/dataobjs/show.html b/archivy/templates/dataobjs/show.html
index aad86636..83781921 100644
--- a/archivy/templates/dataobjs/show.html
+++ b/archivy/templates/dataobjs/show.html
@@ -71,7 +71,7 @@ <h2 id="post-title">
             <svg class="octicon" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 16 16" width="16" height="16"><path fill-rule="evenodd" d="M11.013 1.427a1.75 1.75 0 012.474 0l1.086 1.086a1.75 1.75 0 010 2.474l-8.61 8.61c-.21.21-.47.364-.756.445l-3.251.93a.75.75 0 01-.927-.928l.929-3.25a1.75 1.75 0 01.445-.758l8.61-8.61zm1.414 1.06a.25.25 0 00-.354 0L10.811 3.75l1.439 1.44 1.263-1.263a.25.25 0 000-.354l-1.086-1.086zM11.189 6.25L9.75 4.81l-6.286 6.287a.25.25 0 00-.064.108l-.558 1.953 1.953-.558a.249.249 0 00.108-.064l6.286-6.286z"></path></svg>
             <span>Edit</span>
           </button>
-          <form action="/dataobj/delete/{{ dataobj['id'] }}" method="delete" onsubmit="return confirm('Delete this item permanently?')" novalidate>
+          <form action="/dataobj/delete/{{ dataobj['id'] }}" method="POST" onsubmit="return confirm('Delete this item permanently?')" novalidate>
             {{ form.hidden_tag() }}
             <button class="btn btn-delete">
               <svg class="octicon" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 16 16" width="16" height="16"><path fill-rule="evenodd" d="M6.5 1.75a.25.25 0 01.25-.25h2.5a.25.25 0 01.25.25V3h-3V1.75zm4.5 0V3h2.25a.75.75 0 010 1.5H2.75a.75.75 0 010-1.5H5V1.75C5 .784 5.784 0 6.75 0h2.5C10.216 0 11 .784 11 1.75zM4.496 6.675a.75.75 0 10-1.492.15l.66 6.6A1.75 1.75 0 005.405 15h5.19c.9 0 1.652-.681 1.741-1.576l.66-6.6a.75.75 0 00-1.492-.149l-.66 6.6a.25.25 0 01-.249.225h-5.19a.25.25 0 01-.249-.225l-.66-6.6z"></path></svg>
diff --git a/tests/functional/test_routes.py b/tests/functional/test_routes.py
index 836f38b4..7e59bf42 100644
--- a/tests/functional/test_routes.py
+++ b/tests/functional/test_routes.py
@@ -61,12 +61,12 @@ def test_get_dataobj(test_app, client: FlaskClient, note_fixture):
 
 
 def test_get_delete_dataobj_not_found(test_app, client: FlaskClient):
-    response = client.get("/dataobj/delete/1")
+    response = client.post("/dataobj/delete/1")
     assert response.status_code == 302
 
 
 def test_get_delete_dataobj(test_app, client: FlaskClient, note_fixture):
-    response = client.get("/dataobj/delete/1")
+    response = client.post("/dataobj/delete/1")
     assert response.status_code == 302