diff --git a/crates/core-types/src/auth.rs b/crates/core-types/src/auth.rs index 0a9868d38..87b743a35 100644 --- a/crates/core-types/src/auth.rs +++ b/crates/core-types/src/auth.rs @@ -769,13 +769,7 @@ impl SecurityContext { .authorization .as_ref() .ok_or(AuthenticationError::SecurityContextNotResolved)?; - // Unscoped with no roles is valid. Scoped with no roles OR empty roles list is - // not. - if !matches!(authz.scope, ScopeInfo::Unscoped) - && authz.roles.as_ref().is_none_or(|r| r.is_empty()) - { - return Err(AuthenticationError::SecurityContextNotResolved); - } + Ok(()) } } @@ -2134,24 +2128,6 @@ mod tests { assert!(ctx.fully_resolved().is_ok()); } - #[test] - fn test_fully_resolved_scoped_none_roles() { - let ctx = make_auth_result_project(make_principal("uid"), make_project(), None); - assert!(matches!( - ctx.fully_resolved(), - Err(AuthenticationError::SecurityContextNotResolved) - )); - } - - #[test] - fn test_fully_resolved_scoped_empty_roles() { - let ctx = make_auth_result_project(make_principal("uid"), make_project(), Some(vec![])); - assert!(matches!( - ctx.fully_resolved(), - Err(AuthenticationError::SecurityContextNotResolved) - )); - } - #[test] fn test_fully_resolved_scoped_with_roles() { let ctx = make_auth_result_project( @@ -2168,30 +2144,12 @@ mod tests { assert!(ctx.fully_resolved().is_ok()); } - #[test] - fn test_fully_resolved_system_none_roles() { - let ctx = make_auth_result_system(make_principal("uid"), None); - assert!(matches!( - ctx.fully_resolved(), - Err(AuthenticationError::SecurityContextNotResolved) - )); - } - #[test] fn test_fully_resolved_domain_with_roles() { let ctx = make_auth_result_domain(make_principal("uid"), Some(vec![admin_role()])); assert!(ctx.fully_resolved().is_ok()); } - #[test] - fn test_fully_resolved_domain_none_roles() { - let ctx = make_auth_result_domain(make_principal("uid"), None); - assert!(matches!( - ctx.fully_resolved(), - Err(AuthenticationError::SecurityContextNotResolved) - )); - } - #[test] fn test_try_from_auth_to_security_context() { let ctx = make_auth_result_project( @@ -2589,24 +2547,6 @@ mod tests { assert!(ctx.fully_resolved().is_ok()); } - #[test] - fn test_fully_resolved_trust_none_roles() { - let ctx = make_trust_with_roles(None); - assert!(matches!( - ctx.fully_resolved(), - Err(AuthenticationError::SecurityContextNotResolved) - )); - } - - #[test] - fn test_fully_resolved_trust_empty_roles() { - let ctx = make_trust_with_roles(Some(vec![])); - assert!(matches!( - ctx.fully_resolved(), - Err(AuthenticationError::SecurityContextNotResolved) - )); - } - // --- FernetToken audit_ids propagation --- #[test] diff --git a/crates/core/src/auth.rs b/crates/core/src/auth.rs index 10c8f0b1f..fcde50cac 100644 --- a/crates/core/src/auth.rs +++ b/crates/core/src/auth.rs @@ -201,6 +201,13 @@ impl ValidatedSecurityContext { ctx.set_effective_roles(role_vec); } + if let Some(authz) = ctx.authorization() { + if !matches!(authz.scope, ScopeInfo::Unscoped) + && authz.effective_roles().is_none_or(|r| r.is_empty()) + { + return Err(AuthenticationError::ActorHasNoRolesOnTarget); + } + } Ok(ValidatedSecurityContext(ctx)) } @@ -526,8 +533,9 @@ mod tests { Assignment, AssignmentProviderError, AssignmentType, RoleAssignmentListParameters, }; use openstack_keystone_core_types::auth::{ - AuthenticationContext, IdentityInfo, PrincipalIdentityInfo, PrincipalInfo, ScopeInfo, - SecurityContextTestingBuilder, TrustProjectInfo, UserIdentityInfo, + AuthenticationContext, AuthzInfoBuilder, IdentityInfo, PrincipalIdentityInfo, + PrincipalInfo, ScopeInfo, SecurityContextTestingBuilder, TrustProjectInfo, + UserIdentityInfo, }; use openstack_keystone_core_types::identity::{UserOptions, UserResponse}; use openstack_keystone_core_types::resource::Project; @@ -1016,7 +1024,7 @@ mod tests { && q.project_id.as_deref() == Some(pid) && q.effective == Some(true) }) - .returning(move |_state, _q| Ok(vec![assignment_with_role(rid1)])); + .returning(move |_state, _q| Ok(vec![assignment_with_role("rid1")])); let state = get_mocked_state( None, Some(Provider::mocked_builder().mock_assignment(assignment_mock)), @@ -1703,6 +1711,43 @@ mod tests { )); } + #[tokio::test] + async fn test_new_for_scope_explicit_empty_roles_error() { + let uid = "uid"; + let pid = "pid"; + let mut assignment_mock = MockAssignmentProvider::default(); + assignment_mock + .expect_list_role_assignments() + .withf(|_, q: &RoleAssignmentListParameters| { + q.user_id.as_deref() == Some(uid) + && q.project_id.as_deref() == Some(pid) + && q.effective == Some(true) + && q.include_names == Some(false) + }) + .returning(move |_state, _q| Ok(vec![])); + let state = get_mocked_state( + None, + Some(Provider::mocked_builder().mock_assignment(assignment_mock)), + ) + .await; + let authz = AuthzInfoBuilder::default() + .scope(make_project_scope(pid)) + .roles(Vec::new()) + .build() + .unwrap(); + let ctx = SecurityContextTestingBuilder::default() + .authentication_context(AuthenticationContext::Password) + .principal(make_user_identity(uid)) + .authorization(authz) + .build(); + let result = + ValidatedSecurityContext::new_for_scope(ctx, make_project_scope(pid), &state).await; + assert!(matches!( + result, + Err(AuthenticationError::ActorHasNoRolesOnTarget) + )); + } + // --- Trust scope: expand_implied_roles adds an implied role, trustor has it // --- #[tokio::test] @@ -2276,4 +2321,144 @@ mod tests { Err(AuthenticationError::ActorHasNoRolesOnTarget) )); } + + // Unscoped scope must succeed with zero effective roles. The assignment + // provider must never be called (no mock needed). + #[tokio::test] + async fn test_new_for_scope_unscoped_success() { + let state = get_mocked_state(None, None).await; + // Build a context that already carries an Unscoped authorization so the + // scope-boundary check is skipped entirely (scopes are equal). + let authz = AuthzInfoBuilder::default() + .scope(ScopeInfo::Unscoped) + .roles(Vec::new()) + .build() + .unwrap(); + let ctx = SecurityContextTestingBuilder::default() + .authentication_context(AuthenticationContext::Password) + .principal(make_user_identity("uid")) + .authorization(authz) + .build(); + + let result = + ValidatedSecurityContext::new_for_scope(ctx, ScopeInfo::Unscoped, &state).await; + + // Must succeed and carry zero effective roles — the Unscoped path in + // calculate_effective_roles returns Vec::new() and skips the + // ActorHasNoRolesOnTarget guard. + let validated = result.unwrap(); + + // Access the roles via the authorization state getter + let roles = validated.0.authorization().unwrap().effective_roles(); + assert!(roles.is_none() || roles.unwrap().is_empty()); + } + + // Project-scoped context with a live assignment must succeed and surface + // exactly that one role as an effective role. + #[tokio::test] + async fn test_new_for_scope_project_scoped_success() { + let uid = "uid"; + let pid = "pid"; + let rid = "admin_role"; + + // Strict predicate: must be called once for this user+project combination + // with the exact flags used by resolve_project_default_roles. + let mut assignment_mock = MockAssignmentProvider::default(); + assignment_mock + .expect_list_role_assignments() + .withf(|_, q: &RoleAssignmentListParameters| { + q.user_id.as_deref() == Some(uid) + && q.project_id.as_deref() == Some(pid) + && q.effective == Some(true) + && q.include_names == Some(false) + && q.domain_id.is_none() + && q.system_id.is_none() + }) + .returning(move |_state, _q| Ok(vec![assignment_with_role(rid)])); + + let state = get_mocked_state( + None, + Some(Provider::mocked_builder().mock_assignment(assignment_mock)), + ) + .await; + + // Pre-set the same project scope so the boundary check is skipped. + let authz = AuthzInfoBuilder::default() + .scope(make_project_scope(pid)) + .roles(Vec::new()) + .build() + .unwrap(); + let ctx = SecurityContextTestingBuilder::default() + .authentication_context(AuthenticationContext::Password) + .principal(make_user_identity(uid)) + .authorization(authz) + .build(); + + let result = + ValidatedSecurityContext::new_for_scope(ctx, make_project_scope(pid), &state).await; + + // Must succeed; effective roles must contain exactly the one role the + // assignment provider returned. + let validated = result.unwrap(); + + // Access the roles via the authorization state getter + let roles = validated + .0 + .authorization() + .unwrap() + .effective_roles() + .unwrap(); + assert_eq!(roles.len(), 1); + assert_eq!(roles[0].id, rid); + } + + // Project-scoped context where the assignment provider returns nothing must + // fail with ActorHasNoRolesOnTarget. + #[tokio::test] + async fn test_new_for_scope_project_scoped_no_roles_fails() { + let uid = "uid"; + let pid = "pid"; + + // Same strict predicate as Test 2 — the provider IS called but returns + // an empty list, triggering the ActorHasNoRolesOnTarget guard. + let mut assignment_mock = MockAssignmentProvider::default(); + assignment_mock + .expect_list_role_assignments() + .withf(|_, q: &RoleAssignmentListParameters| { + q.user_id.as_deref() == Some(uid) + && q.project_id.as_deref() == Some(pid) + && q.effective == Some(true) + && q.include_names == Some(false) + && q.domain_id.is_none() + && q.system_id.is_none() + }) + .returning(|_, _| Ok(Vec::::new())); + + let state = get_mocked_state( + None, + Some(Provider::mocked_builder().mock_assignment(assignment_mock)), + ) + .await; + + let authz = AuthzInfoBuilder::default() + .scope(make_project_scope(pid)) + .roles(Vec::new()) + .build() + .unwrap(); + let ctx = SecurityContextTestingBuilder::default() + .authentication_context(AuthenticationContext::Password) + .principal(make_user_identity(uid)) + .authorization(authz) + .build(); + + let result = + ValidatedSecurityContext::new_for_scope(ctx, make_project_scope(pid), &state).await; + + // calculate_effective_roles sees an empty, non-Unscoped result and must + // return ActorHasNoRolesOnTarget. + assert!(matches!( + result, + Err(AuthenticationError::ActorHasNoRolesOnTarget) + )); + } } diff --git a/crates/keystone/src/api/v3/auth/token/create.rs b/crates/keystone/src/api/v3/auth/token/create.rs index 5df0754ab..ef7f161ae 100644 --- a/crates/keystone/src/api/v3/auth/token/create.rs +++ b/crates/keystone/src/api/v3/auth/token/create.rs @@ -131,7 +131,7 @@ mod tests { use openstack_keystone_core_types::auth::*; use openstack_keystone_core_types::identity::UserPasswordAuthRequest; use openstack_keystone_core_types::resource::{Domain, DomainBuilder, Project}; - use openstack_keystone_core_types::token::ProjectScopePayload; + use openstack_keystone_core_types::token::{ProjectScopePayload, TokenProviderError}; use crate::api::v3::auth::token::types::*; use crate::assignment::MockAssignmentProvider; @@ -373,6 +373,160 @@ mod tests { assert_eq!(vec!["password"], res.token.methods); } + #[tokio::test] + #[traced_test] + async fn test_post_explicit_empty_roles_unauthorized() { + let config = Config::default(); + let project = Project { + id: "pid".into(), + domain_id: "pdid".into(), + enabled: true, + ..Default::default() + }; + let user_domain = Domain { + id: "user_domain_id".into(), + enabled: true, + ..Default::default() + }; + let project_domain = Domain { + id: "pdid".into(), + enabled: true, + ..Default::default() + }; + let mut assignment_mock = MockAssignmentProvider::default(); + assignment_mock + .expect_list_role_assignments() + .returning(|_, _| Ok(Vec::new())); + + let auth = AuthenticationResultBuilder::default() + .context(AuthenticationContext::Password) + .principal(PrincipalInfo { + identity: IdentityInfo::User( + UserIdentityInfoBuilder::default() + .user_id("uid") + .build() + .unwrap(), + ), + }) + .build() + .unwrap(); + + let mut identity_mock = MockIdentityProvider::default(); + identity_mock + .expect_authenticate_by_password() + .withf(|_, req: &UserPasswordAuthRequest| { + req.id == Some("uid".to_string()) + && req.password == "pass" + && req.name == Some("uname".to_string()) + }) + .returning(move |_, _| Ok(auth.clone())); + identity_mock.expect_get_user().returning(|_, _| { + use openstack_keystone_core_types::identity::UserResponse; + Ok(Some(UserResponse { + id: "uid".into(), + name: "uname".into(), + domain_id: "user_domain_id".into(), + enabled: true, + default_project_id: None, + extra: std::collections::HashMap::new(), + federated: None, + options: openstack_keystone_core_types::identity::UserOptions::default(), + password_expires_at: None, + })) + }); + + let mut resource_mock = MockResourceProvider::default(); + resource_mock + .expect_get_project() + .withf(|_, id: &'_ str| id == "pid") + .returning(move |_, _| Ok(Some(project.clone()))); + resource_mock + .expect_get_domain() + .withf(|_, id: &'_ str| id == "user_domain_id") + .returning(move |_, _| Ok(Some(user_domain.clone()))); + resource_mock + .expect_get_domain() + .withf(|_, id: &'_ str| id == "pdid") + .returning(move |_, _| Ok(Some(project_domain.clone()))); + + let mut token_mock = MockTokenProvider::default(); + token_mock + .expect_issue_token_context() + .returning(|_, _, _| { + Err(TokenProviderError::Authentication( + AuthenticationError::ActorHasNoRolesOnTarget, + )) + }); + + let provider = Provider::mocked_builder() + .mock_assignment(assignment_mock) + .mock_identity(identity_mock) + .mock_resource(resource_mock) + .mock_token(token_mock) + .build() + .unwrap(); + + let state = Arc::new( + Service::new( + ConfigManager::not_watched(config), + DatabaseConnection::Disconnected, + provider, + Arc::new(MockPolicy::default()), + ) + .await + .unwrap(), + ); + + let mut api = openapi_router() + .layer(TraceLayer::new_for_http()) + .with_state(state.clone()); + + let response = api + .as_service() + .oneshot( + Request::builder() + .uri("/") + .method("POST") + .header(header::CONTENT_TYPE, "application/json") + .body(Body::from( + serde_json::to_vec(&json!({ + "auth": { + "identity": { + "methods": ["password"], + "password": { + "user": { + "id": "uid", + "name": "uname", + "domain": { + "id": "udid", + "name": "udname" + }, + "password": "pass", + }, + }, + }, + "scope": { + "project": { + "id": "pid", + "name": "pname", + "domain": { + "id": "pdid", + "name": "pdname" + } + } + } + } + })) + .unwrap(), + )) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::UNAUTHORIZED); + } + #[tokio::test] #[traced_test] async fn test_post_project_disabled() {