diff --git a/api/libs/handlers/src/metrics/sharing/mod.rs b/api/libs/handlers/src/metrics/sharing/mod.rs index 1a8419a57..f9b313bb4 100644 --- a/api/libs/handlers/src/metrics/sharing/mod.rs +++ b/api/libs/handlers/src/metrics/sharing/mod.rs @@ -1,7 +1,9 @@ pub mod list_sharing_handler; pub mod create_sharing_handler; pub mod delete_sharing_handler; +pub mod update_sharing_handler; pub use list_sharing_handler::*; pub use create_sharing_handler::*; pub use delete_sharing_handler::*; +pub use update_sharing_handler::*; diff --git a/api/libs/handlers/src/metrics/sharing/update_sharing_handler.rs b/api/libs/handlers/src/metrics/sharing/update_sharing_handler.rs new file mode 100644 index 000000000..29e33d990 --- /dev/null +++ b/api/libs/handlers/src/metrics/sharing/update_sharing_handler.rs @@ -0,0 +1,102 @@ +use anyhow::{anyhow, Result}; +use database::{ + enums::{AssetPermissionRole, AssetType, IdentityType}, + helpers::metric_files::fetch_metric_file, +}; +use sharing::{ + check_asset_permission::has_permission, + create_asset_permission::create_share_by_email, +}; +use tracing::info; +use uuid::Uuid; + +/// Handler to update sharing permissions for a metric +/// +/// # Arguments +/// * `metric_id` - The UUID of the metric to update sharing permissions for +/// * `user_id` - The UUID of the user making the request +/// * `emails_and_roles` - List of tuples containing (email, role) pairs to update +/// +/// # Returns +/// * `Result<()>` - Success if all sharing permissions were updated +pub async fn update_metric_sharing_handler( + metric_id: &Uuid, + user_id: &Uuid, + emails_and_roles: Vec<(String, AssetPermissionRole)>, +) -> Result<()> { + info!( + metric_id = %metric_id, + user_id = %user_id, + recipients_count = emails_and_roles.len(), + "Updating sharing permissions for metric" + ); + + // 1. Validate the metric exists + let _metric = match fetch_metric_file(metric_id).await? { + Some(metric) => metric, + None => return Err(anyhow!("Metric not found")), + }; + + // 2. Check if user has permission to update sharing for the metric (Owner or FullAccess) + let has_permission = has_permission( + *metric_id, + AssetType::MetricFile, + *user_id, + IdentityType::User, + AssetPermissionRole::FullAccess, // Owner role implicitly has FullAccess permissions + ).await?; + + if !has_permission { + return Err(anyhow!("User does not have permission to update sharing for this metric")); + } + + // 3. Process each email-role pair and update sharing permissions + for (email, role) in emails_and_roles { + // Validate email format + if !email.contains('@') { + return Err(anyhow!("Invalid email format: {}", email)); + } + + // Update (or create if not exists) the permission using create_share_by_email + // The create_share_by_email function handles both creation and updates with upsert + match create_share_by_email( + &email, + *metric_id, + AssetType::MetricFile, + role, + *user_id, + ).await { + Ok(_) => { + info!("Updated sharing permission for email: {} with role: {:?} on metric: {}", email, role, metric_id); + }, + Err(e) => { + return Err(anyhow!("Failed to update sharing for email {}: {}", email, e)); + } + } + } + + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[tokio::test] + async fn test_update_metric_sharing_invalid_email() { + let metric_id = Uuid::new_v4(); + let user_id = Uuid::new_v4(); + let emails_and_roles = vec![("invalid-email-format".to_string(), AssetPermissionRole::CanView)]; + + let result = update_metric_sharing_handler(&metric_id, &user_id, emails_and_roles).await; + + assert!(result.is_err()); + let error = result.unwrap_err().to_string(); + assert!(error.contains("Invalid email format")); + } + + // Note: For comprehensive tests, we would need to set up proper mocks + // for external dependencies like fetch_metric_file, has_permission, + // and create_share_by_email. This would typically involve using a + // mocking framework that's compatible with async functions. +} \ No newline at end of file diff --git a/api/prds/active/api_metrics_sharing_summary.md b/api/prds/active/api_metrics_sharing_summary.md index e9e91dfb8..0bf83dd1d 100644 --- a/api/prds/active/api_metrics_sharing_summary.md +++ b/api/prds/active/api_metrics_sharing_summary.md @@ -16,8 +16,8 @@ The implementation is broken down into the following components, each with its o 2. **Create Metrics Sharing Endpoint** - POST /metrics/:id/sharing - PRD: [api_metrics_sharing_create.md](/Users/dallin/buster/buster/api/prds/active/api_metrics_sharing_create.md) -3. **Update Metrics Sharing Endpoint** - PUT /metrics/:id/sharing - - PRD: [api_metrics_sharing_update.md](/Users/dallin/buster/buster/api/prds/active/api_metrics_sharing_update.md) +3. **Update Metrics Sharing Endpoint** - PUT /metrics/:id/sharing ✅ + - PRD: [api_metrics_sharing_update.md](/Users/dallin/api_metrics_sharing_update/api/prds/active/api_metrics_sharing_update.md) 4. **Delete Metrics Sharing Endpoint** - DELETE /metrics/:id/sharing - PRD: [api_metrics_sharing_delete.md](/Users/dallin/buster/buster/api/prds/active/api_metrics_sharing_delete.md) @@ -68,7 +68,7 @@ After Phase 1 is complete, the following components can be implemented in parall - Uses `create_share_by_email` from `@[api/libs/sharing/src]/create_asset_permission.rs` - Uses `has_permission` from `@[api/libs/sharing/src]/check_asset_permission.rs` -- **Update Sharing Endpoint** +- **Update Sharing Endpoint** ✅ - Uses `create_share_by_email` from `@[api/libs/sharing/src]/create_asset_permission.rs` - Uses `has_permission` from `@[api/libs/sharing/src]/check_asset_permission.rs` diff --git a/api/prds/active/api_metrics_sharing_update.md b/api/prds/active/api_metrics_sharing_update.md index 3c284b571..26f184ea7 100644 --- a/api/prds/active/api_metrics_sharing_update.md +++ b/api/prds/active/api_metrics_sharing_update.md @@ -1,4 +1,5 @@ # API Metrics Sharing - Update Endpoint PRD +✅ Implemented ## Problem Statement Users need the ability to update sharing permissions for metrics through a REST API endpoint. diff --git a/api/src/routes/rest/routes/collections/sharing/list_sharing.rs b/api/src/routes/rest/routes/collections/sharing/list_sharing.rs index 26335688f..58aae0275 100644 --- a/api/src/routes/rest/routes/collections/sharing/list_sharing.rs +++ b/api/src/routes/rest/routes/collections/sharing/list_sharing.rs @@ -44,7 +44,7 @@ pub async fn list_collection_sharing_rest_handler( role: p.permission.role, }).collect(), }; - Ok(ApiResponse::Success(response)) + Ok(ApiResponse::JsonData(response)) }, Err(e) => { tracing::error!("Error listing sharing permissions: {}", e); diff --git a/api/src/routes/rest/routes/metrics/sharing/mod.rs b/api/src/routes/rest/routes/metrics/sharing/mod.rs index f57767361..e90679c76 100644 --- a/api/src/routes/rest/routes/metrics/sharing/mod.rs +++ b/api/src/routes/rest/routes/metrics/sharing/mod.rs @@ -1,15 +1,17 @@ use axum::{ - routing::{get, post, delete}, + routing::{get, post, delete, put}, Router, }; mod list_sharing; mod create_sharing; mod delete_sharing; +mod update_sharing; pub fn router() -> Router { Router::new() .route("/", get(list_sharing::list_metric_sharing_rest_handler)) .route("/", post(create_sharing::create_metric_sharing_rest_handler)) + .route("/", put(update_sharing::update_metric_sharing_rest_handler)) .route("/", delete(delete_sharing::delete_metric_sharing_rest_handler)) } \ No newline at end of file diff --git a/api/src/routes/rest/routes/metrics/sharing/update_sharing.rs b/api/src/routes/rest/routes/metrics/sharing/update_sharing.rs new file mode 100644 index 000000000..55207257a --- /dev/null +++ b/api/src/routes/rest/routes/metrics/sharing/update_sharing.rs @@ -0,0 +1,65 @@ +use axum::{ + extract::{Json, Path}, + http::StatusCode, + Extension, +}; +use database::enums::AssetPermissionRole; +use handlers::metrics::sharing::update_metric_sharing_handler; +use middleware::AuthenticatedUser; +use serde::Deserialize; +use uuid::Uuid; + +use crate::routes::rest::ApiResponse; + +/// Structure for a single share recipient with their role +#[derive(Debug, Deserialize)] +pub struct ShareRecipient { + pub email: String, + pub role: AssetPermissionRole, +} + +/// REST handler for updating sharing permissions for a metric +pub async fn update_metric_sharing_rest_handler( + Extension(user): Extension, + Path(id): Path, + Json(request): Json>, +) -> Result, (StatusCode, String)> { + tracing::info!( + "Processing PUT request for metric sharing with ID: {}, user_id: {}", + id, + user.id + ); + + let emails_and_roles: Vec<(String, AssetPermissionRole)> = request + .into_iter() + .map(|recipient| (recipient.email, recipient.role)) + .collect(); + + match update_metric_sharing_handler(&id, &user.id, emails_and_roles).await { + Ok(_) => Ok(ApiResponse::JsonData( + "Sharing permissions updated successfully".to_string(), + )), + Err(e) => { + tracing::error!("Error updating sharing permissions: {}", e); + + // Map specific errors to appropriate status codes + let error_message = e.to_string(); + + if error_message.contains("not found") { + return Err((StatusCode::NOT_FOUND, format!("Metric not found: {}", e))); + } else if error_message.contains("permission") { + return Err(( + StatusCode::FORBIDDEN, + format!("Insufficient permissions: {}", e), + )); + } else if error_message.contains("Invalid email") { + return Err((StatusCode::BAD_REQUEST, format!("Invalid email: {}", e))); + } + + Err(( + StatusCode::INTERNAL_SERVER_ERROR, + format!("Failed to update sharing permissions: {}", e), + )) + } + } +} \ No newline at end of file diff --git a/api/tests/integration/metrics/sharing/mod.rs b/api/tests/integration/metrics/sharing/mod.rs index 59e5bac50..bd317d6e2 100644 --- a/api/tests/integration/metrics/sharing/mod.rs +++ b/api/tests/integration/metrics/sharing/mod.rs @@ -1,3 +1,4 @@ pub mod list_sharing_test; pub mod create_sharing_test; pub mod delete_sharing_test; +pub mod update_sharing_test; diff --git a/api/tests/integration/metrics/sharing/update_sharing_test.rs b/api/tests/integration/metrics/sharing/update_sharing_test.rs new file mode 100644 index 000000000..095de3111 --- /dev/null +++ b/api/tests/integration/metrics/sharing/update_sharing_test.rs @@ -0,0 +1,317 @@ +use axum::http::StatusCode; +use chrono::Utc; +use database::{ + enums::{AssetPermissionRole, AssetType, IdentityType}, + models::{AssetPermission}, + pool::get_pg_pool, + schema::{users, metric_files, asset_permissions}, +}; +use diesel::{ExpressionMethods, QueryDsl}; +use diesel_async::RunQueryDsl; +use serde_json::json; +use uuid::Uuid; + +use crate::common::{ + http::client::TestClient, + fixtures::{users::create_test_user, metrics::create_test_metric_file}, +}; + +#[tokio::test] +async fn test_update_metric_sharing_success() { + // Setup test database connection + let mut conn = get_pg_pool().get().await.unwrap(); + + // 1. Create test owner and shared user + let owner = create_test_user("owner@example.com"); + let org_id = Uuid::new_v4(); // Need org id for fixture + diesel::insert_into(users::table) + .values(&owner) + .execute(&mut conn) + .await + .unwrap(); + + let shared_user = create_test_user("shared@example.com"); + diesel::insert_into(users::table) + .values(&shared_user) + .execute(&mut conn) + .await + .unwrap(); + + // 2. Create test metric + let metric = create_test_metric_file(&owner.id, &org_id, None); + diesel::insert_into(metric_files::table) + .values(&metric) + .execute(&mut conn) + .await + .unwrap(); + + // 3. Create owner permission for test user + let now = Utc::now(); + let owner_permission = AssetPermission { + identity_id: owner.id, + identity_type: IdentityType::User, + asset_id: metric.id, + asset_type: AssetType::MetricFile, + role: AssetPermissionRole::Owner, + created_at: now, + updated_at: now, + deleted_at: None, + created_by: owner.id, + updated_by: owner.id, + }; + + // 4. Create existing CanView permission for shared user + let shared_permission = AssetPermission { + identity_id: shared_user.id, + identity_type: IdentityType::User, + asset_id: metric.id, + asset_type: AssetType::MetricFile, + role: AssetPermissionRole::CanView, + created_at: now, + updated_at: now, + deleted_at: None, + created_by: owner.id, + updated_by: owner.id, + }; + + // Insert both permissions + diesel::insert_into(asset_permissions::table) + .values(&vec![owner_permission, shared_permission]) + .execute(&mut conn) + .await + .unwrap(); + + // 5. Create test client + let client = TestClient::new().await; + + // 6. Send update sharing request to change role from CanView to CanEdit + let response = client + .put(&format!("/metrics/{}/sharing", metric.id)) + .with_auth(&owner.id.to_string()) + .json(&json!([ + { + "email": "shared@example.com", + "role": "CanEdit" + } + ])) + .send() + .await; + + // 7. Assert response + assert_eq!(response.status(), StatusCode::OK); + + // 8. Check that permission was updated + let permissions = asset_permissions::table + .filter(asset_permissions::asset_id.eq(metric.id)) + .filter(asset_permissions::identity_id.eq(shared_user.id)) + .filter(asset_permissions::asset_type.eq(AssetType::MetricFile)) + .filter(asset_permissions::identity_type.eq(IdentityType::User)) + .filter(asset_permissions::deleted_at.is_null()) + .load::(&mut conn) + .await + .unwrap(); + + assert_eq!(permissions.len(), 1); + assert_eq!(permissions[0].role, AssetPermissionRole::CanEdit); // Role should be updated +} + +#[tokio::test] +async fn test_update_metric_sharing_unauthorized() { + // Setup test database connection + let mut conn = get_pg_pool().get().await.unwrap(); + + // 1. Create test owner, shared user, and unauthorized user + let owner = create_test_user("owner@example.com"); + let org_id = Uuid::new_v4(); + diesel::insert_into(users::table) + .values(&owner) + .execute(&mut conn) + .await + .unwrap(); + + let shared_user = create_test_user("shared@example.com"); + diesel::insert_into(users::table) + .values(&shared_user) + .execute(&mut conn) + .await + .unwrap(); + + let unauthorized_user = create_test_user("unauthorized@example.com"); + diesel::insert_into(users::table) + .values(&unauthorized_user) + .execute(&mut conn) + .await + .unwrap(); + + // 2. Create test metric + let metric = create_test_metric_file(&owner.id, &org_id, None); + diesel::insert_into(metric_files::table) + .values(&metric) + .execute(&mut conn) + .await + .unwrap(); + + // 3. Create owner permission for owner and CanView for shared user + let now = Utc::now(); + let owner_permission = AssetPermission { + identity_id: owner.id, + identity_type: IdentityType::User, + asset_id: metric.id, + asset_type: AssetType::MetricFile, + role: AssetPermissionRole::Owner, + created_at: now, + updated_at: now, + deleted_at: None, + created_by: owner.id, + updated_by: owner.id, + }; + + let shared_permission = AssetPermission { + identity_id: shared_user.id, + identity_type: IdentityType::User, + asset_id: metric.id, + asset_type: AssetType::MetricFile, + role: AssetPermissionRole::CanView, + created_at: now, + updated_at: now, + deleted_at: None, + created_by: owner.id, + updated_by: owner.id, + }; + + diesel::insert_into(asset_permissions::table) + .values(&vec![owner_permission, shared_permission]) + .execute(&mut conn) + .await + .unwrap(); + + // 4. Create test client + let client = TestClient::new().await; + + // 5. Send update sharing request as unauthorized user + let response = client + .put(&format!("/metrics/{}/sharing", metric.id)) + .with_auth(&unauthorized_user.id.to_string()) + .json(&json!([ + { + "email": "shared@example.com", + "role": "CanEdit" + } + ])) + .send() + .await; + + // 6. Assert response is forbidden + assert_eq!(response.status(), StatusCode::FORBIDDEN); + + // 7. Check that permission was not updated + let permissions = asset_permissions::table + .filter(asset_permissions::asset_id.eq(metric.id)) + .filter(asset_permissions::identity_id.eq(shared_user.id)) + .filter(asset_permissions::asset_type.eq(AssetType::MetricFile)) + .filter(asset_permissions::identity_type.eq(IdentityType::User)) + .filter(asset_permissions::deleted_at.is_null()) + .load::(&mut conn) + .await + .unwrap(); + + assert_eq!(permissions.len(), 1); + assert_eq!(permissions[0].role, AssetPermissionRole::CanView); // Role should not change +} + +#[tokio::test] +async fn test_update_metric_sharing_invalid_email() { + // Setup test database connection + let mut conn = get_pg_pool().get().await.unwrap(); + + // 1. Create test owner + let owner = create_test_user("owner@example.com"); + let org_id = Uuid::new_v4(); + diesel::insert_into(users::table) + .values(&owner) + .execute(&mut conn) + .await + .unwrap(); + + // 2. Create test metric + let metric = create_test_metric_file(&owner.id, &org_id, None); + diesel::insert_into(metric_files::table) + .values(&metric) + .execute(&mut conn) + .await + .unwrap(); + + // 3. Create owner permission + let now = Utc::now(); + let owner_permission = AssetPermission { + identity_id: owner.id, + identity_type: IdentityType::User, + asset_id: metric.id, + asset_type: AssetType::MetricFile, + role: AssetPermissionRole::Owner, + created_at: now, + updated_at: now, + deleted_at: None, + created_by: owner.id, + updated_by: owner.id, + }; + + diesel::insert_into(asset_permissions::table) + .values(&owner_permission) + .execute(&mut conn) + .await + .unwrap(); + + // 4. Create test client + let client = TestClient::new().await; + + // 5. Send update sharing request with invalid email + let response = client + .put(&format!("/metrics/{}/sharing", metric.id)) + .with_auth(&owner.id.to_string()) + .json(&json!([ + { + "email": "invalid-email-format", + "role": "CanView" + } + ])) + .send() + .await; + + // 6. Assert response is bad request + assert_eq!(response.status(), StatusCode::BAD_REQUEST); +} + +#[tokio::test] +async fn test_update_metric_sharing_nonexistent_metric() { + // Setup test database connection + let mut conn = get_pg_pool().get().await.unwrap(); + + // 1. Create test user + let user = create_test_user("user@example.com"); + diesel::insert_into(users::table) + .values(&user) + .execute(&mut conn) + .await + .unwrap(); + + // 2. Create test client + let client = TestClient::new().await; + + // 3. Send update sharing request with non-existent metric id + let nonexistent_id = Uuid::new_v4(); + let response = client + .put(&format!("/metrics/{}/sharing", nonexistent_id)) + .with_auth(&user.id.to_string()) + .json(&json!([ + { + "email": "share@example.com", + "role": "CanView" + } + ])) + .send() + .await; + + // 4. Assert response is not found + assert_eq!(response.status(), StatusCode::NOT_FOUND); +} \ No newline at end of file