changes to allow cors allowed origin and max age to be configurable (#1177)

* allowed cors origin and max_age to be configurable

* e2e test edit

* moved comment

* feedback changes

* e2e test edit

* remove extra line at end of file

Co-authored-by: JR Conlin <jconlin+git@mozilla.com>
This commit is contained in:
Tif Tran 2021-12-06 14:46:11 -08:00 committed by GitHub
parent 94aece75b2
commit 022d235e87
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 137 additions and 24 deletions

5
Cargo.lock generated
View File

@ -1322,9 +1322,9 @@ dependencies = [
[[package]]
name = "http"
version = "0.2.4"
version = "0.2.5"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "527e8c9ac747e28542699a951517aa9a6945af506cd1f2e1b53a576c17b6cc11"
checksum = "1323096b05d41827dadeaee54c9981958c0f94e670bc94ed80037d1a7b8b186b"
dependencies = [
"bytes 1.0.1",
"fnv",
@ -2976,6 +2976,7 @@ dependencies = [
"hkdf",
"hmac",
"hostname",
"http",
"jsonwebtoken",
"lazy_static",
"log",

View File

@ -50,6 +50,7 @@ hex = "0.4.3"
hostname = "0.3.1"
hkdf = "0.11"
hmac = "0.11"
http = "0.2.5"
log = { version = "0.4", features = ["max_level_debug", "release_max_level_info"] }
mime = "0.3"
num_cpus = "1"

View File

@ -23,3 +23,7 @@ tokenserver.fxa_email_domain = "api-accounts.stage.mozaws.net"
tokenserver.fxa_metrics_hash_secret = "INSERT_SECRET_KEY_HERE"
tokenserver.fxa_oauth_server_url = "https://oauth.stage.mozaws.net"
tokenserver.test_mode_enabled = false
# cors settings
# cors_allowed_origin = "localhost"
# cors_max_age = 86400

View File

@ -2,7 +2,6 @@
use std::{sync::Arc, time::Duration};
use actix_cors::Cors;
use actix_web::{
dev, http::header::LOCATION, http::StatusCode, middleware::errhandlers::ErrorHandlers, web,
App, HttpRequest, HttpResponse, HttpServer,
@ -64,7 +63,7 @@ pub struct Server;
#[macro_export]
macro_rules! build_app {
($syncstorage_state: expr, $tokenserver_state: expr, $secrets: expr, $limits: expr) => {
($syncstorage_state: expr, $tokenserver_state: expr, $secrets: expr, $limits: expr, $cors: expr) => {
App::new()
.data($syncstorage_state)
.data($tokenserver_state)
@ -76,12 +75,7 @@ macro_rules! build_app {
.wrap(middleware::weave::WeaveTimestamp::new())
.wrap(middleware::sentry::SentryWrapper::default())
.wrap(middleware::rejectua::RejectUA::default())
// Followed by the "official middleware" so they run first.
// actix is getting increasingly tighter about CORS headers. Our server is
// not a huge risk but does deliver XHR JSON content.
// For now, let's be permissive and use NGINX (the wrapping server)
// for finer grained specification.
.wrap(Cors::permissive())
.wrap($cors)
.service(
web::resource(&cfg_path("/info/collections"))
.route(web::get().to(handlers::get_collections)),
@ -174,7 +168,7 @@ macro_rules! build_app {
#[macro_export]
macro_rules! build_app_without_syncstorage {
($state: expr, $secrets: expr) => {
($state: expr, $secrets: expr, $cors: expr) => {
App::new()
.data($state)
.data($secrets)
@ -189,7 +183,7 @@ macro_rules! build_app_without_syncstorage {
// not a huge risk but does deliver XHR JSON content.
// For now, let's be permissive and use NGINX (the wrapping server)
// for finer grained specification.
.wrap(Cors::permissive())
.wrap($cors)
.service(
web::resource("/1.0/{application}/{version}")
.route(web::get().to(tokenserver::handlers::get_tokenserver_result)),
@ -228,6 +222,7 @@ macro_rules! build_app_without_syncstorage {
impl Server {
pub async fn with_settings(settings: Settings) -> Result<dev::Server, ApiError> {
let settings_copy = settings.clone();
let metrics = metrics::metrics_from_opts(&settings)?;
let host = settings.host.clone();
let port = settings.port;
@ -267,7 +262,8 @@ impl Server {
syncstorage_state,
tokenserver_state.clone(),
Arc::clone(&secrets),
limits
limits,
settings_copy.build_cors()
)
});
@ -285,13 +281,17 @@ impl Server {
pub async fn tokenserver_only_with_settings(
settings: Settings,
) -> Result<dev::Server, ApiError> {
let settings_copy = settings.clone();
let host = settings.host.clone();
let port = settings.port;
let secrets = Arc::new(settings.master_secret);
let tokenserver_state = tokenserver::ServerState::from_settings(&settings.tokenserver)?;
let server = HttpServer::new(move || {
build_app_without_syncstorage!(Some(tokenserver_state.clone()), Arc::clone(&secrets))
build_app_without_syncstorage!(
Some(tokenserver_state.clone()),
Arc::clone(&secrets),
settings_copy.build_cors()
)
});
let server = server

View File

@ -94,7 +94,8 @@ macro_rules! init_app {
get_test_state(&$settings).await,
None::<tokenserver::ServerState>,
Arc::clone(&SECRETS),
limits
limits,
$settings.build_cors()
))
.await
}
@ -216,7 +217,8 @@ where
get_test_state(&settings).await,
None::<tokenserver::ServerState>,
Arc::clone(&SECRETS),
limits
limits,
settings.build_cors()
))
.await;
@ -256,7 +258,8 @@ async fn test_endpoint_with_body(
get_test_state(&settings).await,
None::<tokenserver::ServerState>,
Arc::clone(&SECRETS),
limits
limits,
settings.build_cors()
))
.await;
let req = create_request(method, path, None, Some(body)).to_request();

View File

@ -1,7 +1,9 @@
//! Application settings objects and initialization
use std::{cmp::min, env};
use actix_cors::Cors;
use config::{Config, ConfigError, Environment, File};
use http::method::Method;
use serde::{de::Deserializer, Deserialize, Serialize};
use url::Url;
@ -9,6 +11,10 @@ use crate::db::spanner::models::MAX_SPANNER_LOAD_SIZE;
use crate::error::ApiError;
use crate::tokenserver::settings::Settings as TokenserverSettings;
use crate::web::auth::hkdf_expand_32;
use crate::web::{
X_LAST_MODIFIED, X_VERIFY_CODE, X_WEAVE_BYTES, X_WEAVE_NEXT_OFFSET, X_WEAVE_RECORDS,
X_WEAVE_TIMESTAMP, X_WEAVE_TOTAL_BYTES, X_WEAVE_TOTAL_RECORDS,
};
static DEFAULT_PORT: u16 = 8000;
@ -86,6 +92,12 @@ pub struct Settings {
/// Settings specific to Tokenserver
pub tokenserver: TokenserverSettings,
/// Cors Settings
pub cors_allowed_origin: Option<String>,
pub cors_max_age: Option<usize>,
pub cors_allowed_methods: Option<Vec<String>>,
pub cors_allowed_headers: Option<Vec<String>>,
}
impl Default for Settings {
@ -114,6 +126,10 @@ impl Default for Settings {
spanner_emulator_host: None,
disable_syncstorage: false,
tokenserver: TokenserverSettings::default(),
cors_allowed_origin: None,
cors_allowed_methods: None,
cors_allowed_headers: None,
cors_max_age: None,
}
}
}
@ -177,6 +193,29 @@ impl Settings {
s.set_default("tokenserver.fxa_metrics_hash_secret", "secret")?;
s.set_default("tokenserver.test_mode_enabled", false)?;
// Set Cors defaults
s.set_default(
"cors_allowed_headers",
Some(vec![
"Authorization",
"Content-Type",
"UserAgent",
X_LAST_MODIFIED,
X_WEAVE_TIMESTAMP,
X_WEAVE_NEXT_OFFSET,
X_WEAVE_RECORDS,
X_WEAVE_BYTES,
X_WEAVE_TOTAL_RECORDS,
X_WEAVE_TOTAL_BYTES,
X_VERIFY_CODE,
"TEST_IDLES",
]),
)?;
s.set_default(
"cors_allowed_methods",
Some(vec!["DELETE", "GET", "POST", "PUT"]),
)?;
// Merge the config file if supplied
if let Some(config_filename) = filename {
s.merge(File::with_name(config_filename))?;
@ -272,6 +311,37 @@ impl Settings {
.unwrap_or_else(|_| "<invalid db>".to_owned());
format!("http://{}:{} ({}) {}", self.host, self.port, db, quota)
}
pub fn build_cors(&self) -> Cors {
// Followed by the "official middleware" so they run first.
// actix is getting increasingly tighter about CORS headers. Our server is
// not a huge risk but does deliver XHR JSON content.
// For now, let's be permissive and use NGINX (the wrapping server)
// for finer grained specification.
let mut cors = Cors::default();
if let Some(allowed_origin) = &self.cors_allowed_origin {
cors = cors.allowed_origin(allowed_origin);
}
if let Some(allowed_methods) = &self.cors_allowed_methods {
let mut methods = vec![];
for method_string in allowed_methods {
let method = Method::from_bytes(method_string.as_bytes()).unwrap();
methods.push(method);
}
cors = cors.allowed_methods(methods);
}
if let Some(allowed_headers) = &self.cors_allowed_headers {
cors = cors.allowed_headers(allowed_headers);
}
if let Some(max_age) = &self.cors_max_age {
cors = cors.max_age(*max_age);
}
cors
}
}
/// Server-enforced limits for request payloads.

View File

@ -11,6 +11,10 @@ pub static X_LAST_MODIFIED: &str = "x-last-modified";
pub static X_WEAVE_TIMESTAMP: &str = "x-weave-timestamp";
pub static X_WEAVE_NEXT_OFFSET: &str = "x-weave-next-offset";
pub static X_WEAVE_RECORDS: &str = "x-weave-records";
pub static X_WEAVE_BYTES: &str = "x-weave-bytes";
pub static X_WEAVE_TOTAL_RECORDS: &str = "x-weave-total-records";
pub static X_WEAVE_TOTAL_BYTES: &str = "x-weave-total-bytes";
pub static X_VERIFY_CODE: &str = "x-verify-code";
// Known DockerFlow commands for Ops callbacks
pub const DOCKER_FLOW_ENDPOINTS: [&str; 4] = [

View File

@ -37,9 +37,9 @@ if __name__ == "__main__":
)
def start_server():
the_server_subprocess = subprocess.Popen(target_binary,
shell=True,
env=os.environ)
the_server_subprocess = subprocess.Popen(
target_binary, shell=True, env=os.environ
)
# TODO we should change this to watch for a log message on startup
# to know when to continue instead of sleeping for a fixed amount
@ -47,7 +47,10 @@ if __name__ == "__main__":
return the_server_subprocess
os.environ.setdefault('SYNC_MASTER_SECRET', 'secret0')
os.environ.setdefault("SYNC_MASTER_SECRET", "secret0")
os.environ.setdefault("SYNC_CORS_MAX_AGE", "555")
os.environ.setdefault("SYNC_CORS_ALLOWED_ORIGIN", "localhost")
the_server_subprocess = start_server()
atexit.register(lambda: terminate_process(the_server_subprocess))
res = run_live_functional_tests(TestStorage, sys.argv)

View File

@ -16,6 +16,7 @@ consider it a bug.
import unittest2
import re
import json
import time
@ -29,6 +30,7 @@ import contextlib
import simplejson
from pyramid.interfaces import IAuthenticationPolicy
from webtest.app import AppError
from test_support import StorageFunctionalTestCase
import tokenlib
@ -1746,8 +1748,8 @@ class TestStorage(StorageFunctionalTestCase):
self.assertTrue("max_request_bytes" in limits)
endpoint = self.root + "/storage/xxx_col2?batch=true"
# There are certain obvious constraints on these limits,
# violations of which would be very confusing for clients.
# There are certain obvious constraints on these limits,
# violations of which would be very confusing for clients.
#
# self.assertTrue(
# limits['max_request_bytes'] > limits['max_post_bytes']
@ -2245,3 +2247,28 @@ class TestStorage(StorageFunctionalTestCase):
testEmptyCommit("application/newlines", "\n", status=400)
testEmptyCommit("application/newlines", "{}", status=400)
testEmptyCommit("application/newlines", "[]", status=400)
def test_cors_settings_are_set(self):
res = self.app.options(
self.root + "/__heartbeat__",
headers={
"Access-Control-Request-Method": "GET",
"Origin": "localhost",
"Access-Control-Request-Headers": "Content-Type",
},
)
self.assertEqual(
int(res.headers["access-control-max-age"]), 555
)
self.assertEqual(
res.headers["access-control-allow-origin"], "localhost"
)
# PATCH is not a default allowed method, so request should return 405
def test_patch_is_not_allowed(self):
collection = self.root + "/storage/xxx_col1"
with self.assertRaises(AppError) as error:
self.app.patch_json(collection)
self.assertIn("405", error)