From 1e705c982821e24ffe5597bfae7ca31e6100d0be Mon Sep 17 00:00:00 2001 From: dal Date: Thu, 17 Jul 2025 14:01:45 -0600 Subject: [PATCH] Implement collection access checks for chats and dashboards - Updated `get_chat_handler` to check for collection access when a user lacks direct permission. - Modified `get_dashboard_handler` to first verify collection access before checking chat access. - Added `check_chat_collection_access` function to validate user access to chats via collections. - Enhanced `check_metric_dashboard_access` to include collection access checks concurrently with other permission checks. This change improves the permission model by allowing access to chats and dashboards through associated collections, enhancing user experience and security. --- .../handlers/src/chats/get_chat_handler.rs | 11 +- .../src/dashboards/get_dashboard_handler.rs | 39 ++- .../libs/sharing/src/asset_access_checks.rs | 251 +++++++++++++----- apps/api/libs/sharing/src/lib.rs | 2 +- 4 files changed, 226 insertions(+), 77 deletions(-) diff --git a/apps/api/libs/handlers/src/chats/get_chat_handler.rs b/apps/api/libs/handlers/src/chats/get_chat_handler.rs index b97b3ffbd..60ef7bd75 100644 --- a/apps/api/libs/handlers/src/chats/get_chat_handler.rs +++ b/apps/api/libs/handlers/src/chats/get_chat_handler.rs @@ -17,7 +17,7 @@ use database::{ helpers::chats::fetch_chat_with_permission, pool::get_pg_pool, }; -use sharing::check_permission_access; +use sharing::{check_permission_access, compute_effective_permission}; #[derive(Queryable)] pub struct ChatWithUser { @@ -100,7 +100,14 @@ pub async fn get_chat_handler( let is_creator = chat_with_permission.chat.created_by == user.id; if !has_permission && !is_creator { - return Err(anyhow!("You don't have permission to view this chat")); + // Check if user has access via a collection + let has_collection_access = sharing::check_chat_collection_access(chat_id, &user.id, &user.organizations) + .await + .unwrap_or(false); + + if !has_collection_access { + return Err(anyhow!("You don't have permission to view this chat")); + } } // Run messages query diff --git a/apps/api/libs/handlers/src/dashboards/get_dashboard_handler.rs b/apps/api/libs/handlers/src/dashboards/get_dashboard_handler.rs index d840b4810..81c235081 100644 --- a/apps/api/libs/handlers/src/dashboards/get_dashboard_handler.rs +++ b/apps/api/libs/handlers/src/dashboards/get_dashboard_handler.rs @@ -149,25 +149,37 @@ pub async fn get_dashboard_handler( "Granting access with effective permission (max of direct and workspace sharing)." ); } else { - // No sufficient direct/admin permission, check if user has access via a chat - tracing::debug!(dashboard_id = %dashboard_id, "Insufficient direct/admin permission. Checking chat access."); + // No sufficient direct/admin permission, check if user has access via a collection + tracing::debug!(dashboard_id = %dashboard_id, "Insufficient direct/admin permission. Checking collection access."); - let has_chat_access = sharing::check_dashboard_chat_access(dashboard_id, &user.id, &user.organizations) + let has_collection_access = sharing::check_dashboard_collection_access(dashboard_id, &user.id, &user.organizations) .await .unwrap_or(false); - if has_chat_access { - // User has access to a chat containing this dashboard, grant CanView - tracing::debug!(dashboard_id = %dashboard_id, user_id = %user.id, "User has access via chat. Granting CanView."); + if has_collection_access { + // User has access to a collection containing this dashboard, grant CanView + tracing::debug!(dashboard_id = %dashboard_id, user_id = %user.id, "User has access via collection. Granting CanView."); permission = AssetPermissionRole::CanView; } else { - // No chat access either, check public access rules - tracing::debug!(dashboard_id = %dashboard_id, "No chat access. Checking public access rules."); - if !dashboard_file.publicly_accessible { - tracing::warn!(dashboard_id = %dashboard_id, user_id = %user.id, "Permission denied (not public, no chat access, insufficient direct permission)."); - return Err(anyhow!("You don't have permission to view this dashboard")); - } - tracing::debug!(dashboard_id = %dashboard_id, "Dashboard is publicly accessible."); + // No collection access, check if user has access via a chat + tracing::debug!(dashboard_id = %dashboard_id, "No collection access. Checking chat access."); + + let has_chat_access = sharing::check_dashboard_chat_access(dashboard_id, &user.id, &user.organizations) + .await + .unwrap_or(false); + + if has_chat_access { + // User has access to a chat containing this dashboard, grant CanView + tracing::debug!(dashboard_id = %dashboard_id, user_id = %user.id, "User has access via chat. Granting CanView."); + permission = AssetPermissionRole::CanView; + } else { + // No chat access either, check public access rules + tracing::debug!(dashboard_id = %dashboard_id, "No chat access. Checking public access rules."); + if !dashboard_file.publicly_accessible { + tracing::warn!(dashboard_id = %dashboard_id, user_id = %user.id, "Permission denied (not public, no collection/chat access, insufficient direct permission)."); + return Err(anyhow!("You don't have permission to view this dashboard")); + } + tracing::debug!(dashboard_id = %dashboard_id, "Dashboard is publicly accessible."); // Check if the public access has expired if let Some(expiry_date) = dashboard_file.public_expiry_date { @@ -204,6 +216,7 @@ pub async fn get_dashboard_handler( tracing::debug!(dashboard_id = %dashboard_id, "Public access granted (no password required)."); permission = AssetPermissionRole::CanView; } + } } } diff --git a/apps/api/libs/sharing/src/asset_access_checks.rs b/apps/api/libs/sharing/src/asset_access_checks.rs index a78d4726e..39acb78e2 100644 --- a/apps/api/libs/sharing/src/asset_access_checks.rs +++ b/apps/api/libs/sharing/src/asset_access_checks.rs @@ -1,6 +1,6 @@ use database::enums::{AssetPermissionRole, AssetType, IdentityType, UserOrganizationRole, WorkspaceSharing}; use database::pool::get_pg_pool; -use database::schema::{asset_permissions, dashboard_files, metric_files_to_dashboard_files, collections, collections_to_assets}; +use database::schema::{asset_permissions, dashboard_files, metric_files_to_dashboard_files, collections, collections_to_assets, chats}; use diesel::{BoolExpressionMethods, ExpressionMethods, JoinOnDsl, QueryDsl, OptionalExtension}; use diesel_async::RunQueryDsl; use middleware::OrganizationMembership; @@ -126,6 +126,7 @@ pub fn check_permission_access( /// This function is used to implement permission cascading from dashboards to metrics. /// If a user has access to any dashboard containing the metric (either through direct permissions, /// workspace sharing, or if the dashboard is public), they get at least CanView permission. +/// This also checks if the dashboard itself is accessible via collections (transitive cascading). /// /// # Arguments /// * `metric_id` - UUID of the metric to check @@ -146,81 +147,130 @@ pub async fn check_metric_dashboard_access( ) })?; - // First check if user has direct access to any dashboard containing this metric - let has_direct_access = metric_files_to_dashboard_files::table + // Get all dashboards containing this metric + let dashboard_ids: Vec = metric_files_to_dashboard_files::table .inner_join( dashboard_files::table .on(dashboard_files::id.eq(metric_files_to_dashboard_files::dashboard_file_id)), ) - .inner_join( - asset_permissions::table.on( - asset_permissions::asset_id.eq(dashboard_files::id) - .and(asset_permissions::asset_type.eq(AssetType::DashboardFile)) - .and(asset_permissions::identity_id.eq(user_id)) - .and(asset_permissions::identity_type.eq(IdentityType::User)) - .and(asset_permissions::deleted_at.is_null()) - ), - ) .filter(metric_files_to_dashboard_files::metric_file_id.eq(metric_id)) .filter(dashboard_files::deleted_at.is_null()) .filter(metric_files_to_dashboard_files::deleted_at.is_null()) .select(dashboard_files::id) - .first::(&mut conn) - .await - .optional()?; + .load::(&mut conn) + .await?; - if has_direct_access.is_some() { - return Ok(true); + if dashboard_ids.is_empty() { + return Ok(false); } - // Now check if metric belongs to any PUBLIC dashboard (not expired) - let now = chrono::Utc::now(); - let has_public_access = metric_files_to_dashboard_files::table - .inner_join( - dashboard_files::table - .on(dashboard_files::id.eq(metric_files_to_dashboard_files::dashboard_file_id)), - ) - .filter(metric_files_to_dashboard_files::metric_file_id.eq(metric_id)) - .filter(dashboard_files::deleted_at.is_null()) - .filter(metric_files_to_dashboard_files::deleted_at.is_null()) - .filter(dashboard_files::publicly_accessible.eq(true)) - .filter( - dashboard_files::public_expiry_date - .is_null() - .or(dashboard_files::public_expiry_date.gt(now)), - ) - .select(dashboard_files::id) - .first::(&mut conn) - .await - .optional()?; + // Check multiple access paths concurrently + let user_id_clone = *user_id; + let user_orgs_clone = user_orgs.to_vec(); + let dashboard_ids_clone = dashboard_ids.clone(); + + // 1. Check direct access to dashboards + let direct_access_future = async move { + let mut conn = get_pg_pool().get().await.map_err(|e| { + diesel::result::Error::DatabaseError( + diesel::result::DatabaseErrorKind::UnableToSendCommand, + Box::new(e.to_string()), + ) + })?; + + let has_direct = asset_permissions::table + .filter(asset_permissions::asset_id.eq_any(&dashboard_ids_clone)) + .filter(asset_permissions::asset_type.eq(AssetType::DashboardFile)) + .filter(asset_permissions::identity_id.eq(user_id_clone)) + .filter(asset_permissions::identity_type.eq(IdentityType::User)) + .filter(asset_permissions::deleted_at.is_null()) + .select(asset_permissions::asset_id) + .first::(&mut conn) + .await + .optional()?; + + Ok::(has_direct.is_some()) + }; - if has_public_access.is_some() { - return Ok(true); - } + // 2. Check public access to dashboards + let dashboard_ids_public = dashboard_ids.clone(); + let public_access_future = async move { + let mut conn = get_pg_pool().get().await.map_err(|e| { + diesel::result::Error::DatabaseError( + diesel::result::DatabaseErrorKind::UnableToSendCommand, + Box::new(e.to_string()), + ) + })?; + + let now = chrono::Utc::now(); + let has_public = dashboard_files::table + .filter(dashboard_files::id.eq_any(&dashboard_ids_public)) + .filter(dashboard_files::deleted_at.is_null()) + .filter(dashboard_files::publicly_accessible.eq(true)) + .filter( + dashboard_files::public_expiry_date + .is_null() + .or(dashboard_files::public_expiry_date.gt(now)), + ) + .select(dashboard_files::id) + .first::(&mut conn) + .await + .optional()?; + + Ok::(has_public.is_some()) + }; - // Check if metric belongs to any workspace-shared dashboard - let workspace_shared_dashboard = metric_files_to_dashboard_files::table - .inner_join( - dashboard_files::table - .on(dashboard_files::id.eq(metric_files_to_dashboard_files::dashboard_file_id)), - ) - .filter(metric_files_to_dashboard_files::metric_file_id.eq(metric_id)) - .filter(dashboard_files::deleted_at.is_null()) - .filter(metric_files_to_dashboard_files::deleted_at.is_null()) - .filter(dashboard_files::workspace_sharing.ne(WorkspaceSharing::None)) - .select((dashboard_files::organization_id, dashboard_files::workspace_sharing)) - .first::<(Uuid, WorkspaceSharing)>(&mut conn) - .await - .optional()?; - - if let Some((org_id, _sharing_level)) = workspace_shared_dashboard { - // Check if user is member of that organization - if user_orgs.iter().any(|org| org.id == org_id) { - return Ok(true); + // 3. Check workspace sharing on dashboards + let dashboard_ids_ws = dashboard_ids.clone(); + let user_orgs_ws = user_orgs.to_vec(); + let workspace_access_future = async move { + let mut conn = get_pg_pool().get().await.map_err(|e| { + diesel::result::Error::DatabaseError( + diesel::result::DatabaseErrorKind::UnableToSendCommand, + Box::new(e.to_string()), + ) + })?; + + let workspace_dashboards = dashboard_files::table + .filter(dashboard_files::id.eq_any(&dashboard_ids_ws)) + .filter(dashboard_files::deleted_at.is_null()) + .filter(dashboard_files::workspace_sharing.ne(WorkspaceSharing::None)) + .select((dashboard_files::organization_id, dashboard_files::workspace_sharing)) + .load::<(Uuid, WorkspaceSharing)>(&mut conn) + .await?; + + for (org_id, _) in workspace_dashboards { + if user_orgs_ws.iter().any(|org| org.id == org_id) { + return Ok::(true); + } } - } + + Ok::(false) + }; - Ok(false) + // 4. Check if dashboards are accessible via collections + let dashboard_ids_coll = dashboard_ids.clone(); + let user_id_coll = *user_id; + let user_orgs_coll = user_orgs.to_vec(); + let collection_access_future = async move { + for dashboard_id in &dashboard_ids_coll { + if check_dashboard_collection_access(&dashboard_id, &user_id_coll, &user_orgs_coll).await? { + return Ok::(true); + } + } + Ok::(false) + }; + + // Execute all checks concurrently + let (direct_result, public_result, workspace_result, collection_result) = tokio::join!( + direct_access_future, + public_access_future, + workspace_access_future, + collection_access_future + ); + + // Return true if any check succeeds + Ok(direct_result? || public_result? || workspace_result? || collection_result?) } /// Checks if a user has access to a metric through any associated chat. @@ -619,6 +669,85 @@ pub async fn check_dashboard_collection_access( Ok(false) } +/// Checks if a user has access to a chat through any associated collection. +/// +/// This function is used to implement permission cascading from collections to chats. +/// If a user has access to any collection containing the chat (either through direct permissions +/// or workspace sharing), they get at least CanView permission. +/// +/// # Arguments +/// * `chat_id` - UUID of the chat to check +/// * `user_id` - UUID of the user to check permissions for +/// * `user_orgs` - User's organization memberships +/// +/// # Returns +/// * `Result` - True if the user has access to any collection containing the chat, false otherwise +pub async fn check_chat_collection_access( + chat_id: &Uuid, + user_id: &Uuid, + user_orgs: &[OrganizationMembership], +) -> Result { + let mut conn = get_pg_pool().get().await.map_err(|e| { + diesel::result::Error::DatabaseError( + diesel::result::DatabaseErrorKind::UnableToSendCommand, + Box::new(e.to_string()), + ) + })?; + + // First check if user has direct access to any collection containing this chat + let has_direct_access = collections_to_assets::table + .inner_join( + collections::table + .on(collections::id.eq(collections_to_assets::collection_id)), + ) + .inner_join( + asset_permissions::table.on( + asset_permissions::asset_id.eq(collections::id) + .and(asset_permissions::asset_type.eq(AssetType::Collection)) + .and(asset_permissions::identity_id.eq(user_id)) + .and(asset_permissions::identity_type.eq(IdentityType::User)) + .and(asset_permissions::deleted_at.is_null()) + ), + ) + .filter(collections_to_assets::asset_id.eq(chat_id)) + .filter(collections_to_assets::asset_type.eq(AssetType::Chat)) + .filter(collections::deleted_at.is_null()) + .filter(collections_to_assets::deleted_at.is_null()) + .select(collections::id) + .first::(&mut conn) + .await + .optional()?; + + if has_direct_access.is_some() { + return Ok(true); + } + + // Check if chat belongs to any workspace-shared collection + let workspace_shared_collection = collections_to_assets::table + .inner_join( + collections::table + .on(collections::id.eq(collections_to_assets::collection_id)), + ) + .filter(collections_to_assets::asset_id.eq(chat_id)) + .filter(collections_to_assets::asset_type.eq(AssetType::Chat)) + .filter(collections::deleted_at.is_null()) + .filter(collections_to_assets::deleted_at.is_null()) + .filter(collections::workspace_sharing.ne(WorkspaceSharing::None)) + .select((collections::organization_id, collections::workspace_sharing)) + .first::<(Uuid, WorkspaceSharing)>(&mut conn) + .await + .optional()?; + + if let Some((org_id, _sharing_level)) = workspace_shared_collection { + // Check if user is member of that organization + if user_orgs.iter().any(|org| org.id == org_id) { + return Ok(true); + } + } + + Ok(false) +} + #[cfg(test)] mod tests { use super::*; diff --git a/apps/api/libs/sharing/src/lib.rs b/apps/api/libs/sharing/src/lib.rs index 078a5bd67..6867dd1f1 100644 --- a/apps/api/libs/sharing/src/lib.rs +++ b/apps/api/libs/sharing/src/lib.rs @@ -22,5 +22,5 @@ pub use user_lookup::find_user_by_email; pub use asset_access_checks::{ check_permission_access, check_metric_dashboard_access, check_metric_chat_access, check_dashboard_chat_access, check_metric_collection_access, check_dashboard_collection_access, - compute_effective_permission + check_chat_collection_access, compute_effective_permission };