Repository: Neoteroi/BlackSheep (python)

Issue:
CORS middleware sets invalid `Access-Control-Allow-Origin` header value when multiple `allow_origins` options are set
Set up an app with settings similar to:

```python
app.use_cors(
    allow_methods=("GET", "POST", "PUT", "DELETE"),
    allow_origins=("http://localhost:8000", "http://localhost:9000"),
)
```

Then, try a request from one of the allowed origins. You may see the request fail CORS authorization due to the presence of multiple values for the `Access-Control-Allow-Origin` header.

[Documentation](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Origin#directives) for this header states:

> Specifies an origin. Only a single origin can be specified. If the server supports clients from multiple origins, it must return the origin for the specific client making the request.

Meaning, the server must not set each value provided in the `allow_origins` argument. See [`blacksheep/server/cors.py`](https://github.com/Neoteroi/BlackSheep/blob/main/blacksheep/server/cors.py#L270) where the values are joined together with `", "`.

The correct behavior would be to inspect the value of the `Origin` header sent in the request, and compare it to the values in the `allow_origins` setting. If there is a match, set the `Access-Control-Allow-Origin` response header to the matched value. If there is no match, omit the header.

Relatedly, the port of an origin is significant, but may be omitted, in which case the default port for the scheme is used. For example `http://example.foo` is implied to be `http://example.foo:80`. The current code for matching the origin doesn't seem to take this into account. If you specify an allowed origin of `http://example.foo:80` the CORS middleware will not match requests from `http://example.foo`

Pull request:
Fix #364

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index ecd81db..25c6826 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [2.0a7] - 2023-05-27
+- Fixes bug in CORS handling when [multiple origins are
+  allowed](https://github.com/Neoteroi/BlackSheep/issues/364).
+- Adds a `Vary: Origin` response header for CORS requests when the value of
+  `Access-Control-Allow-Origin` header is a specific URL.
+
 ## [2.0a6] - 2023-04-28 :crown:
 
 - Adds support for automatic import of modules defined under `controllers` and
diff --git a/blacksheep/server/cors.py b/blacksheep/server/cors.py
index f1c4fc8..fafe344 100644
--- a/blacksheep/server/cors.py
+++ b/blacksheep/server/cors.py
@@ -239,6 +239,17 @@ def _get_encoded_value_for_max_age(max_age: int) -> bytes:
     return str(max_age).encode()
 
 
+def _set_cors_origin(response: Response, origin_response: bytes):
+    """
+    Sets a Access-Control-Allow-Origin to the given value, and a `Vary: Origin` header
+    if that value is not "*".
+    """
+    response.set_header(b"Access-Control-Allow-Origin", origin_response)
+
+    if origin_response != b"*":
+        response.add_header(b"Vary", b"Origin")
+
+
 def get_cors_middleware(
     app: BaseApplication,
     strategy: CORSStrategy,
@@ -267,7 +278,6 @@ def get_cors_middleware(
             return not_found()
 
         policy = strategy.get_policy_by_route_or_default(route)
-        allowed_origins = _get_encoded_value_for_set(policy.allow_origins)
         allowed_methods = _get_encoded_value_for_set(policy.allow_methods)
         expose_headers = _get_encoded_value_for_set(policy.expose_headers)
         max_age = _get_encoded_value_for_max_age(policy.max_age)
@@ -278,6 +288,9 @@ def get_cors_middleware(
         ):
             return _get_invalid_origin_response()
 
+        # https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Origin
+        origin_response = b"*" if "*" in policy.allow_origins else origin
+
         if next_request_method:
             # This is a preflight request;
             if (
@@ -297,8 +310,8 @@ def get_cors_middleware(
                         return _get_invalid_header_response(str_value)
 
             response = ok()
+            _set_cors_origin(response, origin_response)
             response.set_header(b"Access-Control-Allow-Methods", allowed_methods)
-            response.set_header(b"Access-Control-Allow-Origin", allowed_origins)
 
             if next_request_headers:
                 response.set_header(
@@ -329,7 +342,7 @@ def get_cors_middleware(
         except Exception as exc:
             response = await app.handle_request_handler_exception(request, exc)
 
-        response.set_header(b"Access-Control-Allow-Origin", allowed_origins)
+        _set_cors_origin(response, origin_response)
         response.set_header(b"Access-Control-Expose-Headers", expose_headers)
 
         return response
diff --git a/pyproject.toml b/pyproject.toml
index 3d038fe..196f3a0 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta"
 
 [project]
 name = "blacksheep"
-version = "2.0a6"
+version = "2.0a7"
 authors = [
     { name = "Roberto Prevato", email = "roberto.prevato@gmail.com" },
 ]


Test patch:
diff --git a/tests/test_cors.py b/tests/test_cors.py
index 2b37e08..c607b93 100644
--- a/tests/test_cors.py
+++ b/tests/test_cors.py
@@ -766,3 +766,57 @@ async def test_cors_preflight_request_handles_404_for_missing_routes(app):
 
     response = app.response
     assert response.status == 404
+
+
+@pytest.mark.asyncio
+async def test_response_to_cors_request_contains_single_allow_origin(app):
+    app.use_cors(
+        allow_methods="GET POST",
+        allow_origins="https://www.neoteroi.dev https://www.neoteroi.xyz",
+    )
+
+    @app.router.post("/")
+    async def home():
+        return text("Hello, World")
+
+    await app.start()
+
+    await app(
+        get_example_scope(
+            "POST",
+            "/",
+            [
+                (b"Origin", b"https://www.neoteroi.dev"),
+            ],
+        ),
+        MockReceive(),
+        MockSend(),
+    )
+
+    response = app.response
+    assert response.status == 200
+    assert (
+        response.headers.get_single(b"Access-Control-Allow-Origin")
+        == b"https://www.neoteroi.dev"
+    )
+    assert response.headers.get_single(b"Access-Control-Expose-Headers") is not None
+
+    await app(
+        get_example_scope(
+            "POST",
+            "/",
+            [
+                (b"Origin", b"https://www.neoteroi.xyz"),
+            ],
+        ),
+        MockReceive(),
+        MockSend(),
+    )
+
+    response = app.response
+    assert response.status == 200
+    assert (
+        response.headers.get_single(b"Access-Control-Allow-Origin")
+        == b"https://www.neoteroi.xyz"
+    )
+    assert response.headers.get_single(b"Access-Control-Expose-Headers") is not None