Fix: Privilege match must be case insensitive.

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),
@@ -234,12 +242,14 @@ 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.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,21 +1410,21 @@ 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"
         );
 
         let url = "http://localhost/test?key1=value1&key2=value3"
             .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 +1437,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 +1454,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"
         );
     }

proxy_agent/src/proxy/authorization_rules.rs

@@ -111,7 +111,21 @@ impl ComputedAuthorizationItem {
                     .collect::<HashMap<String, Identity>>();
                 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::<HashMap<String, Privilege>>();
 
                 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();