-
Notifications
You must be signed in to change notification settings - Fork 432
feat(event_handler): allow multiple CORS origins #2279
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
1f59251
be743c3
61a6581
3ca9aaf
0acb944
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -84,6 +84,7 @@ def with_cors(): | |
|
||
cors_config = CORSConfig( | ||
allow_origin="https://wwww.example.com/", | ||
extra_origins=["https://www1.example.com/"], | ||
expose_headers=["x-exposed-response-header"], | ||
allow_headers=["x-custom-request-header"], | ||
max_age=100, | ||
|
@@ -106,6 +107,7 @@ def without_cors(): | |
def __init__( | ||
self, | ||
allow_origin: str = "*", | ||
extra_origins: Optional[List[str]] = None, | ||
allow_headers: Optional[List[str]] = None, | ||
expose_headers: Optional[List[str]] = None, | ||
max_age: Optional[int] = None, | ||
|
@@ -117,6 +119,8 @@ def __init__( | |
allow_origin: str | ||
The value of the `Access-Control-Allow-Origin` to send in the response. Defaults to "*", but should | ||
only be used during development. | ||
extra_origins: Optional[List[str]] | ||
The list of additional allowed origins. | ||
allow_headers: Optional[List[str]] | ||
The list of additional allowed headers. This list is added to list of | ||
built-in allowed headers: `Authorization`, `Content-Type`, `X-Amz-Date`, | ||
|
@@ -128,16 +132,29 @@ def __init__( | |
allow_credentials: bool | ||
A boolean value that sets the value of `Access-Control-Allow-Credentials` | ||
""" | ||
self.allow_origin = allow_origin | ||
rubenfonseca marked this conversation as resolved.
Show resolved
Hide resolved
|
||
self.allowed_origins = [allow_origin] | ||
if extra_origins: | ||
self.allowed_origins.extend(extra_origins) | ||
self.allow_headers = set(self._REQUIRED_HEADERS + (allow_headers or [])) | ||
self.expose_headers = expose_headers or [] | ||
self.max_age = max_age | ||
self.allow_credentials = allow_credentials | ||
|
||
def to_dict(self) -> Dict[str, str]: | ||
def to_dict(self, origin: Optional[str]) -> Dict[str, str]: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. origin: str = ""? Annotation says it's optional but we are not setting a default value. You can drop the Optional (None), and simply set to an empty str There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this mean I would have to compare There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TIL There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've changed my mind again and I believe it's better to keep the Optional. Reason is, the caller will try to fetch an Origin from the headers. The Origin is not always present, so |
||
"""Builds the configured Access-Control http headers""" | ||
|
||
# If there's no Origin, don't add any CORS headers | ||
if not origin: | ||
return {} | ||
|
||
# If the origin doesn't match any of the allowed origins, and we don't allow all origins ("*"), | ||
# don't add any CORS headers | ||
if origin not in self.allowed_origins and "*" not in self.allowed_origins: | ||
return {} | ||
rubenfonseca marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
# The origin matched an allowed origin, so return the CORS headers | ||
headers: Dict[str, str] = { | ||
"Access-Control-Allow-Origin": self.allow_origin, | ||
"Access-Control-Allow-Origin": origin, | ||
"Access-Control-Allow-Headers": ",".join(sorted(self.allow_headers)), | ||
} | ||
|
||
|
@@ -207,9 +224,9 @@ def __init__(self, response: Response, route: Optional[Route] = None): | |
self.response = response | ||
self.route = route | ||
|
||
def _add_cors(self, cors: CORSConfig): | ||
def _add_cors(self, event: BaseProxyEvent, cors: CORSConfig): | ||
"""Update headers to include the configured Access-Control headers""" | ||
self.response.headers.update(cors.to_dict()) | ||
self.response.headers.update(cors.to_dict(event.get_header_value("Origin"))) | ||
rubenfonseca marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
def _add_cache_control(self, cache_control: str): | ||
"""Set the specified cache control headers for 200 http responses. For non-200 `no-cache` is used.""" | ||
|
@@ -230,7 +247,7 @@ def _route(self, event: BaseProxyEvent, cors: Optional[CORSConfig]): | |
if self.route is None: | ||
return | ||
if self.route.cors: | ||
self._add_cors(cors or CORSConfig()) | ||
self._add_cors(event, cors or CORSConfig()) | ||
if self.route.cache_control: | ||
self._add_cache_control(self.route.cache_control) | ||
if self.route.compress and "gzip" in (event.get_header_value("accept-encoding", "") or ""): | ||
|
@@ -644,7 +661,7 @@ def _not_found(self, method: str) -> ResponseBuilder: | |
headers: Dict[str, Union[str, List[str]]] = {} | ||
if self._cors: | ||
logger.debug("CORS is enabled, updating headers.") | ||
headers.update(self._cors.to_dict()) | ||
headers.update(self._cors.to_dict(self.current_event.get_header_value("Origin"))) | ||
rubenfonseca marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
if method == "OPTIONS": | ||
logger.debug("Pre-flight request detected. Returning CORS with null response") | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -280,7 +280,8 @@ To address this API Gateway behavior, we use `strip_prefixes` parameter to accou | |
|
||
You can configure CORS at the `APIGatewayRestResolver` constructor via `cors` parameter using the `CORSConfig` class. | ||
|
||
This will ensure that CORS headers are always returned as part of the response when your functions match the path invoked. | ||
This will ensure that CORS headers are returned as part of the response when your functions match the path invoked and the `Origin` | ||
rubenfonseca marked this conversation as resolved.
Show resolved
Hide resolved
|
||
matches one of the allowed values. | ||
|
||
???+ tip | ||
Optionally disable CORS on a per path basis with `cors=False` parameter. | ||
|
@@ -310,6 +311,9 @@ For convenience, these are the default values when using `CORSConfig` to enable | |
???+ warning | ||
Always configure `allow_origin` when using in production. | ||
|
||
???+ tip "Multiple allowed origins?" | ||
If you require multiple allowed origins, pass the additional origins using the `extra_origins` key. | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ruben, please add the How about adding a new tab with an example to show how to use this new field? What do you think? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe I'm dizzy in the taxi but this reads odd, perhaps what you meant was: Multiple origins? If you need to allow multiple origins ... As in, Allow-Origins is an explicit CORS terminology, but here we can be more flexible in wording |
||
| Key | Value | Note | | ||
| -------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | ||
| **[allow_origin](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Origin){target="_blank"}**: `str` | `*` | Only use the default value for development. **Never use `*` for production** unless your use case requires it | | ||
|
@@ -331,7 +335,7 @@ You can use the `Response` class to have full control over the response. For exa | |
|
||
=== "fine_grained_responses.py" | ||
|
||
```python hl_lines="9 28-32" | ||
```python hl_lines="9 29-35" | ||
rubenfonseca marked this conversation as resolved.
Show resolved
Hide resolved
|
||
--8<-- "examples/event_handler_rest/src/fine_grained_responses.py" | ||
``` | ||
|
||
|
Uh oh!
There was an error while loading. Please reload this page.