Merge branch 'fix_405_error_and_other_methods' of https://github.com/mig5/onionshare into mig5-fix_405_error_and_other_methods

author: Micah Lee <micah@micahflee.com> 2021-05-25 16:27:26 -0700
committer: Micah Lee <micah@micahflee.com> 2021-05-25 16:27:26 -0700
commit: c682b7ec01bdd395a0bbc8e652473bc9de90067b (patch)
tree: 7e61b1b0ee5e5a9198b7e981a30ebca0a93306f9
parent: 7ef5a99c0a6a5b54798ccd39bd4e604cfd5c004d (diff)
parent: d4d6eea500feb825d5639354b8aad1be6ba451f5 (diff)
download: onionshare-c682b7ec01bdd395a0bbc8e652473bc9de90067b.tar.gz
onionshare-c682b7ec01bdd395a0bbc8e652473bc9de90067b.zip
12 files changed, 182 insertions, 51 deletions
diff --git a/cli/onionshare_cli/resources/templates/500.html b/cli/onionshare_cli/resources/templates/500.html
new file mode 100644
index 00000000..9f6727d2
--- /dev/null
+++ b/cli/onionshare_cli/resources/templates/500.html
@@ -0,0 +1,21 @@
+<!DOCTYPE html>
+<html>
+
+<head>
+  <title>OnionShare: An error occurred</title>
+  <meta charset="utf-8" />
+  <meta name="viewport" content="width=device-width, initial-scale=1"> 
+  <link href="{{ static_url_path }}/img/favicon.ico" rel="icon" type="image/x-icon">
+  <link rel="stylesheet" rel="subresource" type="text/css" href="{{ static_url_path }}/css/style.css" media="all">
+</head>
+
+<body>
+  <div class="info-wrapper">
+    <div class="info">
+      <p><img class="logo" src="{{ static_url_path }}/img/logo_large.png" title="OnionShare"></p>
+      <p class="info-header">Sorry, an unexpected error seems to have occurred, and your request didn't succeed.</p>
+    </div>
+  </div>
+</body>
+
+</html>
diff --git a/cli/onionshare_cli/web/chat_mode.py b/cli/onionshare_cli/web/chat_mode.py
index e9b573dd..f6dc2d1a 100644
--- a/cli/onionshare_cli/web/chat_mode.py
+++ b/cli/onionshare_cli/web/chat_mode.py
@@ -39,6 +39,12 @@ class ChatModeWeb:
         # This tracks the history id
         self.cur_history_id = 0
 
+        # Whether or not we can send REQUEST_INDIVIDUAL_FILE_STARTED
+        # and maybe other events when requests come in to this mode
+        # Chat mode has no concept of individual file requests that
+        # turn into history widgets in the GUI, so set it to False
+        self.supports_file_requests = False
+
         self.define_routes()
 
     def define_routes(self):
@@ -46,7 +52,7 @@ class ChatModeWeb:
         The web app routes for chatting
         """
 
-        @self.web.app.route("/")
+        @self.web.app.route("/", methods=["GET"], provide_automatic_options=False)
         def index():
             history_id = self.cur_history_id
             self.cur_history_id += 1
@@ -72,7 +78,7 @@ class ChatModeWeb:
             )
             return self.web.add_security_headers(r)
 
-        @self.web.app.route("/update-session-username", methods=["POST"])
+        @self.web.app.route("/update-session-username", methods=["POST"], provide_automatic_options=False)
         def update_session_username():
             history_id = self.cur_history_id
             data = request.get_json()
diff --git a/cli/onionshare_cli/web/receive_mode.py b/cli/onionshare_cli/web/receive_mode.py
index f5aae296..76abb0a8 100644
--- a/cli/onionshare_cli/web/receive_mode.py
+++ b/cli/onionshare_cli/web/receive_mode.py
@@ -64,6 +64,10 @@ class ReceiveModeWeb:
         # This tracks the history id
         self.cur_history_id = 0
 
+        # Whether or not we can send REQUEST_INDIVIDUAL_FILE_STARTED
+        # and maybe other events when requests come in to this mode
+        self.supports_file_requests = True
+
         self.define_routes()
 
     def define_routes(self):
@@ -71,7 +75,7 @@ class ReceiveModeWeb:
         The web app routes for receiving files
         """
 
