Skip to content

Commit

Permalink
feat: add optional Access-Control-Max-Age header to CORS plugin (#2331)
Browse files Browse the repository at this point in the history
Hi there

As discussed in this
[issue](#2212), the CORS
plugin in Apollo Router currently lacks setting the
Access-Control-Max-Age header. This PR adds a new option called max_age
that is used like so:
```
cors:
  max_age: 86400
```

This does not solve the Cache-Control header mentioned in issue as it's
a bit separate from CORS.

New test(s) and documentation changes haven't been done yet as I wanted
to confirm this is fine before proceeding further. If so, happy to do
them. Thanks!

(FYI I don't have the ability to assign a reviewer or anything, if we're
able to do that)

Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>
Co-authored-by: Simon Sapin <simon@apollographql.com>
Co-authored-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>
  • Loading branch information
3 people authored Jan 24, 2023
1 parent 8c5f903 commit 84f38fa
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 1 deletion.
12 changes: 12 additions & 0 deletions NEXT_CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,18 @@ Description! And a link to a [reference](http://url)
By [@USERNAME](https://github.com/USERNAME) in https://github.com/apollographql/router/pull/PULL_NUMBER
</KEEP> -->

## 🚀 Features

### Add optional `Access-Control-Max-Age` header to CORS plugin ([Issue #2212](https://github.com/apollographql/router/issues/2212))

Adds new option called `max_age` that is used like this:
```
cors:
max_age: 86400secs
```

By [@osamra-rbi](https://github.com/osamra-rbi) in https://github.com/apollographql/router/pull/2331

## 🐛 Fixes

### Fix panic in schema parse error reporting ([Issue #2269](https://github.com/apollographql/router/issues/2269))
Expand Down
28 changes: 28 additions & 0 deletions apollo-router/src/axum_factory/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use std::str::FromStr;
use std::sync::atomic::AtomicU32;
use std::sync::atomic::Ordering;
use std::sync::Arc;
use std::time::Duration;

use async_compression::tokio::write::GzipDecoder;
use async_compression::tokio::write::GzipEncoder;
Expand All @@ -28,6 +29,7 @@ use reqwest::header::ACCEPT;
use reqwest::header::ACCESS_CONTROL_ALLOW_HEADERS;
use reqwest::header::ACCESS_CONTROL_ALLOW_METHODS;
use reqwest::header::ACCESS_CONTROL_ALLOW_ORIGIN;
use reqwest::header::ACCESS_CONTROL_MAX_AGE;
use reqwest::header::ACCESS_CONTROL_REQUEST_HEADERS;
use reqwest::header::ACCESS_CONTROL_REQUEST_METHOD;
use reqwest::header::ORIGIN;
Expand Down Expand Up @@ -1307,6 +1309,26 @@ async fn cors_origin_default() -> Result<(), ApolloRouterError> {
Ok(())
}

#[tokio::test]
async fn cors_max_age() -> Result<(), ApolloRouterError> {
let conf = Configuration::fake_builder()
.cors(Cors::builder().max_age(Duration::from_secs(100)).build())
.build()
.unwrap();
let (server, client) = init_with_config(
router_service::empty().await,
Arc::new(conf),
MultiMap::new(),
)
.await?;
let url = format!("{}/", server.graphql_listen_address().as_ref().unwrap());

let response = request_cors_with_origin(&client, url.as_str(), "https://thisisatest.com").await;
assert_cors_max_age(response, "100");

Ok(())
}

#[tokio::test]
async fn cors_allow_any_origin() -> Result<(), ApolloRouterError> {
let conf = Configuration::fake_builder()
Expand Down Expand Up @@ -1426,6 +1448,12 @@ fn assert_not_cors_origin(response: reqwest::Response, origin: &str) {
assert!(!origin_valid(headers, origin));
}

fn assert_cors_max_age(response: reqwest::Response, max_age: &str) {
assert!(response.status().is_success());
assert_headers_valid(&response);
assert_header_contains!(response, ACCESS_CONTROL_MAX_AGE, &[max_age]);
}

fn assert_headers_valid(response: &reqwest::Response) {
assert_header_contains!(response, ACCESS_CONTROL_ALLOW_METHODS, &["POST"]);
assert_header_contains!(response, ACCESS_CONTROL_ALLOW_HEADERS, &["content-type"]);
Expand Down
14 changes: 14 additions & 0 deletions apollo-router/src/configuration/cors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// This entire file is license key functionality

use std::str::FromStr;
use std::time::Duration;

use http::request::Parts;
use http::HeaderValue;
Expand Down Expand Up @@ -59,6 +60,11 @@ pub(crate) struct Cors {
/// Allowed request methods. Defaults to GET, POST, OPTIONS.
#[serde(default = "default_cors_methods")]
pub(crate) methods: Vec<String>,

/// The `Access-Control-Max-Age` header value in time units
#[serde(deserialize_with = "humantime_serde::deserialize", default)]
#[schemars(with = "String", default)]
pub(crate) max_age: Option<Duration>,
}

impl Default for Cors {
Expand All @@ -71,6 +77,7 @@ impl Default for Cors {
allow_headers: Default::default(),
expose_headers: Default::default(),
match_origins: Default::default(),
max_age: Default::default(),
}
}
}
Expand All @@ -96,10 +103,12 @@ impl Cors {
origins: Option<Vec<String>>,
match_origins: Option<Vec<String>>,
methods: Option<Vec<String>>,
max_age: Option<Duration>,
) -> Self {
Self {
expose_headers,
match_origins,
max_age,
origins: origins.unwrap_or_else(default_origins),
methods: methods.unwrap_or_else(default_cors_methods),
allow_any_origin: allow_any_origin.unwrap_or_default(),
Expand Down Expand Up @@ -148,6 +157,11 @@ impl Cors {
.ok()
},
)));
let cors = if let Some(max_age) = self.max_age {
cors.max_age(max_age)
} else {
cors
};

if self.allow_any_origin {
Ok(cors.allow_origin(cors::Any))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,8 @@ expression: "&schema"
"GET",
"POST",
"OPTIONS"
]
],
"max_age": null
},
"type": "object",
"properties": {
Expand Down Expand Up @@ -109,6 +110,11 @@ expression: "&schema"
},
"nullable": true
},
"max_age": {
"description": "The `Access-Control-Max-Age` header value in time units",
"default": null,
"type": "string"
},
"methods": {
"description": "Allowed request methods. Defaults to GET, POST, OPTIONS.",
"default": [
Expand Down
5 changes: 5 additions & 0 deletions docs/source/configuration/cors.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,11 @@ cors:
# Which response headers are available to scripts running in the
# browser in response to a cross-origin request.
expose_headers: []

# Adds the Access-Control-Max-Age header
# Can be set to a duration in time units
# If not set, the header is not included
max_age: 2h
```
## Response `Vary` header
Expand Down

0 comments on commit 84f38fa

Please sign in to comment.