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 @@ + + + + + OnionShare: An error occurred + + + + + + + +
+
+

+

Sorry, an unexpected error seems to have occurred, and your request didn't succeed.

+
+
+ + + 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("/") + @self.web.app.route("/", defaults={"path": ""}, methods=["GET"], provide_automatic_options=False) + @self.web.app.route("/", 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("//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("/") + @self.web.app.route("/", defaults={"path": ""}, methods=["GET"], provide_automatic_options=False) + @self.web.app.route("/", 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()