-        @self.web.app.route("/")
+        @self.web.app.route("/", methods=["GET"], provide_automatic_options=False)
         def index():
             history_id = self.cur_history_id
             self.cur_history_id += 1
@@ -93,7 +97,7 @@ class ReceiveModeWeb:
             )
             return self.web.add_security_headers(r)
 
-        @self.web.app.route("/upload", methods=["POST"])
+        @self.web.app.route("/upload", methods=["POST"], provide_automatic_options=False)
         def upload(ajax=False):
             """
             Handle the upload files POST request, though at this point, the files have
@@ -225,7 +229,7 @@ class ReceiveModeWeb:
                     )
                     return self.web.add_security_headers(r)
 
-        @self.web.app.route("/upload-ajax", methods=["POST"])
+        @self.web.app.route("/upload-ajax", methods=["POST"], provide_automatic_options=False)
         def upload_ajax_public():
             if not self.can_upload:
                 return self.web.error403()
diff --git a/cli/onionshare_cli/web/send_base_mode.py b/cli/onionshare_cli/web/send_base_mode.py
index 742f6f75..e448d2dd 100644
--- a/cli/onionshare_cli/web/send_base_mode.py
+++ b/cli/onionshare_cli/web/send_base_mode.py
@@ -52,6 +52,10 @@ class SendBaseModeWeb:
         # This tracks the history id
         self.cur_history_id = 0
 
+        # Whether or not we can send REQUEST_INDIVIDUAL_FILE_STARTED
+        # and maybe other events when requests come in to this mode
+        self.supports_file_requests = True
+
         self.define_routes()
         self.init()
 
@@ -208,10 +212,6 @@ class SendBaseModeWeb:
         history_id = self.cur_history_id
         self.cur_history_id += 1
 
-        # Only GET requests are allowed, any other method should fail
-        if request.method != "GET":
-            return self.web.error405(history_id)
-
         self.web.add_request(
             self.web.REQUEST_INDIVIDUAL_FILE_STARTED,
             path,
diff --git a/cli/onionshare_cli/web/share_mode.py b/cli/onionshare_cli/web/share_mode.py
index 95aec1ba..51ddd674 100644
--- a/cli/onionshare_cli/web/share_mode.py
+++ b/cli/onionshare_cli/web/share_mode.py
@@ -134,8 +134,8 @@ class ShareModeWeb(SendBaseModeWeb):
         The web app routes for sharing files
         """
 
-        @self.web.app.route("/", defaults={"path": ""})
-        @self.web.app.route("/<path:path>")
+        @self.web.app.route("/", defaults={"path": ""}, methods=["GET"], provide_automatic_options=False)
+        @self.web.app.route("/<path:path>", methods=["GET"], provide_automatic_options=False)
         def index(path):
             """
             Render the template for the onionshare landing page.
@@ -160,7 +160,7 @@ class ShareModeWeb(SendBaseModeWeb):
 
             return self.render_logic(path)
 
-        @self.web.app.route("/download")
+        @self.web.app.route("/download", methods=["GET"], provide_automatic_options=False)
         def download():
             """
             Download the zip file.
diff --git a/cli/onionshare_cli/web/web.py b/cli/onionshare_cli/web/web.py
index d88a7e4e..56e307b4 100644
--- a/cli/onionshare_cli/web/web.py
+++ b/cli/onionshare_cli/web/web.py
@@ -229,6 +229,20 @@ class Web:
             mode.cur_history_id += 1
             return self.error404(history_id)
 
