diff --git a/policy/project/user/role/revoke.rego b/policy/project/user/role/revoke.rego new file mode 100644 index 00000000..e4b4cf57 --- /dev/null +++ b/policy/project/user/role/revoke.rego @@ -0,0 +1,27 @@ +package identity.project.user.role.revoke + +import data.identity +import data.identity.assignment + +# Revoke user a role on a project + +default allow := false + +allow if { + "admin" in input.credentials.roles +} + +allow if { + "manager" in input.credentials.roles + assignment.project_role_domain_matches +} + + +violation contains {"field": "domain_id", "msg": "revoking a role from a user on a project requires admin or manager role in the domain scope."} if { + not "admin" in input.credentials.roles + not "manager" in input.credentials.roles +} + +violation contains {"field": "domain_id", "msg": "revoking a role from a user on a project requires domain scope matching the domain_id of the target project and role (or a global role)."} if { + assignment.project_role_domain_matches +} diff --git a/policy/project/user/role/revoke_test.rego b/policy/project/user/role/revoke_test.rego new file mode 100644 index 00000000..fd3149bd --- /dev/null +++ b/policy/project/user/role/revoke_test.rego @@ -0,0 +1,20 @@ +package test_project_user_role_revoke + +import data.identity.project.user.role.revoke + +test_allowed if { + revoke.allow with input as {"credentials": {"roles": ["admin"]}} + revoke.allow with input as {"credentials": {"roles": ["manager"], "domain_id": "foo"}, "target": {"user": {"domain_id": "foo"}, "project": {"domain_id": "foo"}, "role": {"domain_id": null}}} + revoke.allow with input as {"credentials": {"roles": ["manager"], "domain_id": "foo"}, "target": {"user": {"domain_id": "foo"}, "project": {"domain_id": "foo"}, "role": {"domain_id": "foo"}}} +} + +test_forbidden if { + not revoke.allow with input as {"credentials": {"roles": []}} + not revoke.allow with input as {"credentials": {"roles": ["reader"], "system": "foo"}} + not revoke.allow with input as {"credentials": {"roles": ["reader"], "domain_id": "foo"}, "target": {"user": {"domain_id": "foo1"}, "project": {"domain_id": "foo"}, "role": {"domain_id": "foo"}}} + not revoke.allow with input as {"credentials": {"roles": ["reader"], "domain_id": "foo"}, "target": {"user": {"domain_id": "foo1"}, "project": {"domain_id": "foo1"}, "role": {"domain_id": "foo1"}}} + not revoke.allow with input as {"credentials": {"roles": ["reader"], "domain_id": "foo"}, "target": {"user": {"domain_id": "foo"}, "project": {"domain_id": "foo1"}, "role": {"domain_id": "foo1"}}} + not revoke.allow with input as {"credentials": {"roles": ["reader"], "domain_id": "foo"}, "target": {"user": {"domain_id": "foo1"}, "project": {"domain_id": "foo1"}, "role": {"domain_id": "foo1"}}} + not revoke.allow with input as {"credentials": {"roles": ["reader"], "domain_id": "foo"}, "target": {"project": {"domain_id": "foo"}, "role": {"domain_id": "foo"}}} + not revoke.allow with input as {"credentials": {"roles": ["member"], "domain_id": "foo"}, "target": {"project": {"domain_id": "foo"}, "role": {"domain_id": "foo"}, "user": {"domain_id": "foo"}}} +} diff --git a/src/api/v3/role_assignment/project/user/role.rs b/src/api/v3/role_assignment/project/user/role.rs index 3afb8aec..c972d76f 100644 --- a/src/api/v3/role_assignment/project/user/role.rs +++ b/src/api/v3/role_assignment/project/user/role.rs @@ -18,7 +18,8 @@ use crate::keystone::ServiceState; mod check; mod grant; +mod revoke; pub(crate) fn openapi_router() -> OpenApiRouter { - OpenApiRouter::new().routes(routes!(check::check, grant::grant)) + OpenApiRouter::new().routes(routes!(check::check, grant::grant, revoke::revoke)) } diff --git a/src/api/v3/role_assignment/project/user/role/grant.rs b/src/api/v3/role_assignment/project/user/role/grant.rs index e7b7f536..5829403f 100644 --- a/src/api/v3/role_assignment/project/user/role/grant.rs +++ b/src/api/v3/role_assignment/project/user/role/grant.rs @@ -33,15 +33,15 @@ use crate::{ resource::ResourceApi, }; -/// Assign role to group on project +/// Assign role to user on project /// -/// Assigns a role to a group on a project. +/// Assigns a role to a user on a project. #[utoipa::path( put, path = "/projects/{project_id}/users/{user_id}/roles/{role_id}", operation_id = "/project/user/role:put", params( - ("role_id" = String, Path, description = "The user ID."), + ("role_id" = String, Path, description = "The role ID."), ("project_id" = String, Path, description = "The project ID."), ("user_id" = String, Path, description = "The user ID.") ), diff --git a/src/api/v3/role_assignment/project/user/role/revoke.rs b/src/api/v3/role_assignment/project/user/role/revoke.rs new file mode 100644 index 00000000..bad92720 --- /dev/null +++ b/src/api/v3/role_assignment/project/user/role/revoke.rs @@ -0,0 +1,462 @@ +// 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 + +//! Project user role: delete + +use axum::{ + extract::{Path, Request, State}, + http::StatusCode, + response::IntoResponse, +}; +use mockall_double::double; +use serde_json::json; +use tracing::info; + +use crate::api::error::KeystoneApiError; +use crate::keystone::ServiceState; +#[double] +use crate::policy::Policy; +use crate::{ + api::auth::Auth, + assignment::{AssignmentApi, types::AssignmentBuilder, types::AssignmentType}, + identity::IdentityApi, + resource::ResourceApi, +}; + +/// Revoke role from user on project +/// +/// Remove a role assignment for a user on a specific project. +#[utoipa::path( + delete, + path = "/projects/{project_id}/users/{user_id}/roles/{role_id}", + operation_id = "/project/user/role:delete", + params( + ("role_id" = String, Path, description = "The role ID."), + ("project_id" = String, Path, description = "The project ID."), + ("user_id" = String, Path, description = "The user ID."), + ), + responses( + (status = 204, description = "Role revoked successfully"), + (status = 401, description = "Unauthorized"), + (status = 403, description = "Forbidden"), + (status = 404, description = "Grant not found", example = json!(KeystoneApiError::NotFound(String::from("id = 1")))) + ), + security( + ("X-Auth-Token" = [])), + tag="role_assignments" +)] +#[tracing::instrument( + name = "api::v3:project_user_role_revoke", + level = "debug", + skip(state, user_auth, policy), + err(Debug) +)] + +pub(super) async fn revoke( + Auth(user_auth): Auth, + mut policy: Policy, + Path((project_id, user_id, role_id)): Path<(String, String, String)>, + State(state): State, + request: Request, +) -> Result { + let inherited = request.uri().path().contains("inherited_to_projects"); + // Use join instead of try_join to have more constant latency preventing timing + // attacks. + let (user, role, project) = tokio::join!( + state + .provider + .get_identity_provider() + .get_user(&state, &user_id), + state + .provider + .get_assignment_provider() + .get_role(&state, &role_id), + state + .provider + .get_resource_provider() + .get_project(&state, &project_id) + ); + let user = user?.ok_or_else(|| { + info!("User {} was not found", user_id); + KeystoneApiError::NotFound { + resource: "grant".into(), + identifier: "".into(), + } + })?; + let project = project?.ok_or_else(|| { + info!("Project {} was not found", project_id); + KeystoneApiError::NotFound { + resource: "grant".into(), + identifier: "".into(), + } + })?; + let role = role?.ok_or_else(|| { + info!("Role {} was not found", role_id); + KeystoneApiError::NotFound { + resource: "grant".into(), + identifier: "".into(), + } + })?; + + policy + .enforce( + "identity/project/user/role/revoke", + &user_auth, + json!({"user": user, "role": role, "project": project}), + None, + ) + .await?; + + let grant = AssignmentBuilder::default() + .actor_id(user_id) + .role_id(role_id) + .target_id(project_id) + .r#type(AssignmentType::UserProject) + .inherited(inherited) + .build()?; + + state + .provider + .get_assignment_provider() + .revoke_grant(&state, grant) + .await?; + + Ok(StatusCode::NO_CONTENT.into_response()) +} + +#[cfg(test)] +mod tests { + use axum::{ + body::Body, + http::{Request, StatusCode}, + }; + use tower::ServiceExt; + use tower_http::trace::TraceLayer; + use tracing_test::traced_test; + + use crate::api::tests::get_mocked_state; + use crate::api::v3::role_assignment::openapi_router; + use crate::assignment::{MockAssignmentProvider, types::*}; + use crate::identity::{MockIdentityProvider, types::*}; + use crate::provider::Provider; + use crate::resource::{MockResourceProvider, types::Project}; + + #[tokio::test] + #[traced_test] + async fn test_revoke_success() { + let mut identity_mock = MockIdentityProvider::default(); + identity_mock + .expect_get_user() + .withf(|_, id: &'_ str| id == "user_id") + .returning(|_, _| { + Ok(Some(UserResponse { + id: "user_id".into(), + ..Default::default() + })) + }); + + let mut assignment_mock = MockAssignmentProvider::default(); + assignment_mock + .expect_get_role() + .withf(|_, rid: &'_ str| rid == "role_id") + .returning(|_, _| { + Ok(Some(Role { + id: "role_id".into(), + name: "test_role".into(), + ..Default::default() + })) + }); + assignment_mock + .expect_revoke_grant() + .withf(|_, grant: &Assignment| { + grant.role_id == "role_id" + && grant.actor_id == "user_id" + && grant.target_id == "project_id" + && grant.r#type == AssignmentType::UserProject + && !grant.inherited + }) + .returning(|_, _| Ok(())); + + let mut resource_mock = MockResourceProvider::default(); + resource_mock + .expect_get_project() + .withf(|_, pid: &'_ str| pid == "project_id") + .returning(|_, id: &'_ str| { + Ok(Some(Project { + id: id.to_string(), + domain_id: "project_domain_id".into(), + ..Default::default() + })) + }); + + let provider_builder = Provider::mocked_builder() + .assignment(assignment_mock) + .identity(identity_mock) + .resource(resource_mock); + let state = get_mocked_state(provider_builder, true); + let mut api = openapi_router() + .layer(TraceLayer::new_for_http()) + .with_state(state); + + let response = api + .as_service() + .oneshot( + Request::builder() + .method("DELETE") + .uri("/projects/project_id/users/user_id/roles/role_id") + .header("x-auth-token", "foo") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::NO_CONTENT); + } + + #[tokio::test] + #[traced_test] + async fn test_revoke_forbidden() { + let mut identity_mock = MockIdentityProvider::default(); + identity_mock + .expect_get_user() + .withf(|_, id: &'_ str| id == "user_id") + .returning(|_, _| { + Ok(Some(UserResponse { + id: "user_id".into(), + ..Default::default() + })) + }); + + let mut assignment_mock = MockAssignmentProvider::default(); + assignment_mock + .expect_get_role() + .withf(|_, rid: &'_ str| rid == "role_id") + .returning(|_, _| { + Ok(Some(Role { + id: "role_id".into(), + name: "test_role".into(), + ..Default::default() + })) + }); + + let mut resource_mock = MockResourceProvider::default(); + resource_mock + .expect_get_project() + .withf(|_, pid: &'_ str| pid == "project_id") + .returning(|_, id: &'_ str| { + Ok(Some(Project { + id: id.to_string(), + domain_id: "project_domain_id".into(), + ..Default::default() + })) + }); + + let provider_builder = Provider::mocked_builder() + .assignment(assignment_mock) + .identity(identity_mock) + .resource(resource_mock); + let state = get_mocked_state(provider_builder, false); // Policy NOT allowed + let mut api = openapi_router() + .layer(TraceLayer::new_for_http()) + .with_state(state); + + let response = api + .as_service() + .oneshot( + Request::builder() + .method("DELETE") + .uri("/projects/project_id/users/user_id/roles/role_id") + .header("x-auth-token", "foo") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::FORBIDDEN); + } + + #[tokio::test] + #[traced_test] + async fn test_revoke_user_not_found() { + let mut identity_mock = MockIdentityProvider::default(); + identity_mock + .expect_get_user() + .withf(|_, id: &'_ str| id == "user_id") + .returning(|_, _| Ok(None)); // User not found + + let mut assignment_mock = MockAssignmentProvider::default(); + assignment_mock + .expect_get_role() + .withf(|_, rid: &'_ str| rid == "role_id") + .returning(|_, _| { + Ok(Some(Role { + id: "role_id".into(), + name: "test_role".into(), + ..Default::default() + })) + }); + + let mut resource_mock = MockResourceProvider::default(); + resource_mock + .expect_get_project() + .withf(|_, pid: &'_ str| pid == "project_id") + .returning(|_, id: &'_ str| { + Ok(Some(Project { + id: id.to_string(), + domain_id: "project_domain_id".into(), + ..Default::default() + })) + }); + + let provider_builder = Provider::mocked_builder() + .assignment(assignment_mock) + .identity(identity_mock) + .resource(resource_mock); + let state = get_mocked_state(provider_builder, true); + let mut api = openapi_router() + .layer(TraceLayer::new_for_http()) + .with_state(state); + + let response = api + .as_service() + .oneshot( + Request::builder() + .method("DELETE") + .uri("/projects/project_id/users/user_id/roles/role_id") + .header("x-auth-token", "foo") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::NOT_FOUND); + } + + #[tokio::test] + #[traced_test] + async fn test_revoke_project_not_found() { + let mut identity_mock = MockIdentityProvider::default(); + identity_mock + .expect_get_user() + .withf(|_, id: &'_ str| id == "user_id") + .returning(|_, _| { + Ok(Some(UserResponse { + id: "user_id".into(), + ..Default::default() + })) + }); + + let mut assignment_mock = MockAssignmentProvider::default(); + assignment_mock + .expect_get_role() + .withf(|_, rid: &'_ str| rid == "role_id") + .returning(|_, _| { + Ok(Some(Role { + id: "role_id".into(), + name: "test_role".into(), + ..Default::default() + })) + }); + + let mut resource_mock = MockResourceProvider::default(); + resource_mock + .expect_get_project() + .withf(|_, pid: &'_ str| pid == "project_id") + .returning(|_, _| Ok(None)); // Project not found + + let provider_builder = Provider::mocked_builder() + .assignment(assignment_mock) + .identity(identity_mock) + .resource(resource_mock); + let state = get_mocked_state(provider_builder, true); + let mut api = openapi_router() + .layer(TraceLayer::new_for_http()) + .with_state(state); + + let response = api + .as_service() + .oneshot( + Request::builder() + .method("DELETE") + .uri("/projects/project_id/users/user_id/roles/role_id") + .header("x-auth-token", "foo") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::NOT_FOUND); + } + + #[tokio::test] + #[traced_test] + async fn test_revoke_role_not_found() { + let mut identity_mock = MockIdentityProvider::default(); + identity_mock + .expect_get_user() + .withf(|_, id: &'_ str| id == "user_id") + .returning(|_, _| { + Ok(Some(UserResponse { + id: "user_id".into(), + ..Default::default() + })) + }); + + let mut assignment_mock = MockAssignmentProvider::default(); + assignment_mock + .expect_get_role() + .withf(|_, rid: &'_ str| rid == "role_id") + .returning(|_, _| Ok(None)); // Role not found + + let mut resource_mock = MockResourceProvider::default(); + resource_mock + .expect_get_project() + .withf(|_, pid: &'_ str| pid == "project_id") + .returning(|_, id: &'_ str| { + Ok(Some(Project { + id: id.to_string(), + domain_id: "project_domain_id".into(), + ..Default::default() + })) + }); + + let provider_builder = Provider::mocked_builder() + .assignment(assignment_mock) + .identity(identity_mock) + .resource(resource_mock); + let state = get_mocked_state(provider_builder, true); + let mut api = openapi_router() + .layer(TraceLayer::new_for_http()) + .with_state(state); + + let response = api + .as_service() + .oneshot( + Request::builder() + .method("DELETE") + .uri("/projects/project_id/users/user_id/roles/role_id") + .header("x-auth-token", "foo") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::NOT_FOUND); + } +} diff --git a/src/assignment/backend.rs b/src/assignment/backend.rs index 17dd8313..aec134d5 100644 --- a/src/assignment/backend.rs +++ b/src/assignment/backend.rs @@ -86,4 +86,11 @@ pub trait AssignmentBackend: Send + Sync { state: &ServiceState, params: &RoleListParameters, ) -> Result, AssignmentProviderError>; + + /// Revoke assignment grant. + async fn revoke_grant( + &self, + state: &ServiceState, + params: Assignment, + ) -> Result<(), AssignmentProviderError>; } diff --git a/src/assignment/backend/error.rs b/src/assignment/backend/error.rs index 8aebb315..bfc750b4 100644 --- a/src/assignment/backend/error.rs +++ b/src/assignment/backend/error.rs @@ -19,6 +19,10 @@ use crate::error::{BuilderError, DatabaseError}; /// Database driver for the assignments provider. #[derive(Error, Debug)] pub enum AssignmentDatabaseError { + /// Assignment not found. + #[error("assignment not found: {0}")] + AssignmentNotFound(String), + /// Database error. #[error(transparent)] Database { diff --git a/src/assignment/backend/sql.rs b/src/assignment/backend/sql.rs index 5611e29a..ed984434 100644 --- a/src/assignment/backend/sql.rs +++ b/src/assignment/backend/sql.rs @@ -130,4 +130,14 @@ impl AssignmentBackend for SqlBackend { ) -> Result, AssignmentProviderError> { Ok(assignment::list_for_multiple_actors_and_targets(&state.db, params).await?) } + + /// Revoke assignment grant. + #[tracing::instrument(level = "info", skip(self, state))] + async fn revoke_grant( + &self, + state: &ServiceState, + grant: Assignment, + ) -> Result<(), AssignmentProviderError> { + Ok(assignment::delete(&state.db, grant).await?) + } } diff --git a/src/assignment/backend/sql/assignment.rs b/src/assignment/backend/sql/assignment.rs index 98b4cdee..dd1960ea 100644 --- a/src/assignment/backend/sql/assignment.rs +++ b/src/assignment/backend/sql/assignment.rs @@ -23,10 +23,12 @@ use crate::db::entity::{ mod check; mod create; +mod delete; mod list; pub use check::check; pub use create::create; +pub use delete::delete; pub use list::list; pub use list::list_for_multiple_actors_and_targets; diff --git a/src/assignment/backend/sql/assignment/delete.rs b/src/assignment/backend/sql/assignment/delete.rs new file mode 100644 index 00000000..2f9b581f --- /dev/null +++ b/src/assignment/backend/sql/assignment/delete.rs @@ -0,0 +1,389 @@ +// 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 sea_orm::entity::*; +use sea_orm::{DatabaseConnection, EntityTrait, QueryFilter}; + +use crate::assignment::backend::error::AssignmentDatabaseError; +use crate::assignment::types::*; +use crate::db::entity::{ + assignment as db_assignment, sea_orm_active_enums::Type as DbAssignmentType, + system_assignment as db_system_assignment, +}; +use crate::error::DbContextExt; + +/// Delete assignment grant. +pub async fn delete( + db: &DatabaseConnection, + grant: Assignment, +) -> Result<(), AssignmentDatabaseError> { + let rows_affected = match &grant.r#type { + AssignmentType::GroupDomain + | AssignmentType::GroupProject + | AssignmentType::UserDomain + | AssignmentType::UserProject => { + db_assignment::Entity::delete_many() + .filter(db_assignment::Column::RoleId.eq(&grant.role_id)) + .filter(db_assignment::Column::TargetId.eq(&grant.target_id)) + .filter(db_assignment::Column::ActorId.eq(&grant.actor_id)) + .filter(db_assignment::Column::Type.eq(DbAssignmentType::try_from(&grant.r#type)?)) + .filter(db_assignment::Column::Inherited.eq(grant.inherited)) + .exec(db) + .await + .context("deleting assignment")? + .rows_affected + } + AssignmentType::GroupSystem | AssignmentType::UserSystem => { + db_system_assignment::Entity::delete_many() + .filter(db_system_assignment::Column::RoleId.eq(&grant.role_id)) + .filter(db_system_assignment::Column::TargetId.eq(&grant.target_id)) + .filter(db_system_assignment::Column::ActorId.eq(&grant.actor_id)) + .filter(db_system_assignment::Column::Type.eq(grant.r#type.to_string())) + .filter(db_system_assignment::Column::Inherited.eq(grant.inherited)) + .exec(db) + .await + .context("deleting system assignment")? + .rows_affected + } + }; + + if rows_affected == 0 { + return Err(AssignmentDatabaseError::AssignmentNotFound(format!( + "actor={}, target={}, role={}, type={:?}, inherited={}", + grant.actor_id, grant.target_id, grant.role_id, grant.r#type, grant.inherited + ))); + } + + Ok(()) +} + +#[cfg(test)] +mod tests { + use sea_orm::{DatabaseBackend, MockDatabase, MockExecResult, Transaction}; + + use super::*; + + #[tokio::test] + async fn test_delete_user_project_success() { + // Create MockDatabase with mock query results + let db = MockDatabase::new(DatabaseBackend::Postgres) + .append_exec_results([MockExecResult { + last_insert_id: 0, + rows_affected: 1, // 1 row deleted + }]) + .into_connection(); + + let grant = Assignment { + role_id: "role_id".into(), + actor_id: "actor_id".into(), + target_id: "target_id".into(), + r#type: AssignmentType::UserProject, + inherited: false, + role_name: None, + implied_via: None, + }; + + // Should succeed + delete(&db, grant).await.unwrap(); + + // Verify SQL was executed correctly + assert_eq!( + db.into_transaction_log(), + [Transaction::from_sql_and_values( + DatabaseBackend::Postgres, + r#"DELETE FROM "assignment" WHERE "assignment"."role_id" = $1 AND "assignment"."target_id" = $2 AND "assignment"."actor_id" = $3 AND "assignment"."type" = (CAST($4 AS "type")) AND "assignment"."inherited" = $5"#, + [ + "role_id".into(), + "target_id".into(), + "actor_id".into(), + "UserProject".into(), + false.into(), + ] + ),] + ); + } + + #[tokio::test] + async fn test_delete_group_project_success() { + let db = MockDatabase::new(DatabaseBackend::Postgres) + .append_exec_results([MockExecResult { + last_insert_id: 0, + rows_affected: 1, + }]) + .into_connection(); + + let grant = Assignment { + role_id: "role_id".into(), + actor_id: "group_id".into(), + target_id: "project_id".into(), + r#type: AssignmentType::GroupProject, + inherited: false, + role_name: None, + implied_via: None, + }; + + delete(&db, grant).await.unwrap(); + + assert_eq!( + db.into_transaction_log(), + [Transaction::from_sql_and_values( + DatabaseBackend::Postgres, + r#"DELETE FROM "assignment" WHERE "assignment"."role_id" = $1 AND "assignment"."target_id" = $2 AND "assignment"."actor_id" = $3 AND "assignment"."type" = (CAST($4 AS "type")) AND "assignment"."inherited" = $5"#, + [ + "role_id".into(), + "project_id".into(), + "group_id".into(), + "GroupProject".into(), + false.into(), + ] + ),] + ); + } + + #[tokio::test] + async fn test_delete_user_domain_success() { + let db = MockDatabase::new(DatabaseBackend::Postgres) + .append_exec_results([MockExecResult { + last_insert_id: 0, + rows_affected: 1, + }]) + .into_connection(); + + let grant = Assignment { + role_id: "role_id".into(), + actor_id: "user_id".into(), + target_id: "domain_id".into(), + r#type: AssignmentType::UserDomain, + inherited: false, + role_name: None, + implied_via: None, + }; + + delete(&db, grant).await.unwrap(); + + assert_eq!( + db.into_transaction_log(), + [Transaction::from_sql_and_values( + DatabaseBackend::Postgres, + r#"DELETE FROM "assignment" WHERE "assignment"."role_id" = $1 AND "assignment"."target_id" = $2 AND "assignment"."actor_id" = $3 AND "assignment"."type" = (CAST($4 AS "type")) AND "assignment"."inherited" = $5"#, + [ + "role_id".into(), + "domain_id".into(), + "user_id".into(), + "UserDomain".into(), + false.into(), + ] + ),] + ); + } + + #[tokio::test] + async fn test_delete_user_system_success() { + let db = MockDatabase::new(DatabaseBackend::Postgres) + .append_exec_results([MockExecResult { + last_insert_id: 0, + rows_affected: 1, + }]) + .into_connection(); + + let grant = Assignment { + role_id: "role_id".into(), + actor_id: "user_id".into(), + target_id: "system".into(), + r#type: AssignmentType::UserSystem, + inherited: false, + role_name: None, + implied_via: None, + }; + + delete(&db, grant).await.unwrap(); + + assert_eq!( + db.into_transaction_log(), + [Transaction::from_sql_and_values( + DatabaseBackend::Postgres, + r#"DELETE FROM "system_assignment" WHERE "system_assignment"."role_id" = $1 AND "system_assignment"."target_id" = $2 AND "system_assignment"."actor_id" = $3 AND "system_assignment"."type" = $4 AND "system_assignment"."inherited" = $5"#, + [ + "role_id".into(), + "system".into(), + "user_id".into(), + "UserSystem".into(), + false.into(), + ] + ),] + ); + } + + #[tokio::test] + async fn test_delete_group_system_success() { + let db = MockDatabase::new(DatabaseBackend::Postgres) + .append_exec_results([MockExecResult { + last_insert_id: 0, + rows_affected: 1, + }]) + .into_connection(); + + let grant = Assignment { + role_id: "role_id".into(), + actor_id: "group_id".into(), + target_id: "system".into(), + r#type: AssignmentType::GroupSystem, + inherited: false, + role_name: None, + implied_via: None, + }; + + delete(&db, grant).await.unwrap(); + + assert_eq!( + db.into_transaction_log(), + [Transaction::from_sql_and_values( + DatabaseBackend::Postgres, + r#"DELETE FROM "system_assignment" WHERE "system_assignment"."role_id" = $1 AND "system_assignment"."target_id" = $2 AND "system_assignment"."actor_id" = $3 AND "system_assignment"."type" = $4 AND "system_assignment"."inherited" = $5"#, + [ + "role_id".into(), + "system".into(), + "group_id".into(), + "GroupSystem".into(), + false.into(), + ] + ),] + ); + } + + #[tokio::test] + async fn test_delete_inherited_assignment_success() { + // Inherited assignments CAN now be deleted (if they exist in DB) + let db = MockDatabase::new(DatabaseBackend::Postgres) + .append_exec_results([MockExecResult { + last_insert_id: 0, + rows_affected: 1, // ← 1 row deleted + }]) + .into_connection(); + + let grant = Assignment { + role_id: "role_id".into(), + actor_id: "user_id".into(), + target_id: "project_id".into(), + r#type: AssignmentType::UserProject, + inherited: true, // ← Inherited assignment + role_name: None, + implied_via: None, + }; + + // Should succeed + delete(&db, grant).await.unwrap(); + + // Verify correct SQL with inherited=true + assert_eq!( + db.into_transaction_log(), + [Transaction::from_sql_and_values( + DatabaseBackend::Postgres, + r#"DELETE FROM "assignment" WHERE "assignment"."role_id" = $1 AND "assignment"."target_id" = $2 AND "assignment"."actor_id" = $3 AND "assignment"."type" = (CAST($4 AS "type")) AND "assignment"."inherited" = $5"#, + [ + "role_id".into(), + "project_id".into(), + "user_id".into(), + "UserProject".into(), + true.into(), // ← inherited=true + ] + ),] + ); + } + + #[tokio::test] + async fn test_delete_not_found_returns_error() { + // Deleting non-existent grant should return AssignmentNotFound error + let db = MockDatabase::new(DatabaseBackend::Postgres) + .append_exec_results([MockExecResult { + last_insert_id: 0, + rows_affected: 0, // ← 0 rows deleted + }]) + .into_connection(); + + let grant = Assignment { + role_id: "nonexistent_role".into(), + actor_id: "user_id".into(), + target_id: "project_id".into(), + r#type: AssignmentType::UserProject, + inherited: false, + role_name: None, + implied_via: None, + }; + + // Should return error + let result = delete(&db, grant).await; + + assert!(result.is_err()); + + // Verify it's the correct error type + match result { + Err(AssignmentDatabaseError::AssignmentNotFound(msg)) => { + assert!(msg.contains("nonexistent_role")); + assert!(msg.contains("user_id")); + assert!(msg.contains("project_id")); + } + _ => panic!("Expected AssignmentNotFound error, got: {:?}", result), + } + + // Verify SQL was still executed + assert_eq!( + db.into_transaction_log(), + [Transaction::from_sql_and_values( + DatabaseBackend::Postgres, + r#"DELETE FROM "assignment" WHERE "assignment"."role_id" = $1 AND "assignment"."target_id" = $2 AND "assignment"."actor_id" = $3 AND "assignment"."type" = (CAST($4 AS "type")) AND "assignment"."inherited" = $5"#, + [ + "nonexistent_role".into(), + "project_id".into(), + "user_id".into(), + "UserProject".into(), + false.into(), + ] + ),] + ); + } + + #[tokio::test] + async fn test_delete_inherited_not_found_returns_error() { + // Deleting non-existent inherited grant should also return error + let db = MockDatabase::new(DatabaseBackend::Postgres) + .append_exec_results([MockExecResult { + last_insert_id: 0, + rows_affected: 0, // ← 0 rows deleted + }]) + .into_connection(); + + let grant = Assignment { + role_id: "role_id".into(), + actor_id: "user_id".into(), + target_id: "project_id".into(), + r#type: AssignmentType::UserProject, + inherited: true, // ← Inherited + role_name: None, + implied_via: None, + }; + + // Should return error + let result = delete(&db, grant).await; + + assert!(result.is_err()); + + match result { + Err(AssignmentDatabaseError::AssignmentNotFound(msg)) => { + assert!(msg.contains("inherited=true")); + } + _ => panic!("Expected AssignmentNotFound error"), + } + } +} diff --git a/src/assignment/error.rs b/src/assignment/error.rs index 977349ac..00979434 100644 --- a/src/assignment/error.rs +++ b/src/assignment/error.rs @@ -21,6 +21,10 @@ use crate::resource::error::ResourceProviderError; /// Assignment provider error. #[derive(Error, Debug)] pub enum AssignmentProviderError { + /// Assignment not found. + #[error("assignment not found: {0}")] + AssignmentNotFound(String), + /// Assignment provider error #[error(transparent)] Backend { source: AssignmentDatabaseError }, @@ -71,6 +75,9 @@ pub enum AssignmentProviderError { impl From for AssignmentProviderError { fn from(source: AssignmentDatabaseError) -> Self { match source { + AssignmentDatabaseError::AssignmentNotFound(msg) => { + AssignmentProviderError::AssignmentNotFound(msg) + } AssignmentDatabaseError::Database { source } => match source { cfl @ crate::error::DatabaseError::Conflict { .. } => { Self::Conflict(cfl.to_string()) diff --git a/src/assignment/mock.rs b/src/assignment/mock.rs index b24bb7fb..9092d85c 100644 --- a/src/assignment/mock.rs +++ b/src/assignment/mock.rs @@ -63,5 +63,11 @@ mock! { state: &ServiceState, params: &RoleAssignmentListParameters, ) -> Result, AssignmentProviderError>; + + async fn revoke_grant( + &self, + state: &ServiceState, + params: Assignment, + ) -> Result<(), AssignmentProviderError>; } } diff --git a/src/assignment/mod.rs b/src/assignment/mod.rs index aae1debd..87f27b28 100644 --- a/src/assignment/mod.rs +++ b/src/assignment/mod.rs @@ -230,4 +230,14 @@ impl AssignmentApi for AssignmentProvider { .list_assignments_for_multiple_actors_and_targets(state, &request.build()?) .await } + + /// Revoke grant + #[tracing::instrument(level = "info", skip(self, state))] + async fn revoke_grant( + &self, + state: &ServiceState, + grant: Assignment, + ) -> Result<(), AssignmentProviderError> { + self.backend_driver.revoke_grant(state, grant).await + } } diff --git a/src/assignment/types/provider_api.rs b/src/assignment/types/provider_api.rs index 5714e7c1..3cf5a3aa 100644 --- a/src/assignment/types/provider_api.rs +++ b/src/assignment/types/provider_api.rs @@ -62,4 +62,11 @@ pub trait AssignmentApi: Send + Sync { state: &ServiceState, params: &RoleAssignmentListParameters, ) -> Result, AssignmentProviderError>; + + /// Revoke assignment grant. + async fn revoke_grant( + &self, + state: &ServiceState, + params: Assignment, + ) -> Result<(), AssignmentProviderError>; }