diff --git a/backend/ee-repo-ref.txt b/backend/ee-repo-ref.txt index 4c7dc9a5ac..fef2825da4 100644 --- a/backend/ee-repo-ref.txt +++ b/backend/ee-repo-ref.txt @@ -1 +1 @@ -67c80ba9088de8cec92a3a528b637648f9b9b3a0 +9a61895fa24f1e5a13ee174353ccbe2ad76a24f5 diff --git a/backend/windmill-api-workspaces/src/datatable_acl.rs b/backend/windmill-api-workspaces/src/datatable_acl.rs index 81d8a0ad67..fb86dd73a2 100644 --- a/backend/windmill-api-workspaces/src/datatable_acl.rs +++ b/backend/windmill-api-workspaces/src/datatable_acl.rs @@ -294,6 +294,123 @@ async fn connect_as_caller( Ok((client, CallerConnection { dbname, current_user })) } +/// The first object a change reaches that the connection's role does not own, if +/// any — the reason a caller may be refused even on a target they do own. +/// +/// The scopes that read `IN SCHEMA` cover the whole schema whoever owns what is +/// in it, and `SET OWNER` on a schema moves every object in it; a revoke may +/// also name objects one at a time. `Target` scope on a table or a schema is the +/// target itself, which [`can_manage_target`] has already answered. +async fn first_unmanageable_object( + client: &tokio_postgres::Client, + target: &AclTarget, + change: &AclChange, +) -> Result> { + let Some(schema) = target.schema() else { + return Ok(None); + }; + + // A revoke that names its objects reaches exactly those. + if let AclChange::Revoke { objects, .. } = change { + if !objects.is_empty() { + let named: Vec = objects + .iter() + .map(|o| match &o.args { + Some(args) => format!("{}({args})", o.name), + None => o.name.clone(), + }) + .collect(); + let row = client + .query_opt( + "SELECT name FROM ( + SELECT c.relname AS name, pg_has_role(c.relowner, 'USAGE') AS mine + FROM pg_class c JOIN pg_namespace n ON n.oid = c.relnamespace + WHERE n.nspname = $1 AND c.relname = ANY($2) + UNION ALL + SELECT p.proname || '(' || pg_get_function_identity_arguments(p.oid) || ')', + pg_has_role(p.proowner, 'USAGE') + FROM pg_proc p JOIN pg_namespace n ON n.oid = p.pronamespace + WHERE n.nspname = $1 + AND p.proname || '(' || pg_get_function_identity_arguments(p.oid) || ')' + = ANY($2) + ) o WHERE NOT o.mine ORDER BY name LIMIT 1", + &[&schema, &named], + ) + .await + .map_err(|e| { + Error::internal_err(format!( + "Failed to read the owners of schema '{schema}': {}", + pg_error_message(&e) + )) + })?; + return Ok(row.map(|row| format!("{schema}.{}", row.get::<_, String>(0)))); + } + } + + // The object classes the change touches, as `relkind`s and whether routines + // are included — Postgres groups views and foreign tables under TABLES. + let (relkinds, routines): (&[&str], bool) = match change { + AclChange::SetOwner { .. } if matches!(target, AclTarget::Schema { .. }) => { + (&["r", "p", "v", "m", "S", "f"], true) + } + AclChange::SetOwner { .. } => return Ok(None), + AclChange::Grant { scope, .. } | AclChange::Revoke { scope, .. } => match scope { + GrantScope::AllTables => (&["r", "p", "v", "f"], false), + GrantScope::AllSequences => (&["S"], false), + GrantScope::AllFunctions => (&[], true), + // A future grant creates no statement about an existing object. + GrantScope::FutureTables + | GrantScope::FutureSequences + | GrantScope::FutureFunctions + | GrantScope::Target => return Ok(None), + }, + }; + + let relkinds: Vec = relkinds.iter().map(|k| k.to_string()).collect(); + let row = client + .query_opt( + "SELECT name FROM ( + SELECT c.relname AS name, pg_has_role(c.relowner, 'USAGE') AS mine + FROM pg_class c JOIN pg_namespace n ON n.oid = c.relnamespace + WHERE n.nspname = $1 AND c.relkind::text = ANY($2) + UNION ALL + SELECT p.proname || '(' || pg_get_function_identity_arguments(p.oid) || ')', + pg_has_role(p.proowner, 'USAGE') + FROM pg_proc p JOIN pg_namespace n ON n.oid = p.pronamespace + WHERE $3 AND n.nspname = $1 + ) o WHERE NOT o.mine ORDER BY name LIMIT 1", + &[&schema, &relkinds, &routines], + ) + .await + .map_err(|e| { + Error::internal_err(format!( + "Failed to read the owners of schema '{schema}': {}", + pg_error_message(&e) + )) + })?; + Ok(row.map(|row| format!("{schema}.{}", row.get::<_, String>(0)))) +} + +/// The data table roles `authed` may themselves run as. +async fn usable_role_names( + db: &DB, + w_id: &str, + datatable_name: &str, + authed: &ApiAuthed, +) -> Result> { + let authed_ref = authed.to_authed_ref(); + let datatable = read_datatable(db, w_id, datatable_name).await?; + Ok(match datatable.permissions.filter(|p| p.enabled) { + Some(p) => p + .roles + .iter() + .filter(|(_, role)| can_use_datatable_role(role, &authed_ref)) + .map(|(name, _)| name.clone()) + .collect(), + None => vec![ADMIN_DATATABLE_ROLE.to_string()], + }) +} + /// Whether the connection's role is a member of the target's owner, which is /// what "may change its access" means here. async fn can_manage_target(client: &tokio_postgres::Client, target: &AclTarget) -> Result { @@ -632,17 +749,7 @@ async fn get_datatable_acl( // Every role is named, since the list is not what is private — what each of // them may reach is. - let authed_ref = authed.to_authed_ref(); - let datatable = read_datatable(&db, &w_id, &datatable_name).await?; - let usable_roles: Vec = match datatable.permissions.filter(|p| p.enabled) { - Some(p) => p - .roles - .iter() - .filter(|(_, role)| can_use_datatable_role(role, &authed_ref)) - .map(|(name, _)| name.clone()) - .collect(), - None => vec![ADMIN_DATATABLE_ROLE.to_string()], - }; + let usable_roles = usable_role_names(&db, &w_id, &datatable_name, &authed).await?; Ok(Json(DatatableAclInfo { owner: windmill_role_of(&roles, &owner), @@ -813,15 +920,38 @@ async fn build_acl_plan( crate::datatable_permissions::require_datatable_permissions_license().await?; let (client, conn) = connect_as_caller(db, authed, w_id, datatable_name, req.role.as_deref()).await?; + // The objects a revoke names come from the request; the ones it is checked + // and planned against come from the catalog. + let change = match &req.change { + AclChange::Revoke { role, privileges, scope, objects } => AclChange::Revoke { + role: role.clone(), + privileges: privileges.clone(), + scope: *scope, + objects: resolve_acl_objects(&client, &req.target, objects).await?, + }, + change => change.clone(), + }; // What the caller's own role may change. Postgres cannot enforce the rule we // want on its own — handing an object to a role you are not a member of is // refused outright, and granting on one you own needs the grant option — so // this is the check, and the statements run as the data table's admin below. - if !authed.is_admin && !can_manage_target(&client, &req.target).await? { - return Err(Error::NotAuthorized(format!( - "{} is owned by a role you are not a member of", - req.target.label(&conn.dbname) - ))); + if !authed.is_admin { + if !can_manage_target(&client, &req.target).await? { + return Err(Error::NotAuthorized(format!( + "{} is owned by a role you are not a member of", + req.target.label(&conn.dbname) + ))); + } + // Owning the target is not owning what a change through it reaches: a + // schema-wide scope names every object in the schema, and handing a + // schema over takes them all with it. Postgres would have skipped the + // ones the caller does not own; these statements run as admin, so it + // will not. + if let Some(unreachable) = first_unmanageable_object(&client, &req.target, &change).await? { + return Err(Error::NotAuthorized(format!( + "This change also covers {unreachable}, which is owned by a role you are not a member of" + ))); + } } let roles = role_map( db, @@ -835,10 +965,18 @@ async fn build_acl_plan( AclChange::Grant { role, .. } | AclChange::Revoke { role, .. } => role, }; let pg_role = pg_role_of(&roles, role_name)?; + // The roles a plan may also write about: what a schema's new owner is kept + // in reach of, and whose future objects a `created later` grant covers. Both + // are `ALTER DEFAULT PRIVILEGES FOR ROLE `, which speaks for that role + // — so a non-admin only gets the ones they may run as. + let usable = usable_role_names(db, w_id, datatable_name, authed).await?; let other_pg_roles: Vec = roles - .values() - .filter(|pg| pg.as_str() != pg_role.as_str()) - .cloned() + .iter() + .filter(|(name, pg)| { + pg.as_str() != pg_role.as_str() + && (authed.is_admin || usable.iter().any(|u| u == name.as_str())) + }) + .map(|(_, pg)| pg.clone()) .collect(); let existing_objects = match (&req.change, &req.target) { (AclChange::SetOwner { .. }, AclTarget::Schema { schema }) => { @@ -846,17 +984,6 @@ async fn build_acl_plan( } _ => vec![], }; - // The objects a revoke names come from the request; the ones it plans with - // come from the catalog. - let change = match &req.change { - AclChange::Revoke { role, privileges, scope, objects } => AclChange::Revoke { - role: role.clone(), - privileges: privileges.clone(), - scope: *scope, - objects: resolve_acl_objects(&client, &req.target, objects).await?, - }, - change => change.clone(), - }; let plan = crate::datatable_acl_oss::plan_statements( &req.target, &change, diff --git a/backend/windmill-api-workspaces/src/workspaces.rs b/backend/windmill-api-workspaces/src/workspaces.rs index 4fa4d32c84..5cc09c12fb 100644 --- a/backend/windmill-api-workspaces/src/workspaces.rs +++ b/backend/windmill-api-workspaces/src/workspaces.rs @@ -2987,9 +2987,48 @@ pub(crate) async fn resolve_pg_source_checked( authed: &ApiAuthed, w_id: &str, source: &str, +) -> Result { + resolve_pg_source_as(db, user_db, authed, w_id, source, false).await +} + +/// As [`resolve_pg_source_checked`], but a workspace admin reaches a data table +/// as `admin` rather than as the role it defaults to. +/// +/// For the paths that copy a whole database: dumping as a restricted default +/// role silently leaves out every table that role cannot read, which is a +/// truncated copy rather than an error. A non-admin still resolves as their own +/// role — `admin` is not theirs to ask for — so this hands out nothing. +pub(crate) async fn resolve_pg_source_for_copy( + db: &DB, + user_db: &UserDB, + authed: &ApiAuthed, + w_id: &str, + source: &str, +) -> Result { + resolve_pg_source_as(db, user_db, authed, w_id, source, authed.is_admin).await +} + +async fn resolve_pg_source_as( + db: &DB, + user_db: &UserDB, + authed: &ApiAuthed, + w_id: &str, + source: &str, + as_admin: bool, ) -> Result { let db_resource = if let Some(name) = source.strip_prefix("datatable://") { - get_datatable_resource_as_default_role(db, authed, w_id, name).await? + if as_admin { + get_datatable_resource_from_db( + db, + w_id, + name, + Some(ADMIN_DATATABLE_ROLE), + DatatableAccess::Authed(authed.to_authed_ref()), + ) + .await? + } else { + get_datatable_resource_as_default_role(db, authed, w_id, name).await? + } } else if let Some(path) = source.strip_prefix("$res:") { let db_with_authed = windmill_common::db::DbWithOptAuthed::from_authed( authed, @@ -3466,9 +3505,9 @@ async fn import_pg_database( } let schema_only = req.fork_behavior == DataTableForkBehavior::SchemaOnly; - let source_pg = resolve_pg_source_checked(&db, &user_db, &authed, &w_id, &req.source).await?; + let source_pg = resolve_pg_source_for_copy(&db, &user_db, &authed, &w_id, &req.source).await?; let mut target_pg = - resolve_pg_source_checked(&db, &user_db, &authed, &w_id, &req.target).await?; + resolve_pg_source_for_copy(&db, &user_db, &authed, &w_id, &req.target).await?; if let Some(ref override_dbname) = req.target_dbname_override { if !windmill_api_auth::is_super_admin_authed(&db, &authed).await? { diff --git a/backend/windmill-common/src/workspaces.rs b/backend/windmill-common/src/workspaces.rs index 709da03314..41a288e8f5 100644 --- a/backend/windmill-common/src/workspaces.rs +++ b/backend/windmill-common/src/workspaces.rs @@ -1297,10 +1297,11 @@ pub async fn get_datatable_resource_from_db_unchecked( .await } -/// Resolve a data table's connection credentials as `role` (default `admin`), -/// checking that `access` is allowed to use it when the data table is -/// permissioned. On an unpermissioned data table only `admin` is accepted, and -/// the resolution is the same as the unchecked one. +/// Resolve a data table's connection credentials as `role`, checking that +/// `access` is allowed to use it when the data table is permissioned. Absent +/// means the data table's configured default role, which is `admin` only until a +/// workspace names another one. On an unpermissioned data table only `admin` is +/// accepted, and the resolution is the same as the unchecked one. pub async fn get_datatable_resource_from_db( db: &DB, w_id: &str, @@ -1336,12 +1337,6 @@ pub async fn get_datatable_replication_resource_from_db_unchecked( .await } -/// Resolve which postgres login the data table should be reached through, and -/// authorize it. -/// -/// Returns the `(user, password)` to swap into the connection, or `None` when -/// the data table's own credentials are to be used — which is every -/// unpermissioned data table, and the `admin` role of a permissioned one. /// Look up the role a resolution asks for, without authorizing it. /// /// `Ok(None)` means the data table is unpermissioned and resolves through its own @@ -1379,6 +1374,12 @@ fn datatable_role_entry<'a>( Ok(Some((role_name.as_str(), role_entry))) } +/// Resolve which postgres login the data table should be reached through, and +/// authorize it. +/// +/// Returns the `(user, password)` to swap into the connection, or `None` when +/// the data table's own credentials are to be used — which is every +/// unpermissioned data table, and the `admin` role of a permissioned one. async fn resolve_datatable_role( db: &DB, w_id: &str, diff --git a/frontend/src/lib/components/datatableAcl/PgAclEditor.svelte b/frontend/src/lib/components/datatableAcl/PgAclEditor.svelte index 3f83346658..febecce58b 100644 --- a/frontend/src/lib/components/datatableAcl/PgAclEditor.svelte +++ b/frontend/src/lib/components/datatableAcl/PgAclEditor.svelte @@ -191,7 +191,9 @@ {grantScopeLabel(grant)} {@const revokeScope = revokeScopeOf(grant)} - {#if info.roles.includes(grant.grantee) && revokeScope} + + {#if info.roles.includes(grant.grantee) && revokeScope && info.can_manage}