+        @self.app.errorhandler(405)
+        def method_not_allowed(e):
+            mode = self.get_mode()
+            history_id = mode.cur_history_id
+            mode.cur_history_id += 1
+            return self.error405(history_id)
+
+        @self.app.errorhandler(500)
+        def method_not_allowed(e):
+            mode = self.get_mode()
+            history_id = mode.cur_history_id
+            mode.cur_history_id += 1
+            return self.error500(history_id)
+
         @self.app.route("/<password_candidate>/shutdown")
         def shutdown(password_candidate):
             """
@@ -280,11 +294,13 @@ class Web:
         return self.add_security_headers(r)
 
     def error404(self, history_id):
-        self.add_request(
-            self.REQUEST_INDIVIDUAL_FILE_STARTED,
-            request.path,
-            {"id": history_id, "status_code": 404},
-        )
+        mode = self.get_mode()
+        if mode.supports_file_requests:
+            self.add_request(
+                self.REQUEST_INDIVIDUAL_FILE_STARTED,
+                request.path,
+                {"id": history_id, "status_code": 404},
+            )
 
         self.add_request(Web.REQUEST_OTHER, request.path)
         r = make_response(
@@ -293,11 +309,13 @@ class Web:
         return self.add_security_headers(r)
 
     def error405(self, history_id):
-        self.add_request(
-            self.REQUEST_INDIVIDUAL_FILE_STARTED,
-            request.path,
-            {"id": history_id, "status_code": 405},
-        )
+        mode = self.get_mode()
+        if mode.supports_file_requests:
+            self.add_request(
+                self.REQUEST_INDIVIDUAL_FILE_STARTED,
+                request.path,
+                {"id": history_id, "status_code": 405},
+            )
 
         self.add_request(Web.REQUEST_OTHER, request.path)
         r = make_response(
@@ -305,6 +323,21 @@ class Web:
         )
         return self.add_security_headers(r)
 
+    def error500(self, history_id):
+        mode = self.get_mode()
+        if mode.supports_file_requests:
+            self.add_request(
+                self.REQUEST_INDIVIDUAL_FILE_STARTED,
+                request.path,
+                {"id": history_id, "status_code": 500},
+            )
+
+        self.add_request(Web.REQUEST_OTHER, request.path)
+        r = make_response(
+            render_template("500.html", static_url_path=self.static_url_path), 500
+        )
+        return self.add_security_headers(r)
+
     def add_security_headers(self, r):
         """
         Add security headers to a request
