Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 71 additions & 5 deletions crates/core-types/src/auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -679,11 +679,30 @@ impl SecurityContext {
}
}
ScopeInfo::Project { project, .. } => {
if let Some(token_restriction) = &self.token_restriction

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this change is unnecessary (at least within this change). The test you modify is not dealing with restricted token, it only uses regular authentication tokens. Moreover I plan on removing token restrictions as a concept completely since they are now superseded by a more universal solution.

&& 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 {
Expand Down Expand Up @@ -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();
Expand Down
280 changes: 272 additions & 8 deletions crates/keystone/src/api/v3/auth/token/create.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,14 +84,12 @@ async fn create_inner(
let provider_scope: Option<ProviderScope> = 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()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am very unsure in that . The lines that the issue were referring to (iirc) were removed with 466a398 so you should not be required to change anything here

&& !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
Expand Down Expand Up @@ -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);
}
}
Loading
Loading