Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 50 additions & 1 deletion CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,56 @@
[Unreleased](https://github.com/bird-house/birdhouse-deploy/tree/master) (latest)
------------------------------------------------------------------------------------------------------------------

[//]: # (list changes here, using '-' for each new entry, remove this when items are added)
## Changes

- Allow each service to specify values for `Access-Control-Allow-Origin`

Previously, if a `location` block in the `nginx` configuration for a given service included the cors helper
configuration (with `include /etc/nginx/conf.d/cors.include;`) then all origins were allowed by default.

This was done by setting the header `Access-Control-Allow-Origin: *` which works well but is a bit too permissive
since it allowed __all__ origins.

This change introduces a mechanism to specify specific additional allowed origins by setting the
`$access_control_allow_origin` nginx variable in the `location` block before including the `cors.include` file.

For example:

```
set $access_control_allow_origin http://example.com;
include /etc/nginx/conf.d/cors.include;
```

will set the value of the `Access-Control-Allow-Origin` response header to `http://example.com`.

By default, the header value will be `*` if `$access_control_allow_origin` is not set (to maintain backwards
compatibility).

To specify multiple allowed origins, use a `map` directive (see the implementation for `components/stac` for an
example).

- Set allowed CORS origins for `stac` through an environment variable

This change implements this flexibility for the `components/stac` component. By setting the `STAC_CORS_ORIGINS`
variable a user can specify allowed origins for responses from the `components/stac` component.

For example, setting the following:

```
export STAC_CORS_ORIGINS='https://example.com ~^https?://(www\.)?other\.example\.com$'
```

then requests from https://example.com and http://other.example.com will get a response with the
`Access-Control-Allow-Origin header` set to their origin, but http://example.ca will not.

Note that this breaks backwards compatibility slightly since previously all origins were allowed for `/stac` by
default. To keep the backwards compatible behaviour you can set:

```
export STAC_CORS_ORIGINS='~.*'
```
Comment on lines 65 to 70
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't export STAC_CORS_ORIGINS='~.*' be set in components/stac/default.env to make it work like the other components?

I think the https://geojson.io case could simply be documented in comment around the default value to indicate that, if * is not desired to be more strict, that specific Origin might need to be considered for stac-browser. It is not required given that stac-browser is not combined with stac anymore.

The backward incompatibility note might be slightly misleading due to the subtlety of CORS. Before, Access-Control-Allow-Origin was omitted entirely, rather than Access-Control-Allow-Origin: *, so CORS was not applied at all. If * was the default, it would behave the same for clients that don't consider it, but it impacts where CORS are actually considered, which is relevant for some Origins like https://geojson.io.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure I can update that to keep the default. I've also updated the STAC and STAC Browser documentation since it was out of date.

The backward incompatibility note might be slightly misleading due to the subtlety of CORS

I don't think so. Previously, the cors.include file was enabled so the Access-Control-Allow-Origin was already set to * (it was not omitted)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even if it was included and set with *, wasn't it omitted because of ˋproxy_hide_header`?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe that was the intention originally but it wasn't actually hidden. proxy_hide_header only hides header values set by the upstream server, not by nginx (#599 (comment)).

Would you rather that we remain backwards compatible with the intended behaviour (no Access-Control-Allow-Origin) or with the actual behaviour (Access-Control-Allow-Origin=*)?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is fine with Access-Control-Allow-Origin=*.


to match all origins.

[2.18.7](https://github.com/bird-house/birdhouse-deploy/tree/2.18.7) (2025-10-17)
------------------------------------------------------------------------------------------------------------------
Expand Down
10 changes: 7 additions & 3 deletions birdhouse/components/proxy/conf.d/cors.include
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
proxy_hide_header 'Access-Control-Allow-Origin';

if ( $access_control_allow_origin ~ "^$" ) {
set $access_control_allow_origin '*';
}

if ($request_method = 'OPTIONS') {
add_header 'Access-Control-Allow-Origin' '*';
add_header 'Access-Control-Allow-Origin' $access_control_allow_origin;
add_header 'Access-Control-Allow-Methods' 'GET, POST, OPTIONS';
#
# Custom headers and headers various browsers *should* be OK with but aren't
Expand All @@ -16,13 +20,13 @@
return 204;
}
if ($request_method = 'POST') {
add_header 'Access-Control-Allow-Origin' '*';
add_header 'Access-Control-Allow-Origin' $access_control_allow_origin;
add_header 'Access-Control-Allow-Methods' 'GET, POST, OPTIONS';
add_header 'Access-Control-Allow-Headers' 'DNT,X-CustomHeader,Keep-Alive,User-Agent,X-Requested-With,If-Modified-Since,Cache-Control,Content-Type,Content-Range,Range';
add_header 'Access-Control-Expose-Headers' 'DNT,X-CustomHeader,Keep-Alive,User-Agent,X-Requested-With,If-Modified-Since,Cache-Control,Content-Type,Content-Range,Range';
}
if ($request_method = 'GET') {
add_header 'Access-Control-Allow-Origin' '*';
add_header 'Access-Control-Allow-Origin' $access_control_allow_origin;
add_header 'Access-Control-Allow-Methods' 'GET, POST, OPTIONS';
add_header 'Access-Control-Allow-Headers' 'DNT,X-CustomHeader,Keep-Alive,User-Agent,X-Requested-With,If-Modified-Since,Cache-Control,Content-Type,Content-Range,Range';
add_header 'Access-Control-Expose-Headers' 'DNT,X-CustomHeader,Keep-Alive,User-Agent,X-Requested-With,If-Modified-Since,Cache-Control,Content-Type,Content-Range,Range';
Expand Down
2 changes: 2 additions & 0 deletions birdhouse/components/proxy/conf.d/frontend.conf.template
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ map $http_x_forwarded_proto $real_scheme {
'' $scheme;
}

include /etc/nginx/conf.extra-directives.d/*/*.conf;

server {
listen 80 ${PROXY_LISTEN_80_PARAMS};
server_name localhost;
Expand Down
4 changes: 4 additions & 0 deletions birdhouse/components/stac-browser/default.env
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ export STAC_BROWSER_IMAGE_URI='${STAC_BROWSER_IMAGE}'
# - https://github.com/radiantearth/stac-browser/blob/main/docs/docker.md
export STAC_BROWSER_PATH_PREFIX='/stac-browser/'

# Allows requests from https://geojson.io to access the /stac endpoint according to the CORS headers.
# See components/stac/default.env for more information.
export STAC_CORS_ORIGINS='https://geojson.io'

export DELAYED_EVAL="
$DELAYED_EVAL
STAC_BROWSER_IMAGE
Expand Down
1 change: 1 addition & 0 deletions birdhouse/components/stac/.gitignore
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
config/magpie/config.yml
config/proxy/conf.extra-service.d/stac.conf
config/proxy/conf.extra-directives.d/stac.conf
config/canarie-api/canarie_api_monitoring.py
service-config.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
map $http_origin $stac_origin_allowed {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remind me what the Nginx map directive do again?

This block means if the http_origin variable exists, create stac_origin_allowed variable with value ${BIRDHOUSE_PROXY_SCHEME}://${BIRDHOUSE_FQDN_PUBLIC}; ${STAC_ADDITIONAL_CORS_ORIGINS} ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's mostly right...

This block creates a new variable stac_origin_allowed whose value is based on the value of the http_origin variable.
If the value of http_origin matches any of the patterns defined in ${STAC_ADDITIONAL_CORS_ORIGINS} then the stac_origin_allowed value will be set to http_origin.
Otherwise stac_origin_allowed will be set to ${BIRDHOUSE_PROXY_SCHEME}://${BIRDHOUSE_FQDN_PUBLIC} (the default).

# default should not be set to the empty string because the cors.include file will interpret
# that as "unset" and will change it to * by default. To get around this, set this to birdhouse's
# own origin (which has the same effect as being unset since same-origin requests are already allowed).
default ${BIRDHOUSE_PROXY_SCHEME}://${BIRDHOUSE_FQDN_PUBLIC};
${STAC_ADDITIONAL_CORS_ORIGINS}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@
proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
proxy_set_header X-Forwarded-Host $host:$server_port;
proxy_buffering off;
set $access_control_allow_origin $stac_origin_allowed;
include /etc/nginx/conf.d/cors.include;
add_header Vary Origin;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does the Vary header do again? Why we need to add it here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh interesting so as soon as we do not return Access-Control-Allow-Origin with value * but with a specific list of domain, then this Vary header is needed ! So should this Vary be moved to proxy/conf.d/cors.include? I fear we will forget to add this manually for other components.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The thing though is that Vary should be added only if the Origin (or any other header) actually affects the response (assuming no other header already distinguishes the response like the Content-Type). Adding it systematically (eg: when Access-Control-Allow-Origin: *) could result in duplicate cache entries for each domain accessing the server although nothing changes between them. It is used purposely to generate a "diff" in the cached response according to its domain-specific value, so that each domain is managed separately. I think we don't have a choice to apply it on case-by-case basis.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding it systematically (eg: when Access-Control-Allow-Origin: *)

I was suggesting to add Vary only when Access-Control-Allow-Origin is not *. Hopefully the if-statement in Nginx allows this. Otherwise we should put a big reminder somewhere so other users notice it.

Copy link
Collaborator Author

@mishaschwartz mishaschwartz Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fmigneault @tlvu What about 942cc07 ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about 942cc07 ?

Looks good too. So I assume when $vary_origin is "" (empty), we return add_header Vary $vary_origin; ==> add_header Vary ""; the empty value won't cause any side-effect and is a legit value allowed by HTTP?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add_header Vary ""; the empty value won't cause any side-effect and is a legit value allowed by HTTP?

I just re-read Francis response earlier (#599 (comment)) ? "nginx omits header if empty". Could you add this comment to the change so it is clear for other readers too.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For sure! Added in 2aa0cae

}

# Automatically redirect to /stac/stac and exclude redirect when already using /stac
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ services:
volumes:
- ./components/stac/service-config.json:/static/services/stac.json:ro
- ./components/stac/config/proxy/conf.extra-service.d:/etc/nginx/conf.extra-service.d/stac:ro
- ./components/stac/config/proxy/conf.extra-directives.d:/etc/nginx/conf.extra-directives.d/stac:ro
12 changes: 12 additions & 0 deletions birdhouse/components/stac/default.env
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,15 @@ export STAC_POPULATOR_BACKUP_IMAGE='${STAC_POPULATOR_BACKUP_DOCKER}:${STAC_POPUL
# This must match the "stac_version" value in the current STAC catalog.
export PYSTAC_STAC_VERSION_OVERRIDE=1.0.0

# Add additional origins that are allowed to access the /stac endpoint (other than the same origin) according to CORS rules.
# The values in the space delimited list set by STAC_CORS_ORIGINS are origins that will be allowed to access responses
# from /stac. These values can either be strings which will be matched directly or regular expressions prefixed by ~.
#
# For example, if STAC_CORS_ORIGINS='https://example.com ~^https?://(www\.)?other\.example\.com$' then requests from
# https://example.com and http://other.example.com will get a response with the Access-Control-Allow-Origin header set
# to their origin, but http://example.ca will not.
export STAC_ADDITIONAL_CORS_ORIGINS='$(for origin in $STAC_CORS_ORIGINS; do echo "$origin \$http_origin;"; done;)'

# add any new variables not already in 'VARS' or 'OPTIONAL_VARS' that must be replaced in templates here
# single quotes are important in below list to keep variable names intact until 'birdhouse-compose' parses them
EXTRA_VARS='
Expand All @@ -71,6 +80,7 @@ export DELAYED_EVAL="
STAC_MIGRATION_DOCKER
STAC_MIGRATION_IMAGE
STAC_POPULATOR_BACKUP_IMAGE
STAC_ADDITIONAL_CORS_ORIGINS
"

OPTIONAL_VARS="
Expand All @@ -86,4 +96,6 @@ OPTIONAL_VARS="
\$STAC_MIGRATION_TAGGED
\$STAC_MIGRATION_DOCKER
\$STAC_MIGRATION_IMAGE
\$STAC_ADDITIONAL_CORS_ORIGINS
\$STAC_CORS_ORIGIN_ALLOW_ALL
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

STAC_CORS_ORIGIN_ALLOW_ALL not used anywhere in this PR, ancien relic of some renaming in this PR that you forgot to remove?

"
Loading