diff --git a/docs/_newsfragments/2325.newandimproved.rst b/docs/_newsfragments/2325.newandimproved.rst new file mode 100644 index 000000000..62a61b81e --- /dev/null +++ b/docs/_newsfragments/2325.newandimproved.rst @@ -0,0 +1,4 @@ +The :class:`~CORSMiddleware` now properly handles the missing ``Allow`` +header case, by denying the preflight CORS request. +The static resource has been updated to properly support CORS request, +by allowing GET requests. diff --git a/falcon/app.py b/falcon/app.py index b66247051..38a40dc3c 100644 --- a/falcon/app.py +++ b/falcon/app.py @@ -759,6 +759,15 @@ def add_sink(self, sink: SinkCallable, prefix: SinkPrefix = r'/') -> None: impractical. For example, you might use a sink to create a smart proxy that forwards requests to one or more backend services. + Note: + To support CORS preflight requests when using the default CORS middleware, + either by setting ``App.cors_enable=True`` or by adding the + :class:`~.CORSMiddleware` to the ``App.middleware``, the sink should + set the ``Allow`` header in the request to the allowed + method values when serving an ``OPTIONS`` request. If the ``Allow`` header is + missing from the response, the default CORS middleware will deny the + preflight request. + Args: sink (callable): A callable taking the form ``func(req, resp, **kwargs)``. @@ -1007,9 +1016,7 @@ def _prepare_middleware( middleware=middleware, independent_middleware=independent_middleware ) - def _get_responder( - self, req: Request - ) -> Tuple[ + def _get_responder(self, req: Request) -> Tuple[ Union[ResponderCallable, AsgiResponderCallable, AsgiResponderWsCallable], Dict[str, Any], object, diff --git a/falcon/middleware.py b/falcon/middleware.py index 0e87275ed..d457a44b8 100644 --- a/falcon/middleware.py +++ b/falcon/middleware.py @@ -17,6 +17,15 @@ class CORSMiddleware(object): * https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS * https://www.w3.org/TR/cors/#resource-processing-model + Note: + Falcon will automatically add OPTIONS responders if they are missing from the + responder instances added to the routes. When providing a custom ``on_options`` + method, the ``Allow`` headers in the response should be set to the allowed + method values. If the ``Allow`` header is missing from the response, + this middleware will deny the preflight request. + + This is also valid when using a sink function. + Keyword Arguments: allow_origins (Union[str, Iterable[str]]): List of origins to allow (case sensitive). The string ``'*'`` acts as a wildcard, matching every origin. @@ -120,9 +129,17 @@ def process_response( 'Access-Control-Request-Headers', default='*' ) - resp.set_header('Access-Control-Allow-Methods', str(allow)) - resp.set_header('Access-Control-Allow-Headers', allow_headers) - resp.set_header('Access-Control-Max-Age', '86400') # 24 hours + if allow is None: + # there is no allow set, remove all access control headers + resp.delete_header('Access-Control-Allow-Methods') + resp.delete_header('Access-Control-Allow-Headers') + resp.delete_header('Access-Control-Max-Age') + resp.delete_header('Access-Control-Expose-Headers') + resp.delete_header('Access-Control-Allow-Origin') + else: + resp.set_header('Access-Control-Allow-Methods', allow) + resp.set_header('Access-Control-Allow-Headers', allow_headers) + resp.set_header('Access-Control-Max-Age', '86400') # 24 hours async def process_response_async(self, *args: Any) -> None: self.process_response(*args) diff --git a/falcon/routing/static.py b/falcon/routing/static.py index 76ef4ed14..cc2a5ab92 100644 --- a/falcon/routing/static.py +++ b/falcon/routing/static.py @@ -183,6 +183,12 @@ def match(self, path: str) -> bool: def __call__(self, req: Request, resp: Response, **kw: Any) -> None: """Resource responder for this route.""" assert not kw + if req.method == 'OPTIONS': + # it's likely a CORS request. Set the allow header to the appropriate value. + resp.set_header('Allow', 'GET') + resp.set_header('Content-Length', '0') + return + without_prefix = req.path[len(self._prefix) :] # NOTE(kgriffs): Check surrounding whitespace and strip trailing @@ -247,9 +253,9 @@ class StaticRouteAsync(StaticRoute): async def __call__(self, req: asgi.Request, resp: asgi.Response, **kw: Any) -> None: # type: ignore[override] super().__call__(req, resp, **kw) - - # NOTE(kgriffs): Fixup resp.stream so that it is non-blocking - resp.stream = _AsyncFileReader(resp.stream) # type: ignore[assignment,arg-type] + if resp.stream is not None: # None when in an option request + # NOTE(kgriffs): Fixup resp.stream so that it is non-blocking + resp.stream = _AsyncFileReader(resp.stream) # type: ignore[assignment,arg-type] class _AsyncFileReader: diff --git a/tests/test_cors_middleware.py b/tests/test_cors_middleware.py index 4594242be..34fec7e53 100644 --- a/tests/test_cors_middleware.py +++ b/tests/test_cors_middleware.py @@ -1,3 +1,4 @@ +from pathlib import Path import pytest import falcon @@ -86,8 +87,7 @@ def test_enabled_cors_handles_preflighting(self, cors_client): result.headers['Access-Control-Max-Age'] == '86400' ) # 24 hours in seconds - @pytest.mark.xfail(reason='will be fixed in 2325') - def test_enabled_cors_handles_preflighting_custom_option(self, cors_client): + def test_enabled_cors_handles_preflight_custom_option(self, cors_client): cors_client.app.add_route('/', CORSOptionsResource()) result = cors_client.simulate_options( headers=( @@ -97,11 +97,10 @@ def test_enabled_cors_handles_preflighting_custom_option(self, cors_client): ) ) assert 'Access-Control-Allow-Methods' not in result.headers - assert ( - result.headers['Access-Control-Allow-Headers'] - == 'X-PINGOTHER, Content-Type' - ) - assert result.headers['Access-Control-Max-Age'] == '86400' + assert 'Access-Control-Allow-Headers' not in result.headers + assert 'Access-Control-Max-Age' not in result.headers + assert 'Access-Control-Expose-Headers' not in result.headers + assert 'Access-Control-Allow-Origin' not in result.headers def test_enabled_cors_handles_preflighting_no_headers_in_req(self, cors_client): cors_client.app.add_route('/', CORSHeaderResource()) @@ -117,6 +116,50 @@ def test_enabled_cors_handles_preflighting_no_headers_in_req(self, cors_client): result.headers['Access-Control-Max-Age'] == '86400' ) # 24 hours in seconds + def test_enabled_cors_static_route(self, cors_client): + cors_client.app.add_static_route('/static', Path(__file__).parent) + result = cors_client.simulate_options( + f'/static/{Path(__file__).name}', + headers=( + ('Origin', 'localhost'), + ('Access-Control-Request-Method', 'GET'), + ), + ) + + assert result.headers['Access-Control-Allow-Methods'] == 'GET' + assert result.headers['Access-Control-Allow-Headers'] == '*' + assert result.headers['Access-Control-Max-Age'] == '86400' + assert result.headers['Access-Control-Allow-Origin'] == '*' + + @pytest.mark.parametrize('support_options', [True, False]) + def test_enabled_cors_sink_route(self, cors_client, support_options): + def my_sink(req, resp): + if req.method == 'OPTIONS' and support_options: + resp.set_header('ALLOW', 'GET') + else: + resp.text = 'my sink' + + cors_client.app.add_sink(my_sink, '/sink') + result = cors_client.simulate_options( + f'/sink/123', + headers=( + ('Origin', 'localhost'), + ('Access-Control-Request-Method', 'GET'), + ), + ) + + if support_options: + assert result.headers['Access-Control-Allow-Methods'] == 'GET' + assert result.headers['Access-Control-Allow-Headers'] == '*' + assert result.headers['Access-Control-Max-Age'] == '86400' + assert result.headers['Access-Control-Allow-Origin'] == '*' + else: + assert 'Access-Control-Allow-Methods' not in result.headers + assert 'Access-Control-Allow-Headers' not in result.headers + assert 'Access-Control-Max-Age' not in result.headers + assert 'Access-Control-Expose-Headers' not in result.headers + assert 'Access-Control-Allow-Origin' not in result.headers + @pytest.fixture(scope='function') def make_cors_client(asgi, util): diff --git a/tests/test_static.py b/tests/test_static.py index cfeffc3b3..7e063072a 100644 --- a/tests/test_static.py +++ b/tests/test_static.py @@ -1,3 +1,4 @@ +from email import header import errno import io import os @@ -617,3 +618,19 @@ def test_file_closed(client, patch_open): assert patch_open.current_file is not None assert patch_open.current_file.closed + + +def test_options_request(util, asgi, patch_open): + patch_open() + app = util.create_app(asgi, cors_enable=True) + app.add_static_route('/static', '/var/www/statics') + client = testing.TestClient(app) + + resp = client.simulate_options( + path='/static/foo/bar.txt', + headers={'Origin': 'localhost', 'Access-Control-Request-Method': 'GET'}, + ) + assert resp.status_code == 200 + assert resp.text == '' + assert int(resp.headers['Content-Length']) == 0 + assert resp.headers['Access-Control-Allow-Methods'] == 'GET'