From 8f5d7302acb7f6d15ba7051df7fd7fda7375a29e Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Wed, 15 Jan 2020 15:58:55 +0000 Subject: Implement RedirectException (#6687) Allow REST endpoint implemnentations to raise a RedirectException, which will redirect the user's browser to a given location. --- tests/test_server.py | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 77 insertions(+), 2 deletions(-) (limited to 'tests/test_server.py') diff --git a/tests/test_server.py b/tests/test_server.py index 98fef21d55..0d57eed268 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -23,8 +23,12 @@ from twisted.test.proto_helpers import AccumulatingProtocol from twisted.web.resource import Resource from twisted.web.server import NOT_DONE_YET -from synapse.api.errors import Codes, SynapseError -from synapse.http.server import JsonResource +from synapse.api.errors import Codes, RedirectException, SynapseError +from synapse.http.server import ( + DirectServeResource, + JsonResource, + wrap_html_request_handler, +) from synapse.http.site import SynapseSite, logger from synapse.logging.context import make_deferred_yieldable from synapse.util import Clock @@ -164,6 +168,77 @@ class JsonResourceTests(unittest.TestCase): self.assertEqual(channel.json_body["errcode"], "M_UNRECOGNIZED") +class WrapHtmlRequestHandlerTests(unittest.TestCase): + class TestResource(DirectServeResource): + callback = None + + @wrap_html_request_handler + async def _async_render_GET(self, request): + return await self.callback(request) + + def setUp(self): + self.reactor = ThreadedMemoryReactorClock() + + def test_good_response(self): + def callback(request): + request.write(b"response") + request.finish() + + res = WrapHtmlRequestHandlerTests.TestResource() + res.callback = callback + + request, channel = make_request(self.reactor, b"GET", b"/path") + render(request, res, self.reactor) + + self.assertEqual(channel.result["code"], b"200") + body = channel.result["body"] + self.assertEqual(body, b"response") + + def test_redirect_exception(self): + """ + If the callback raises a RedirectException, it is turned into a 30x + with the right location. + """ + + def callback(request, **kwargs): + raise RedirectException(b"/look/an/eagle", 301) + + res = WrapHtmlRequestHandlerTests.TestResource() + res.callback = callback + + request, channel = make_request(self.reactor, b"GET", b"/path") + render(request, res, self.reactor) + + self.assertEqual(channel.result["code"], b"301") + headers = channel.result["headers"] + location_headers = [v for k, v in headers if k == b"Location"] + self.assertEqual(location_headers, [b"/look/an/eagle"]) + + def test_redirect_exception_with_cookie(self): + """ + If the callback raises a RedirectException which sets a cookie, that is + returned too + """ + + def callback(request, **kwargs): + e = RedirectException(b"/no/over/there", 304) + e.cookies.append(b"session=yespls") + raise e + + res = WrapHtmlRequestHandlerTests.TestResource() + res.callback = callback + + request, channel = make_request(self.reactor, b"GET", b"/path") + render(request, res, self.reactor) + + self.assertEqual(channel.result["code"], b"304") + headers = channel.result["headers"] + location_headers = [v for k, v in headers if k == b"Location"] + self.assertEqual(location_headers, [b"/no/over/there"]) + cookies_headers = [v for k, v in headers if k == b"Set-Cookie"] + self.assertEqual(cookies_headers, [b"session=yespls"]) + + class SiteTestCase(unittest.HomeserverTestCase): def test_lose_connection(self): """ -- cgit 1.5.1 From 4429764c9f00bd1d266f08e08a20daac1b849d46 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 22 May 2020 09:30:07 -0400 Subject: Return 200 OK for all OPTIONS requests (#7534) --- changelog.d/7534.misc | 1 + synapse/app/generic_worker.py | 5 ++-- synapse/app/homeserver.py | 14 ++++++++---- synapse/http/server.py | 23 ++++++++++++++++--- tests/test_server.py | 53 +++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 85 insertions(+), 11 deletions(-) create mode 100644 changelog.d/7534.misc (limited to 'tests/test_server.py') diff --git a/changelog.d/7534.misc b/changelog.d/7534.misc new file mode 100644 index 0000000000..9088fb65b8 --- /dev/null +++ b/changelog.d/7534.misc @@ -0,0 +1 @@ +All endpoints now respond with a 200 OK for `OPTIONS` requests. \ No newline at end of file diff --git a/synapse/app/generic_worker.py b/synapse/app/generic_worker.py index a45c876213..a37520000a 100644 --- a/synapse/app/generic_worker.py +++ b/synapse/app/generic_worker.py @@ -22,7 +22,6 @@ from typing import Dict, Iterable from typing_extensions import ContextManager from twisted.internet import defer, reactor -from twisted.web.resource import NoResource import synapse import synapse.events @@ -41,7 +40,7 @@ from synapse.config.logger import setup_logging from synapse.federation import send_queue from synapse.federation.transport.server import TransportLayerServer from synapse.handlers.presence import BasePresenceHandler, get_interested_parties -from synapse.http.server import JsonResource +from synapse.http.server import JsonResource, OptionsResource from synapse.http.servlet import RestServlet, parse_json_object_from_request from synapse.http.site import SynapseSite from synapse.logging.context import LoggingContext @@ -566,7 +565,7 @@ class GenericWorkerServer(HomeServer): if name == "replication": resources[REPLICATION_PREFIX] = ReplicationRestResource(self) - root_resource = create_resource_tree(resources, NoResource()) + root_resource = create_resource_tree(resources, OptionsResource()) _base.listen_tcp( bind_addresses, diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index d7f337e586..93a5ba2100 100644 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -31,7 +31,7 @@ from prometheus_client import Gauge from twisted.application import service from twisted.internet import defer, reactor from twisted.python.failure import Failure -from twisted.web.resource import EncodingResourceWrapper, IResource, NoResource +from twisted.web.resource import EncodingResourceWrapper, IResource from twisted.web.server import GzipEncoderFactory from twisted.web.static import File @@ -52,7 +52,11 @@ from synapse.config._base import ConfigError from synapse.config.homeserver import HomeServerConfig from synapse.federation.transport.server import TransportLayerServer from synapse.http.additional_resource import AdditionalResource -from synapse.http.server import RootRedirect +from synapse.http.server import ( + OptionsResource, + RootOptionsRedirectResource, + RootRedirect, +) from synapse.http.site import SynapseSite from synapse.logging.context import LoggingContext from synapse.metrics import METRICS_PREFIX, MetricsResource, RegistryProxy @@ -121,11 +125,11 @@ class SynapseHomeServer(HomeServer): # try to find something useful to redirect '/' to if WEB_CLIENT_PREFIX in resources: - root_resource = RootRedirect(WEB_CLIENT_PREFIX) + root_resource = RootOptionsRedirectResource(WEB_CLIENT_PREFIX) elif STATIC_PREFIX in resources: - root_resource = RootRedirect(STATIC_PREFIX) + root_resource = RootOptionsRedirectResource(STATIC_PREFIX) else: - root_resource = NoResource() + root_resource = OptionsResource() root_resource = create_resource_tree(resources, root_resource) diff --git a/synapse/http/server.py b/synapse/http/server.py index 042a605198..33fcfbea6e 100644 --- a/synapse/http/server.py +++ b/synapse/http/server.py @@ -350,9 +350,6 @@ class JsonResource(HttpServer, resource.Resource): register_paths, so will return (possibly via Deferred) either None, or a tuple of (http code, response body). """ - if request.method == b"OPTIONS": - return _options_handler, "options_request_handler", {} - request_path = request.path.decode("ascii") # Loop through all the registered callbacks to check if the method @@ -448,6 +445,26 @@ class RootRedirect(resource.Resource): return resource.Resource.getChild(self, name, request) +class OptionsResource(resource.Resource): + """Responds to OPTION requests for itself and all children.""" + + def render_OPTIONS(self, request): + code, response_json_object = _options_handler(request) + + return respond_with_json( + request, code, response_json_object, send_cors=False, canonical_json=False, + ) + + def getChildWithDefault(self, path, request): + if request.method == b"OPTIONS": + return self # select ourselves as the child to render + return resource.Resource.getChildWithDefault(self, path, request) + + +class RootOptionsRedirectResource(OptionsResource, RootRedirect): + pass + + def respond_with_json( request, code, diff --git a/tests/test_server.py b/tests/test_server.py index 0d57eed268..437f925bf9 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -27,6 +27,7 @@ from synapse.api.errors import Codes, RedirectException, SynapseError from synapse.http.server import ( DirectServeResource, JsonResource, + OptionsResource, wrap_html_request_handler, ) from synapse.http.site import SynapseSite, logger @@ -168,6 +169,58 @@ class JsonResourceTests(unittest.TestCase): self.assertEqual(channel.json_body["errcode"], "M_UNRECOGNIZED") +class OptionsResourceTests(unittest.TestCase): + def setUp(self): + self.reactor = ThreadedMemoryReactorClock() + + class DummyResource(Resource): + isLeaf = True + + def render(self, request): + return request.path + + # Setup a resource with some children. + self.resource = OptionsResource() + self.resource.putChild(b"res", DummyResource()) + + def _make_request(self, method, path): + """Create a request from the method/path and return a channel with the response.""" + request, channel = make_request(self.reactor, method, path, shorthand=False) + request.prepath = [] # This doesn't get set properly by make_request. + + # Create a site and query for the resource. + site = SynapseSite("test", "site_tag", {}, self.resource, "1.0") + request.site = site + resource = site.getResourceFor(request) + + # Finally, render the resource and return the channel. + render(request, resource, self.reactor) + return channel + + def test_unknown_options_request(self): + """An OPTIONS requests to an unknown URL still returns 200 OK.""" + channel = self._make_request(b"OPTIONS", b"/foo/") + self.assertEqual(channel.result["code"], b"200") + self.assertEqual(channel.result["body"], b"{}") + + def test_known_options_request(self): + """An OPTIONS requests to an known URL still returns 200 OK.""" + channel = self._make_request(b"OPTIONS", b"/res/") + self.assertEqual(channel.result["code"], b"200") + self.assertEqual(channel.result["body"], b"{}") + + def test_unknown_request(self): + """A non-OPTIONS request to an unknown URL should 404.""" + channel = self._make_request(b"GET", b"/foo/") + self.assertEqual(channel.result["code"], b"404") + + def test_known_request(self): + """A non-OPTIONS request to an known URL should query the proper resource.""" + channel = self._make_request(b"GET", b"/res/") + self.assertEqual(channel.result["code"], b"200") + self.assertEqual(channel.result["body"], b"/res/") + + class WrapHtmlRequestHandlerTests(unittest.TestCase): class TestResource(DirectServeResource): callback = None -- cgit 1.5.1 From 2901f54359bba1ccbe2aac52fd9ff255aa6072b7 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 22 May 2020 17:42:39 +0100 Subject: Fix missing CORS headers on OPTION responses (#7560) Broke in #7534. --- changelog.d/7560.misc | 1 + synapse/http/server.py | 2 +- tests/test_server.py | 28 ++++++++++++++++++++++++++++ 3 files changed, 30 insertions(+), 1 deletion(-) create mode 100644 changelog.d/7560.misc (limited to 'tests/test_server.py') diff --git a/changelog.d/7560.misc b/changelog.d/7560.misc new file mode 100644 index 0000000000..9088fb65b8 --- /dev/null +++ b/changelog.d/7560.misc @@ -0,0 +1 @@ +All endpoints now respond with a 200 OK for `OPTIONS` requests. \ No newline at end of file diff --git a/synapse/http/server.py b/synapse/http/server.py index 33fcfbea6e..9cc2e2e154 100644 --- a/synapse/http/server.py +++ b/synapse/http/server.py @@ -452,7 +452,7 @@ class OptionsResource(resource.Resource): code, response_json_object = _options_handler(request) return respond_with_json( - request, code, response_json_object, send_cors=False, canonical_json=False, + request, code, response_json_object, send_cors=True, canonical_json=False, ) def getChildWithDefault(self, path, request): diff --git a/tests/test_server.py b/tests/test_server.py index 437f925bf9..e9a43b1e45 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -203,12 +203,40 @@ class OptionsResourceTests(unittest.TestCase): self.assertEqual(channel.result["code"], b"200") self.assertEqual(channel.result["body"], b"{}") + # Ensure the correct CORS headers have been added + self.assertTrue( + channel.headers.hasHeader(b"Access-Control-Allow-Origin"), + "has CORS Origin header", + ) + self.assertTrue( + channel.headers.hasHeader(b"Access-Control-Allow-Methods"), + "has CORS Methods header", + ) + self.assertTrue( + channel.headers.hasHeader(b"Access-Control-Allow-Headers"), + "has CORS Headers header", + ) + def test_known_options_request(self): """An OPTIONS requests to an known URL still returns 200 OK.""" channel = self._make_request(b"OPTIONS", b"/res/") self.assertEqual(channel.result["code"], b"200") self.assertEqual(channel.result["body"], b"{}") + # Ensure the correct CORS headers have been added + self.assertTrue( + channel.headers.hasHeader(b"Access-Control-Allow-Origin"), + "has CORS Origin header", + ) + self.assertTrue( + channel.headers.hasHeader(b"Access-Control-Allow-Methods"), + "has CORS Methods header", + ) + self.assertTrue( + channel.headers.hasHeader(b"Access-Control-Allow-Headers"), + "has CORS Headers header", + ) + def test_unknown_request(self): """A non-OPTIONS request to an unknown URL should 404.""" channel = self._make_request(b"GET", b"/foo/") -- cgit 1.5.1