Merge pull request #361 from buster-so/staging

Permissions Bug for Org Admins
This commit is contained in:
dal 2025-06-24 08:26:11 -07:00 committed by GitHub
commit b569d26f0c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 39 additions and 15 deletions

View File

@ -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
let is_admin = user.organizations.iter().any(|org| {
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/admin permission.");
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.");

View File

@ -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
let is_admin = user.organizations.iter().any(|org| {
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/admin permission.");
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.");

View File

@ -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(())

View File

@ -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,
// 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.
// 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(&[metric.id]).await?;
Ok(())

View File

@ -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,