mirror of https://github.com/buster-so/buster.git
refactor: update permission handling for dashboard and metric access
- Enhanced permission logic in `get_dashboard_handler` and `get_metric_handler` to grant Owner permissions to users with WorkspaceAdmin or DataAdmin roles. - Updated related tests to reflect the new permission structure, ensuring that admin users receive the correct access level. - Removed unused `fastembed` dependency from multiple Cargo.toml files to streamline the project.
This commit is contained in:
parent
152b901f8f
commit
2cc9e0396b
|
@ -110,7 +110,6 @@ diesel_migrations = "2.0.0"
|
||||||
html-escape = "0.2.13"
|
html-escape = "0.2.13"
|
||||||
tokio-cron-scheduler = "0.13.0"
|
tokio-cron-scheduler = "0.13.0"
|
||||||
tokio-retry = "0.3.0"
|
tokio-retry = "0.3.0"
|
||||||
fastembed = "4.8.0"
|
|
||||||
|
|
||||||
[profile.release]
|
[profile.release]
|
||||||
debug = false
|
debug = false
|
||||||
|
|
|
@ -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");
|
tracing::debug!(dashboard_id = %dashboard_id, ?direct_permission_level, has_sufficient_direct_permission, "Direct permission check result");
|
||||||
|
|
||||||
if has_sufficient_direct_permission {
|
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 {
|
} else {
|
||||||
// No sufficient direct/admin permission, check public access rules
|
// No sufficient direct/admin permission, check public access rules
|
||||||
tracing::debug!(dashboard_id = %dashboard_id, "Insufficient direct/admin permission. Checking public access rules.");
|
tracing::debug!(dashboard_id = %dashboard_id, "Insufficient direct/admin permission. Checking public access rules.");
|
||||||
|
|
|
@ -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");
|
tracing::debug!(metric_id = %metric_id, ?direct_permission_level, has_sufficient_direct_permission, "Direct permission check result");
|
||||||
|
|
||||||
if has_sufficient_direct_permission {
|
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 {
|
} else {
|
||||||
// No sufficient direct/admin permission, check public access rules
|
// No sufficient direct/admin permission, check public access rules
|
||||||
tracing::debug!(metric_id = %metric_id, "Insufficient direct/admin permission. Checking public access rules.");
|
tracing::debug!(metric_id = %metric_id, "Insufficient direct/admin permission. Checking public access rules.");
|
||||||
|
|
|
@ -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
|
let result = get_dashboard_handler(&dashboard.id, &auth_user, None, None).await; // No password
|
||||||
|
|
||||||
assert!(result.is_ok());
|
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();
|
let response = result.unwrap();
|
||||||
assert_eq!(response.permission, AssetPermissionRole::CanView);
|
assert_eq!(response.permission, AssetPermissionRole::Owner);
|
||||||
|
|
||||||
cleanup_test_data(&[dashboard.id]).await?;
|
cleanup_test_data(&[dashboard.id]).await?;
|
||||||
Ok(())
|
Ok(())
|
||||||
|
|
|
@ -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
|
let result = get_metric_handler(&metric.id, &auth_user, None, None).await; // No password provided
|
||||||
|
|
||||||
assert!(result.is_ok());
|
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();
|
let response = result.unwrap();
|
||||||
assert_eq!(response.permission, AssetPermissionRole::CanView);
|
assert_eq!(response.permission, AssetPermissionRole::Owner);
|
||||||
|
|
||||||
cleanup_test_data(&[metric.id]).await?;
|
cleanup_test_data(&[metric.id]).await?;
|
||||||
Ok(())
|
Ok(())
|
||||||
|
|
|
@ -8,7 +8,6 @@ reqwest = { workspace = true }
|
||||||
serde = { workspace = true }
|
serde = { workspace = true }
|
||||||
serde_json = { workspace = true }
|
serde_json = { workspace = true }
|
||||||
dotenv = { workspace = true }
|
dotenv = { workspace = true }
|
||||||
fastembed = "4.8.0"
|
|
||||||
|
|
||||||
[dev-dependencies]
|
[dev-dependencies]
|
||||||
dotenv = { workspace = true }
|
dotenv = { workspace = true }
|
||||||
|
|
|
@ -1,8 +1,8 @@
|
||||||
|
use dotenv::dotenv;
|
||||||
use reqwest::Client;
|
use reqwest::Client;
|
||||||
use serde::{Deserialize, Serialize};
|
use serde::{Deserialize, Serialize};
|
||||||
use std::error::Error;
|
|
||||||
use dotenv::dotenv;
|
|
||||||
use std::env;
|
use std::env;
|
||||||
|
use std::error::Error;
|
||||||
|
|
||||||
pub struct Reranker {
|
pub struct Reranker {
|
||||||
api_key: String,
|
api_key: String,
|
||||||
|
@ -44,11 +44,6 @@ impl Reranker {
|
||||||
documents: &[&str],
|
documents: &[&str],
|
||||||
top_n: usize,
|
top_n: usize,
|
||||||
) -> Result<Vec<RerankResult>, Box<dyn Error>> {
|
) -> 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
|
// Otherwise use the remote API
|
||||||
let request_body = RerankRequest {
|
let request_body = RerankRequest {
|
||||||
query: query.to_string(),
|
query: query.to_string(),
|
||||||
|
@ -66,37 +61,6 @@ impl Reranker {
|
||||||
let response_body: RerankResponse = response.json().await?;
|
let response_body: RerankResponse = response.json().await?;
|
||||||
Ok(response_body.results)
|
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)]
|
#[derive(Serialize)]
|
||||||
|
|
|
@ -39,7 +39,6 @@ tower-http = { workspace = true }
|
||||||
tracing = { workspace = true }
|
tracing = { workspace = true }
|
||||||
tracing-subscriber = { workspace = true }
|
tracing-subscriber = { workspace = true }
|
||||||
uuid = { workspace = true }
|
uuid = { workspace = true }
|
||||||
fastembed = { workspace = true }
|
|
||||||
|
|
||||||
# Local dependencies
|
# Local dependencies
|
||||||
handlers = { path = "../libs/handlers" }
|
handlers = { path = "../libs/handlers" }
|
||||||
|
|
|
@ -22,7 +22,6 @@ use tower::ServiceBuilder;
|
||||||
use tower_http::{compression::CompressionLayer, trace::TraceLayer};
|
use tower_http::{compression::CompressionLayer, trace::TraceLayer};
|
||||||
use tracing::{error, info, warn};
|
use tracing::{error, info, warn};
|
||||||
use tracing_subscriber::{layer::SubscriberExt, util::SubscriberInitExt, EnvFilter};
|
use tracing_subscriber::{layer::SubscriberExt, util::SubscriberInitExt, EnvFilter};
|
||||||
use fastembed::{InitOptions, RerankInitOptions, RerankerModel, TextRerank};
|
|
||||||
|
|
||||||
pub const MIGRATIONS: EmbeddedMigrations = embed_migrations!();
|
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 environment = env::var("ENVIRONMENT").unwrap_or_else(|_| "development".to_string());
|
||||||
let is_development = environment == "development";
|
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()
|
ring::default_provider()
|
||||||
.install_default()
|
.install_default()
|
||||||
.expect("Failed to install default crypto provider");
|
.expect("Failed to install default crypto provider");
|
||||||
|
|
Loading…
Reference in New Issue