diff --git a/cli/onionshare_cli/web/website_mode.py b/cli/onionshare_cli/web/website_mode.py
index 6badd399..5ab1b184 100644
--- a/cli/onionshare_cli/web/website_mode.py
+++ b/cli/onionshare_cli/web/website_mode.py
@@ -37,8 +37,8 @@ class WebsiteModeWeb(SendBaseModeWeb):
         The web app routes for sharing a website
         """
 
-        @self.web.app.route("/", defaults={"path": ""})
-        @self.web.app.route("/<path:path>")
+        @self.web.app.route("/", defaults={"path": ""}, methods=["GET"], provide_automatic_options=False)
+        @self.web.app.route("/<path:path>", methods=["GET"], provide_automatic_options=False)
         def path_public(path):
             return path_logic(path)
 
diff --git a/desktop/tests/gui_base_test.py b/desktop/tests/gui_base_test.py
index 3a38ff8e..acaa9739 100644
--- a/desktop/tests/gui_base_test.py
+++ b/desktop/tests/gui_base_test.py
@@ -465,6 +465,20 @@ class GuiBaseTest(unittest.TestCase):
         # We should have timed out now
         self.assertEqual(tab.get_mode().server_status.status, 0)
 
+    def hit_405(self, url, expected_resp, data = {}, methods = [] ):
+        """Test various HTTP methods and the response"""
+        for method in methods:
+            if method == "put":
+                r = requests.put(url, data = data)
+            if method == "post":
+                r = requests.post(url, data = data)
+            if method == "delete":
+                r = requests.delete(url)
+            if method == "options":
+                r = requests.options(url)
+            self.assertTrue(expected_resp in r.text)
+            self.assertFalse('Werkzeug' in r.headers)
+
     # Grouped tests follow from here
 
     def run_all_common_setup_tests(self):
diff --git a/desktop/tests/test_gui_chat.py b/desktop/tests/test_gui_chat.py
index 08c619c6..15ecaa44 100644
--- a/desktop/tests/test_gui_chat.py
+++ b/desktop/tests/test_gui_chat.py
@@ -30,7 +30,7 @@ class TestChat(GuiBaseTest):
     def change_username(self, tab):
         """Test that we can change our username"""
         url = f"http://127.0.0.1:{tab.app.port}/update-session-username"
-        data = {"username":"oniontest"}
+        data = {"username": "oniontest"}
         if tab.settings.get("general", "public"):
             r = requests.post(url, json=data)
         else:
@@ -47,28 +47,7 @@ class TestChat(GuiBaseTest):
         self.assertTrue(jsonResponse["success"])
         self.assertEqual(jsonResponse["username"], "oniontest")
 
-    def change_username_too_long(self, tab):
-        """Test that we can't set our username to something 128 chars or longer"""
-        url = f"http://127.0.0.1:{tab.app.port}/update-session-username"
-        bad_username = "sduBB9yEMkyQpwkMM4A9nUbQwNUbPU2PQuJYN26zCQ4inELpB76J5i5oRUnD3ESVaE9NNE8puAtBj2DiqDaZdVqhV8MonyxSSGHRv87YgM5dzwBYPBxttoQSKZAUkFjo"
-        data = {"username":bad_username}
-        if tab.settings.get("general", "public"):
-            r = requests.post(url, json=data)
-        else:
-            r = requests.post(
-                url,
-                json=data,
-                auth=requests.auth.HTTPBasicAuth(
-                    "onionshare", tab.get_mode().server_status.web.password
-                ),
-            )
-
-        QtTest.QTest.qWait(500, self.gui.qtapp)
-        jsonResponse = r.json()
-        self.assertFalse(jsonResponse["success"])
-        self.assertNotEqual(jsonResponse["username"], bad_username)
-
-    def run_all_chat_mode_tests(self, tab):
+    def run_all_chat_mode_started_tests(self, tab):
         """Tests in chat mode after starting a chat"""
         self.server_working_on_start_button_pressed(tab)
         self.server_status_indicator_says_starting(tab)
@@ -79,9 +58,9 @@ class TestChat(GuiBaseTest):
         self.have_copy_url_button(tab)
         self.have_show_qr_code_button(tab)
         self.server_status_indicator_says_started(tab)
-        self.view_chat(tab)
-        self.change_username(tab)
-        self.change_username_too_long(tab)
+
+    def run_all_chat_mode_stopping_tests(self, tab):
+        """Tests stopping a chat"""
         self.server_is_stopped(tab)
         self.web_server_is_stopped(tab)
         self.server_status_indicator_says_closed(tab)
