diff options
| author | Alex Auvolat <alex@adnab.me> | 2022-03-01 11:15:16 +0100 |
|---|---|---|
| committer | Alex Auvolat <alex@adnab.me> | 2022-03-01 11:15:16 +0100 |
| commit | 8a5bbc3b0b1c6ab252d0c98950456a9d4cc2e9fe (patch) | |
| tree | fc58f965981debc989108c39379dfca3c4da5e93 | |
| parent | 97f245f218836c699088ab51d810f33ae947c903 (diff) | |
| download | garage-8a5bbc3b0b1c6ab252d0c98950456a9d4cc2e9fe.tar.gz garage-8a5bbc3b0b1c6ab252d0c98950456a9d4cc2e9fe.zip | |
More permissive OPTIONS on S3 APIv0.6.1better-cors
| -rw-r--r-- | src/api/api_server.rs | 2 | ||||
| -rw-r--r-- | src/api/s3_cors.rs | 64 | ||||
| -rw-r--r-- | src/web/web_server.rs | 4 |
3 files changed, 50 insertions, 20 deletions
diff --git a/src/api/api_server.rs b/src/api/api_server.rs index 5ac78bf4..322ea298 100644 --- a/src/api/api_server.rs +++ b/src/api/api_server.rs @@ -116,7 +116,7 @@ async fn handler_inner(garage: Arc<Garage>, req: Request<Body>) -> Result<Respon return handle_post_object(garage, req, bucket_name.unwrap()).await; } if let Endpoint::Options = endpoint { - return handle_options(garage, &req, bucket_name).await; + return handle_options_s3api(garage, &req, bucket_name).await; } let (api_key, content_sha256) = check_payload_signature(&garage, &req).await?; diff --git a/src/api/s3_cors.rs b/src/api/s3_cors.rs index 7dc48d8e..e3deaeda 100644 --- a/src/api/s3_cors.rs +++ b/src/api/s3_cors.rs @@ -100,33 +100,63 @@ pub async fn handle_put_cors( .body(Body::empty())?) } -pub async fn handle_options( +pub async fn handle_options_s3api( garage: Arc<Garage>, req: &Request<Body>, bucket_name: Option<String>, ) -> Result<Response<Body>, Error> { - let bucket = if let Some(bn) = bucket_name { + // FIXME: CORS rules of buckets with local aliases are + // not taken into account. + + // If the bucket name is a global bucket name, + // we try to apply the CORS rules of that bucket. + // If a user has a local bucket name that has + // the same name, its CORS rules won't be applied + // and will be shadowed by the rules of the globally + // existing bucket (but this is inevitable because + // OPTIONS calls are not auhtenticated). + if let Some(bn) = bucket_name { let helper = garage.bucket_helper(); - let bucket_id = helper - .resolve_global_bucket_name(&bn) - .await? - .ok_or(Error::NoSuchBucket)?; - garage - .bucket_table - .get(&EmptyKey, &bucket_id) - .await? - .filter(|b| !b.state.is_deleted()) - .ok_or(Error::NoSuchBucket)? + let bucket_id = helper.resolve_global_bucket_name(&bn).await?; + if let Some(id) = bucket_id { + let bucket = garage + .bucket_table + .get(&EmptyKey, &id) + .await? + .filter(|b| !b.state.is_deleted()) + .ok_or(Error::NoSuchBucket)?; + handle_options_for_bucket(req, &bucket) + } else { + // If there is a bucket name in the request, but that name + // does not correspond to a global alias for a bucket, + // then it's either a non-existing bucket or a local bucket. + // We have no way of knowing, because the request is not + // authenticated and thus we can't resolve local aliases. + // We take the permissive approach of allowing everything, + // because we don't want to prevent web apps that use + // local bucket names from making API calls. + Ok(Response::builder() + .header(ACCESS_CONTROL_ALLOW_ORIGIN, "*") + .header(ACCESS_CONTROL_ALLOW_METHODS, "*") + .status(StatusCode::OK) + .body(Body::empty())?) + } } else { - // The only supported API call that doesn't use a bucket name is ListBuckets, - // which we want to allow in all cases - return Ok(Response::builder() + // If there is no bucket name in the request, + // we are doing a ListBuckets call, which we want to allow + // for all origins. + Ok(Response::builder() .header(ACCESS_CONTROL_ALLOW_ORIGIN, "*") .header(ACCESS_CONTROL_ALLOW_METHODS, "GET") .status(StatusCode::OK) - .body(Body::empty())?); - }; + .body(Body::empty())?) + } +} +pub fn handle_options_for_bucket( + req: &Request<Body>, + bucket: &Bucket, +) -> Result<Response<Body>, Error> { let origin = req .headers() .get("Origin") diff --git a/src/web/web_server.rs b/src/web/web_server.rs index 15935cba..80d2feb9 100644 --- a/src/web/web_server.rs +++ b/src/web/web_server.rs @@ -13,7 +13,7 @@ use crate::error::*; use garage_api::error::{Error as ApiError, OkOrBadRequest, OkOrInternalError}; use garage_api::helpers::{authority_to_host, host_to_bucket}; -use garage_api::s3_cors::{add_cors_headers, find_matching_cors_rule, handle_options}; +use garage_api::s3_cors::{add_cors_headers, find_matching_cors_rule, handle_options_for_bucket}; use garage_api::s3_get::{handle_get, handle_head}; use garage_model::garage::Garage; @@ -133,7 +133,7 @@ async fn serve_file(garage: Arc<Garage>, req: &Request<Body>) -> Result<Response ); let ret_doc = match *req.method() { - Method::OPTIONS => handle_options(garage.clone(), req, Some(bucket_name.to_string())).await, + Method::OPTIONS => handle_options_for_bucket(req, &bucket), Method::HEAD => handle_head(garage.clone(), req, bucket_id, &key, None).await, Method::GET => handle_get(garage.clone(), req, bucket_id, &key, None).await, _ => Err(ApiError::BadRequest("HTTP method not supported".into())), |