Merge pull request #360 from buster-so/dal/permissions-bug

refactor: update permission handling for dashboard and metric access

api/Cargo.toml

@@ -110,7 +110,6 @@ diesel_migrations = "2.0.0"
 html-escape = "0.2.13"
 tokio-cron-scheduler = "0.13.0"
 tokio-retry = "0.3.0"
-fastembed = "4.8.0"
 
 [profile.release]
 debug = false

api/libs/handlers/src/dashboards/get_dashboard_handler.rs

@@ -130,9 +130,22 @@ pub async fn get_dashboard_handler(
     tracing::debug!(dashboard_id = %dashboard_id, ?direct_permission_level, has_sufficient_direct_permission, "Direct permission check result");
 
     if has_sufficient_direct_permission {
-        // User has direct/admin permission, use that role
+        // Check if user is WorkspaceAdmin or DataAdmin for this organization
-        permission = direct_permission_level.unwrap_or(AssetPermissionRole::CanView); // Default just in case
+        let is_admin = user.organizations.iter().any(|org| {
-        tracing::debug!(dashboard_id = %dashboard_id, user_id = %user.id, ?permission, "Granting access via direct/admin permission.");
+            org.id == dashboard_file.organization_id
+                && (org.role == database::enums::UserOrganizationRole::WorkspaceAdmin
+                    || org.role == database::enums::UserOrganizationRole::DataAdmin)
+        });
+
+        if is_admin {
+            // Admin users get Owner permissions
+            permission = AssetPermissionRole::Owner;
+            tracing::debug!(dashboard_id = %dashboard_id, user_id = %user.id, ?permission, "Granting Owner access to admin user.");
+        } else {
+            // User has direct permission, use that role
+            permission = direct_permission_level.unwrap_or(AssetPermissionRole::CanView); // Default just in case
+            tracing::debug!(dashboard_id = %dashboard_id, user_id = %user.id, ?permission, "Granting access via direct permission.");
+        }
     } else {
         // No sufficient direct/admin permission, check public access rules
         tracing::debug!(dashboard_id = %dashboard_id, "Insufficient direct/admin permission. Checking public access rules.");

api/libs/handlers/src/metrics/get_metric_handler.rs

@@ -138,9 +138,22 @@ pub async fn get_metric_handler(
     tracing::debug!(metric_id = %metric_id, ?direct_permission_level, has_sufficient_direct_permission, "Direct permission check result");
 
     if has_sufficient_direct_permission {
-        // User has direct/admin permission, use that role
+        // Check if user is WorkspaceAdmin or DataAdmin for this organization
-        permission = direct_permission_level.unwrap_or(AssetPermissionRole::CanView); // Default just in case
+        let is_admin = user.organizations.iter().any(|org| {
-        tracing::debug!(metric_id = %metric_id, user_id = %user.id, ?permission, "Granting access via direct/admin permission.");
+            org.id == metric_file.organization_id
+                && (org.role == database::enums::UserOrganizationRole::WorkspaceAdmin
+                    || org.role == database::enums::UserOrganizationRole::DataAdmin)
+        });
+
+        if is_admin {
+            // Admin users get Owner permissions
+            permission = AssetPermissionRole::Owner;
+            tracing::debug!(metric_id = %metric_id, user_id = %user.id, ?permission, "Granting Owner access to admin user.");
+        } else {
+            // User has direct permission, use that role
+            permission = direct_permission_level.unwrap_or(AssetPermissionRole::CanView); // Default just in case
+            tracing::debug!(metric_id = %metric_id, user_id = %user.id, ?permission, "Granting access via direct permission.");
+        }
     } else {
         // No sufficient direct/admin permission, check public access rules
         tracing::debug!(metric_id = %metric_id, "Insufficient direct/admin permission. Checking public access rules.");

api/libs/handlers/tests/dashboards/get_dashboard_handler_permission_test.rs

@@ -202,9 +202,9 @@ async fn test_get_dashboard_admin_role_public_password() -> Result<()> {
     let result = get_dashboard_handler(&dashboard.id, &auth_user, None, None).await; // No password
 
     assert!(result.is_ok());
-    // Admins currently default to CanView if no explicit permission exists on the asset itself
+    // Admins should get Owner permissions regardless of explicit asset permissions
     let response = result.unwrap();
-    assert_eq!(response.permission, AssetPermissionRole::CanView);
+    assert_eq!(response.permission, AssetPermissionRole::Owner);
 
     cleanup_test_data(&[dashboard.id]).await?;
     Ok(())

api/libs/handlers/tests/metrics/get_metric_handler_permission_test.rs

@@ -232,11 +232,9 @@ async fn test_get_metric_admin_role_public_password() -> Result<()> {
     let result = get_metric_handler(&metric.id, &auth_user, None, None).await; // No password provided
 
     assert!(result.is_ok());
-    // Admins currently default to CanView if no explicit permission exists on the asset itself,
+    // Admins should get Owner permissions regardless of explicit asset permissions
-    // even though check_permission_access returns true. This might be desired or not.
-    // Let's assert CanView for now, reflecting current check_permission_access behavior combined with handler logic.
     let response = result.unwrap();
-    assert_eq!(response.permission, AssetPermissionRole::CanView);
+    assert_eq!(response.permission, AssetPermissionRole::Owner);
 
     cleanup_test_data(&[metric.id]).await?;
     Ok(())

api/libs/rerank/Cargo.toml

@@ -8,7 +8,6 @@ reqwest = { workspace = true }
 serde = { workspace = true }
 serde_json = { workspace = true }
 dotenv = { workspace = true }
-fastembed = "4.8.0"
 
 [dev-dependencies]
 dotenv = { workspace = true }

api/libs/rerank/src/lib.rs

@@ -1,8 +1,8 @@
+use dotenv::dotenv;
 use reqwest::Client;
 use serde::{Deserialize, Serialize};
-use std::error::Error;
-use dotenv::dotenv;
 use std::env;
+use std::error::Error;
 
 pub struct Reranker {
     api_key: String,
@@ -16,7 +16,7 @@ impl Reranker {
     pub fn new() -> Result<Self, Box<dyn Error>> {
         dotenv().ok();
         let environment = env::var("ENVIRONMENT").unwrap_or_else(|_| "production".to_string());
-        
+
         // If local environment, we don't need these values
         let (api_key, model, base_url) = if environment == "local" {
             (String::new(), String::new(), String::new())
@@ -27,7 +27,7 @@ impl Reranker {
                 env::var("RERANK_BASE_URL")?,
             )
         };
-        
+
         let client = Client::new();
         Ok(Self {
             api_key,
@@ -44,11 +44,6 @@ impl Reranker {
         documents: &[&str],
         top_n: usize,
     ) -> Result<Vec<RerankResult>, Box<dyn Error>> {
-        // Use local fastembed reranking if ENVIRONMENT is set to local
-        if self.environment == "local" {
-            return self.local_rerank(query, documents, top_n).await;
-        }
-        
         // Otherwise use the remote API
         let request_body = RerankRequest {
             query: query.to_string(),
@@ -66,37 +61,6 @@ impl Reranker {
         let response_body: RerankResponse = response.json().await?;
         Ok(response_body.results)
     }
-    
-    async fn local_rerank(
-        &self,
-        query: &str,
-        documents: &[&str],
-        top_n: usize,
-    ) -> Result<Vec<RerankResult>, Box<dyn Error>> {
-        use fastembed::{TextRerank, RerankInitOptions, RerankerModel};
-        
-        // Initialize the reranker model
-        let model = TextRerank::try_new(
-            RerankInitOptions::new(RerankerModel::JINARerankerV1TurboEn).with_show_download_progress(true),
-        )?;
-        
-        // Limit top_n to the number of documents
-        let actual_top_n = std::cmp::min(top_n, documents.len());
-
-        // Perform reranking
-        let fastembed_results = model.rerank(query, documents.to_vec(),false, Some(actual_top_n))?;
-        
-        // Convert fastembed results to our RerankResult format
-        let results = fastembed_results
-            .iter()
-            .map(|result| RerankResult {
-                index: result.index,
-                relevance_score: result.score,
-            })
-            .collect();
-        
-        Ok(results)
-    }
 }
 
 #[derive(Serialize)]
@@ -116,4 +80,4 @@ struct RerankResponse {
 pub struct RerankResult {
     pub index: usize,
     pub relevance_score: f32,
-} 
+}

api/server/Cargo.toml

@@ -39,7 +39,6 @@ tower-http = { workspace = true }
 tracing = { workspace = true }
 tracing-subscriber = { workspace = true }
 uuid = { workspace = true }
-fastembed = { workspace = true }
 
 # Local dependencies
 handlers = { path = "../libs/handlers" }

api/server/src/main.rs

@@ -22,7 +22,6 @@ use tower::ServiceBuilder;
 use tower_http::{compression::CompressionLayer, trace::TraceLayer};
 use tracing::{error, info, warn};
 use tracing_subscriber::{layer::SubscriberExt, util::SubscriberInitExt, EnvFilter};
-use fastembed::{InitOptions, RerankInitOptions, RerankerModel, TextRerank};
 
 pub const MIGRATIONS: EmbeddedMigrations = embed_migrations!();
 
@@ -34,13 +33,6 @@ async fn main() -> Result<(), anyhow::Error> {
     let environment = env::var("ENVIRONMENT").unwrap_or_else(|_| "development".to_string());
     let is_development = environment == "development";
 
-    if environment == "local" {
-        let options =
-            RerankInitOptions::new(RerankerModel::JINARerankerV1TurboEn).with_show_download_progress(true);
-        let model = TextRerank::try_new(options)?;
-        println!("Model loaded and ready!");
-    }
-
     ring::default_provider()
         .install_default()
         .expect("Failed to install default crypto provider");