@@ -93,5 +72,27 @@ class TestChat(GuiBaseTest):
         Test chat mode
         """
         tab = self.new_chat_tab()
-        self.run_all_chat_mode_tests(tab)
+        self.run_all_chat_mode_started_tests(tab)
+        self.view_chat(tab)
+        self.change_username(tab)
+        self.run_all_chat_mode_stopping_tests(tab)
+        self.close_all_tabs()
+
+    def test_405_page_returned_for_invalid_methods(self):
+        """
+        Our custom 405 page should return for invalid methods
+        """
+        tab = self.new_chat_tab()
+
+        tab.get_mode().mode_settings_widget.public_checkbox.click()
+
+        self.run_all_chat_mode_started_tests(tab)
+        url = f"http://127.0.0.1:{tab.app.port}/"
+        self.hit_405(
+            url,
+            expected_resp="OnionShare: 405 Method Not Allowed",
+            data={"foo": "bar"},
+            methods=["put", "post", "delete", "options"],
+        )
+        self.run_all_chat_mode_stopping_tests(tab)
         self.close_all_tabs()
diff --git a/desktop/tests/test_gui_receive.py b/desktop/tests/test_gui_receive.py
index 6e14ae67..b523b0fa 100644
--- a/desktop/tests/test_gui_receive.py
+++ b/desktop/tests/test_gui_receive.py
@@ -286,3 +286,22 @@ class TestReceive(GuiBaseTest):
         self.run_all_upload_non_writable_dir_tests(tab)
 
         self.close_all_tabs()
+
+    def test_405_page_returned_for_invalid_methods(self):
+        """
+        Our custom 405 page should return for invalid methods
+        """
+        tab = self.new_receive_tab()
+
+        tab.get_mode().mode_settings_widget.public_checkbox.click()
+
+        self.run_all_common_setup_tests()
+        self.run_all_receive_mode_setup_tests(tab)
+        self.upload_file(tab, self.tmpfile_test, "test.txt")
+        url = f"http://127.0.0.1:{tab.app.port}/"
+        self.hit_405(url, expected_resp="OnionShare: 405 Method Not Allowed", data = {'foo':'bar'}, methods = ["put", "post", "delete", "options"])
+
+        self.server_is_stopped(tab)
+        self.web_server_is_stopped(tab)
+        self.server_status_indicator_says_closed(tab)
+        self.close_all_tabs()
diff --git a/desktop/tests/test_gui_share.py b/desktop/tests/test_gui_share.py
index 380d63f6..531e456f 100644
--- a/desktop/tests/test_gui_share.py
+++ b/desktop/tests/test_gui_share.py
@@ -608,3 +608,20 @@ class TestShare(GuiBaseTest):
         self.hit_401(tab)
 
         self.close_all_tabs()
+
+    def test_405_page_returned_for_invalid_methods(self):
+        """
+        Our custom 405 page should return for invalid methods
+        """
+        tab = self.new_share_tab()
+
+        tab.get_mode().autostop_sharing_checkbox.click()
+        tab.get_mode().mode_settings_widget.public_checkbox.click()
+
+        self.run_all_common_setup_tests()
+        self.run_all_share_mode_setup_tests(tab)
+        self.run_all_share_mode_started_tests(tab)
+        url = f"http://127.0.0.1:{tab.app.port}/"
+        self.hit_405(url, expected_resp="OnionShare: 405 Method Not Allowed", data = {'foo':'bar'}, methods = ["put", "post", "delete", "options"])
+        self.history_widgets_present(tab)
+        self.close_all_tabs()
diff --git a/desktop/tests/test_gui_website.py b/desktop/tests/test_gui_website.py
index a838cb96..f526756a 100644
--- a/desktop/tests/test_gui_website.py
+++ b/desktop/tests/test_gui_website.py
@@ -99,3 +99,19 @@ class TestWebsite(GuiBaseTest):
         tab.get_mode().disable_csp_checkbox.click()
         self.run_all_website_mode_download_tests(tab)
         self.close_all_tabs()
+
+    def test_405_page_returned_for_invalid_methods(self):
+        """
+        Our custom 405 page should return for invalid methods
+        """
+        tab = self.new_website_tab()
+
+        tab.get_mode().mode_settings_widget.public_checkbox.click()
+
+        self.run_all_common_setup_tests()
+        self.run_all_website_mode_setup_tests(tab)
+        self.run_all_website_mode_started_tests(tab)
+        url = f"http://127.0.0.1:{tab.app.port}/"
+        self.hit_405(url, expected_resp="OnionShare: 405 Method Not Allowed", data = {'foo':'bar'}, methods = ["put", "post", "delete", "options"])
+
+        self.close_all_tabs()
author	Micah Lee <micah@micahflee.com>	2021-05-25 16:27:26 -0700
committer	Micah Lee <micah@micahflee.com>	2021-05-25 16:27:26 -0700
commit	c682b7ec01bdd395a0bbc8e652473bc9de90067b (patch)
tree	7e61b1b0ee5e5a9198b7e981a30ebca0a93306f9
parent	7ef5a99c0a6a5b54798ccd39bd4e604cfd5c004d (diff)
parent	d4d6eea500feb825d5639354b8aad1be6ba451f5 (diff)
download	onionshare-c682b7ec01bdd395a0bbc8e652473bc9de90067b.tar.gz onionshare-c682b7ec01bdd395a0bbc8e652473bc9de90067b.zip