diff --git a/Cargo.lock b/Cargo.lock index 3f7f8fbea..798c86975 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8096,6 +8096,7 @@ dependencies = [ "log", "openid", "reqwest 0.13.2", + "rstest", "schemars 1.2.1", "serde", "serde_json", diff --git a/common/auth/Cargo.toml b/common/auth/Cargo.toml index c29ad742a..167757429 100644 --- a/common/auth/Cargo.toml +++ b/common/auth/Cargo.toml @@ -37,6 +37,9 @@ url = { workspace = true } utoipa = { workspace = true, features = ["actix_extras"], optional = true } utoipa-swagger-ui = { workspace = true, features = ["actix-web"], optional = true } +[dev-dependencies] +rstest = { workspace = true } + [features] actix = ["actix-web", "actix-http", "actix-web-httpauth"] swagger = ["utoipa", "utoipa-swagger-ui", "actix"] diff --git a/common/auth/schema/auth.json b/common/auth/schema/auth.json index d7cef5de8..3285c800e 100644 --- a/common/auth/schema/auth.json +++ b/common/auth/schema/auth.json @@ -77,6 +77,11 @@ ], "default": null }, + "scopeSelector": { + "description": "JSON path extracting scopes from the access token (default: $['scope','scp'])", + "type": "string", + "default": "$['scope','scp']" + }, "groupMappings": { "description": "Mapping table for groups returned found through the `groups_selector` to permissions.", "type": "object", diff --git a/common/auth/src/authenticator/claims.rs b/common/auth/src/authenticator/claims.rs index 5d8ce0bfc..a65b8f3ba 100644 --- a/common/auth/src/authenticator/claims.rs +++ b/common/auth/src/authenticator/claims.rs @@ -24,9 +24,6 @@ pub struct AccessTokenClaims { #[serde(flatten)] pub extended_claims: Value, - - #[serde(default, skip_serializing_if = "String::is_empty")] - pub scope: String, } impl CompactJson for AccessTokenClaims {} diff --git a/common/auth/src/authenticator/config.rs b/common/auth/src/authenticator/config.rs index 3ab9473ec..3b92f049d 100644 --- a/common/auth/src/authenticator/config.rs +++ b/common/auth/src/authenticator/config.rs @@ -44,6 +44,7 @@ impl AuthenticatorConfig { additional_permissions: Default::default(), required_audience: None, group_selector: None, + scope_selector: default_scope_selector(), group_mappings: Default::default(), tls_insecure: false, tls_ca_certificates: Default::default(), @@ -132,6 +133,10 @@ pub struct AuthenticatorClientConfig { #[serde(default)] pub group_selector: Option, + /// JSON path extracting scopes from the access token (default: $['scope','scp']) + #[serde(default = "default_scope_selector")] + pub scope_selector: String, + /// Mapping table for groups returned found through the `groups_selector` to permissions. #[serde(default, skip_serializing_if = "HashMap::is_empty")] pub group_mappings: HashMap>, @@ -145,6 +150,12 @@ pub struct AuthenticatorClientConfig { pub tls_ca_certificates: Vec, } +pub const DEFAULT_SCOPE_SELECTOR: &str = "$['scope','scp']"; + +fn default_scope_selector() -> String { + DEFAULT_SCOPE_SELECTOR.to_string() +} + impl SingleAuthenticatorClientConfig { pub fn expand(self) -> impl Iterator { self.client_ids @@ -157,6 +168,7 @@ impl SingleAuthenticatorClientConfig { required_audience: self.required_audience.clone(), scope_mappings: default_scope_mappings(), group_selector: None, + scope_selector: default_scope_selector(), group_mappings: Default::default(), additional_permissions: Default::default(), }) diff --git a/common/auth/src/authenticator/mod.rs b/common/auth/src/authenticator/mod.rs index 82262ade2..fe7020ae7 100644 --- a/common/auth/src/authenticator/mod.rs +++ b/common/auth/src/authenticator/mod.rs @@ -192,6 +192,14 @@ async fn create_client(config: AuthenticatorClientConfig) -> anyhow::Result anyhow::Result, group_selector: Option, group_mappings: HashMap>, + scope_selector: JpQuery, } impl AuthenticatorClient { /// Convert from a set of (verified!) access token claims into a [`ValidatedAccessToken`] struct. pub fn convert_token(&self, access_token: AccessTokenClaims) -> ValidatedAccessToken { - let mut permissions = Self::map_scopes(&access_token.scope, &self.scope_mappings); + let extra_values = &access_token.extended_claims; + let mut permissions = Self::map_items( + Self::extract_scopes(extra_values, &self.scope_selector), + &self.scope_mappings, + ); permissions.extend(self.additional_permissions.clone()); let groups = self .group_selector .as_ref() - .map(|selector| Self::extract_groups(&access_token.extended_claims, selector)) + .map(|selector| Self::extract_groups(extra_values, selector)) .unwrap_or_default(); - permissions.extend(Self::map_groups(groups, &self.group_mappings)); + permissions.extend(Self::map_items(groups, &self.group_mappings)); ValidatedAccessToken { access_token, @@ -231,6 +245,28 @@ impl AuthenticatorClient { } } + /// Extract scopes from the value/access token + fn extract_scopes(value: &Value, selector: &JpQuery) -> Vec { + let mut result = Vec::new(); + for qr in js_path_process(selector, value).ok().into_iter().flatten() { + match qr.val() { + Value::String(s) => { + result.extend(s.split_ascii_whitespace().map(str::to_string)); + } + Value::Array(arr) => { + result.extend( + arr.iter() + .filter_map(|v| v.as_str()) + .flat_map(|s| s.split_ascii_whitespace()) + .map(str::to_string), + ); + } + _ => {} + } + } + result + } + /// Extract the groups from the value/access token fn extract_groups(value: &Value, selector: &JpQuery) -> Vec { js_path_process(selector, value) @@ -242,31 +278,18 @@ impl AuthenticatorClient { .collect() } - /// Run the scopes through the scope mapping configuration - fn map_scopes(scopes: &str, scope_mappings: &HashMap>) -> Vec { - scopes - .split(' ') - .flat_map(|scope| { - scope_mappings - .get(scope) - .cloned() - .unwrap_or_else(|| vec![scope.to_string()]) - }) - .collect() - } - - /// Run the groups through the group mapping configuration - fn map_groups( - groups: Vec, - group_mappings: &HashMap>, + fn map_items( + items: impl IntoIterator>, + table: &HashMap>, ) -> Vec { - groups - .into_iter() - .flat_map(|group| match group_mappings.get(&group) { - Some(permissions) => permissions.clone(), - None => vec![group], - }) - .collect() + let mut result = Vec::new(); + for item in items { + match table.get(item.as_ref()) { + Some(mapped) => result.extend_from_slice(mapped), + None => result.push(item.as_ref().to_string()), + } + } + result } } @@ -281,6 +304,8 @@ impl Deref for AuthenticatorClient { #[cfg(test)] mod test { use super::*; + use rstest::rstest; + use serde_json::json; fn assert_scope_mapping(scopes: &str, mappings: &[(&str, &[&str])], expected: &[&str]) { let mappings = mappings @@ -291,7 +316,7 @@ mod test { .iter() .map(|item| item.to_string()) .collect::>(); - let result = AuthenticatorClient::map_scopes(scopes, &mappings); + let result = AuthenticatorClient::map_items(scopes.split_whitespace(), &mappings); assert_eq!(result, expected); } @@ -337,4 +362,59 @@ mod test { let groups = AuthenticatorClient::extract_groups(&value, &selector); assert_eq!(&groups, &["manager", "reader"]); } + + #[rstest] + #[case::scope_only(json!({"scope": "read:document create:document"}), vec!["read:document", "create:document"])] + #[case::scp_string(json!({"scp": "read:document"}), vec!["read:document"])] + #[case::scp_array(json!({"scp": ["api://app/read:document", "api://app/create:document"]}), vec!["api://app/read:document", "api://app/create:document"])] + #[case::both_claims(json!({"scope": "openid profile", "scp": ["api://app/read:document", "api://app/create:document"]}), vec!["openid", "profile", "api://app/read:document", "api://app/create:document"])] + fn test_extract_scopes(#[case] value: Value, #[case] expected: Vec<&str>) { + let selector = parse_json_path(config::DEFAULT_SCOPE_SELECTOR).unwrap(); + assert_eq!( + AuthenticatorClient::extract_scopes(&value, &selector), + expected + ); + } + + #[test] + fn test_extract_scopes_custom_selector() { + let selector = parse_json_path("$.scp").unwrap(); + let value = json!({ + "scope": "openid profile", + "scp": ["read:document", "create:document"] + }); + // Should extract only from scp, ignoring scope + assert_eq!( + AuthenticatorClient::extract_scopes(&value, &selector), + &["read:document", "create:document"] + ); + } + + #[rstest] + #[case::null_value(json!({"scope": null}), vec![])] + #[case::number_value(json!({"scope": 42}), vec![])] + #[case::boolean_value(json!({"scope": true}), vec![])] + #[case::object_value(json!({"scope": {"nested": "value"}}), vec![])] + #[case::empty_string(json!({"scope": ""}), vec![])] + #[case::empty_array(json!({"scp": []}), vec![])] + #[case::no_matching_fields(json!({"other": "value"}), vec![])] + fn test_extract_scopes_non_string_and_empty(#[case] value: Value, #[case] expected: Vec<&str>) { + let selector = parse_json_path(config::DEFAULT_SCOPE_SELECTOR).unwrap(); + assert_eq!( + AuthenticatorClient::extract_scopes(&value, &selector), + expected + ); + } + + #[rstest] + #[case::mixed_types(json!({"scp": ["read:document", 42, null, "write:document", true]}), vec!["read:document", "write:document"])] + #[case::string_multiple_whitespace(json!({"scope": " read:document write:document "}), vec!["read:document", "write:document"])] + #[case::array_with_whitespace(json!({"scp": [" read:document write:document ", " delete:document "]}), vec!["read:document", "write:document", "delete:document"])] + fn test_extract_scopes_edge_cases(#[case] value: Value, #[case] expected: Vec<&str>) { + let selector = parse_json_path(config::DEFAULT_SCOPE_SELECTOR).unwrap(); + assert_eq!( + AuthenticatorClient::extract_scopes(&value, &selector), + expected + ); + } }