From 1e3715148ba1bd8d4862e5ac66a738e3666b3ea8 Mon Sep 17 00:00:00 2001 From: fl0lli Date: Thu, 17 Jul 2025 15:06:14 +0200 Subject: [PATCH 1/4] feat: add claim import for dynamically setting can_request_admin user attribute; add test --- crates/cli/src/sync.rs | 4 + crates/config/src/sections/upstream_oauth2.rs | 25 +++ .../src/upstream_oauth2/provider.rs | 3 + .../handlers/src/upstream_oauth2/callback.rs | 155 +++++++++++++++++- docs/config.schema.json | 26 +++ docs/reference/configuration.md | 5 + 6 files changed, 217 insertions(+), 1 deletion(-) diff --git a/crates/cli/src/sync.rs b/crates/cli/src/sync.rs index 363c2a0f8..0db542733 100644 --- a/crates/cli/src/sync.rs +++ b/crates/cli/src/sync.rs @@ -59,6 +59,10 @@ fn map_claims_imports( account_name: mas_data_model::UpstreamOAuthProviderSubjectPreference { template: config.account_name.template.clone(), }, + is_admin: mas_data_model::UpstreamOAuthProviderImportPreference { + action: map_import_action(config.is_admin.action), + template: config.is_admin.template.clone(), + }, } } diff --git a/crates/config/src/sections/upstream_oauth2.rs b/crates/config/src/sections/upstream_oauth2.rs index 8d6229848..b2b4ea192 100644 --- a/crates/config/src/sections/upstream_oauth2.rs +++ b/crates/config/src/sections/upstream_oauth2.rs @@ -283,6 +283,26 @@ impl AccountNameImportPreference { } } +/// What should be done for the `ìs_admin` attribute +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Default, JsonSchema)] +pub struct IsAdminImportPreference { + /// How to handle the claim + #[serde(default, skip_serializing_if = "ImportAction::is_default")] + pub action: ImportAction, + + /// The Jinja2 template to use for the admin attribute. + /// + /// If not provided, it will be ignored. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub template: Option, +} + +impl IsAdminImportPreference { + const fn is_default(&self) -> bool { + self.action.is_default() && self.template.is_none() + } +} + /// How claims should be imported #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Default, JsonSchema)] pub struct ClaimsImports { @@ -312,6 +332,11 @@ pub struct ClaimsImports { skip_serializing_if = "AccountNameImportPreference::is_default" )] pub account_name: AccountNameImportPreference, + + /// Import the `can_request_admin` attribute of the user based on the + /// defined claim (i.e. `is_admin`) + #[serde(default, skip_serializing_if = "IsAdminImportPreference::is_default")] + pub is_admin: IsAdminImportPreference, } impl ClaimsImports { diff --git a/crates/data-model/src/upstream_oauth2/provider.rs b/crates/data-model/src/upstream_oauth2/provider.rs index 3a71c03c3..ed4e7b644 100644 --- a/crates/data-model/src/upstream_oauth2/provider.rs +++ b/crates/data-model/src/upstream_oauth2/provider.rs @@ -323,6 +323,9 @@ pub struct ClaimsImports { #[serde(default)] pub account_name: SubjectPreference, + + #[serde(default)] + pub is_admin: ImportPreference, } // XXX: this should have another name diff --git a/crates/handlers/src/upstream_oauth2/callback.rs b/crates/handlers/src/upstream_oauth2/callback.rs index e368c9b72..82ee55ff1 100644 --- a/crates/handlers/src/upstream_oauth2/callback.rs +++ b/crates/handlers/src/upstream_oauth2/callback.rs @@ -27,6 +27,7 @@ use mas_storage::{ }, }; use mas_templates::{FormPostContext, Templates}; +use minijinja::Value; use oauth2_types::{errors::ClientErrorCode, requests::AccessTokenRequest}; use opentelemetry::{Key, KeyValue, metrics::Counter}; use serde::{Deserialize, Serialize}; @@ -458,7 +459,7 @@ pub(crate) async fn handler( .account_name .template .as_deref() - .and_then(|template| match env.render_str(template, context) { + .and_then(|template| match env.render_str(template, context.clone()) { Ok(name) => Some(name), Err(e) => { tracing::warn!( @@ -487,6 +488,20 @@ pub(crate) async fn handler( ) .await?; + // Try to set can_request_admin + if let Some(user_id) = link.user_id { + let is_admin = determine_admin_flag(&provider, &env, context) + .await + .unwrap_or(false); + let user = repo.user().lookup(user_id).await?; + + if let Some(user) = user { + repo.user() + .set_can_request_admin(user.clone(), is_admin) + .await?; + } + } + let cookie_jar = sessions_cookie .add_link_to_session(session.id, link.id)? .save(cookie_jar, &clock); @@ -499,3 +514,141 @@ pub(crate) async fn handler( ) .into_response()) } + +async fn determine_admin_flag( + provider: &UpstreamOAuthProvider, + env: &minijinja::Environment<'_>, + context: Value, +) -> Option { + provider + .claims_imports + .is_admin + .template + .as_deref() + .and_then(|template| match env.render_str(template, context) { + Ok(is_admin) => match is_admin.parse() { + Ok(is_admin) => Some(is_admin), + Err(e) => { + tracing::warn!( + error = &e as &dyn std::error::Error, + "Failed to parse is_admin" + ); + None + } + }, + Err(e) => { + tracing::warn!( + error = &e as &dyn std::error::Error, + "Failed to render is_admin" + ); + None + } + }) +} + +#[cfg(test)] +mod tests { + use std::collections::HashMap; + + use mas_data_model::{ + UpstreamOAuthProviderClaimsImports, UpstreamOAuthProviderImportAction, + UpstreamOAuthProviderImportPreference, UpstreamOAuthProviderOnBackchannelLogout, + UpstreamOAuthProviderTokenAuthMethod, + }; + use mas_iana::jose::JsonWebSignatureAlg; + use minijinja::Environment; + + use super::*; + + #[tokio::test] + async fn test_determine_admin_flag() { + let provider = UpstreamOAuthProvider { + id: Default::default(), + issuer: None, + human_name: None, + brand_name: None, + discovery_mode: Default::default(), + pkce_mode: Default::default(), + jwks_uri_override: None, + authorization_endpoint_override: None, + scope: "openid profile".parse().unwrap(), + token_endpoint_override: None, + userinfo_endpoint_override: None, + fetch_userinfo: false, + userinfo_signed_response_alg: None, + client_id: "".to_string(), + encrypted_client_secret: None, + token_endpoint_signing_alg: None, + token_endpoint_auth_method: UpstreamOAuthProviderTokenAuthMethod::None, + id_token_signed_response_alg: JsonWebSignatureAlg::Hs256, + response_mode: None, + created_at: Default::default(), + disabled_at: None, + claims_imports: UpstreamOAuthProviderClaimsImports { + is_admin: UpstreamOAuthProviderImportPreference { + action: UpstreamOAuthProviderImportAction::Force, + template: Some("{{ user.is_admin }}".to_string()), + }, + ..Default::default() + }, + additional_authorization_parameters: vec![], + forward_login_hint: false, + on_backchannel_logout: UpstreamOAuthProviderOnBackchannelLogout::DoNothing, + }; + + let env = Environment::new(); + + let mut id_token_claims = HashMap::new(); + + // Test with is_admin set to true + id_token_claims.insert("is_admin".to_owned(), serde_json::Value::Bool(true)); + + let context = AttributeMappingContext::new() + .with_id_token_claims(id_token_claims.clone()) + .build(); + + let result = determine_admin_flag(&provider, &env, context) + .await + .unwrap(); + assert!(result); + + id_token_claims.clear(); + + // Test with is_admin set to false + id_token_claims.insert("is_admin".to_owned(), serde_json::Value::Bool(false)); + + let context = AttributeMappingContext::new() + .with_id_token_claims(id_token_claims.clone()) + .build(); + + let result = determine_admin_flag(&provider, &env, context) + .await + .unwrap(); + assert!(!result); + + id_token_claims.clear(); + + // Test with invalid admin field set to true + id_token_claims.insert( + "can_request_admin".to_owned(), + serde_json::Value::Bool(true), + ); + + let context = AttributeMappingContext::new() + .with_id_token_claims(id_token_claims.clone()) + .build(); + + let result = determine_admin_flag(&provider, &env, context).await; + assert!(result.is_none()); + + id_token_claims.clear(); + + // Test with no claims + let context = AttributeMappingContext::new() + .with_id_token_claims(id_token_claims.clone()) + .build(); + + let result = determine_admin_flag(&provider, &env, context).await; + assert!(result.is_none()); + } +} diff --git a/docs/config.schema.json b/docs/config.schema.json index abb811fca..242fb18ae 100644 --- a/docs/config.schema.json +++ b/docs/config.schema.json @@ -2333,6 +2333,14 @@ "$ref": "#/definitions/AccountNameImportPreference" } ] + }, + "is_admin": { + "description": "Import the `can_request_admin` attribute of the user based on the defined claim (i.e. `is_admin`)", + "allOf": [ + { + "$ref": "#/definitions/IsAdminImportPreference" + } + ] } } }, @@ -2443,6 +2451,24 @@ } } }, + "IsAdminImportPreference": { + "description": "What should be done for the `ìs_admin` attribute", + "type": "object", + "properties": { + "action": { + "description": "How to handle the claim", + "allOf": [ + { + "$ref": "#/definitions/ImportAction" + } + ] + }, + "template": { + "description": "The Jinja2 template to use for the admin attribute.\n\nIf not provided, it will be ignored.", + "type": "string" + } + } + }, "OnBackchannelLogout": { "description": "What to do when receiving an OIDC Backchannel logout request.", "oneOf": [ diff --git a/docs/reference/configuration.md b/docs/reference/configuration.md index 4dad3d6a0..15bba8440 100644 --- a/docs/reference/configuration.md +++ b/docs/reference/configuration.md @@ -800,6 +800,11 @@ upstream_oauth2: # This helps end user identify what account they are using account_name: #template: "@{{ user.preferred_username }}" + + # Whether the user is an admin and can request the admin scope + is_admin: + #action: force + #template: "{{ user.is_admin }}" ``` ## `experimental` From ff3849e878799e8bd82fc9cd7082743036f16d1b Mon Sep 17 00:00:00 2001 From: fl0lli Date: Fri, 18 Jul 2025 19:22:35 +0200 Subject: [PATCH 2/4] test: extend test cases for determine_admin_flag Signed-off-by: fl0lli --- .../handlers/src/upstream_oauth2/callback.rs | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/crates/handlers/src/upstream_oauth2/callback.rs b/crates/handlers/src/upstream_oauth2/callback.rs index 82ee55ff1..613c99adb 100644 --- a/crates/handlers/src/upstream_oauth2/callback.rs +++ b/crates/handlers/src/upstream_oauth2/callback.rs @@ -650,5 +650,54 @@ mod tests { let result = determine_admin_flag(&provider, &env, context).await; assert!(result.is_none()); + + id_token_claims.clear(); + + // Test with String value type + id_token_claims.insert( + "is_admin".to_owned(), + serde_json::Value::String("true".to_owned()), + ); + + let context = AttributeMappingContext::new() + .with_id_token_claims(id_token_claims.clone()) + .build(); + + let result = determine_admin_flag(&provider, &env, context) + .await + .unwrap(); + assert!(result); + + id_token_claims.clear(); + + // Test with String value type + id_token_claims.insert( + "is_admin".to_owned(), + serde_json::Value::String("false".to_owned()), + ); + + let context = AttributeMappingContext::new() + .with_id_token_claims(id_token_claims.clone()) + .build(); + + let result = determine_admin_flag(&provider, &env, context) + .await + .unwrap(); + assert!(!result); + + id_token_claims.clear(); + + // Test with invalid value + id_token_claims.insert( + "is_admin".to_owned(), + serde_json::Value::String("something".to_owned()), + ); + + let context = AttributeMappingContext::new() + .with_id_token_claims(id_token_claims.clone()) + .build(); + + let result = determine_admin_flag(&provider, &env, context).await; + assert!(result.is_none()); } } From ac6c9cb2de019f6362d2a7b072d100d3e9d08a9d Mon Sep 17 00:00:00 2001 From: fl0lli Date: Fri, 18 Jul 2025 19:48:31 +0200 Subject: [PATCH 3/4] test: create tests using wiremock server for testing can_request_admin feature of callback handler Signed-off-by: fl0lli --- .../handlers/src/upstream_oauth2/callback.rs | 627 +++++++++++++++++- 1 file changed, 620 insertions(+), 7 deletions(-) diff --git a/crates/handlers/src/upstream_oauth2/callback.rs b/crates/handlers/src/upstream_oauth2/callback.rs index 613c99adb..d79fdde03 100644 --- a/crates/handlers/src/upstream_oauth2/callback.rs +++ b/crates/handlers/src/upstream_oauth2/callback.rs @@ -548,17 +548,32 @@ async fn determine_admin_flag( #[cfg(test)] mod tests { - use std::collections::HashMap; - + use super::*; + use crate::test_utils::{setup, CookieHelper, RequestBuilderExt, ResponseExt, TestState}; + use axum::http::Request; + use axum::Json; + use chrono::Duration; use mas_data_model::{ - UpstreamOAuthProviderClaimsImports, UpstreamOAuthProviderImportAction, - UpstreamOAuthProviderImportPreference, UpstreamOAuthProviderOnBackchannelLogout, - UpstreamOAuthProviderTokenAuthMethod, + UpstreamOAuthAuthorizationSession, UpstreamOAuthProviderClaimsImports, + UpstreamOAuthProviderImportAction, UpstreamOAuthProviderImportPreference, + UpstreamOAuthProviderOnBackchannelLogout, UpstreamOAuthProviderTokenAuthMethod, User, }; use mas_iana::jose::JsonWebSignatureAlg; + use mas_iana::oauth::OAuthAccessTokenType; + use mas_jose::claims; + use mas_jose::constraints::Constrainable; + use mas_jose::jwt::{JsonWebSignatureHeader, Jwt}; + use mas_router::Route; + use mas_storage::upstream_oauth2::UpstreamOAuthProviderParams; + use mas_storage::user::UserRepository; + use mas_storage::RepositoryAccess; use minijinja::Environment; - - use super::*; + use oauth2_types::scope::{Scope, OPENID}; + use sqlx::PgPool; + use std::collections::HashMap; + use url::Url; + use wiremock::matchers::{method, path}; + use wiremock::{Mock, MockServer, ResponseTemplate}; #[tokio::test] async fn test_determine_admin_flag() { @@ -700,4 +715,602 @@ mod tests { let result = determine_admin_flag(&provider, &env, context).await; assert!(result.is_none()); } + + async fn setup_mock_server(state: &TestState, id_token: String, mock_server: &MockServer) { + // Set up the mock server to respond to the token endpoint + let _mock_guard = Mock::given(method("POST")) + .and(path("/token")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "access_token": "test_access_token", + "token_type": OAuthAccessTokenType::Bearer, + "expires_in": 3600, + "scope": "openid profile", + "id_token": id_token, + }))) + .mount(&mock_server) + .await; + + // Set up the mock server to respond to the jwks endpoint + // For test purposes re-use the JWKs MAS uses as the JWKs an upstream IDP would publish + let jwks = Json(state.key_store.public_jwks()); + let _mock_guard = Mock::given(method("GET")) + .and(path("/jwks")) + .respond_with(ResponseTemplate::new(200).set_body_json(jwks.0)) + .mount(&mock_server) + .await; + } + + async fn setup_user_with_session( + state: &TestState, + repo: &mut BoxRepository, + provider: &UpstreamOAuthProvider, + subject: String, + ) -> (User, UpstreamOAuthAuthorizationSession) { + // Create the user using the subject + let user = repo + .user() + .add(&mut state.rng(), &state.clock, subject.to_owned()) + .await + .unwrap(); + + // Create OAuth Link attached to the subject + let link = repo + .upstream_oauth_link() + .add( + &mut state.rng(), + &state.clock, + &provider, + subject.to_owned(), + None, + ) + .await + .unwrap(); + + // Link the OAuth Link to the created user + repo.upstream_oauth_link() + .associate_to_user(&link, &user) + .await + .unwrap(); + + let session = repo + .upstream_oauth_session() + .add( + &mut state.rng(), + &state.clock, + &provider, + "teststate".to_owned(), + None, + None, + ) + .await + .unwrap(); + + (user, session) + } + + fn create_id_token( + state: &TestState, + issuer: String, + subject: String, + client_id: String, + additional_claims: HashMap, + ) -> String { + let mut id_token_claims = HashMap::new(); + + // Create the default claims for the ID token + claims::ISS.insert(&mut id_token_claims, issuer).unwrap(); + claims::SUB.insert(&mut id_token_claims, subject).unwrap(); + claims::AUD.insert(&mut id_token_claims, client_id).unwrap(); + + let issued_at = state.clock.now(); + claims::IAT.insert(&mut id_token_claims, issued_at).unwrap(); + + let expires = issued_at + Duration::try_hours(1).unwrap(); + claims::EXP.insert(&mut id_token_claims, expires).unwrap(); + + // Add additional claims an upstream IDP may have provided + for (key, value) in additional_claims { + id_token_claims.insert(key, value); + } + + // Create the ID token and sign it + let key = state + .key_store + .signing_key_for_algorithm(&JsonWebSignatureAlg::Rs256) + .unwrap(); + let signer = key + .params() + .signing_key_for_alg(&JsonWebSignatureAlg::Rs256) + .unwrap(); + let header = + JsonWebSignatureHeader::new(JsonWebSignatureAlg::Rs256).with_kid(key.kid().unwrap()); + Jwt::sign_with_rng(&mut state.rng(), header, id_token_claims, &signer) + .unwrap() + .into_string() + } + + async fn create_provider( + state: &TestState, + repo: &mut BoxRepository, + issuer: String, + client_id: String, + claims_imports: UpstreamOAuthProviderClaimsImports, + ) -> UpstreamOAuthProvider { + let issuer_url = Url::parse(&*issuer).unwrap(); + + // Create an upstream IDP + repo.upstream_oauth_provider() + .add( + &mut state.rng(), + &state.clock, + UpstreamOAuthProviderParams { + issuer: Some(issuer.clone()), + human_name: Some("Example Ltd.".to_owned()), + brand_name: None, + scope: Scope::from_iter([OPENID]), + token_endpoint_auth_method: UpstreamOAuthProviderTokenAuthMethod::None, + token_endpoint_signing_alg: None, + id_token_signed_response_alg: JsonWebSignatureAlg::Rs256, + client_id: client_id.to_owned(), + encrypted_client_secret: None, + claims_imports, + authorization_endpoint_override: None, + token_endpoint_override: Some(issuer_url.join("token").unwrap()), + userinfo_endpoint_override: None, + fetch_userinfo: false, + userinfo_signed_response_alg: None, + jwks_uri_override: Some(issuer_url.join("jwks").unwrap()), + discovery_mode: mas_data_model::UpstreamOAuthProviderDiscoveryMode::Disabled, + pkce_mode: mas_data_model::UpstreamOAuthProviderPkceMode::Auto, + response_mode: None, + additional_authorization_parameters: Vec::new(), + forward_login_hint: false, + ui_order: 0, + on_backchannel_logout: UpstreamOAuthProviderOnBackchannelLogout::DoNothing, + }, + ) + .await + .unwrap() + } + + #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")] + async fn test_upstream_oauth2_callback_user_can_request_admin_is_admin(pool: PgPool) { + setup(); + + let state = TestState::from_pool(pool).await.unwrap(); + let mut repo = state.repository().await.unwrap(); + let cookies = CookieHelper::new(); + let cookie_jar = state.cookie_jar(); + + let mock_server = MockServer::start().await; + + let client_id = "test-client-id"; + let subject = "testuser123"; + + let mut additional_claims: HashMap = HashMap::new(); + additional_claims.insert("is_admin".to_owned(), serde_json::Value::Bool(true)); + + // Create an ID token with the is_admin claim set to true + let id_token = create_id_token( + &state, + mock_server.uri(), + subject.to_owned(), + client_id.to_owned(), + additional_claims, + ); + + setup_mock_server(&state, id_token, &mock_server).await; + + // Create the upstream OAuth provider with the is_admin claim import + let provider = create_provider( + &state, + &mut repo, + mock_server.uri(), + client_id.to_owned(), + UpstreamOAuthProviderClaimsImports { + is_admin: UpstreamOAuthProviderImportPreference { + action: UpstreamOAuthProviderImportAction::Force, + template: Some("{{ user.is_admin }}".to_string()), + }, + ..UpstreamOAuthProviderClaimsImports::default() + }, + ) + .await; + + let (user, session) = + setup_user_with_session(&state, &mut repo, &provider, subject.to_owned()).await; + // Assert the user was created with the expected username + assert_eq!(user.username, subject); + + // Assert that initially the user can not request admin + assert!(!user.can_request_admin); + + // Save the repository state + repo.save().await.unwrap(); + + // Set up the cookie jar for the session + let cookie_jar = UpstreamSessionsCookie::default() + .add(session.id, provider.id, session.state_str.clone(), None) + .save(cookie_jar, &state.clock); + cookies.import(cookie_jar); + + let callback_uri = format!( + "{}?state={}&code=testcode", + mas_router::UpstreamOAuth2Callback::new(provider.id).path(), + session.state_str, + ); + + // Request the callback handler with the session cookie + let request = cookies.with_cookies(Request::get(callback_uri).empty()); + let response = state.request(request).await; + + // Assert the response status is SEE_OTHER (303) + response.assert_status(StatusCode::SEE_OTHER); + + // Retrieve the changed user from the repository + let user = state + .repository() + .await + .unwrap() + .user() + .lookup(user.id) + .await + .unwrap() + .unwrap(); + + // Assert that the user can now request admin access + assert!(user.can_request_admin); + } + + #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")] + async fn test_upstream_oauth2_callback_user_can_request_admin_is_not_admin(pool: PgPool) { + setup(); + + let state = TestState::from_pool(pool).await.unwrap(); + let mut repo = state.repository().await.unwrap(); + let cookies = CookieHelper::new(); + let cookie_jar = state.cookie_jar(); + + let mock_server = MockServer::start().await; + + let client_id = "test-client-id"; + let subject = "testuser123"; + + let mut additional_claims: HashMap = HashMap::new(); + additional_claims.insert("is_admin".to_owned(), serde_json::Value::Bool(false)); + + // Create an ID token with the is_admin claim set to true + let id_token = create_id_token( + &state, + mock_server.uri(), + subject.to_owned(), + client_id.to_owned(), + additional_claims, + ); + + setup_mock_server(&state, id_token, &mock_server).await; + + // Create the upstream OAuth provider with the is_admin claim import + let provider = create_provider( + &state, + &mut repo, + mock_server.uri(), + client_id.to_owned(), + UpstreamOAuthProviderClaimsImports { + is_admin: UpstreamOAuthProviderImportPreference { + action: UpstreamOAuthProviderImportAction::Force, + template: Some("{{ user.is_admin }}".to_string()), + }, + ..UpstreamOAuthProviderClaimsImports::default() + }, + ) + .await; + + let (user, session) = + setup_user_with_session(&state, &mut repo, &provider, subject.to_owned()).await; + // Assert the user was created with the expected username + assert_eq!(user.username, subject); + + // Assert that initially the user can not request admin + assert!(!user.can_request_admin); + + // Save the repository state + repo.save().await.unwrap(); + + // Set up the cookie jar for the session + let cookie_jar = UpstreamSessionsCookie::default() + .add(session.id, provider.id, session.state_str.clone(), None) + .save(cookie_jar, &state.clock); + cookies.import(cookie_jar); + + let callback_uri = format!( + "{}?state={}&code=testcode", + mas_router::UpstreamOAuth2Callback::new(provider.id).path(), + session.state_str, + ); + + // Request the callback handler with the session cookie + let request = cookies.with_cookies(Request::get(callback_uri).empty()); + let response = state.request(request).await; + + // Assert the response status is SEE_OTHER (303) + response.assert_status(StatusCode::SEE_OTHER); + + // Retrieve the changed user from the repository + let user = state + .repository() + .await + .unwrap() + .user() + .lookup(user.id) + .await + .unwrap() + .unwrap(); + + // Assert that the user can now request admin access + assert!(!user.can_request_admin); + } + + #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")] + async fn test_upstream_oauth2_callback_user_can_request_admin_default(pool: PgPool) { + setup(); + + let state = TestState::from_pool(pool).await.unwrap(); + let mut repo = state.repository().await.unwrap(); + let cookies = CookieHelper::new(); + let cookie_jar = state.cookie_jar(); + + let mock_server = MockServer::start().await; + + let client_id = "test-client-id"; + let subject = "testuser123"; + + let mut additional_claims: HashMap = HashMap::new(); + additional_claims.insert("is_admin".to_owned(), serde_json::Value::Bool(true)); + + // Create an ID token with the is_admin claim set to true + let id_token = create_id_token( + &state, + mock_server.uri(), + subject.to_owned(), + client_id.to_owned(), + additional_claims, + ); + + setup_mock_server(&state, id_token, &mock_server).await; + + // Create the upstream OAuth provider with the is_admin claim import + let provider = create_provider( + &state, + &mut repo, + mock_server.uri(), + client_id.to_owned(), + UpstreamOAuthProviderClaimsImports { + ..UpstreamOAuthProviderClaimsImports::default() + }, + ) + .await; + + let (user, session) = + setup_user_with_session(&state, &mut repo, &provider, subject.to_owned()).await; + // Assert the user was created with the expected username + assert_eq!(user.username, subject); + + // Assert that initially the user can not request admin + assert!(!user.can_request_admin); + + // Save the repository state + repo.save().await.unwrap(); + + // Set up the cookie jar for the session + let cookie_jar = UpstreamSessionsCookie::default() + .add(session.id, provider.id, session.state_str.clone(), None) + .save(cookie_jar, &state.clock); + cookies.import(cookie_jar); + + let callback_uri = format!( + "{}?state={}&code=testcode", + mas_router::UpstreamOAuth2Callback::new(provider.id).path(), + session.state_str, + ); + + // Request the callback handler with the session cookie + let request = cookies.with_cookies(Request::get(callback_uri).empty()); + let response = state.request(request).await; + + // Assert the response status is SEE_OTHER (303) + response.assert_status(StatusCode::SEE_OTHER); + + // Retrieve the changed user from the repository + let user = state + .repository() + .await + .unwrap() + .user() + .lookup(user.id) + .await + .unwrap() + .unwrap(); + + // Assert that the user can now request admin access + assert!(!user.can_request_admin); + } + + #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")] + async fn test_upstream_oauth2_callback_user_can_request_admin_invalid_config_claim(pool: PgPool) { + setup(); + + let state = TestState::from_pool(pool).await.unwrap(); + let mut repo = state.repository().await.unwrap(); + let cookies = CookieHelper::new(); + let cookie_jar = state.cookie_jar(); + + let mock_server = MockServer::start().await; + + let client_id = "test-client-id"; + let subject = "testuser123"; + + let mut additional_claims: HashMap = HashMap::new(); + additional_claims.insert("is_admin".to_owned(), serde_json::Value::Bool(true)); + + // Create an ID token with the is_admin claim set to true + let id_token = create_id_token( + &state, + mock_server.uri(), + subject.to_owned(), + client_id.to_owned(), + additional_claims, + ); + + setup_mock_server(&state, id_token, &mock_server).await; + + // Create the upstream OAuth provider with the is_admin claim import + let provider = create_provider( + &state, + &mut repo, + mock_server.uri(), + client_id.to_owned(), + UpstreamOAuthProviderClaimsImports { + is_admin: UpstreamOAuthProviderImportPreference { + action: UpstreamOAuthProviderImportAction::Force, + template: Some("{{ user.some_random_value }}".to_string()), + }, + ..UpstreamOAuthProviderClaimsImports::default() + }, + ) + .await; + + let (user, session) = + setup_user_with_session(&state, &mut repo, &provider, subject.to_owned()).await; + // Assert the user was created with the expected username + assert_eq!(user.username, subject); + + // Assert that initially the user can not request admin + assert!(!user.can_request_admin); + + // Save the repository state + repo.save().await.unwrap(); + + // Set up the cookie jar for the session + let cookie_jar = UpstreamSessionsCookie::default() + .add(session.id, provider.id, session.state_str.clone(), None) + .save(cookie_jar, &state.clock); + cookies.import(cookie_jar); + + let callback_uri = format!( + "{}?state={}&code=testcode", + mas_router::UpstreamOAuth2Callback::new(provider.id).path(), + session.state_str, + ); + + // Request the callback handler with the session cookie + let request = cookies.with_cookies(Request::get(callback_uri).empty()); + let response = state.request(request).await; + + // Assert the response status is SEE_OTHER (303) + response.assert_status(StatusCode::SEE_OTHER); + + // Retrieve the changed user from the repository + let user = state + .repository() + .await + .unwrap() + .user() + .lookup(user.id) + .await + .unwrap() + .unwrap(); + + // Assert that the user can now request admin access + assert!(!user.can_request_admin); + } + + #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")] + async fn test_upstream_oauth2_callback_user_can_request_admin_invalid_upstream_claim(pool: PgPool) { + setup(); + + let state = TestState::from_pool(pool).await.unwrap(); + let mut repo = state.repository().await.unwrap(); + let cookies = CookieHelper::new(); + let cookie_jar = state.cookie_jar(); + + let mock_server = MockServer::start().await; + + let client_id = "test-client-id"; + let subject = "testuser123"; + + let mut additional_claims: HashMap = HashMap::new(); + additional_claims.insert("is_this_user_admin".to_owned(), serde_json::Value::Bool(true)); + + // Create an ID token with the is_admin claim set to true + let id_token = create_id_token( + &state, + mock_server.uri(), + subject.to_owned(), + client_id.to_owned(), + additional_claims, + ); + + setup_mock_server(&state, id_token, &mock_server).await; + + // Create the upstream OAuth provider with the is_admin claim import + let provider = create_provider( + &state, + &mut repo, + mock_server.uri(), + client_id.to_owned(), + UpstreamOAuthProviderClaimsImports { + is_admin: UpstreamOAuthProviderImportPreference { + action: UpstreamOAuthProviderImportAction::Force, + template: Some("{{ user.is_admin }}".to_string()), + }, + ..UpstreamOAuthProviderClaimsImports::default() + }, + ) + .await; + + let (user, session) = + setup_user_with_session(&state, &mut repo, &provider, subject.to_owned()).await; + // Assert the user was created with the expected username + assert_eq!(user.username, subject); + + // Assert that initially the user can not request admin + assert!(!user.can_request_admin); + + // Save the repository state + repo.save().await.unwrap(); + + // Set up the cookie jar for the session + let cookie_jar = UpstreamSessionsCookie::default() + .add(session.id, provider.id, session.state_str.clone(), None) + .save(cookie_jar, &state.clock); + cookies.import(cookie_jar); + + let callback_uri = format!( + "{}?state={}&code=testcode", + mas_router::UpstreamOAuth2Callback::new(provider.id).path(), + session.state_str, + ); + + // Request the callback handler with the session cookie + let request = cookies.with_cookies(Request::get(callback_uri).empty()); + let response = state.request(request).await; + + // Assert the response status is SEE_OTHER (303) + response.assert_status(StatusCode::SEE_OTHER); + + // Retrieve the changed user from the repository + let user = state + .repository() + .await + .unwrap() + .user() + .lookup(user.id) + .await + .unwrap() + .unwrap(); + + // Assert that the user can now request admin access + assert!(!user.can_request_admin); + } } From 3f5ec23c8a84b3a983f3f4e33d0f1558dfab7b27 Mon Sep 17 00:00:00 2001 From: fl0lli Date: Fri, 18 Jul 2025 19:49:06 +0200 Subject: [PATCH 4/4] chore: formatting Signed-off-by: fl0lli --- .../handlers/src/upstream_oauth2/callback.rs | 60 +++++++++++-------- 1 file changed, 36 insertions(+), 24 deletions(-) diff --git a/crates/handlers/src/upstream_oauth2/callback.rs b/crates/handlers/src/upstream_oauth2/callback.rs index d79fdde03..9320ccda5 100644 --- a/crates/handlers/src/upstream_oauth2/callback.rs +++ b/crates/handlers/src/upstream_oauth2/callback.rs @@ -548,32 +548,36 @@ async fn determine_admin_flag( #[cfg(test)] mod tests { - use super::*; - use crate::test_utils::{setup, CookieHelper, RequestBuilderExt, ResponseExt, TestState}; - use axum::http::Request; - use axum::Json; + use std::collections::HashMap; + + use axum::{Json, http::Request}; use chrono::Duration; use mas_data_model::{ UpstreamOAuthAuthorizationSession, UpstreamOAuthProviderClaimsImports, UpstreamOAuthProviderImportAction, UpstreamOAuthProviderImportPreference, UpstreamOAuthProviderOnBackchannelLogout, UpstreamOAuthProviderTokenAuthMethod, User, }; - use mas_iana::jose::JsonWebSignatureAlg; - use mas_iana::oauth::OAuthAccessTokenType; - use mas_jose::claims; - use mas_jose::constraints::Constrainable; - use mas_jose::jwt::{JsonWebSignatureHeader, Jwt}; + use mas_iana::{jose::JsonWebSignatureAlg, oauth::OAuthAccessTokenType}; + use mas_jose::{ + claims, + constraints::Constrainable, + jwt::{JsonWebSignatureHeader, Jwt}, + }; use mas_router::Route; - use mas_storage::upstream_oauth2::UpstreamOAuthProviderParams; - use mas_storage::user::UserRepository; - use mas_storage::RepositoryAccess; + use mas_storage::{ + RepositoryAccess, upstream_oauth2::UpstreamOAuthProviderParams, user::UserRepository, + }; use minijinja::Environment; - use oauth2_types::scope::{Scope, OPENID}; + use oauth2_types::scope::{OPENID, Scope}; use sqlx::PgPool; - use std::collections::HashMap; use url::Url; - use wiremock::matchers::{method, path}; - use wiremock::{Mock, MockServer, ResponseTemplate}; + use wiremock::{ + Mock, MockServer, ResponseTemplate, + matchers::{method, path}, + }; + + use super::*; + use crate::test_utils::{CookieHelper, RequestBuilderExt, ResponseExt, TestState, setup}; #[tokio::test] async fn test_determine_admin_flag() { @@ -731,7 +735,8 @@ mod tests { .await; // Set up the mock server to respond to the jwks endpoint - // For test purposes re-use the JWKs MAS uses as the JWKs an upstream IDP would publish + // For test purposes re-use the JWKs MAS uses as the JWKs an upstream IDP would + // publish let jwks = Json(state.key_store.public_jwks()); let _mock_guard = Mock::given(method("GET")) .and(path("/jwks")) @@ -1004,7 +1009,7 @@ mod tests { ..UpstreamOAuthProviderClaimsImports::default() }, ) - .await; + .await; let (user, session) = setup_user_with_session(&state, &mut repo, &provider, subject.to_owned()).await; @@ -1089,7 +1094,7 @@ mod tests { ..UpstreamOAuthProviderClaimsImports::default() }, ) - .await; + .await; let (user, session) = setup_user_with_session(&state, &mut repo, &provider, subject.to_owned()).await; @@ -1137,7 +1142,9 @@ mod tests { } #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")] - async fn test_upstream_oauth2_callback_user_can_request_admin_invalid_config_claim(pool: PgPool) { + async fn test_upstream_oauth2_callback_user_can_request_admin_invalid_config_claim( + pool: PgPool, + ) { setup(); let state = TestState::from_pool(pool).await.unwrap(); @@ -1178,7 +1185,7 @@ mod tests { ..UpstreamOAuthProviderClaimsImports::default() }, ) - .await; + .await; let (user, session) = setup_user_with_session(&state, &mut repo, &provider, subject.to_owned()).await; @@ -1226,7 +1233,9 @@ mod tests { } #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")] - async fn test_upstream_oauth2_callback_user_can_request_admin_invalid_upstream_claim(pool: PgPool) { + async fn test_upstream_oauth2_callback_user_can_request_admin_invalid_upstream_claim( + pool: PgPool, + ) { setup(); let state = TestState::from_pool(pool).await.unwrap(); @@ -1240,7 +1249,10 @@ mod tests { let subject = "testuser123"; let mut additional_claims: HashMap = HashMap::new(); - additional_claims.insert("is_this_user_admin".to_owned(), serde_json::Value::Bool(true)); + additional_claims.insert( + "is_this_user_admin".to_owned(), + serde_json::Value::Bool(true), + ); // Create an ID token with the is_admin claim set to true let id_token = create_id_token( @@ -1267,7 +1279,7 @@ mod tests { ..UpstreamOAuthProviderClaimsImports::default() }, ) - .await; + .await; let (user, session) = setup_user_with_session(&state, &mut repo, &provider, subject.to_owned()).await;