From 50cde38ef89cdf06756ff501ecdf1e310037db50 Mon Sep 17 00:00:00 2001 From: Christian <37517470+Christian-Toney@users.noreply.github.com> Date: Wed, 25 Feb 2026 14:44:09 -0500 Subject: [PATCH 1/2] Add feature: Update groups --- .../groups/initialize_groups_table.sql | 4 +- src/queries/groups/insert_group_row.sql | 8 +- src/resources/group/mod.rs | 65 ++- src/resources/group/tests.rs | 12 +- src/routes/groups/{group_id}/mod.rs | 123 ++--- src/routes/groups/{group_id}/tests.rs | 498 +++++++++--------- src/tests.rs | 4 +- 7 files changed, 336 insertions(+), 378 deletions(-) diff --git a/src/queries/groups/initialize_groups_table.sql b/src/queries/groups/initialize_groups_table.sql index c899cd72..aa52ed3b 100644 --- a/src/queries/groups/initialize_groups_table.sql +++ b/src/queries/groups/initialize_groups_table.sql @@ -12,9 +12,7 @@ BEGIN id UUID DEFAULT uuidv7() PRIMARY KEY, name TEXT NOT NULL UNIQUE, display_name TEXT NOT NULL, - description TEXT, - parent_resource_type group_parent_resource_type NOT NULL, - parent_group_id UUID references groups(id) ON DELETE CASCADE + description TEXT ); END diff --git a/src/queries/groups/insert_group_row.sql b/src/queries/groups/insert_group_row.sql index 73e4dd6a..75c9b3a2 100644 --- a/src/queries/groups/insert_group_row.sql +++ b/src/queries/groups/insert_group_row.sql @@ -1,13 +1,9 @@ INSERT INTO groups ( name, display_name, - description, - parent_resource_type, - parent_group_id + description ) VALUES ( $1, $2, - $3, - $4, - $5 + $3 ) RETURNING *; \ No newline at end of file diff --git a/src/resources/group/mod.rs b/src/resources/group/mod.rs index cb12c023..c8bd8c07 100644 --- a/src/resources/group/mod.rs +++ b/src/resources/group/mod.rs @@ -12,13 +12,10 @@ pub const ALLOWED_QUERY_KEYS: &[&str] = &[ "id", "name", "display_name", - "description", - "parent_resource_type", - "parent_group_id" + "description" ]; pub const UUID_QUERY_KEYS: &[&str] = &[ - "id", - "parent_group_id" + "id" ]; pub const RESOURCE_NAME: &str = "Group"; pub const DATABASE_TABLE_NAME: &str = "groups"; @@ -42,13 +39,21 @@ pub struct InitialGroupProperties { pub display_name: String, /// The group's description, if applicable. - pub description: Option, + pub description: Option - /// The group's parent resource type. - pub parent_resource_type: GroupParentResourceType, +} - /// The group's parent group ID, if applicable. - pub parent_group_id: Option +#[derive(Debug, Clone, ToSql, FromSql, Default, Serialize, Deserialize)] +pub struct EditableGroupProperties { + + /// The group's name. + pub name: Option, + + /// The group's display name. + pub display_name: Option, + + /// The group's description, if applicable. + pub description: Option> } @@ -65,13 +70,7 @@ pub struct Group { pub display_name: String, /// The group's description, if applicable. - pub description: Option, - - /// The group's parent resource type. - pub parent_resource_type: GroupParentResourceType, - - /// The group's parent group ID, if applicable. - pub parent_group_id: Option + pub description: Option } @@ -134,9 +133,7 @@ impl Group { id: row.get("id"), name: row.get("name"), display_name: row.get("display_name"), - description: row.get("description"), - parent_resource_type: row.get("parent_resource_type"), - parent_group_id: row.get("parent_group_id") + description: row.get("description") }; } @@ -158,9 +155,7 @@ impl Group { let parameters: &[&(dyn ToSql + Sync)] = &[ &initial_properties.name, &initial_properties.display_name, - &initial_properties.description, - &initial_properties.parent_resource_type, - &initial_properties.parent_group_id + &initial_properties.description ]; let database_client = database_pool.get().await?; let row = database_client.query_one(query, parameters).await.map_err(|error| { @@ -219,6 +214,30 @@ impl Group { } + /// Updates this group and returns a new instance of the group. + pub async fn update(&self, properties: &EditableGroupProperties, database_pool: &deadpool_postgres::Pool) -> Result { + + let query = String::from("UPDATE groups SET "); + let parameter_boxes: Vec> = Vec::new(); + let database_client = database_pool.get().await?; + + database_client.query("BEGIN;", &[]).await?; + let (parameter_boxes, query) = slashstepql::add_parameter_to_query(parameter_boxes, query, "name", properties.name.as_ref()); + let (parameter_boxes, query) = slashstepql::add_parameter_to_query(parameter_boxes, query, "display_name", properties.display_name.as_ref()); + let (parameter_boxes, query) = slashstepql::add_parameter_to_query(parameter_boxes, query, "description", properties.description.as_ref()); + let (mut parameter_boxes, mut query) = (parameter_boxes, query); + + query.push_str(format!(" WHERE id = ${} RETURNING *;", parameter_boxes.len() + 1).as_str()); + parameter_boxes.push(Box::new(&self.id)); + let parameters: Vec<&(dyn ToSql + Sync)> = parameter_boxes.iter().map(|parameter| parameter.as_ref() as &(dyn ToSql + Sync)).collect(); + let row = database_client.query_one(&query, ¶meters).await?; + database_client.query("COMMIT;", &[]).await?; + + let group = Self::convert_from_row(&row); + return Ok(group); + + } + } impl DeletableResource for Group { diff --git a/src/resources/group/tests.rs b/src/resources/group/tests.rs index b85834e2..fd477d63 100644 --- a/src/resources/group/tests.rs +++ b/src/resources/group/tests.rs @@ -1,10 +1,10 @@ use uuid::Uuid; use crate::{ - initialize_required_tables, predefinitions::initialize_predefined_actions, initialize_predefined_configurations, resources::{ + initialize_required_tables, predefinitions::initialize_predefined_actions, resources::{ DeletableResource, ResourceError, access_policy::{AccessPolicy, InitialAccessPolicyProperties}, action::{ Action, DEFAULT_ACTION_LIST_LIMIT - }, group::GroupParentResourceType + } }, tests::{TestEnvironment, TestSlashstepServerError} }; use super::{DEFAULT_RESOURCE_LIST_LIMIT, GET_RESOURCE_ACTION_NAME, Group, InitialGroupProperties}; @@ -15,8 +15,6 @@ fn assert_groups_are_equal(group_1: &Group, group_2: &Group) { assert_eq!(group_1.name, group_2.name); assert_eq!(group_1.display_name, group_2.display_name); assert_eq!(group_1.description, group_2.description); - assert_eq!(group_1.parent_resource_type, group_2.parent_resource_type); - assert_eq!(group_1.parent_group_id, group_2.parent_group_id); } @@ -25,8 +23,6 @@ fn assert_group_is_equal_to_initial_properties(group: &Group, initial_properties assert_eq!(group.name, initial_properties.name); assert_eq!(group.display_name, initial_properties.display_name); assert_eq!(group.description, initial_properties.description); - assert_eq!(group.parent_resource_type, initial_properties.parent_resource_type); - assert_eq!(group.parent_group_id, initial_properties.parent_group_id); } @@ -62,9 +58,7 @@ async fn verify_creation() -> Result<(), TestSlashstepServerError> { let group_properties = InitialGroupProperties { name: Uuid::now_v7().to_string(), display_name: Uuid::now_v7().to_string(), - description: Some(Uuid::now_v7().to_string()), - parent_resource_type: GroupParentResourceType::Server, - parent_group_id: None + description: Some(Uuid::now_v7().to_string()) }; let group = Group::create(&group_properties, &test_environment.database_pool).await?; diff --git a/src/routes/groups/{group_id}/mod.rs b/src/routes/groups/{group_id}/mod.rs index 8999365e..25eef6dc 100644 --- a/src/routes/groups/{group_id}/mod.rs +++ b/src/routes/groups/{group_id}/mod.rs @@ -10,16 +10,16 @@ */ use std::sync::Arc; -use axum::{Extension, Json, Router, extract::{Path, State}}; +use axum::{Extension, Json, Router, extract::{Path, State, rejection::JsonRejection}}; use reqwest::StatusCode; use crate::{ AppState, HTTPError, middleware::{authentication_middleware, http_request_middleware}, resources::{ - access_policy::{AccessPolicyResourceType, ActionPermissionLevel}, action_log_entry::{ActionLogEntry, ActionLogEntryActorType, ActionLogEntryTargetResourceType, InitialActionLogEntryProperties}, app::App, app_authorization::AppAuthorization, group::Group, http_transaction::HTTPTransaction, server_log_entry::ServerLogEntry, user::User + access_policy::{AccessPolicyResourceType, ActionPermissionLevel}, action_log_entry::{ActionLogEntry, ActionLogEntryActorType, ActionLogEntryTargetResourceType, InitialActionLogEntryProperties}, app::App, app_authorization::AppAuthorization, group::{EditableGroupProperties, Group}, http_transaction::HTTPTransaction, server_log_entry::ServerLogEntry, user::User }, - utilities::{reusable_route_handlers::delete_resource, route_handler_utilities::{AuthenticatedPrincipal, get_action_by_name, get_action_log_entry_expiration_timestamp, get_authenticated_principal, get_group_by_id, get_resource_hierarchy, get_uuid_from_string, verify_delegate_permissions, verify_principal_permissions}} + utilities::{reusable_route_handlers::delete_resource, route_handler_utilities::{AuthenticatedPrincipal, get_action_by_name, get_action_log_entry_expiration_timestamp, get_authenticated_principal, get_group_by_id, get_request_body_without_json_rejection, get_resource_hierarchy, get_uuid_from_string, verify_delegate_permissions, verify_principal_permissions}} }; #[path = "./access-policies/mod.rs"] @@ -99,94 +99,67 @@ async fn handle_delete_group_request( } -// /// PATCH /groups/{group_id} -// /// -// /// Updates an app by its ID. -// #[axum::debug_handler] -// async fn handle_patch_app_request( -// Path(group_id): Path, -// State(state): State, -// Extension(http_transaction): Extension>, -// Extension(authenticated_user): Extension>>, -// Extension(authenticated_app): Extension>>, -// Extension(authenticated_app_authorization): Extension>>, -// body: Result, JsonRejection> -// ) -> Result, HTTPError> { - -// let http_transaction = http_transaction.clone(); - -// ServerLogEntry::trace("Verifying request body...", Some(&http_transaction.id), &state.database_pool).await.ok(); -// let updated_app_properties = match body { - -// Ok(updated_app_properties) => updated_app_properties, - -// Err(error) => { - -// let http_error = match error { - -// JsonRejection::JsonDataError(error) => HTTPError::BadRequestError(Some(error.to_string())), - -// JsonRejection::JsonSyntaxError(_) => HTTPError::BadRequestError(Some(format!("Failed to parse request body. Ensure the request body is valid JSON."))), - -// JsonRejection::MissingJsonContentType(_) => HTTPError::BadRequestError(Some(format!("Missing request body content type. It should be \"application/json\"."))), - -// JsonRejection::BytesRejection(error) => HTTPError::InternalServerError(Some(format!("Failed to parse request body: {:?}", error))), - -// _ => HTTPError::InternalServerError(Some(error.to_string())) - -// }; - -// ServerLogEntry::from_http_error(&http_error, Some(&http_transaction.id), &state.database_pool).await.ok(); -// return Err(http_error); - -// } - -// }; +/// PATCH /groups/{group_id} +/// +/// Updates a group by its ID. +#[axum::debug_handler] +async fn handle_patch_group_request( + Path(group_id): Path, + State(state): State, + Extension(http_transaction): Extension>, + Extension(authenticated_user): Extension>>, + Extension(authenticated_app): Extension>>, + Extension(authenticated_app_authorization): Extension>>, + body: Result, JsonRejection> +) -> Result, HTTPError> { -// let original_target_field = get_app_by_id(&group_id, &http_transaction, &state.database_pool).await?; -// let resource_hierarchy = get_resource_hierarchy(&original_target_field, &AccessPolicyResourceType::App, &original_target_field.id, &http_transaction, &state.database_pool).await?; -// let update_access_policy_action = get_action_by_name("apps.update", &http_transaction, &state.database_pool).await?; -// verify_delegate_permissions(authenticated_app_authorization.as_ref().map(|app_authorization| &app_authorization.id), &update_access_policy_action.id, &http_transaction.id, &ActionPermissionLevel::User, &state.database_pool).await?; -// let authenticated_principal = get_authenticated_principal(authenticated_user.as_ref(), authenticated_app.as_ref())?; -// verify_principal_permissions(&authenticated_principal, &update_access_policy_action, &resource_hierarchy, &http_transaction, &ActionPermissionLevel::User, &state.database_pool).await?; + let http_transaction = http_transaction.clone(); + let updated_group_properties = get_request_body_without_json_rejection(body, &http_transaction, &state.database_pool).await?; + let group_id = get_uuid_from_string(&group_id, "group", &http_transaction, &state.database_pool).await?; + let original_target_group = get_group_by_id(&group_id, &http_transaction, &state.database_pool).await?; + let resource_hierarchy = get_resource_hierarchy(&original_target_group, &AccessPolicyResourceType::Group, &original_target_group.id, &http_transaction, &state.database_pool).await?; + let update_access_policy_action = get_action_by_name("groups.update", &http_transaction, &state.database_pool).await?; + verify_delegate_permissions(authenticated_app_authorization.as_ref().map(|app_authorization| &app_authorization.id), &update_access_policy_action.id, &http_transaction.id, &ActionPermissionLevel::User, &state.database_pool).await?; + let authenticated_principal = get_authenticated_principal(authenticated_user.as_ref(), authenticated_app.as_ref())?; + verify_principal_permissions(&authenticated_principal, &update_access_policy_action, &resource_hierarchy, &http_transaction, &ActionPermissionLevel::User, &state.database_pool).await?; -// ServerLogEntry::trace(&format!("Updating authenticated_app {}...", original_target_field.id), Some(&http_transaction.id), &state.database_pool).await.ok(); -// let updated_target_action = match original_target_field.update(&updated_app_properties, &state.database_pool).await { + ServerLogEntry::trace(&format!("Updating group {}...", original_target_group.id), Some(&http_transaction.id), &state.database_pool).await.ok(); + let updated_target_group = match original_target_group.update(&updated_group_properties, &state.database_pool).await { -// Ok(updated_target_action) => updated_target_action, + Ok(updated_target_group) => updated_target_group, -// Err(error) => { + Err(error) => { -// let http_error = HTTPError::InternalServerError(Some(format!("Failed to update authenticated_app: {:?}", error))); -// ServerLogEntry::from_http_error(&http_error, Some(&http_transaction.id), &state.database_pool).await.ok(); -// return Err(http_error); + let http_error = HTTPError::InternalServerError(Some(format!("Failed to update group {}: {:?}", original_target_group.id, error))); + ServerLogEntry::from_http_error(&http_error, Some(&http_transaction.id), &state.database_pool).await.ok(); + return Err(http_error); -// } + } -// }; + }; -// ActionLogEntry::create(&InitialActionLogEntryProperties { -// action_id: update_access_policy_action.id, -// http_transaction_id: Some(http_transaction.id), -// actor_type: if let AuthenticatedPrincipal::User(_) = &authenticated_principal { ActionLogEntryActorType::User } else { ActionLogEntryActorType::App }, -// actor_user_id: if let AuthenticatedPrincipal::User(authenticated_user) = &authenticated_principal { Some(authenticated_user.id.clone()) } else { None }, -// actor_group_id: if let AuthenticatedPrincipal::App(authenticated_app) = &authenticated_principal { Some(authenticated_app.id.clone()) } else { None }, -// target_resource_type: ActionLogEntryTargetResourceType::Action, -// target_action_id: Some(updated_target_action.id), -// ..Default::default() -// }, &state.database_pool).await.ok(); -// ServerLogEntry::success(&format!("Successfully updated action {}.", updated_target_action.id), Some(&http_transaction.id), &state.database_pool).await.ok(); + ActionLogEntry::create(&InitialActionLogEntryProperties { + action_id: update_access_policy_action.id, + http_transaction_id: Some(http_transaction.id), + actor_type: if let AuthenticatedPrincipal::User(_) = &authenticated_principal { ActionLogEntryActorType::User } else { ActionLogEntryActorType::App }, + actor_user_id: if let AuthenticatedPrincipal::User(authenticated_user) = &authenticated_principal { Some(authenticated_user.id.clone()) } else { None }, + actor_app_id: if let AuthenticatedPrincipal::App(authenticated_app) = &authenticated_principal { Some(authenticated_app.id.clone()) } else { None }, + target_resource_type: ActionLogEntryTargetResourceType::Group, + target_group_id: Some(updated_target_group.id), + ..Default::default() + }, &state.database_pool).await.ok(); + ServerLogEntry::success(&format!("Successfully updated group {}.", updated_target_group.id), Some(&http_transaction.id), &state.database_pool).await.ok(); -// return Ok(Json(updated_target_action)); + return Ok(Json(updated_target_group)); -// } +} pub fn get_router(state: AppState) -> Router { let router = Router::::new() .route("/groups/{group_id}", axum::routing::get(handle_get_group_request)) .route("/groups/{group_id}", axum::routing::delete(handle_delete_group_request)) - // .route("/groups/{group_id}", axum::routing::patch(handle_patch_app_request)) + .route("/groups/{group_id}", axum::routing::patch(handle_patch_group_request)) .layer(axum::middleware::from_fn_with_state(state.clone(), authentication_middleware::authenticate_user)) .layer(axum::middleware::from_fn_with_state(state.clone(), authentication_middleware::authenticate_app)) .layer(axum::middleware::from_fn_with_state(state.clone(), http_request_middleware::create_http_request)) diff --git a/src/routes/groups/{group_id}/tests.rs b/src/routes/groups/{group_id}/tests.rs index 7474f0e5..272da566 100644 --- a/src/routes/groups/{group_id}/tests.rs +++ b/src/routes/groups/{group_id}/tests.rs @@ -14,13 +14,14 @@ use axum_extra::extract::cookie::Cookie; use axum_test::TestServer; use ntest::timeout; use reqwest::StatusCode; +use uuid::Uuid; use crate::{ Action, AppState, get_json_web_token_private_key, initialize_required_tables, predefinitions::{ initialize_predefined_actions, initialize_predefined_configurations, initialize_predefined_roles }, resources::{ - ResourceError, access_policy::ActionPermissionLevel, group::Group + ResourceError, access_policy::ActionPermissionLevel, group::{EditableGroupProperties, Group} }, tests::{TestEnvironment, TestSlashstepServerError} }; @@ -64,8 +65,6 @@ async fn verify_returned_resource_by_id() -> Result<(), TestSlashstepServerError assert_eq!(response_group.name, group.name); assert_eq!(response_group.display_name, group.display_name); assert_eq!(response_group.description, group.description); - assert_eq!(response_group.parent_resource_type, group.parent_resource_type); - assert_eq!(response_group.parent_group_id, group.parent_group_id); return Ok(()); @@ -368,278 +367,259 @@ async fn verify_resource_exists_when_deleting_by_id() -> Result<(), TestSlashste } -// /// Verifies that the router can return a 200 status code if the resource is successfully patched. -// #[tokio::test] -// async fn verify_successful_patch_by_id() -> Result<(), TestSlashstepServerError> { +/// Verifies that the router can return a 200 status code if the resource is successfully patched. +#[tokio::test] +async fn verify_successful_patch_by_id() -> Result<(), TestSlashstepServerError> { -// let test_environment = TestEnvironment::new().await?; -// initialize_required_tables(&test_environment.database_pool).await?; -// initialize_predefined_actions(&test_environment.database_pool).await?; -// initialize_predefined_roles(&test_environment.database_pool).await?; - -// // Create the user and the session. -// let user = test_environment.create_random_user().await?; -// let session = test_environment.create_random_session(Some(&user.id)).await?; -// let json_web_token_private_key = get_json_web_token_private_key().await?; -// let session_token = session.generate_json_web_token(&json_web_token_private_key).await?; -// let update_fields_action = Action::get_by_name("groups.update", &test_environment.database_pool).await?; -// AccessPolicy::create(&InitialAccessPolicyProperties { -// action_id: update_fields_action.id, -// permission_level: ActionPermissionLevel::Editor, -// is_inheritance_enabled: true, -// principal_type: AccessPolicyPrincipalType::User, -// principal_user_id: Some(user.id), -// scoped_resource_type: AccessPolicyResourceType::Server, -// ..Default::default() -// }, &test_environment.database_pool).await?; - -// // Set up the server and send the request. -// let original_app = test_environment.create_random_app().await?; -// let new_name = Uuid::now_v7().to_string(); -// let new_display_name = Uuid::now_v7().to_string(); -// let new_description = Some(Uuid::now_v7().to_string()); -// let new_client_type = AppClientType::Confidential; - -// let state = AppState { -// database_pool: test_environment.database_pool.clone(), -// }; -// let router = super::get_router(state.clone()) -// .with_state(state) -// .into_make_service_with_connect_info::(); -// let test_server = TestServer::new(router)?; -// let response = test_server.patch(&format!("/groups/{}", original_field.id)) -// .add_cookie(Cookie::new("sessionToken", format!("Bearer {}", session_token))) -// .json(&serde_json::json!({ -// "name": new_name.clone(), -// "display_name": new_display_name.clone(), -// "description": new_description.clone(), -// "client_type": new_client_type.clone() -// })) -// .await; + let test_environment = TestEnvironment::new().await?; + initialize_required_tables(&test_environment.database_pool).await?; + initialize_predefined_actions(&test_environment.database_pool).await?; + initialize_predefined_roles(&test_environment.database_pool).await?; + initialize_predefined_configurations(&test_environment.database_pool).await?; + + // Create the user and the session. + let user = test_environment.create_random_user().await?; + let session = test_environment.create_random_session(Some(&user.id)).await?; + let json_web_token_private_key = get_json_web_token_private_key().await?; + let session_token = session.generate_json_web_token(&json_web_token_private_key).await?; + let update_groups_action = Action::get_by_name("groups.update", &test_environment.database_pool).await?; + test_environment.create_server_access_policy(&user.id, &update_groups_action.id, &ActionPermissionLevel::User).await?; + + // Set up the server and send the request. + let original_group = test_environment.create_random_group().await?; + let updated_group_properties = EditableGroupProperties { + name: Some(Uuid::now_v7().to_string()), + display_name: Some(Uuid::now_v7().to_string()), + description: Some(Some(Uuid::now_v7().to_string())) + }; + + let state = AppState { + database_pool: test_environment.database_pool.clone(), + }; + let router = super::get_router(state.clone()) + .with_state(state) + .into_make_service_with_connect_info::(); + let test_server = TestServer::new(router)?; + let response = test_server.patch(&format!("/groups/{}", original_group.id)) + .add_cookie(Cookie::new("sessionToken", format!("Bearer {}", session_token))) + .json(&serde_json::json!(updated_group_properties)) + .await; -// // Verify the response. -// assert_eq!(response.status_code(), StatusCode::OK); - -// let updated_app: Field = response.json(); -// assert_eq!(original_field.id, updated_field.id); -// assert_eq!(updated_app.name, new_name); -// assert_eq!(updated_app.display_name, new_display_name); -// assert_eq!(updated_app.description, new_description); -// assert_eq!(updated_app.client_type, new_client_type); -// assert_eq!(original_app.parent_resource_type, updated_app.parent_resource_type); -// assert_eq!(original_app.parent_workspace_id, updated_app.parent_workspace_id); -// assert_eq!(original_app.parent_user_id, updated_app.parent_user_id); - -// return Ok(()); - -// } - -// /// Verifies that the router can return a 400 status code if the request doesn't have a valid content type. -// #[tokio::test] -// async fn verify_content_type_when_patching_by_id() -> Result<(), TestSlashstepServerError> { - -// let test_environment = TestEnvironment::new().await?; -// initialize_required_tables(&test_environment.database_pool).await?; -// initialize_predefined_actions(&test_environment.database_pool).await?; -// initialize_predefined_roles(&test_environment.database_pool).await?; - -// // Set up the server and send the request. -// let state = AppState { -// database_pool: test_environment.database_pool.clone(), -// }; -// let router = super::get_router(state.clone()) -// .with_state(state) -// .into_make_service_with_connect_info::(); -// let test_server = TestServer::new(router)?; -// let response = test_server.patch("/groups/not-a-uuid") -// .await; + // Verify the response. + assert_eq!(response.status_code(), StatusCode::OK); + + let updated_group: Group = response.json(); + assert_eq!(updated_group.id, original_group.id); + assert_eq!(updated_group.name, updated_group_properties.name.unwrap()); + assert_eq!(updated_group.display_name, updated_group_properties.display_name.unwrap()); + assert_eq!(updated_group.description, updated_group_properties.description.unwrap()); + + return Ok(()); + +} + +/// Verifies that the router can return a 400 status code if the request doesn't have a valid content type. +#[tokio::test] +async fn verify_content_type_when_patching_by_id() -> Result<(), TestSlashstepServerError> { + + let test_environment = TestEnvironment::new().await?; + initialize_required_tables(&test_environment.database_pool).await?; + initialize_predefined_actions(&test_environment.database_pool).await?; + initialize_predefined_roles(&test_environment.database_pool).await?; + initialize_predefined_configurations(&test_environment.database_pool).await?; + + // Set up the server and send the request. + let state = AppState { + database_pool: test_environment.database_pool.clone(), + }; + let router = super::get_router(state.clone()) + .with_state(state) + .into_make_service_with_connect_info::(); + let test_server = TestServer::new(router)?; + let response = test_server.patch("/groups/not-a-uuid") + .await; -// // Verify the response. -// assert_eq!(response.status_code(), StatusCode::BAD_REQUEST); -// return Ok(()); - -// } - -// /// Verifies that the router can return a 400 status code if the request body is not valid JSON. -// #[tokio::test] -// async fn verify_request_body_exists_when_patching_by_id() -> Result<(), TestSlashstepServerError> { - -// let test_environment = TestEnvironment::new().await?; -// initialize_required_tables(&test_environment.database_pool).await?; -// initialize_predefined_actions(&test_environment.database_pool).await?; -// initialize_predefined_roles(&test_environment.database_pool).await?; - -// // Set up the server and send the request. -// let state = AppState { -// database_pool: test_environment.database_pool.clone(), -// }; -// let router = super::get_router(state.clone()) -// .with_state(state) -// .into_make_service_with_connect_info::(); -// let test_server = TestServer::new(router)?; -// let response = test_server.patch("/groups/not-a-uuid") -// .add_header("Content-Type", "application/json") -// .await; + // Verify the response. + assert_eq!(response.status_code(), StatusCode::BAD_REQUEST); + return Ok(()); + +} + +/// Verifies that the router can return a 400 status code if the request body is not valid JSON. +#[tokio::test] +async fn verify_request_body_exists_when_patching_by_id() -> Result<(), TestSlashstepServerError> { + + let test_environment = TestEnvironment::new().await?; + initialize_required_tables(&test_environment.database_pool).await?; + initialize_predefined_actions(&test_environment.database_pool).await?; + initialize_predefined_roles(&test_environment.database_pool).await?; + initialize_predefined_configurations(&test_environment.database_pool).await?; + + // Set up the server and send the request. + let state = AppState { + database_pool: test_environment.database_pool.clone(), + }; + let router = super::get_router(state.clone()) + .with_state(state) + .into_make_service_with_connect_info::(); + let test_server = TestServer::new(router)?; + let response = test_server.patch("/groups/not-a-uuid") + .add_header("Content-Type", "application/json") + .await; -// // Verify the response. -// assert_eq!(response.status_code(), StatusCode::BAD_REQUEST); -// return Ok(()); + // Verify the response. + assert_eq!(response.status_code(), StatusCode::BAD_REQUEST); + return Ok(()); -// } +} -// /// Verifies that the router can return a 400 status code if the request body includes unwanted data. -// #[tokio::test] -// async fn verify_request_body_json_when_patching_by_id() -> Result<(), TestSlashstepServerError> { +/// Verifies that the router can return a 400 status code if the request body includes unwanted data. +#[tokio::test] +async fn verify_request_body_json_when_patching_by_id() -> Result<(), TestSlashstepServerError> { -// let test_environment = TestEnvironment::new().await?; -// initialize_required_tables(&test_environment.database_pool).await?; -// initialize_predefined_actions(&test_environment.database_pool).await?; -// initialize_predefined_roles(&test_environment.database_pool).await?; + let test_environment = TestEnvironment::new().await?; + initialize_required_tables(&test_environment.database_pool).await?; + initialize_predefined_actions(&test_environment.database_pool).await?; + initialize_predefined_roles(&test_environment.database_pool).await?; + initialize_predefined_configurations(&test_environment.database_pool).await?; -// // Set up the server and send the request. -// let state = AppState { -// database_pool: test_environment.database_pool.clone(), -// }; -// let router = super::get_router(state.clone()) -// .with_state(state) -// .into_make_service_with_connect_info::(); -// let test_server = TestServer::new(router)?; -// let response = test_server.patch(&format!("/groups/{}", uuid::Uuid::now_v7())) -// .add_header("Content-Type", "application/json") -// .json(&serde_json::json!({ -// "name": "Super Duper Admin", -// "display_name": "true", -// "description": true, -// })) -// .await; + // Create a dummy delegation policy to patch. + let group = test_environment.create_random_group().await?; + + // Set up the server and send the request. + let state = AppState { + database_pool: test_environment.database_pool.clone(), + }; + let router = super::get_router(state.clone()) + .with_state(state) + .into_make_service_with_connect_info::(); + let test_server = TestServer::new(router)?; + let response = test_server.patch(&format!("/groups/{}", group.id)) + .add_header("Content-Type", "application/json") + .json(&serde_json::json!({ + "name": true + })) + .await; -// // Verify the response. -// assert_eq!(response.status_code(), StatusCode::BAD_REQUEST); -// return Ok(()); - -// } - -// /// Verifies that the router can return a 400 status code if the resource ID is not a UUID. -// #[tokio::test] -// async fn verify_uuid_when_patching_by_id() -> Result<(), TestSlashstepServerError> { - -// let test_environment = TestEnvironment::new().await?; -// initialize_required_tables(&test_environment.database_pool).await?; -// initialize_predefined_actions(&test_environment.database_pool).await?; -// initialize_predefined_roles(&test_environment.database_pool).await?; -// let state = AppState { -// database_pool: test_environment.database_pool.clone(), -// }; -// let router = super::get_router(state.clone()) -// .with_state(state) -// .into_make_service_with_connect_info::(); -// let test_server = TestServer::new(router)?; -// let response = test_server.patch("/groups/not-a-uuid") -// .add_header("Content-Type", "application/json") -// .json(&serde_json::json!({ -// "display_name": Uuid::now_v7().to_string() -// })) -// .await; + // Verify the response. + assert_eq!(response.status_code(), StatusCode::BAD_REQUEST); + return Ok(()); + +} + +/// Verifies that the router can return a 400 status code if the resource ID is not a UUID. +#[tokio::test] +async fn verify_uuid_when_patching_by_id() -> Result<(), TestSlashstepServerError> { + + let test_environment = TestEnvironment::new().await?; + initialize_required_tables(&test_environment.database_pool).await?; + initialize_predefined_actions(&test_environment.database_pool).await?; + initialize_predefined_roles(&test_environment.database_pool).await?; + let state = AppState { + database_pool: test_environment.database_pool.clone(), + }; + let router = super::get_router(state.clone()) + .with_state(state) + .into_make_service_with_connect_info::(); + let test_server = TestServer::new(router)?; + let response = test_server.patch("/groups/not-a-uuid") + .add_header("Content-Type", "application/json") + .json(&serde_json::json!({})) + .await; -// assert_eq!(response.status_code(), StatusCode::BAD_REQUEST); -// return Ok(()); + assert_eq!(response.status_code(), StatusCode::BAD_REQUEST); + return Ok(()); -// } +} -// /// Verifies that the router can return a 401 status code if the user needs authentication. -// #[tokio::test] -// async fn verify_authentication_when_patching_by_id() -> Result<(), TestSlashstepServerError> { +/// Verifies that the router can return a 401 status code if the user needs authentication. +#[tokio::test] +async fn verify_authentication_when_patching_by_id() -> Result<(), TestSlashstepServerError> { -// let test_environment = TestEnvironment::new().await?; -// initialize_required_tables(&test_environment.database_pool).await?; -// initialize_predefined_actions(&test_environment.database_pool).await?; -// initialize_predefined_roles(&test_environment.database_pool).await?; + let test_environment = TestEnvironment::new().await?; + initialize_required_tables(&test_environment.database_pool).await?; + initialize_predefined_actions(&test_environment.database_pool).await?; + initialize_predefined_roles(&test_environment.database_pool).await?; -// // Set up the server and send the request. -// let group = test_environment.create_random_group().await?; -// let state = AppState { -// database_pool: test_environment.database_pool.clone(), -// }; -// let router = super::get_router(state.clone()) -// .with_state(state) -// .into_make_service_with_connect_info::(); -// let test_server = TestServer::new(router)?; -// let response = test_server.patch(&format!("/groups/{}", group.id)) -// .json(&serde_json::json!({ -// "display_name": Uuid::now_v7().to_string() -// })) -// .await; + // Set up the server and send the request. + let group = test_environment.create_random_group().await?; + let state = AppState { + database_pool: test_environment.database_pool.clone(), + }; + let router = super::get_router(state.clone()) + .with_state(state) + .into_make_service_with_connect_info::(); + let test_server = TestServer::new(router)?; + let response = test_server.patch(&format!("/groups/{}", group.id)) + .json(&serde_json::json!({})) + .await; -// assert_eq!(response.status_code(), StatusCode::UNAUTHORIZED); - -// return Ok(()); - -// } - -// /// Verifies that the router can return a 403 status code if the user does not have permission to patch the resource. -// #[tokio::test] -// async fn verify_permission_when_patching() -> Result<(), TestSlashstepServerError> { - -// let test_environment = TestEnvironment::new().await?; -// initialize_required_tables(&test_environment.database_pool).await?; -// initialize_predefined_actions(&test_environment.database_pool).await?; -// initialize_predefined_roles(&test_environment.database_pool).await?; - -// // Create the user and the session. -// let user = test_environment.create_random_user().await?; -// let session = test_environment.create_random_session(Some(&user.id)).await?; -// let json_web_token_private_key = get_json_web_token_private_key().await?; -// let session_token = session.generate_json_web_token(&json_web_token_private_key).await?; - -// // Set up the server and send the request. -// let group = test_environment.create_random_group().await?; -// let state = AppState { -// database_pool: test_environment.database_pool.clone(), -// }; -// let router = super::get_router(state.clone()) -// .with_state(state) -// .into_make_service_with_connect_info::(); -// let test_server = TestServer::new(router)?; -// let response = test_server.patch(&format!("/groups/{}", group.id)) -// .add_cookie(Cookie::new("sessionToken", format!("Bearer {}", session_token))) -// .json(&serde_json::json!({ -// "display_name": Uuid::now_v7().to_string() -// })) -// .await; + assert_eq!(response.status_code(), StatusCode::UNAUTHORIZED); + + return Ok(()); + +} + +/// Verifies that the router can return a 403 status code if the user does not have permission to patch the resource. +#[tokio::test] +async fn verify_permission_when_patching() -> Result<(), TestSlashstepServerError> { + + let test_environment = TestEnvironment::new().await?; + initialize_required_tables(&test_environment.database_pool).await?; + initialize_predefined_actions(&test_environment.database_pool).await?; + initialize_predefined_roles(&test_environment.database_pool).await?; + + // Create the user and the session. + let user = test_environment.create_random_user().await?; + let session = test_environment.create_random_session(Some(&user.id)).await?; + let json_web_token_private_key = get_json_web_token_private_key().await?; + let session_token = session.generate_json_web_token(&json_web_token_private_key).await?; + + // Set up the server and send the request. + let group = test_environment.create_random_group().await?; + let state = AppState { + database_pool: test_environment.database_pool.clone(), + }; + let router = super::get_router(state.clone()) + .with_state(state) + .into_make_service_with_connect_info::(); + let test_server = TestServer::new(router)?; + let response = test_server.patch(&format!("/groups/{}", group.id)) + .add_cookie(Cookie::new("sessionToken", format!("Bearer {}", session_token))) + .json(&serde_json::json!({})) + .await; -// // Verify the response. -// assert_eq!(response.status_code(), StatusCode::FORBIDDEN); - -// return Ok(()); - -// } - -// /// Verifies that the router can return a 404 status code if the resource does not exist. -// #[tokio::test] -// async fn verify_resource_exists_when_patching() -> Result<(), TestSlashstepServerError> { - -// let test_environment = TestEnvironment::new().await?; -// initialize_required_tables(&test_environment.database_pool).await?; -// initialize_predefined_actions(&test_environment.database_pool).await?; -// initialize_predefined_roles(&test_environment.database_pool).await?; - -// // Set up the server and send the request. -// let state = AppState { -// database_pool: test_environment.database_pool.clone(), -// }; -// let router = super::get_router(state.clone()) -// .with_state(state) -// .into_make_service_with_connect_info::(); -// let test_server = TestServer::new(router)?; -// let response = test_server.patch(&format!("/groups/{}", Uuid::now_v7())) -// .json(&serde_json::json!({ -// "display_name": Uuid::now_v7().to_string() -// })) -// .await; + // Verify the response. + assert_eq!(response.status_code(), StatusCode::FORBIDDEN); + + return Ok(()); + +} + +/// Verifies that the router can return a 404 status code if the resource does not exist. +#[tokio::test] +async fn verify_resource_exists_when_patching() -> Result<(), TestSlashstepServerError> { + + let test_environment = TestEnvironment::new().await?; + initialize_required_tables(&test_environment.database_pool).await?; + initialize_predefined_actions(&test_environment.database_pool).await?; + initialize_predefined_roles(&test_environment.database_pool).await?; + + // Set up the server and send the request. + let state = AppState { + database_pool: test_environment.database_pool.clone(), + }; + let router = super::get_router(state.clone()) + .with_state(state) + .into_make_service_with_connect_info::(); + let test_server = TestServer::new(router)?; + let response = test_server.patch(&format!("/groups/{}", Uuid::now_v7())) + .json(&serde_json::json!({})) + .await; -// // Verify the response. -// assert_eq!(response.status_code(), StatusCode::NOT_FOUND); + // Verify the response. + assert_eq!(response.status_code(), StatusCode::NOT_FOUND); -// return Ok(()); + return Ok(()); -// } \ No newline at end of file +} \ No newline at end of file diff --git a/src/tests.rs b/src/tests.rs index a5053944..c7fb53d8 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -320,9 +320,7 @@ impl TestEnvironment { let group_properties = InitialGroupProperties { name: Uuid::now_v7().to_string(), display_name: Uuid::now_v7().to_string(), - description: Some(Uuid::now_v7().to_string()), - parent_resource_type: GroupParentResourceType::Server, - parent_group_id: None + description: Some(Uuid::now_v7().to_string()) }; let group = Group::create(&group_properties, &self.database_pool).await?; From d58a25f586fd2701ad5bad9d79cdd1b2c3622aa4 Mon Sep 17 00:00:00 2001 From: Christian <37517470+Christian-Toney@users.noreply.github.com> Date: Wed, 25 Feb 2026 15:07:07 -0500 Subject: [PATCH 2/2] Fix group hierarchy --- ...te_function_can_principal_get_resource.sql | 44 ++----------------- 1 file changed, 3 insertions(+), 41 deletions(-) diff --git a/src/queries/access_policies/create_function_can_principal_get_resource.sql b/src/queries/access_policies/create_function_can_principal_get_resource.sql index 16d2ec2c..c11bed61 100644 --- a/src/queries/access_policies/create_function_can_principal_get_resource.sql +++ b/src/queries/access_policies/create_function_can_principal_get_resource.sql @@ -797,7 +797,7 @@ CREATE OR REPLACE FUNCTION can_principal_get_resource( ELSIF selected_resource_type = 'Group' THEN - -- Group -> (Group | Server) + -- Group -> Server -- Check if the group has an associated access policy. SELECT permission_level, @@ -827,46 +827,8 @@ CREATE OR REPLACE FUNCTION can_principal_get_resource( -- Use the parent resource type. needs_inheritance := TRUE; - - SELECT - parent_resource_type - INTO - selected_resource_parent_type - FROM - groups - WHERE - groups.id = selected_resource_id; - - IF selected_resource_parent_type = 'Group' THEN - - SELECT - parent_group_id - INTO - selected_resource_parent_id - FROM - groups - WHERE - groups.id = selected_resource_id; - - IF selected_resource_parent_id IS NULL THEN - - RAISE EXCEPTION 'Couldn''t find a parent group for group %.', selected_resource_id; - - END IF; - - selected_resource_type := 'Group'; - selected_resource_id := selected_resource_parent_id; - - ELSIF selected_resource_parent_type = 'Server' THEN - - selected_resource_type := 'Server'; - selected_resource_id := NULL; - - ELSE - - RAISE EXCEPTION 'Unknown parent resource type % for group %.', selected_resource_parent_type, selected_resource_id; - - END IF; + selected_resource_type := 'Server'; + selected_resource_id := NULL; ELSIF selected_resource_type = 'HTTPTransaction' THEN