From 79c6767abd3af4a7630e58019fdfc057eac5b69a Mon Sep 17 00:00:00 2001 From: "Zhidong Peng (HE/HIM)" Date: Fri, 20 Feb 2026 10:36:28 -0800 Subject: [PATCH 1/3] Fix: Privilege match must be case insensitive. --- proxy_agent/src/key_keeper/key.rs | 26 ++++++++++++------- proxy_agent/src/proxy/authorization_rules.rs | 27 +++++++++++++++++--- 2 files changed, 40 insertions(+), 13 deletions(-) diff --git a/proxy_agent/src/key_keeper/key.rs b/proxy_agent/src/key_keeper/key.rs index b413e3e2..58f75c98 100644 --- a/proxy_agent/src/key_keeper/key.rs +++ b/proxy_agent/src/key_keeper/key.rs @@ -213,12 +213,20 @@ impl Clone for Privilege { } impl Privilege { - pub fn is_match(&self, logger: &mut ConnectionLogger, request_url: &Uri) -> bool { + /// Note: `self.path` and `self.queryParameters` keys/values are expected to be + /// pre-lowercased (done in `ComputedAuthorizationItem::from_authorization_item`). + /// `lowered_request_path` should be `request_url.path().to_lowercase()`, hoisted by the caller. + pub fn is_match( + &self, + logger: &mut ConnectionLogger, + request_url: &Uri, + lowered_request_path: &str, + ) -> bool { logger.write( LoggerLevel::Trace, format!("Start to match privilege '{}'", self.name), ); - if request_url.path().to_lowercase().starts_with(&self.path) { + if lowered_request_path.starts_with(&self.path) { logger.write( LoggerLevel::Trace, format!("Matched privilege path '{}'", self.path), @@ -236,10 +244,10 @@ impl Privilege { for (key, value) in query_parameters { match hyper_client::query_pairs(request_url) .into_iter() - .find(|(k, _)| k.to_lowercase() == key.to_lowercase()) + .find(|(k, _)| k.to_lowercase() == *key) { Some((_, v)) => { - if v.to_lowercase() == value.to_lowercase() { + if v.to_lowercase() == *value { logger.write( LoggerLevel::Trace, format!( @@ -1400,7 +1408,7 @@ mod tests { .parse() .unwrap(); assert!( - privilege.is_match(&mut logger, &url), + privilege.is_match(&mut logger, &url, &url.path().to_lowercase()), "privilege should be matched" ); @@ -1408,13 +1416,13 @@ mod tests { .parse() .unwrap(); assert!( - !privilege.is_match(&mut logger, &url), + !privilege.is_match(&mut logger, &url, &url.path().to_lowercase()), "privilege should not be matched" ); let url = "http://localhost/test?key1=value1".parse().unwrap(); assert!( - !privilege.is_match(&mut logger, &url), + !privilege.is_match(&mut logger, &url, &url.path().to_lowercase()), "privilege should not be matched" ); @@ -1427,7 +1435,7 @@ mod tests { .parse() .unwrap(); assert!( - privilege1.is_match(&mut logger, &url), + privilege1.is_match(&mut logger, &url, &url.path().to_lowercase()), "privilege should be matched" ); @@ -1444,7 +1452,7 @@ mod tests { .parse() .unwrap(); assert!( - !privilege2.is_match(&mut logger, &url), + !privilege2.is_match(&mut logger, &url, &url.path().to_lowercase()), "privilege should not be matched" ); } diff --git a/proxy_agent/src/proxy/authorization_rules.rs b/proxy_agent/src/proxy/authorization_rules.rs index 746efee0..3c706083 100644 --- a/proxy_agent/src/proxy/authorization_rules.rs +++ b/proxy_agent/src/proxy/authorization_rules.rs @@ -111,7 +111,21 @@ impl ComputedAuthorizationItem { .collect::>(); privilege_dict = privileges .into_iter() - .map(|privilege| (privilege.name.clone(), privilege)) + .map(|privilege| { + // case insensitive for path and query parameters key/values, + // to make it easier for users to write the rules without worrying about the case sensitivity. + // The name of the privilege is case sensitive, as it is used as the key in the privilege_dict and privilege_assignments. + let normalized = Privilege { + name: privilege.name, + path: privilege.path.to_lowercase(), + queryParameters: privilege.queryParameters.map(|qp| { + qp.into_iter() + .map(|(k, v)| (k.to_lowercase(), v.to_lowercase())) + .collect() + }), + }; + (normalized.name.clone(), normalized) + }) .collect::>(); for role_assignment in role_assignments { @@ -177,10 +191,11 @@ impl ComputedAuthorizationItem { return true; } + let lowered_request_path = request_url.path().to_lowercase(); let mut any_privilege_matched = false; for privilege in self.privileges.values() { let privilege_name = &privilege.name; - if privilege.is_match(logger, &request_url) { + if privilege.is_match(logger, &request_url, &lowered_request_path) { any_privilege_matched = true; logger.write( LoggerLevel::Trace, @@ -374,7 +389,7 @@ mod tests { }]), privileges: Some(vec![Privilege { name: "test".to_string(), - path: "/test".to_string(), + path: "/TEST".to_string(), // test the case insensitivity of the path queryParameters: None, }]), identities: Some(vec![Identity { @@ -416,8 +431,12 @@ mod tests { runAsElevated: true, }; // assert the claim is allowed given the rules above - let url = hyper::Uri::from_str("http://localhost/test/test").unwrap(); + + // test the case insensitivity of the path + let url = hyper::Uri::from_str("http://localhost/tESt/test").unwrap(); assert!(rules.is_allowed(&mut test_logger, url, claims.clone())); + + // test the case insensitivity of the path and the relative url let relative_url = hyper::Uri::from_str("/test/test").unwrap(); assert!(rules.is_allowed(&mut test_logger, relative_url.clone(), claims.clone())); claims.userName = "test1".to_string(); From ade1a4bb4178a4f08456b59c82076c301fe9ce70 Mon Sep 17 00:00:00 2001 From: "Zhidong Peng (HE/HIM)" Date: Fri, 20 Feb 2026 10:42:16 -0800 Subject: [PATCH 2/3] Add comments to explain why not optimize the query parameters to lower case for the request_url. --- proxy_agent/src/key_keeper/key.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/proxy_agent/src/key_keeper/key.rs b/proxy_agent/src/key_keeper/key.rs index 58f75c98..26d3f429 100644 --- a/proxy_agent/src/key_keeper/key.rs +++ b/proxy_agent/src/key_keeper/key.rs @@ -242,6 +242,8 @@ impl Privilege { ); for (key, value) in query_parameters { + // We may need to optimize this like `lowered_request_path` if there are too many query parameters in the future, + // but currently we expect only a few query parameters at most, so the performance impact should be minimal. match hyper_client::query_pairs(request_url) .into_iter() .find(|(k, _)| k.to_lowercase() == *key) From 6649a402f0c7bda11127279834a64d8641e96f50 Mon Sep 17 00:00:00 2001 From: "Zhidong Peng (HE/HIM)" Date: Fri, 20 Feb 2026 10:44:16 -0800 Subject: [PATCH 3/3] fix format --- proxy_agent/src/key_keeper/key.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proxy_agent/src/key_keeper/key.rs b/proxy_agent/src/key_keeper/key.rs index 26d3f429..e8c2b61d 100644 --- a/proxy_agent/src/key_keeper/key.rs +++ b/proxy_agent/src/key_keeper/key.rs @@ -242,7 +242,7 @@ impl Privilege { ); for (key, value) in query_parameters { - // We may need to optimize this like `lowered_request_path` if there are too many query parameters in the future, + // We may need to optimize this like `lowered_request_path` if there are too many query parameters in the future, // but currently we expect only a few query parameters at most, so the performance impact should be minimal. match hyper_client::query_pairs(request_url) .into_iter()