From 22d606b3e6fa03bea67642f4bcaf9a818c36165d Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Tue, 7 Apr 2026 13:17:20 -0700 Subject: [PATCH 1/3] feat: support rewritten paths for auth handling --- crates/core/src/proxy.rs | 9 ++++++--- crates/core/src/route_handler.rs | 16 ++++++++++++++++ crates/core/src/router.rs | 1 + 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/crates/core/src/proxy.rs b/crates/core/src/proxy.rs index 9cfabb9..7a91726 100644 --- a/crates/core/src/proxy.rs +++ b/crates/core/src/proxy.rs @@ -268,6 +268,7 @@ where req.query, req.headers, req.source_ip, + req.signing_path, ) .await; @@ -362,7 +363,7 @@ where source_ip: Option, ) -> HandlerAction { let (action, _metadata) = self - .resolve_request_with_metadata(method, path, query, headers, source_ip) + .resolve_request_with_metadata(method, path, query, headers, source_ip, None) .await; action } @@ -376,6 +377,7 @@ where query: Option<&str>, headers: &HeaderMap, source_ip: Option, + signing_path: Option<&str>, ) -> (HandlerAction, RequestMetadata) { let request_id = Uuid::new_v4().to_string(); @@ -397,10 +399,11 @@ where }; tracing::debug!(operation = ?operation, "parsed S3 operation"); - // Resolve identity + // Resolve identity — use the original client-facing path for signature + // verification when a signing_path is provided (e.g. path-mapping rewrites). let identity = match auth::resolve_identity( &method, - path, + signing_path.unwrap_or(path), query.unwrap_or(""), headers, &self.credential_registry, diff --git a/crates/core/src/route_handler.rs b/crates/core/src/route_handler.rs index db6c91b..6a8238a 100644 --- a/crates/core/src/route_handler.rs +++ b/crates/core/src/route_handler.rs @@ -218,6 +218,12 @@ 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>, } impl<'a> RequestInfo<'a> { @@ -236,8 +242,18 @@ impl<'a> RequestInfo<'a> { headers, source_ip, params: Params::default(), + signing_path: 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 + } } /// 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..3c32112 100644 --- a/crates/core/src/router.rs +++ b/crates/core/src/router.rs @@ -66,6 +66,7 @@ impl Router { query: req.query, headers: req.headers, source_ip: req.source_ip, + signing_path: req.signing_path, }; matched .value From 719f7c82dd0ce4154b548f25307d69314ba4509b Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Tue, 7 Apr 2026 14:02:58 -0700 Subject: [PATCH 2/3] feat: enhance request rewriting to support signing query for signature verification --- crates/core/src/proxy.rs | 10 ++- crates/core/src/route_handler.rs | 17 ++++ crates/core/src/router.rs | 1 + crates/path-mapping/src/lib.rs | 58 ++++++++++--- crates/path-mapping/tests/mapping_tests.rs | 98 ++++++++++++---------- 5 files changed, 126 insertions(+), 58 deletions(-) diff --git a/crates/core/src/proxy.rs b/crates/core/src/proxy.rs index 7a91726..597cf76 100644 --- a/crates/core/src/proxy.rs +++ b/crates/core/src/proxy.rs @@ -269,6 +269,7 @@ where req.headers, req.source_ip, req.signing_path, + req.signing_query, ) .await; @@ -363,7 +364,7 @@ where source_ip: Option, ) -> HandlerAction { let (action, _metadata) = self - .resolve_request_with_metadata(method, path, query, headers, source_ip, None) + .resolve_request_with_metadata(method, path, query, headers, source_ip, None, None) .await; action } @@ -378,6 +379,7 @@ where headers: &HeaderMap, source_ip: Option, signing_path: Option<&str>, + signing_query: Option<&str>, ) -> (HandlerAction, RequestMetadata) { let request_id = Uuid::new_v4().to_string(); @@ -399,12 +401,12 @@ where }; tracing::debug!(operation = ?operation, "parsed S3 operation"); - // Resolve identity — use the original client-facing path for signature - // verification when a signing_path is provided (e.g. path-mapping rewrites). + // 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, signing_path.unwrap_or(path), - query.unwrap_or(""), + signing_query.or(query).unwrap_or(""), headers, &self.credential_registry, self.credential_resolver.as_deref(), diff --git a/crates/core/src/route_handler.rs b/crates/core/src/route_handler.rs index 6a8238a..4a919b7 100644 --- a/crates/core/src/route_handler.rs +++ b/crates/core/src/route_handler.rs @@ -224,6 +224,12 @@ pub struct RequestInfo<'a> { /// 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> { @@ -243,6 +249,7 @@ impl<'a> RequestInfo<'a> { source_ip, params: Params::default(), signing_path: None, + signing_query: None, } } @@ -254,6 +261,16 @@ impl<'a> RequestInfo<'a> { 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 3c32112..b6bcc44 100644 --- a/crates/core/src/router.rs +++ b/crates/core/src/router.rs @@ -67,6 +67,7 @@ impl Router { 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..f9b4c63 100644 --- a/crates/path-mapping/tests/mapping_tests.rs +++ b/crates/path-mapping/tests/mapping_tests.rs @@ -115,104 +115,114 @@ 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())); } From 93a678a0a527a5f6a8d95e66e4d21f8cb6c4d1c7 Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Wed, 8 Apr 2026 09:18:49 -0700 Subject: [PATCH 3/3] cleanup --- crates/core/src/proxy.rs | 62 ++++++++-------------- crates/path-mapping/tests/mapping_tests.rs | 3 +- 2 files changed, 24 insertions(+), 41 deletions(-) diff --git a/crates/core/src/proxy.rs b/crates/core/src/proxy.rs index 597cf76..8fe51ff 100644 --- a/crates/core/src/proxy.rs +++ b/crates/core/src/proxy.rs @@ -261,17 +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, - req.signing_path, - req.signing_query, - ) - .await; + let (action, metadata) = self.resolve_request_with_metadata(req).await; // Helper to extract response body size fn response_body_bytes(body: &ProxyResponseBody) -> Option { @@ -363,9 +353,8 @@ where headers: &HeaderMap, source_ip: Option, ) -> HandlerAction { - let (action, _metadata) = self - .resolve_request_with_metadata(method, path, query, headers, source_ip, None, None) - .await; + let req = RequestInfo::new(&method, path, query, headers, source_ip); + let (action, _metadata) = self.resolve_request_with_metadata(&req).await; action } @@ -373,48 +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, - signing_path: Option<&str>, - signing_query: Option<&str>, + 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 — 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, - signing_path.unwrap_or(path), - signing_query.or(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"); @@ -434,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 @@ -445,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()), @@ -459,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 { @@ -488,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/path-mapping/tests/mapping_tests.rs b/crates/path-mapping/tests/mapping_tests.rs index f9b4c63..68b4c46 100644 --- a/crates/path-mapping/tests/mapping_tests.rs +++ b/crates/path-mapping/tests/mapping_tests.rs @@ -158,8 +158,7 @@ fn rewrite_prefix_routed_list() { #[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/")); + 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");