diff --git a/crates/core/src/proxy.rs b/crates/core/src/proxy.rs index 9cfabb9..8fe51ff 100644 --- a/crates/core/src/proxy.rs +++ b/crates/core/src/proxy.rs @@ -261,15 +261,7 @@ where } // Resolve via proxy pipeline (with metadata for after_dispatch) - let (action, metadata) = self - .resolve_request_with_metadata( - req.method.clone(), - req.path, - req.query, - req.headers, - req.source_ip, - ) - .await; + let (action, metadata) = self.resolve_request_with_metadata(req).await; // Helper to extract response body size fn response_body_bytes(body: &ProxyResponseBody) -> Option { @@ -361,9 +353,8 @@ where headers: &HeaderMap, source_ip: Option, ) -> HandlerAction { - let (action, _metadata) = self - .resolve_request_with_metadata(method, path, query, headers, source_ip) - .await; + let req = RequestInfo::new(&method, path, query, headers, source_ip); + let (action, _metadata) = self.resolve_request_with_metadata(&req).await; action } @@ -371,45 +362,43 @@ where /// [`RequestMetadata`] for post-dispatch callbacks (e.g. metering). pub(crate) async fn resolve_request_with_metadata( &self, - method: Method, - path: &str, - query: Option<&str>, - headers: &HeaderMap, - source_ip: Option, + req: &RequestInfo<'_>, ) -> (HandlerAction, RequestMetadata) { let request_id = Uuid::new_v4().to_string(); tracing::info!( request_id = %request_id, - method = %method, - path = %path, - query = ?query, + method = %req.method, + path = %req.path, + query = ?req.query, "incoming request" ); // Determine host style - let host_style = determine_host_style(headers, self.virtual_host_domain.as_deref()); + let host_style = determine_host_style(req.headers, self.virtual_host_domain.as_deref()); // Parse the S3 operation - let operation = match request::parse_s3_request(&method, path, query, headers, host_style) { - Ok(op) => op, - Err(err) => return self.error_result(err, path, &request_id, source_ip), - }; + let operation = + match request::parse_s3_request(req.method, req.path, req.query, req.headers, host_style) { + Ok(op) => op, + Err(err) => return self.error_result(err, req.path, &request_id, req.source_ip), + }; tracing::debug!(operation = ?operation, "parsed S3 operation"); - // Resolve identity + // Resolve identity — use the original client-facing path and query for + // signature verification when provided (e.g. path-mapping rewrites). let identity = match auth::resolve_identity( - &method, - path, - query.unwrap_or(""), - headers, + req.method, + req.signing_path.unwrap_or(req.path), + req.signing_query.or(req.query).unwrap_or(""), + req.headers, &self.credential_registry, self.credential_resolver.as_deref(), ) .await { Ok(id) => id, - Err(err) => return self.error_result(err, path, &request_id, source_ip), + Err(err) => return self.error_result(err, req.path, &request_id, req.source_ip), }; tracing::debug!(identity = ?identity, "resolved identity"); @@ -429,7 +418,7 @@ where tracing::trace!("authorization passed"); Some(resolved) } - Err(err) => return self.error_result(err, path, &request_id, source_ip), + Err(err) => return self.error_result(err, req.path, &request_id, req.source_ip), } } else { None @@ -440,8 +429,8 @@ where identity: &identity, operation: &operation, bucket_config: resolved.as_ref().map(|r| Cow::Borrowed(&r.config)), - headers, - source_ip, + headers: req.headers, + source_ip: req.source_ip, request_id: &request_id, list_rewrite: resolved.as_ref().and_then(|r| r.list_rewrite.as_ref()), display_name: resolved.as_ref().and_then(|r| r.display_name.as_deref()), @@ -454,7 +443,7 @@ where identity: Some(identity.clone()), operation: Some(operation.clone()), bucket: operation.bucket().map(str::to_string), - source_ip, + source_ip: req.source_ip, }; match next.run(ctx).await { @@ -483,7 +472,7 @@ where } (action, metadata) } - Err(err) => self.error_result(err, path, &request_id, source_ip), + Err(err) => self.error_result(err, req.path, &request_id, req.source_ip), } } diff --git a/crates/core/src/route_handler.rs b/crates/core/src/route_handler.rs index db6c91b..4a919b7 100644 --- a/crates/core/src/route_handler.rs +++ b/crates/core/src/route_handler.rs @@ -218,6 +218,18 @@ pub struct RequestInfo<'a> { /// Populated by the router when a route pattern matches. Empty when the /// request is constructed via [`RequestInfo::new`]. pub params: Params, + /// The original path as seen by the client, used for SigV4 signature + /// verification when the proxy rewrites paths before dispatch. + /// + /// When `None`, `path` is used for both operation parsing and signature + /// verification. + pub signing_path: Option<&'a str>, + /// The original query string as seen by the client, used for SigV4 + /// signature verification when the proxy rewrites query parameters. + /// + /// When `None`, `query` is used for both operation parsing and signature + /// verification. + pub signing_query: Option<&'a str>, } impl<'a> RequestInfo<'a> { @@ -236,8 +248,29 @@ impl<'a> RequestInfo<'a> { headers, source_ip, params: Params::default(), + signing_path: None, + signing_query: None, } } + + /// Set the original client-facing path for SigV4 signature verification. + /// + /// Use this when the proxy rewrites paths (e.g. path-mapping) so that + /// signature verification uses the path the client actually signed. + pub fn with_signing_path(mut self, signing_path: &'a str) -> Self { + self.signing_path = Some(signing_path); + self + } + + /// Set the original client-facing query string for SigV4 signature verification. + /// + /// Use this when the proxy rewrites query parameters (e.g. path-mapping + /// strips prefix segments from the `prefix` parameter) so that signature + /// verification uses the query string the client actually signed. + pub fn with_signing_query(mut self, signing_query: Option<&'a str>) -> Self { + self.signing_query = signing_query; + self + } } /// A pluggable handler that can intercept requests before proxy dispatch. diff --git a/crates/core/src/router.rs b/crates/core/src/router.rs index ef98187..b6bcc44 100644 --- a/crates/core/src/router.rs +++ b/crates/core/src/router.rs @@ -66,6 +66,8 @@ impl Router { query: req.query, headers: req.headers, source_ip: req.source_ip, + signing_path: req.signing_path, + signing_query: req.signing_query, }; matched .value diff --git a/crates/path-mapping/src/lib.rs b/crates/path-mapping/src/lib.rs index a60e3f7..6df3dec 100644 --- a/crates/path-mapping/src/lib.rs +++ b/crates/path-mapping/src/lib.rs @@ -39,20 +39,22 @@ //! assert_eq!(mapped.display_bucket, "acme"); //! //! // Full request rewrite (path + query) -//! let (path, query) = mapping.rewrite_request( +//! let result = mapping.rewrite_request( //! "/acme/data/report.csv", //! None, //! ); -//! assert_eq!(path, "/acme:data/report.csv"); -//! assert_eq!(query, None); +//! assert_eq!(result.path, "/acme:data/report.csv"); +//! assert_eq!(result.query, None); +//! assert_eq!(result.signing_path, "/acme/data/report.csv"); //! //! // Prefix-based list rewrite -//! let (path, query) = mapping.rewrite_request( +//! let result = mapping.rewrite_request( //! "/acme", //! Some("list-type=2&prefix=data/subdir/"), //! ); -//! assert_eq!(path, "/acme:data"); -//! assert_eq!(query, Some("list-type=2&prefix=subdir/".to_string())); +//! assert_eq!(result.path, "/acme:data"); +//! assert_eq!(result.query, Some("list-type=2&prefix=subdir/".to_string())); +//! assert_eq!(result.signing_query, Some("list-type=2&prefix=data/subdir/".to_string())); //! ``` use multistore::api::list_rewrite::ListRewrite; @@ -74,6 +76,22 @@ pub struct PathMapping { pub display_bucket_segments: usize, } +/// Result of rewriting a request path and query string. +/// +/// Contains both the rewritten values (for S3 operation parsing) and the +/// original values (for SigV4 signature verification). +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct RewriteResult { + /// Rewritten path for S3 operation parsing. + pub path: String, + /// Rewritten query for operation parsing. + pub query: Option, + /// Original client path for SigV4 verification. + pub signing_path: String, + /// Original client query for SigV4 verification. + pub signing_query: Option, +} + /// Result of mapping a request path. #[derive(Debug, Clone, PartialEq, Eq)] pub struct MappedPath { @@ -208,14 +226,22 @@ impl PathMapping { /// /// 3. **Pass-through**: all other paths are returned unchanged. Route handlers /// or the gateway itself will handle them. - pub fn rewrite_request(&self, path: &str, query: Option<&str>) -> (String, Option) { + pub fn rewrite_request(&self, path: &str, query: Option<&str>) -> RewriteResult { + let signing_path = path.to_string(); + let signing_query = query.map(|q| q.to_string()); + // Case 1: enough path segments to map directly if let Some(mapped) = self.parse(path) { let rewritten_path = match mapped.key { Some(ref key) => format!("/{}/{}", mapped.bucket, key), None => format!("/{}", mapped.bucket), }; - return (rewritten_path, query.map(|q| q.to_string())); + return RewriteResult { + path: rewritten_path, + query: query.map(|q| q.to_string()), + signing_path, + signing_query, + }; } // Case 2: single-segment path with a list-type query and non-empty prefix @@ -225,14 +251,26 @@ impl PathMapping { if is_list_request(query_str) { if let Some(prefix) = extract_query_param(query_str, "prefix") { if !prefix.is_empty() { - return self.rewrite_prefix_to_bucket(trimmed, &prefix, query_str); + let (rewritten_path, rewritten_query) = + self.rewrite_prefix_to_bucket(trimmed, &prefix, query_str); + return RewriteResult { + path: rewritten_path, + query: rewritten_query, + signing_path, + signing_query, + }; } } } } // Case 3: pass through unchanged - (path.to_string(), query.map(|q| q.to_string())) + RewriteResult { + path: path.to_string(), + query: query.map(|q| q.to_string()), + signing_path, + signing_query, + } } /// Fold the first prefix component into the bucket name. diff --git a/crates/path-mapping/tests/mapping_tests.rs b/crates/path-mapping/tests/mapping_tests.rs index 0cedbf0..68b4c46 100644 --- a/crates/path-mapping/tests/mapping_tests.rs +++ b/crates/path-mapping/tests/mapping_tests.rs @@ -115,104 +115,113 @@ fn trailing_slash_on_bucket_segments() { #[test] fn rewrite_multi_segment_path() { let mapping = default_mapping(); - assert_eq!( - mapping.rewrite_request("/account/product/file.parquet", None), - ("/account--product/file.parquet".to_string(), None) - ); + let result = mapping.rewrite_request("/account/product/file.parquet", None); + assert_eq!(result.path, "/account--product/file.parquet"); + assert_eq!(result.query, None); + assert_eq!(result.signing_path, "/account/product/file.parquet"); + assert_eq!(result.signing_query, None); } #[test] fn rewrite_multi_segment_nested_key() { let mapping = default_mapping(); - assert_eq!( - mapping.rewrite_request("/account/product/dir/sub/file.parquet", None), - ("/account--product/dir/sub/file.parquet".to_string(), None) - ); + let result = mapping.rewrite_request("/account/product/dir/sub/file.parquet", None); + assert_eq!(result.path, "/account--product/dir/sub/file.parquet"); + assert_eq!(result.query, None); + assert_eq!(result.signing_path, "/account/product/dir/sub/file.parquet"); + assert_eq!(result.signing_query, None); } #[test] fn rewrite_bucket_only_no_key() { let mapping = default_mapping(); - assert_eq!( - mapping.rewrite_request("/account/product", Some("list-type=2")), - ( - "/account--product".to_string(), - Some("list-type=2".to_string()) - ) - ); + let result = mapping.rewrite_request("/account/product", Some("list-type=2")); + assert_eq!(result.path, "/account--product"); + assert_eq!(result.query, Some("list-type=2".to_string())); + assert_eq!(result.signing_path, "/account/product"); + assert_eq!(result.signing_query, Some("list-type=2".to_string())); } #[test] fn rewrite_prefix_routed_list() { let mapping = default_mapping(); + let result = mapping.rewrite_request("/account", Some("list-type=2&prefix=product/")); + assert_eq!(result.path, "/account--product"); + assert_eq!(result.query, Some("list-type=2&prefix=".to_string())); + assert_eq!(result.signing_path, "/account"); assert_eq!( - mapping.rewrite_request("/account", Some("list-type=2&prefix=product/")), - ( - "/account--product".to_string(), - Some("list-type=2&prefix=".to_string()) - ) + result.signing_query, + Some("list-type=2&prefix=product/".to_string()) ); } #[test] fn rewrite_prefix_routed_list_with_subdir() { let mapping = default_mapping(); + let result = mapping.rewrite_request("/account", Some("list-type=2&prefix=product/subdir/")); + assert_eq!(result.path, "/account--product"); + assert_eq!(result.query, Some("list-type=2&prefix=subdir/".to_string())); + assert_eq!(result.signing_path, "/account"); assert_eq!( - mapping.rewrite_request("/account", Some("list-type=2&prefix=product/subdir/")), - ( - "/account--product".to_string(), - Some("list-type=2&prefix=subdir/".to_string()) - ) + result.signing_query, + Some("list-type=2&prefix=product/subdir/".to_string()) ); } #[test] fn rewrite_url_encoded_prefix() { let mapping = default_mapping(); + let result = + mapping.rewrite_request("/account", Some("list-type=2&prefix=my%20product/subdir/")); + assert_eq!(result.path, "/account--my product"); + assert_eq!(result.query, Some("list-type=2&prefix=subdir/".to_string())); + assert_eq!(result.signing_path, "/account"); assert_eq!( - mapping.rewrite_request("/account", Some("list-type=2&prefix=my%20product/subdir/")), - ( - "/account--my product".to_string(), - Some("list-type=2&prefix=subdir/".to_string()) - ) + result.signing_query, + Some("list-type=2&prefix=my%20product/subdir/".to_string()) ); } #[test] fn rewrite_single_segment_no_list_passes_through() { let mapping = default_mapping(); - assert_eq!( - mapping.rewrite_request("/account", None), - ("/account".to_string(), None) - ); + let result = mapping.rewrite_request("/account", None); + assert_eq!(result.path, "/account"); + assert_eq!(result.query, None); + assert_eq!(result.signing_path, "/account"); + assert_eq!(result.signing_query, None); } #[test] fn rewrite_single_segment_list_no_prefix_passes_through() { let mapping = default_mapping(); - assert_eq!( - mapping.rewrite_request("/account", Some("list-type=2")), - ("/account".to_string(), Some("list-type=2".to_string())) - ); + let result = mapping.rewrite_request("/account", Some("list-type=2")); + assert_eq!(result.path, "/account"); + assert_eq!(result.query, Some("list-type=2".to_string())); + assert_eq!(result.signing_path, "/account"); + assert_eq!(result.signing_query, Some("list-type=2".to_string())); } #[test] fn rewrite_root_passes_through() { let mapping = default_mapping(); - assert_eq!(mapping.rewrite_request("/", None), ("/".to_string(), None)); + let result = mapping.rewrite_request("/", None); + assert_eq!(result.path, "/"); + assert_eq!(result.query, None); } #[test] fn rewrite_empty_passes_through() { let mapping = default_mapping(); - assert_eq!(mapping.rewrite_request("", None), ("".to_string(), None)); + let result = mapping.rewrite_request("", None); + assert_eq!(result.path, ""); + assert_eq!(result.query, None); } #[test] fn rewrite_trailing_slash_passes_through() { let mapping = default_mapping(); - assert_eq!( - mapping.rewrite_request("/account/", Some("list-type=2")), - ("/account/".to_string(), Some("list-type=2".to_string())) - ); + let result = mapping.rewrite_request("/account/", Some("list-type=2")); + assert_eq!(result.path, "/account/"); + assert_eq!(result.query, Some("list-type=2".to_string())); }