From 73554a351d4065a60cc2ffad1cefe3f61f4fc3e8 Mon Sep 17 00:00:00 2001 From: Sihao Jiang Date: Thu, 2 Jul 2026 09:44:28 -0400 Subject: [PATCH] fix: allow token rescope to accessible project The token create handler rejected any (re)authentication that carried a token restriction with `allow_rescope = false` whenever a scope was requested. That guard was redundant and too broad: `token_restriction` is only populated for `FernetToken::Restricted` auth, and the real boundary is already enforced by `SecurityContext::validate_scope_boundaries` (a restricted token is pinned to its project, since `RestrictedPayload` always carries a concrete `project_id`), while `issue_token_context` rejects the request when the user holds no role on the target scope. The guard even blocked re-scoping to the token's own bound project, which the boundary check permits. Remove the guard from `create_inner` so rescoping with regular tokens is allowed and restricted tokens remain governed by the security gate. The `AuthenticationRescopeForbidden` variant is kept (still used by the audit name and HTTP-status mappings) but is no longer produced. Move restricted-token rescope enforcement into scope boundary validation so allow_rescope=false only permits the current project, while project-pinned restrictions still reject other projects. Add a `test_api::assignment::add_project_grant` helper and an API test `test_rescope_to_newly_granted_project` that creates a project, grants a fresh user a role on it and verifies the user can rescope onto it. Add a handler unit test asserting a restricted-token rescope is no longer rejected. Closes: #763 Signed-off-by: Sihao Jiang --- crates/core-types/src/auth.rs | 76 ++++- .../keystone/src/api/v3/auth/token/create.rs | 280 +++++++++++++++++- tests/api/src/assignment.rs | 81 +++++ tests/api/src/lib.rs | 1 + tests/api/tests/api_v3/auth/token/token.rs | 160 +++++++--- 5 files changed, 547 insertions(+), 51 deletions(-) create mode 100644 tests/api/src/assignment.rs diff --git a/crates/core-types/src/auth.rs b/crates/core-types/src/auth.rs index 62614a960..12a108ea7 100644 --- a/crates/core-types/src/auth.rs +++ b/crates/core-types/src/auth.rs @@ -679,11 +679,30 @@ impl SecurityContext { } } ScopeInfo::Project { project, .. } => { - if let Some(token_restriction) = &self.token_restriction - && let Some(tr_pid) = &token_restriction.project_id - && *tr_pid != project.id - { - return Err(AuthenticationError::ScopeNotAllowed); + if let Some(token_restriction) = &self.token_restriction { + if let Some(tr_pid) = &token_restriction.project_id + && *tr_pid != project.id + { + return Err(AuthenticationError::ScopeNotAllowed); + } + + // If rescope is restricted, then only when the current token is scoped to + // a project, and this request is the same project, will the rescope be accepted + if !token_restriction.allow_rescope { + let same_project = self.authorization().is_some_and(|authz| { + matches!( + &authz.scope, + ScopeInfo::Project { + project: current_project, + .. + } if current_project.id == project.id + ) + }); + + if !same_project { + return Err(AuthenticationError::ScopeNotAllowed); + } + } } match &self.authentication_context { AuthenticationContext::ApplicationCredential { @@ -1991,6 +2010,53 @@ mod tests { )); } + #[test] + fn test_validate_scope_boundaries_no_rescope_restriction_allows_only_current_project() { + let s = AllScopes::new(); + let restriction = TokenRestrictionBuilder::default() + .allow_rescope(false) + .allow_renew(true) + .id("tr_id") + .domain_id("did") + .role_ids(vec![]) + .build() + .unwrap(); + + let mut ctx = make_token_ctx(make_principal("uid")); + ctx.set_token_restriction(restriction); + ctx.set_authorization(AuthzInfo { + scope: s.project.clone(), + roles: None, + }); + + assert!(ctx.validate_scope_boundaries(&s.project).is_ok()); + assert!(matches!( + ctx.validate_scope_boundaries(&s.project2), + Err(AuthenticationError::ScopeNotAllowed) + )); + } + + #[test] + fn test_validate_scope_boundaries_no_rescope_restriction_requires_current_project_scope() { + let s = AllScopes::new(); + let restriction = TokenRestrictionBuilder::default() + .allow_rescope(false) + .allow_renew(true) + .id("tr_id") + .domain_id("did") + .role_ids(vec![]) + .build() + .unwrap(); + + let mut ctx = make_token_ctx(make_principal("uid")); + ctx.set_token_restriction(restriction); + + assert!(matches!( + ctx.validate_scope_boundaries(&s.project), + Err(AuthenticationError::ScopeNotAllowed) + )); + } + #[test] fn test_validate_scope_boundaries_app_cred() { let s = AllScopes::new(); diff --git a/crates/keystone/src/api/v3/auth/token/create.rs b/crates/keystone/src/api/v3/auth/token/create.rs index ccf118209..68439fb69 100644 --- a/crates/keystone/src/api/v3/auth/token/create.rs +++ b/crates/keystone/src/api/v3/auth/token/create.rs @@ -84,14 +84,12 @@ async fn create_inner( let provider_scope: Option = req.auth.scope.clone().map(Into::into); let authz_info = get_authz_info(state, provider_scope.as_ref()).await?; - // This is a new authentication/reauthentication. Check if that is allowed at - // all - if let Some(token_restriction) = ctx.token_restriction() - && !token_restriction.allow_rescope - && req.auth.scope.is_some() - { - return Err(KeystoneApiError::AuthenticationRescopeForbidden); - } + // No rescope guard here: scope constraints are enforced downstream by + // issue_token_context. It calls SecurityContext::validate_scope_boundaries, + // which keeps a restricted token within its bounds (pinned project, or + // allow_rescope == false => current project only) and rejects disallowed scope + // types, then validates the user actually holds a role on the target scope before + // the token is issued. let vsc = state .provider @@ -669,4 +667,270 @@ mod tests { assert_eq!(response.status(), StatusCode::UNAUTHORIZED); } + + #[tokio::test] + #[traced_test] + async fn test_post_token_rescope_not_rejected_with_restriction() { + // Regression test for #763. A token (re)authentication carrying a + // restriction with `allow_rescope = false` and requesting a scope must no + // longer be rejected by the handler; whether the scope is permissible is + // decided by `issue_token_context` (mocked here to succeed). + use openstack_keystone_core_types::auth::AuthzInfoBuilder; + use openstack_keystone_core_types::identity::UserResponseBuilder; + use openstack_keystone_core_types::resource::ProjectBuilder; + use openstack_keystone_core_types::role::RoleRefBuilder; + use openstack_keystone_core_types::token::{ + FernetToken, ProjectScopePayload, TokenRestriction, + }; + + let config = Config::default(); + let project = Project { + id: "pid".into(), + domain_id: "pdid".into(), + enabled: true, + ..Default::default() + }; + let project_domain = Domain { + id: "pdid".into(), + enabled: true, + ..Default::default() + }; + + // Context returned by token authentication: bound to `pid` and explicitly + // forbidding rescope. + let auth_vsc = { + let user_resp = UserResponseBuilder::default() + .id("uid") + .name("uname".to_string()) + .domain_id("user_domain_id".to_string()) + .enabled(true) + .build() + .unwrap(); + + let restriction = TokenRestriction { + allow_rescope: false, + allow_renew: true, + id: "tr".into(), + domain_id: "user_domain_id".into(), + project_id: Some("pid".into()), + user_id: Some("uid".into()), + ..Default::default() + }; + + let authz = AuthzInfoBuilder::default() + .roles(vec![ + RoleRefBuilder::default() + .id("admin") + .name("admin") + .build() + .unwrap(), + ]) + .scope(ScopeInfo::Project { + project: ProjectBuilder::default() + .id("pid") + .domain_id("pdid") + .enabled(true) + .name("pname") + .build() + .unwrap(), + project_domain: DomainBuilder::default() + .id("pdid") + .name("pdname") + .enabled(true) + .build() + .unwrap(), + }) + .build() + .unwrap(); + + let sc = SecurityContext::test_build() + .authentication_context(AuthenticationContext::Token(FernetToken::ProjectScope( + ProjectScopePayload { + user_id: "uid".into(), + methods: Vec::from(["password".to_string()]), + project_id: "pid".into(), + ..Default::default() + }, + ))) + .principal(PrincipalInfo { + identity: IdentityInfo::User( + UserIdentityInfoBuilder::default() + .user_id("uid") + .user(user_resp) + .user_domain( + DomainBuilder::default() + .id("user_domain_id") + .name("user_domain_name") + .enabled(true) + .build() + .unwrap(), + ) + .build() + .unwrap(), + ), + }) + .authorization(authz) + .token_restriction(restriction) + .build(); + openstack_keystone_core::auth::ValidatedSecurityContext::test_new(sc) + }; + + // Project-scoped context produced by a successful issue_token_context + // (identical shape to `test_post`'s fixture). + let issued_vsc = { + let user_resp = UserResponseBuilder::default() + .id("uid") + .name("uname".to_string()) + .domain_id("user_domain_id".to_string()) + .enabled(true) + .build() + .unwrap(); + + let fernet_payload = ProjectScopePayload { + user_id: "uid".into(), + methods: Vec::from(["token".to_string()]), + project_id: "pid".into(), + ..Default::default() + }; + + let authz = AuthzInfoBuilder::default() + .roles(vec![ + RoleRefBuilder::default() + .id("admin") + .name("admin") + .build() + .unwrap(), + ]) + .scope(ScopeInfo::Project { + project: ProjectBuilder::default() + .id("pid") + .domain_id("pdid") + .enabled(true) + .name("pname") + .build() + .unwrap(), + project_domain: DomainBuilder::default() + .id("pdid") + .name("pdname") + .enabled(true) + .build() + .unwrap(), + }) + .build() + .unwrap(); + + let sc = SecurityContext::test_build() + .authentication_context(AuthenticationContext::Token(FernetToken::ProjectScope( + fernet_payload.clone(), + ))) + .principal(PrincipalInfo { + identity: IdentityInfo::User( + UserIdentityInfoBuilder::default() + .user_id("uid") + .user(user_resp) + .user_domain( + DomainBuilder::default() + .id("user_domain_id") + .name("user_domain_name") + .enabled(true) + .build() + .unwrap(), + ) + .build() + .unwrap(), + ), + }) + .token(FernetToken::ProjectScope(fernet_payload)) + .authorization(authz) + .build(); + openstack_keystone_core::auth::ValidatedSecurityContext::test_new(sc) + }; + + let mut token_mock = MockTokenProvider::default(); + let auth_vsc_clone = auth_vsc.clone(); + token_mock + .expect_authorize_by_token() + .returning(move |_, _, _, _| Ok(auth_vsc_clone.clone())); + let issued_clone = issued_vsc.clone(); + token_mock + .expect_issue_token_context() + .once() + .returning(move |_, _, _| Ok(issued_clone.clone())); + token_mock + .expect_encode_token() + .returning(|_| Ok("token".to_string())); + + 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 == "pdid") + .returning(move |_, _| Ok(Some(project_domain.clone()))); + + let mut catalog_mock = MockCatalogProvider::default(); + catalog_mock + .expect_get_catalog() + .returning(|_, _| Ok(Vec::new())); + + let provider = Provider::mocked_builder() + .mock_resource(resource_mock) + .mock_token(token_mock) + .mock_catalog(catalog_mock) + .build() + .unwrap(); + + let state = Arc::new( + Service::new( + ConfigManager::not_watched(config), + DatabaseConnection::Disconnected, + provider, + Arc::new(MockPolicy::default()), + AuditDispatcher::noop(), + None, + ) + .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": ["token"], + "token": { "id": "parent_token" } + }, + "scope": { + "project": { + "id": "pid", + "name": "pname", + "domain": { "id": "pdid", "name": "pdname" } + } + } + } + })) + .unwrap(), + )) + .unwrap(), + ) + .await + .unwrap(); + + // Previously this returned 400 (AuthenticationRescopeForbidden). With the + // guard removed it reaches issue_token_context and succeeds. + assert_eq!(response.status(), StatusCode::OK); + } } diff --git a/tests/api/src/assignment.rs b/tests/api/src/assignment.rs new file mode 100644 index 000000000..211c83ae4 --- /dev/null +++ b/tests/api/src/assignment.rs @@ -0,0 +1,81 @@ +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + +use std::{borrow::Cow, sync::Arc}; + +use derive_builder::Builder; +use eyre::Result; + +use openstack_sdk::api::rest_endpoint_prelude::*; +use openstack_sdk::{AsyncOpenStack, api::QueryAsync}; + +/// Grant a role to a user on a project. +pub mod grant { + use super::*; + + #[derive(Builder, Clone, Debug)] + #[builder(setter(strip_option, into))] + struct ProjectUserRoleGrant<'a> { + project_id: Cow<'a, str>, + user_id: Cow<'a, str>, + role_id: Cow<'a, str>, + } + + impl RestEndpoint for ProjectUserRoleGrant<'_> { + fn method(&self) -> http::Method { + http::Method::PUT + } + + fn endpoint(&self) -> Cow<'static, str> { + format!( + "projects/{}/users/{}/roles/{}", + self.project_id, self.user_id, self.role_id + ) + .into() + } + + fn service_type(&self) -> ServiceType { + ServiceType::Identity + } + + fn api_version(&self) -> Option { + Some(ApiVersion::new(3, 0)) + } + } + + /// Grant `role_id` to `user_id` on `project_id`. The grant is a PUT and is + /// cleaned up implicitly when the project or user is deleted. + pub async fn add_project_grant( + client: &Arc, + project_id: P, + user_id: U, + role_id: R, + ) -> Result<()> + where + P: AsRef, + U: AsRef, + R: AsRef, + { + openstack_sdk::api::ignore( + ProjectUserRoleGrantBuilder::default() + .project_id(project_id.as_ref()) + .user_id(user_id.as_ref()) + .role_id(role_id.as_ref()) + .build()?, + ) + .query_async(client.as_ref()) + .await?; + Ok(()) + } +} diff --git a/tests/api/src/lib.rs b/tests/api/src/lib.rs index 0468ad113..35ac2aed3 100644 --- a/tests/api/src/lib.rs +++ b/tests/api/src/lib.rs @@ -11,6 +11,7 @@ // limitations under the License. // // SPDX-License-Identifier: Apache-2.0 +pub mod assignment; pub mod auth; pub mod common; pub mod guard; diff --git a/tests/api/tests/api_v3/auth/token/token.rs b/tests/api/tests/api_v3/auth/token/token.rs index a53e6eaea..bd70eccc0 100644 --- a/tests/api/tests/api_v3/auth/token/token.rs +++ b/tests/api/tests/api_v3/auth/token/token.rs @@ -12,54 +12,138 @@ // // SPDX-License-Identifier: Apache-2.0 -use std::sync::Arc; - -use eyre::Result; +use std::{collections::HashMap, sync::Arc}; //use openstack_keystone_api_types::scope::*; +use eyre::Result; +use openstack_keystone_api_types::scope::{DomainBuilder, Scope, ScopeProjectBuilder}; +use openstack_keystone_api_types::v3::{project::ProjectCreateBuilder, user::UserCreateBuilder}; use openstack_sdk::{AsyncOpenStack, config::CloudConfig}; +use uuid::Uuid; +use test_api::assignment::grant::add_project_grant; use test_api::auth::project::list_auth_projects; +use test_api::common::{TestClient, get_password_auth, get_session_by_user_password}; +use test_api::guard::ResourceGuard; +use test_api::identity::user::create_user; +use test_api::resource::project::create_project; +use test_api::role::list_roles; #[tokio::test] async fn test_rescope_project_scope() -> Result<()> { - let mut test_client = AsyncOpenStack::new(&CloudConfig::from_env()?).await?; + let admin = Arc::new(AsyncOpenStack::new(&CloudConfig::from_env()?).await?); + let password = "TestPassword123!"; + + let user = create_user( + &admin, + UserCreateBuilder::default() + .name(format!("usr_{}", Uuid::new_v4().simple())) + .domain_id("default") + .enabled(true) + .password(password) + .build()?, + ) + .await?; + + let source = create_project( + &admin, + ProjectCreateBuilder::default() + .domain_id("default") + .parent_id("default") + .name(format!("src_{}", Uuid::new_v4().simple())) + .is_domain(false) + .enabled(true) + .build()?, + ) + .await?; + + let target = create_project( + &admin, + ProjectCreateBuilder::default() + .domain_id("default") + .parent_id("default") + .name(format!("dst_{}", Uuid::new_v4().simple())) + .is_domain(false) + .enabled(true) + .build()?, + ) + .await?; + + let roles: HashMap = list_roles(&admin) + .await? + .into_iter() + .map(|r| (r.name, r.id)) + .collect(); + let member = roles.get("member").expect("member role must exist"); - let projects = list_auth_projects(&Arc::new(test_client.clone())).await?; + add_project_grant(&admin, &source.id, &user.id, member).await?; + add_project_grant(&admin, &target.id, &user.id, member).await?; - for project in projects { - // auth with project_id - test_client - .authorize( - Some(openstack_sdk::auth::authtoken::AuthTokenScope::Project( - openstack_sdk::types::identity::v3::Project { - id: Some(project.id.clone()), - name: None, - domain: None, - }, - )), - false, - false, - ) - .await?; - // auth with project name and domain_id - test_client - .authorize( - Some(openstack_sdk::auth::authtoken::AuthTokenScope::Project( - openstack_sdk::types::identity::v3::Project { - id: None, - name: Some(project.name.clone()), - domain: Some(openstack_sdk::types::identity::v3::Domain { - id: Some(project.domain_id.clone()), - name: None, - }), - }, - )), - false, - false, - ) - .await?; - } + let user_sdk = get_session_by_user_password(&user.name, &user.domain_id, password).await?; + let projects = list_auth_projects(&user_sdk).await?; + assert!(projects.iter().any(|p| p.id == target.id)); + let mut client = TestClient::default()?; + client + .auth_password( + get_password_auth(&user.name, password, &user.domain_id)?, + Some(project_scope_id(&source.id)?), + ) + .await?; + client.rescope(Some(project_scope_id(&target.id)?)).await?; + assert_eq!( + target.id, + client + .auth + .as_ref() + .unwrap() + .token + .project + .as_ref() + .unwrap() + .id + ); + + let mut client = TestClient::default()?; + client + .auth_password( + get_password_auth(&user.name, password, &user.domain_id)?, + Some(project_scope_id(&source.id)?), + ) + .await?; + client + .rescope(Some(project_scope_name(&target.name, &target.domain_id)?)) + .await?; + assert_eq!( + target.id, + client + .auth + .as_ref() + .unwrap() + .token + .project + .as_ref() + .unwrap() + .id + ); + + user.delete().await?; + source.delete().await?; + target.delete().await?; Ok(()) } + +fn project_scope_id(project_id: &str) -> Result { + Ok(Scope::Project( + ScopeProjectBuilder::default().id(project_id).build()?, + )) +} + +fn project_scope_name(name: &str, domain_id: &str) -> Result { + Ok(Scope::Project( + ScopeProjectBuilder::default() + .name(name) + .domain(DomainBuilder::default().id(domain_id).build()?) + .build()?, + )) +}