fix(auth): reject ambiguous case-insensitive claim matches (#2386)

crates/policy/src/policy.rs

@@ -35,6 +35,7 @@ pub use policy::*;
 pub use principal::Principal;
 pub use resource::ResourceSet;
 pub use statement::Statement;
+pub use utils::{ClaimLookup, get_claim_case_insensitive};
 
 #[derive(thiserror::Error, Debug)]
 #[cfg_attr(test, derive(Eq, PartialEq))]

crates/policy/src/policy/policy.rs

@@ -13,8 +13,8 @@
 // limitations under the License.
 
 use super::{
-    Effect, Error as IamError, Functions, ID, Statement, action::Action, statement::BPStatement,
-    statement::variable_resolver_for_policy_args,
+    ClaimLookup, Effect, Error as IamError, Functions, ID, Statement, action::Action, get_claim_case_insensitive,
+    statement::BPStatement, statement::variable_resolver_for_policy_args,
 };
 use crate::error::{Error, Result};
 use serde::{Deserialize, Serialize};
@@ -239,42 +239,37 @@ impl Validator for BucketPolicy {
     }
 }
 
-fn get_claim_case_insensitive<'a>(claims: &'a HashMap<String, Value>, claim_name: &str) -> Option<&'a Value> {
-    if let Some(v) = claims.get(claim_name) {
-        return Some(v);
-    }
-    let claim_name_lower = claim_name.to_lowercase();
-    claims
-        .iter()
-        .find(|(k, _)| k.to_lowercase() == claim_name_lower)
-        .map(|(_, v)| v)
-}
-
 fn get_values_from_claims(claims: &HashMap<String, Value>, claim_name: &str) -> (HashSet<String>, bool) {
     let mut s = HashSet::new();
-    if let Some(pname) = get_claim_case_insensitive(claims, claim_name) {
-        if let Some(pnames) = pname.as_array() {
-            for pname in pnames {
-                if let Some(pname_str) = pname.as_str() {
-                    for pname in pname_str.split(',') {
-                        let pname = pname.trim();
-                        if !pname.is_empty() {
-                            s.insert(pname.to_string());
+    match get_claim_case_insensitive(claims, claim_name) {
+        ClaimLookup::Found(pname) => {
+            if let Some(pnames) = pname.as_array() {
+                for pname in pnames {
+                    if let Some(pname_str) = pname.as_str() {
+                        for pname in pname_str.split(',') {
+                            let pname = pname.trim();
+                            if !pname.is_empty() {
+                                s.insert(pname.to_string());
+                            }
                         }
                     }
                 }
+                return (s, true);
             }
-            return (s, true);
-        } else if let Some(pname_str) = pname.as_str() {
-            for pname in pname_str.split(',') {
-                let pname = pname.trim();
-                if !pname.is_empty() {
-                    s.insert(pname.to_string());
+
+            if let Some(pname_str) = pname.as_str() {
+                for pname in pname_str.split(',') {
+                    let pname = pname.trim();
+                    if !pname.is_empty() {
+                        s.insert(pname.to_string());
+                    }
                 }
+                return (s, true);
             }
-            return (s, true);
         }
+        ClaimLookup::Missing | ClaimLookup::Ambiguous => {}
     }
+
     (s, false)
 }
 
@@ -1744,4 +1739,26 @@ mod test {
         assert!(policies.contains("consoleAdmin"));
         assert!(policies.contains("readwrite"));
     }
+
+    #[test]
+    fn test_get_values_from_claims_ambiguous_case_insensitive_match_returns_missing() {
+        let mut claims = HashMap::new();
+        claims.insert("Policy".to_string(), Value::Array(vec![Value::String("exact_match".to_string())]));
+        claims.insert("policy".to_string(), Value::Array(vec![Value::String("lowercase".to_string())]));
+
+        let (policies, found) = get_values_from_claims(&claims, "POLICY");
+        assert!(!found);
+        assert!(policies.is_empty());
+    }
+
+    #[test]
+    fn test_get_policies_from_claims_ambiguous_case_insensitive_match_returns_missing() {
+        let mut claims = HashMap::new();
+        claims.insert("Policy".to_string(), Value::String("consoleAdmin".to_string()));
+        claims.insert("policy".to_string(), Value::String("readwrite".to_string()));
+
+        let (policies, found) = get_policies_from_claims(&claims, "POLICY");
+        assert!(!found);
+        assert!(policies.is_empty());
+    }
 }

crates/policy/src/policy/utils.rs

@@ -19,6 +19,41 @@ use serde_json::Value;
 pub mod path;
 pub mod wildcard;
 
+#[derive(Clone, Copy, Debug, Eq, PartialEq)]
+pub enum ClaimLookup<'a> {
+    Missing,
+    Found(&'a Value),
+    Ambiguous,
+}
+
+fn case_insensitive_eq(left: &str, right: &str) -> bool {
+    left.chars()
+        .flat_map(char::to_lowercase)
+        .eq(right.chars().flat_map(char::to_lowercase))
+}
+
+pub fn get_claim_case_insensitive<'a>(claims: &'a HashMap<String, Value>, claim_name: &str) -> ClaimLookup<'a> {
+    if let Some(value) = claims.get(claim_name) {
+        return ClaimLookup::Found(value);
+    }
+
+    let mut matched = None;
+
+    for (candidate, value) in claims {
+        if case_insensitive_eq(candidate, claim_name) {
+            if matched.is_some() {
+                return ClaimLookup::Ambiguous;
+            }
+            matched = Some(value);
+        }
+    }
+
+    match matched {
+        Some(value) => ClaimLookup::Found(value),
+        None => ClaimLookup::Missing,
+    }
+}
+
 pub fn _get_values_from_claims(claim: &HashMap<String, Value>, chaim_name: &str) -> (Vec<String>, bool) {
     let mut result = vec![];
     let Some(pname) = claim.get(chaim_name) else {
@@ -77,7 +112,9 @@ pub fn _split_path(path: &str, second_index: bool) -> (&str, &str) {
 
 #[cfg(test)]
 mod tests {
-    use super::_split_path;
+    use super::{_split_path, ClaimLookup, get_claim_case_insensitive};
+    use serde_json::{Value, json};
+    use std::collections::HashMap;
 
     #[test_case::test_case("format.json", false => ("format.json", ""))]
     #[test_case::test_case("users/tester.json", false => ("users/", "tester.json"))]
@@ -98,4 +135,36 @@ mod tests {
     fn test_split_path(path: &str, second_index: bool) -> (&str, &str) {
         _split_path(path, second_index)
     }
+
+    #[test]
+    fn test_get_claim_case_insensitive_prefers_exact_match() {
+        let mut claims = HashMap::new();
+        claims.insert("Policy".to_string(), json!("exact_match"));
+        claims.insert("policy".to_string(), json!("lowercase"));
+
+        assert_eq!(
+            get_claim_case_insensitive(&claims, "Policy"),
+            ClaimLookup::Found(&Value::String("exact_match".to_string()))
+        );
+    }
+
+    #[test]
+    fn test_get_claim_case_insensitive_returns_ambiguous_for_multiple_folded_matches() {
+        let mut claims = HashMap::new();
+        claims.insert("Policy".to_string(), json!("exact_match"));
+        claims.insert("policy".to_string(), json!("lowercase"));
+
+        assert_eq!(get_claim_case_insensitive(&claims, "POLICY"), ClaimLookup::Ambiguous);
+    }
+
+    #[test]
+    fn test_get_claim_case_insensitive_matches_unicode_without_allocation() {
+        let mut claims = HashMap::new();
+        claims.insert("Straße".to_string(), json!("value"));
+
+        assert_eq!(
+            get_claim_case_insensitive(&claims, "straße"),
+            ClaimLookup::Found(&Value::String("value".to_string()))
+        );
+    }
 }