mirror of https://github.com/buster-so/buster.git
cascading permissions from dash to metrics
This commit is contained in:
parent
d00313131e
commit
7cee45916d
|
@ -1,11 +1,10 @@
|
||||||
use anyhow::{anyhow, Result};
|
use anyhow::{anyhow, Result};
|
||||||
use chrono::Utc;
|
|
||||||
use database::{
|
use database::{
|
||||||
pool::get_pg_pool,
|
pool::get_pg_pool,
|
||||||
schema::{dashboard_files, metric_files, metric_files_to_dashboard_files},
|
schema::metric_files,
|
||||||
types::{data_metadata::DataMetadata, MetricYml},
|
types::{data_metadata::DataMetadata, MetricYml},
|
||||||
};
|
};
|
||||||
use diesel::{BoolExpressionMethods, ExpressionMethods, JoinOnDsl, QueryDsl};
|
use diesel::{ExpressionMethods, QueryDsl};
|
||||||
use diesel_async::RunQueryDsl;
|
use diesel_async::RunQueryDsl;
|
||||||
use indexmap::IndexMap;
|
use indexmap::IndexMap;
|
||||||
use middleware::AuthenticatedUser;
|
use middleware::AuthenticatedUser;
|
||||||
|
@ -68,41 +67,18 @@ pub async fn get_metric_data_handler(
|
||||||
|
|
||||||
if is_permission_error {
|
if is_permission_error {
|
||||||
tracing::warn!(
|
tracing::warn!(
|
||||||
"Initial metric access failed due to potential permission issue: {}. Checking public dashboard access.",
|
"Initial metric access failed due to potential permission issue: {}. Checking dashboard access.",
|
||||||
e
|
e
|
||||||
);
|
);
|
||||||
|
|
||||||
// --- Step 3: Check if metric belongs to a valid public dashboard ---
|
// Check if user has access to ANY dashboard containing this metric (including public dashboards)
|
||||||
let mut conn_check = get_pg_pool().get().await?;
|
let has_dashboard_access = sharing::check_metric_dashboard_access(&request.metric_id, &user.id)
|
||||||
let now = Utc::now();
|
|
||||||
|
|
||||||
let public_dashboard_exists = match 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(request.metric_id))
|
|
||||||
.filter(dashboard_files::publicly_accessible.eq(true))
|
|
||||||
.filter(dashboard_files::deleted_at.is_null())
|
|
||||||
.filter(
|
|
||||||
dashboard_files::public_expiry_date
|
|
||||||
.is_null()
|
|
||||||
.or(dashboard_files::public_expiry_date.gt(now)),
|
|
||||||
)
|
|
||||||
.select(dashboard_files::id) // Select any column to check existence
|
|
||||||
.first::<Uuid>(&mut conn_check) // Try to get the first matching ID
|
|
||||||
.await
|
.await
|
||||||
{
|
.unwrap_or(false);
|
||||||
Ok(id) => Some(id),
|
|
||||||
Err(diesel::NotFound) => None,
|
|
||||||
Err(e) => {
|
|
||||||
tracing::error!("Error checking if public dashboard exists: {}", e);
|
|
||||||
return Err(anyhow!("Error checking if public dashboard exists: {}", e));
|
|
||||||
}
|
|
||||||
};
|
|
||||||
|
|
||||||
if public_dashboard_exists.is_some() {
|
if has_dashboard_access {
|
||||||
// --- Step 4: Public dashboard found, fetch metric bypassing permissions ---
|
// User has access to a dashboard containing this metric
|
||||||
tracing::info!("Found associated public dashboard. Fetching metric definition without direct permissions.");
|
tracing::info!("Found associated dashboard with user access. Fetching metric with dashboard context.");
|
||||||
match get_metric_for_dashboard_handler(
|
match get_metric_for_dashboard_handler(
|
||||||
&request.metric_id,
|
&request.metric_id,
|
||||||
&user,
|
&user,
|
||||||
|
@ -113,19 +89,19 @@ pub async fn get_metric_data_handler(
|
||||||
{
|
{
|
||||||
Ok(metric_via_dashboard) => {
|
Ok(metric_via_dashboard) => {
|
||||||
tracing::debug!(
|
tracing::debug!(
|
||||||
"Successfully retrieved metric via public dashboard association."
|
"Successfully retrieved metric via dashboard association."
|
||||||
);
|
);
|
||||||
metric_via_dashboard // Use this metric definition
|
metric_via_dashboard // Use this metric definition
|
||||||
}
|
}
|
||||||
Err(fetch_err) => {
|
Err(fetch_err) => {
|
||||||
// If fetching via dashboard fails unexpectedly, return that error
|
// If fetching via dashboard fails unexpectedly, return that error
|
||||||
tracing::error!("Failed to fetch metric via dashboard context even though public dashboard exists: {}", fetch_err);
|
tracing::error!("Failed to fetch metric via dashboard context: {}", fetch_err);
|
||||||
return Err(fetch_err);
|
return Err(fetch_err);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
// No public dashboard association found, return the original permission error
|
// No dashboard access, return the original permission error
|
||||||
tracing::warn!("No valid public dashboard association found for metric. Returning original error.");
|
tracing::warn!("No dashboard association found for metric. Returning original error.");
|
||||||
return Err(e);
|
return Err(e);
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
|
|
|
@ -157,10 +157,22 @@ pub async fn get_metric_for_dashboard_handler(
|
||||||
tracing::debug!(metric_id = %metric_id, user_id = %user.id, ?permission, "Granting access via direct permission.");
|
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 if user has access via a dashboard
|
||||||
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 dashboard access.");
|
||||||
|
|
||||||
|
let has_dashboard_access = sharing::check_metric_dashboard_access(metric_id, &user.id)
|
||||||
|
.await
|
||||||
|
.unwrap_or(false);
|
||||||
|
|
||||||
|
if has_dashboard_access {
|
||||||
|
// User has access to a dashboard containing this metric, grant CanView
|
||||||
|
tracing::debug!(metric_id = %metric_id, user_id = %user.id, "User has access via dashboard. Granting CanView.");
|
||||||
|
permission = AssetPermissionRole::CanView;
|
||||||
|
} else {
|
||||||
|
// No dashboard access, check public access rules
|
||||||
|
tracing::debug!(metric_id = %metric_id, "No dashboard access. Checking public access rules.");
|
||||||
if !metric_file.publicly_accessible {
|
if !metric_file.publicly_accessible {
|
||||||
tracing::warn!(metric_id = %metric_id, user_id = %user.id, "Permission denied (not public, insufficient direct permission).");
|
tracing::warn!(metric_id = %metric_id, user_id = %user.id, "Permission denied (not public, no dashboard access, insufficient direct permission).");
|
||||||
return Err(anyhow!("You don't have permission to view this metric"));
|
return Err(anyhow!("You don't have permission to view this metric"));
|
||||||
}
|
}
|
||||||
tracing::debug!(metric_id = %metric_id, "Metric is publicly accessible.");
|
tracing::debug!(metric_id = %metric_id, "Metric is publicly accessible.");
|
||||||
|
@ -201,6 +213,7 @@ pub async fn get_metric_for_dashboard_handler(
|
||||||
permission = AssetPermissionRole::CanView;
|
permission = AssetPermissionRole::CanView;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// Declare variables to hold potentially versioned data
|
// Declare variables to hold potentially versioned data
|
||||||
let resolved_name: String;
|
let resolved_name: String;
|
||||||
|
|
|
@ -155,10 +155,22 @@ pub async fn get_metric_handler(
|
||||||
tracing::debug!(metric_id = %metric_id, user_id = %user.id, ?permission, "Granting access via direct permission.");
|
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 if user has access via a dashboard
|
||||||
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 dashboard access.");
|
||||||
|
|
||||||
|
let has_dashboard_access = sharing::check_metric_dashboard_access(metric_id, &user.id)
|
||||||
|
.await
|
||||||
|
.unwrap_or(false);
|
||||||
|
|
||||||
|
if has_dashboard_access {
|
||||||
|
// User has access to a dashboard containing this metric, grant CanView
|
||||||
|
tracing::debug!(metric_id = %metric_id, user_id = %user.id, "User has access via dashboard. Granting CanView.");
|
||||||
|
permission = AssetPermissionRole::CanView;
|
||||||
|
} else {
|
||||||
|
// No dashboard access, check public access rules
|
||||||
|
tracing::debug!(metric_id = %metric_id, "No dashboard access. Checking public access rules.");
|
||||||
if !metric_file.publicly_accessible {
|
if !metric_file.publicly_accessible {
|
||||||
tracing::warn!(metric_id = %metric_id, user_id = %user.id, "Permission denied (not public, insufficient direct permission).");
|
tracing::warn!(metric_id = %metric_id, user_id = %user.id, "Permission denied (not public, no dashboard access, insufficient direct permission).");
|
||||||
return Err(anyhow!("You don't have permission to view this metric"));
|
return Err(anyhow!("You don't have permission to view this metric"));
|
||||||
}
|
}
|
||||||
tracing::debug!(metric_id = %metric_id, "Metric is publicly accessible.");
|
tracing::debug!(metric_id = %metric_id, "Metric is publicly accessible.");
|
||||||
|
@ -199,6 +211,7 @@ pub async fn get_metric_handler(
|
||||||
permission = AssetPermissionRole::CanView;
|
permission = AssetPermissionRole::CanView;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// Declare variables to hold potentially versioned data
|
// Declare variables to hold potentially versioned data
|
||||||
let resolved_name: String;
|
let resolved_name: String;
|
||||||
|
|
|
@ -1,4 +1,8 @@
|
||||||
use database::enums::{AssetPermissionRole, UserOrganizationRole};
|
use database::enums::{AssetPermissionRole, AssetType, IdentityType, UserOrganizationRole};
|
||||||
|
use database::pool::get_pg_pool;
|
||||||
|
use database::schema::{asset_permissions, dashboard_files, metric_files_to_dashboard_files};
|
||||||
|
use diesel::{BoolExpressionMethods, ExpressionMethods, JoinOnDsl, QueryDsl, OptionalExtension};
|
||||||
|
use diesel_async::RunQueryDsl;
|
||||||
use middleware::OrganizationMembership;
|
use middleware::OrganizationMembership;
|
||||||
use uuid::Uuid;
|
use uuid::Uuid;
|
||||||
|
|
||||||
|
@ -39,6 +43,80 @@ pub fn check_permission_access(
|
||||||
false
|
false
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Checks if a user has access to a metric through any associated dashboard.
|
||||||
|
///
|
||||||
|
/// 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
|
||||||
|
/// or if the dashboard is public), they get at least CanView permission.
|
||||||
|
///
|
||||||
|
/// # Arguments
|
||||||
|
/// * `metric_id` - UUID of the metric to check
|
||||||
|
/// * `user_id` - UUID of the user to check permissions for
|
||||||
|
///
|
||||||
|
/// # Returns
|
||||||
|
/// * `Result<bool>` - True if the user has access to any dashboard containing the metric, false otherwise
|
||||||
|
pub async fn check_metric_dashboard_access(
|
||||||
|
metric_id: &Uuid,
|
||||||
|
user_id: &Uuid,
|
||||||
|
) -> Result<bool, diesel::result::Error> {
|
||||||
|
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 dashboard containing this metric
|
||||||
|
let has_direct_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)),
|
||||||
|
)
|
||||||
|
.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::<Uuid>(&mut conn)
|
||||||
|
.await
|
||||||
|
.optional()?;
|
||||||
|
|
||||||
|
if has_direct_access.is_some() {
|
||||||
|
return Ok(true);
|
||||||
|
}
|
||||||
|
|
||||||
|
// 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::<Uuid>(&mut conn)
|
||||||
|
.await
|
||||||
|
.optional()?;
|
||||||
|
|
||||||
|
Ok(has_public_access.is_some())
|
||||||
|
}
|
||||||
|
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
mod tests {
|
mod tests {
|
||||||
use super::*;
|
use super::*;
|
||||||
|
|
|
@ -19,4 +19,4 @@ pub use types::{
|
||||||
SerializableAssetPermission, UserInfo,
|
SerializableAssetPermission, UserInfo,
|
||||||
};
|
};
|
||||||
pub use user_lookup::find_user_by_email;
|
pub use user_lookup::find_user_by_email;
|
||||||
pub use asset_access_checks::check_permission_access;
|
pub use asset_access_checks::{check_permission_access, check_metric_dashboard_access};
|
||||||
|
|
Loading…
Reference in New Issue