Skip to content

Commit

Permalink
Merge pull request #66 from uclahs-cds/aholmes-middleware-adjustments
Browse files Browse the repository at this point in the history
Alter how CORS is handled in BL_Python.
  • Loading branch information
aholmes authored May 17, 2024
2 parents 85ddfb0 + 51d8590 commit 1a30368
Show file tree
Hide file tree
Showing 12 changed files with 71 additions and 49 deletions.
15 changes: 9 additions & 6 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,20 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm

---

## [Unreleased]
# `BL_Python.all` [0.2.4] - 2024-05-17

# `BL_Python.web` [0.2.2] - 2024-05-16
- [BL_Python.web v0.2.2](https://github.com/uclahs-cds/BL_Python/blob/BL_Python.web-v0.2.2/src/web/CHANGELOG.md#022---2024-05-16)

# `BL_Python.platform` [0.2.2] - 2024-05-16
- [BL_Python.platform v0.2.2](https://github.com/uclahs-cds/BL_Python/blob/BL_Python.platform-v0.2.2/src/platform/CHANGELOG.md#022---2024-05-16)
## `BL_Python.web` [0.2.3] - 2024-05-17
- [BL_Python.web v0.2.3](https://github.com/uclahs-cds/BL_Python/blob/BL_Python.web-v0.2.3/src/web/CHANGELOG.md#023---2024-05-17)

# `BL_Python.all` [0.2.3] - 2024-05-16
- Contains all libraries up to v0.2.1, and `BL_Python.platform` v0.2.2 and `BL_Python.web` v0.2.2

## `BL_Python.web` [0.2.2] - 2024-05-16
- [BL_Python.web v0.2.2](https://github.com/uclahs-cds/BL_Python/blob/BL_Python.web-v0.2.2/src/web/CHANGELOG.md#022---2024-05-16)

## `BL_Python.platform` [0.2.2] - 2024-05-16
- [BL_Python.platform v0.2.2](https://github.com/uclahs-cds/BL_Python/blob/BL_Python.platform-v0.2.2/src/platform/CHANGELOG.md#022---2024-05-16)

# [0.2.0] - 2024-05-14
### Added
- New package `BL_Python.identity` for SSO
Expand Down
2 changes: 1 addition & 1 deletion src/__init__.py
Original file line number Diff line number Diff line change
@@ -1 +1 @@
__version__: str = "0.2.3"
__version__: str = "0.2.4"
2 changes: 1 addition & 1 deletion src/web/BL_Python/web/__init__.py
Original file line number Diff line number Diff line change
@@ -1 +1 @@
__version__: str = "0.2.2"
__version__: str = "0.2.3"
5 changes: 3 additions & 2 deletions src/web/BL_Python/web/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ class LoggingConfig(BaseModel):


class WebSecurityCorsConfig(BaseModel):
origin: str | None = None
allow_credentials: bool = True
origins: list[str] | None = None
allow_credentials: bool = False
allow_methods: list[
Literal[
"GET",
Expand All @@ -27,6 +27,7 @@ class WebSecurityCorsConfig(BaseModel):
"TRACE",
]
] = field(default_factory=lambda: ["GET", "POST", "OPTIONS"])
allow_headers: list[str | Literal["*"]] = field(default_factory=lambda: ["*"])


class WebSecurityConfig(BaseModel):
Expand Down
4 changes: 3 additions & 1 deletion src/web/BL_Python/web/middleware/dependency_injection.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,9 @@ def configure_dependencies(
module, "register_middleware", None
)
if register_callback is not None and callable(register_callback):
register_callback(app)
flask_injector.injector.call_with_injection(
register_callback, kwargs={"app": app}
)

# this binds all BL_Python middlewares with Injector
_configure_openapi_middleware_dependencies(app, flask_injector)
Expand Down
30 changes: 21 additions & 9 deletions src/web/BL_Python/web/middleware/flask/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -161,17 +161,29 @@ def _ordered_api_response_handers(response: Response, config: Config, log: Logge
def _wrap_all_api_responses(response: Response, config: Config, log: Logger):
correlation_id = _get_correlation_id(log)

cors_domain: str | None = None
if config.web.security.cors.origin:
cors_domain = config.web.security.cors.origin
cors_domains: list[str] | None = None
if config.web.security.cors.origins:
cors_domains = config.web.security.cors.origins
else:
if not response.headers.get(CORS_ACCESS_CONTROL_ALLOW_ORIGIN_HEADER):
cors_domain = request.headers.get(ORIGIN_HEADER)
if not cors_domain:
cors_domain = request.headers.get(HOST_HEADER)

if cors_domain:
response.headers[CORS_ACCESS_CONTROL_ALLOW_ORIGIN_HEADER] = cors_domain
cors_domains = (
(header_value := request.headers.get(ORIGIN_HEADER))
and [header_value]
or None
)
if not cors_domains:
cors_domains = (
(header_value := request.headers.get(HOST_HEADER))
and [header_value]
or None
)

if cors_domains:
# although the config allows multiple, the header may only contain one.
# either only one will have been set, or we take the first from the config.
# this may not be valid in all cases, and so the ASGI CORSMiddleware sould
# be used instead
response.headers[CORS_ACCESS_CONTROL_ALLOW_ORIGIN_HEADER] = cors_domains[0]

response.headers[CORS_ACCESS_CONTROL_ALLOW_CREDENTIALS_HEADER] = str(
config.web.security.cors.allow_credentials
Expand Down
26 changes: 0 additions & 26 deletions src/web/BL_Python/web/middleware/openapi/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,7 @@
from ..consts import (
CONTENT_SECURITY_POLICY_HEADER,
CORRELATION_ID_HEADER,
CORS_ACCESS_CONTROL_ALLOW_CREDENTIALS_HEADER,
CORS_ACCESS_CONTROL_ALLOW_METHODS_HEADER,
CORS_ACCESS_CONTROL_ALLOW_ORIGIN_HEADER,
HOST_HEADER,
INCOMING_REQUEST_MESSAGE,
ORIGIN_HEADER,
OUTGOING_RESPONSE_MESSAGE,
REQUEST_COOKIE_HEADER,
RESPONSE_COOKIE_HEADER,
Expand Down Expand Up @@ -234,27 +229,6 @@ def _wrap_all_api_responses(
correlation_id = _get_correlation_id(request, response, log)
response_headers = _headers_as_dict(response)

cors_domain: str | None = None
if config.web.security.cors.origin:
cors_domain = config.web.security.cors.origin
else:
if not response_headers.get(CORS_ACCESS_CONTROL_ALLOW_ORIGIN_HEADER):
request_headers = _headers_as_dict(request)
cors_domain = request_headers.get(ORIGIN_HEADER)
if not cors_domain:
cors_domain = request_headers.get(HOST_HEADER)

if cors_domain:
response_headers[CORS_ACCESS_CONTROL_ALLOW_ORIGIN_HEADER] = cors_domain

response_headers[CORS_ACCESS_CONTROL_ALLOW_CREDENTIALS_HEADER] = str(
config.web.security.cors.allow_credentials
)

response_headers[CORS_ACCESS_CONTROL_ALLOW_METHODS_HEADER] = ",".join(
config.web.security.cors.allow_methods
)

response_headers[CORRELATION_ID_HEADER] = correlation_id

if config.web.security.csp:
Expand Down
20 changes: 20 additions & 0 deletions src/web/BL_Python/web/middleware/openapi/cors.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
from BL_Python.web.config import Config
from connexion import FlaskApp
from connexion.middleware import MiddlewarePosition
from injector import Module, inject
from starlette.middleware.cors import CORSMiddleware


class CORSMiddlewareModule(Module):
@inject
def register_middleware(self, app: FlaskApp, config: Config):
cors_config = config.web.security.cors

app.add_middleware(
CORSMiddleware,
position=MiddlewarePosition.BEFORE_EXCEPTION,
allow_origins=cors_config.origins,
allow_credentials=cors_config.allow_credentials,
allow_methods=cors_config.allow_methods,
allow_headers=cors_config.allow_headers,
)
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ log_level = 'INFO'
format = 'JSON'

[web.security.cors]
origin = 'http://localhost:5000'
origins = ['http://localhost:5000']
allow_credentials = true
allow_methods = ['GET', 'POST', 'PATCH', 'OPTIONS']

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ log_level = 'INFO'
format = 'plaintext'

[web.security.cors]
origin = 'http://localhost:5000'
origins = ['http://localhost:5000']
allow_credentials = true
allow_methods = ['GET', 'POST', 'PATCH', 'OPTIONS']

Expand Down
10 changes: 10 additions & 0 deletions src/web/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,16 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
Review the `BL_Python` [CHANGELOG.md](https://github.com/uclahs-cds/BL_Python/blob/main/CHANGELOG.md) for full monorepo notes.

---
## Unreleased

## [0.2.3] - 2024-05-17
### Changed
- Flask and OpenAPI apps require configuration of CORS origins as an array
- Flask apps still only supports a single origin
- OpenAPI apps use `CORSMiddleware` in favor of the custom CORS handlers

### Added
- Custom middleware modules can now use Injector to get interfaces that have been previously registered

## [0.2.2] - 2024-05-16
### Changed
Expand Down
2 changes: 1 addition & 1 deletion src/web/test/unit/middleware/test_api_response_handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ def test__wrap_all_api_responses__sets_CSP_header(
@pytest.mark.parametrize(
"header,value,config_attribute_name",
[
(CORS_ACCESS_CONTROL_ALLOW_ORIGIN_HEADER, "example.com", "origin"),
(CORS_ACCESS_CONTROL_ALLOW_ORIGIN_HEADER, ["example.com"], "origins"),
(
CORS_ACCESS_CONTROL_ALLOW_CREDENTIALS_HEADER,
"False",
Expand Down

0 comments on commit 1a30368

Please sign in to comment.