diff --git a/backend/windmill-api-workspaces/src/datatable_migrations.rs b/backend/windmill-api-workspaces/src/datatable_migrations.rs index b4c0611b03..386c577829 100644 --- a/backend/windmill-api-workspaces/src/datatable_migrations.rs +++ b/backend/windmill-api-workspaces/src/datatable_migrations.rs @@ -123,7 +123,12 @@ async fn datatable_database_arg( .await? .ok_or_else(|| Error::internal_err(format!("datatable {datatable_name} not found")))?; - Ok(to_raw_value(&format!("datatable://{datatable_name}"))) + // Migrations change the schema and are recorded against objects root owns, so + // they run as root rather than the data table's default role. + Ok(to_raw_value(&format!( + "datatable://{datatable_name}?role={}", + windmill_common::workspaces::ROOT_DATATABLE_ROLE + ))) } /// Run a migration's SQL as a normal Windmill `postgresql` job, permissioned as diff --git a/backend/windmill-api-workspaces/src/datatable_permissions.rs b/backend/windmill-api-workspaces/src/datatable_permissions.rs index d180f484e7..4456d518e6 100644 --- a/backend/windmill-api-workspaces/src/datatable_permissions.rs +++ b/backend/windmill-api-workspaces/src/datatable_permissions.rs @@ -31,8 +31,8 @@ use windmill_common::error::{pg_error_message, Error, JsonResult, Result}; use windmill_common::query_builders::{render_db_quoted_identifier, DbType}; use windmill_common::utils::{rd_string, require_admin}; use windmill_common::workspaces::{ - datatable_pg_role_name, get_datatable_resource_from_db_unchecked, DataTable, - DataTablePermissions, DataTableRole, ROOT_DATATABLE_ROLE, + can_use_datatable_role, datatable_pg_role_name, get_datatable_resource_from_db_unchecked, + DataTable, DataTablePermissions, DataTableRole, DATATABLE_TENANT_WILDCARD, ROOT_DATATABLE_ROLE, }; use windmill_common::{PgDatabase, DB}; @@ -46,6 +46,10 @@ pub(crate) fn routes() -> Router { "/datatable_permissions/{datatable_name}/preview", post(preview_datatable_permissions), ) + .route( + "/datatable_usable_roles/{datatable_name}", + get(list_usable_datatable_roles), + ) } /// A data table role as the UI sees it: the generated password never leaves the @@ -90,6 +94,15 @@ pub struct DatatableRoleRename { pub to: String, } +/// The roles the caller may actually run as, for pickers. Unlike the admin-only +/// permissions view this exposes no tenant lists — only what the caller can use. +#[derive(Serialize, Debug)] +pub struct UsableDatatableRoles { + pub enabled: bool, + pub roles: Vec, + pub default_role: String, +} + #[derive(Serialize, Debug)] pub struct DatatablePermissionsPreview { pub statements: Vec, @@ -141,10 +154,13 @@ fn validate_role_name(name: &str) -> Result<()> { } fn validate_tenant(tenant: &str) -> Result<()> { + if tenant == DATATABLE_TENANT_WILDCARD { + return Ok(()); + } match tenant.split_once('/') { Some(("u" | "g" | "f", rest)) if !rest.is_empty() => Ok(()), _ => Err(Error::BadRequest(format!( - "Invalid tenant '{tenant}': expected u/, g/ or f/" + "Invalid tenant '{tenant}': expected '*', u/, g/ or f/" ))), } } @@ -175,24 +191,23 @@ fn plan_role_changes( let mut warnings = Vec::new(); let mut dropped_pg_roles: HashSet = HashSet::new(); - let drop_role = |statements: &mut Vec, - dropped: &mut HashSet, - pg_role: &str| { - if !existing_pg_roles.contains(pg_role) { - return; - } - dropped.insert(pg_role.to_string()); - let q = quote_ident(pg_role); - // Give the objects back to root before dropping, else the DROP fails on - // anything the role still owns. DROP OWNED then clears what is left: - // privileges granted to it and its default-privilege entries. - statements.push(PlannedStatement::plain(format!( - "REASSIGN OWNED BY {q} TO {};", - quote_ident(root_pg_role) - ))); - statements.push(PlannedStatement::plain(format!("DROP OWNED BY {q};"))); - statements.push(PlannedStatement::plain(format!("DROP ROLE {q};"))); - }; + let drop_role = + |statements: &mut Vec, dropped: &mut HashSet, pg_role: &str| { + if !existing_pg_roles.contains(pg_role) { + return; + } + dropped.insert(pg_role.to_string()); + let q = quote_ident(pg_role); + // Give the objects back to root before dropping, else the DROP fails on + // anything the role still owns. DROP OWNED then clears what is left: + // privileges granted to it and its default-privilege entries. + statements.push(PlannedStatement::plain(format!( + "REASSIGN OWNED BY {q} TO {};", + quote_ident(root_pg_role) + ))); + statements.push(PlannedStatement::plain(format!("DROP OWNED BY {q};"))); + statements.push(PlannedStatement::plain(format!("DROP ROLE {q};"))); + }; if !req.enabled { for (name, role) in old_roles.iter() { @@ -359,7 +374,11 @@ fn plan_role_changes( .unwrap_or_else(|| rd_string(32)); if old_pg != pg_rolename { if existing_pg_roles.contains(&old_pg) { - pending_renames.push((old_pg.clone(), pg_rolename.clone(), password.clone())); + pending_renames.push(( + old_pg.clone(), + pg_rolename.clone(), + password.clone(), + )); } else { warnings.push(format!( "Role '{from}' was expected to exist in the database as '{old_pg}' but does not; it will be created as '{pg_rolename}'." @@ -412,7 +431,11 @@ fn plan_role_changes( } } - statements.extend(order_renames(pending_renames, existing_pg_roles, &dropped_pg_roles)?); + statements.extend(order_renames( + pending_renames, + existing_pg_roles, + &dropped_pg_roles, + )?); statements.extend(creates_sql); Ok(RolePlan { @@ -420,8 +443,7 @@ fn plan_role_changes( permissions: DataTablePermissions { enabled: true, roles, - default_role: (default_role != ROOT_DATATABLE_ROLE) - .then(|| default_role.to_string()), + default_role: (default_role != ROOT_DATATABLE_ROLE).then(|| default_role.to_string()), }, warnings, }) @@ -678,6 +700,34 @@ async fn get_datatable_permissions( })) } +/// List the roles `authed` may run this data table as. An unpermissioned data +/// table reports `enabled: false` and no roles, so a picker can hide itself. +async fn list_usable_datatable_roles( + authed: ApiAuthed, + Extension(db): Extension, + Path((w_id, datatable_name)): Path<(String, String)>, +) -> JsonResult { + let datatable = read_datatable(&db, &w_id, &datatable_name).await?; + let Some(permissions) = datatable.permissions.filter(|p| p.enabled) else { + return Ok(Json(UsableDatatableRoles { + enabled: false, + roles: vec![], + default_role: ROOT_DATATABLE_ROLE.to_string(), + })); + }; + let authed_ref = authed.to_authed_ref(); + Ok(Json(UsableDatatableRoles { + enabled: true, + default_role: permissions.default_role().to_string(), + roles: permissions + .roles + .iter() + .filter(|(_, role)| can_use_datatable_role(role, &authed_ref)) + .map(|(name, _)| name.clone()) + .collect(), + })) +} + async fn preview_datatable_permissions( authed: ApiAuthed, Extension(db): Extension, @@ -895,11 +945,11 @@ mod tests { .map(|r| datatable_pg_role_name(W_ID, DT, r)) .collect(); let req = SetDatatablePermissions { - enabled: false, - roles: vec![], - default_role: None, - renames: vec![], - }; + enabled: false, + roles: vec![], + default_role: None, + renames: vec![], + }; let plan = plan( Some(&old), &req, @@ -1033,11 +1083,11 @@ mod tests { fn a_role_the_config_lost_track_of_is_not_dropped() { let old = enabled_with(&["analyst"]); let req = SetDatatablePermissions { - enabled: false, - roles: vec![], - default_role: None, - renames: vec![], - }; + enabled: false, + roles: vec![], + default_role: None, + renames: vec![], + }; // The Postgres role is already gone, so planning its drop would fail the // whole transaction and wedge the opt-out. let plan = plan(Some(&old), &req, &[]).unwrap(); @@ -1104,7 +1154,16 @@ mod tests { "{bad_role} should be rejected" ); } - for bad_tenant in ["alice", "x/alice", "u/", ""] { + // The wildcard is the one tenant with no prefix. + let req = SetDatatablePermissions { + enabled: true, + roles: vec![role("root", &["*"])], + default_role: None, + renames: vec![], + }; + assert!(plan(None, &req, &[]).is_ok()); + + for bad_tenant in ["alice", "x/alice", "u/", "", "*/alice", "**"] { let req = SetDatatablePermissions { enabled: true, roles: vec![role("root", &[bad_tenant])], diff --git a/backend/windmill-api/openapi.yaml b/backend/windmill-api/openapi.yaml index e56a680187..6b23467657 100644 --- a/backend/windmill-api/openapi.yaml +++ b/backend/windmill-api/openapi.yaml @@ -5013,6 +5013,37 @@ paths: schema: type: string + /w/{workspace}/workspaces/datatable_usable_roles/{datatable_name}: + get: + summary: list the datatable roles the caller may run as + operationId: listUsableDatatableRoles + tags: + - workspace + parameters: + - $ref: "#/components/parameters/WorkspaceId" + - name: datatable_name + in: path + required: true + schema: + type: string + responses: + "200": + description: usable roles + content: + application/json: + schema: + type: object + required: [enabled, roles, default_role] + properties: + enabled: + type: boolean + roles: + type: array + items: + type: string + default_role: + type: string + /w/{workspace}/workspaces/datatable_permissions/{datatable_name}/preview: post: summary: preview the SQL a datatable permissions change would run (admins only) @@ -31472,7 +31503,7 @@ components: name: type: string tenants: - description: who may run as this role, as u/, g/ or f/ + description: who may run as this role, as '*' (everyone), u/, g/ or f/ type: array items: type: string diff --git a/backend/windmill-common/src/workspaces.rs b/backend/windmill-common/src/workspaces.rs index 252fab7c69..0e1ae448cf 100644 --- a/backend/windmill-common/src/workspaces.rs +++ b/backend/windmill-common/src/workspaces.rs @@ -1013,6 +1013,10 @@ pub struct DataTable { /// cannot be created, renamed or dropped. pub const ROOT_DATATABLE_ROLE: &str = "root"; +/// Tenant matching every workspace member. Distinct from listing the `all` group, +/// whose membership is bookkeeping that can drift; this one cannot. +pub const DATATABLE_TENANT_WILDCARD: &str = "*"; + #[derive(Deserialize, Serialize, Debug, Default, Clone)] pub struct DataTablePermissions { pub enabled: bool, @@ -1140,6 +1144,9 @@ pub fn datatable_pg_role_name(w_id: &str, datatable: &str, role: &str) -> String pub fn can_use_datatable_role(role: &DataTableRole, authed: &crate::db::AuthedRef<'_>) -> bool { *authed.is_admin || role.tenants.iter().any(|tenant| { + if tenant == DATATABLE_TENANT_WILDCARD { + return true; + } match tenant.split_once('/') { Some(("u", user)) => authed.username == user, Some(("g", group)) => authed.groups.iter().any(|g| g == group), @@ -1202,6 +1209,22 @@ fn datatable_not_found_error(name: &str, datatables: Option<&serde_json::Value>) )) } +/// Split a `?role=` data table reference (the part after +/// `datatable://`, or the whole thing for the implicit `datatable` form). +/// +/// The role rides in the reference rather than only in a `-- role` annotation +/// because a DuckDB script can attach several data tables under different roles, +/// and because generated SQL — the database manager's, for instance — has no +/// natural place to put a file-level annotation. +pub fn parse_datatable_ref(reference: &str) -> (&str, Option<&str>) { + let (name, query) = reference.split_once('?').unwrap_or((reference, "")); + let role = query + .split('&') + .find_map(|param| param.strip_prefix("role=")) + .filter(|role| !role.is_empty()); + (name, role) +} + /// Who a data table is being resolved for, when it is permissioned. pub enum DatatableAccess<'a> { /// Internal callers that have already authorized the access (or for which @@ -1225,15 +1248,28 @@ pub enum DatatableAccess<'a> { NoIdentity, } -/// Resolve a data table's connection credentials without authorizing the caller. -/// Callers MUST have already authorized the access; anything running on behalf -/// of a user should go through [`get_datatable_resource_from_db`] instead. +/// Resolve a data table's connection credentials as `root`, without authorizing +/// the caller. +/// +/// Always `root`, never the configured default role: this is the connection that +/// owns every object, and the internal machinery built on it — role DDL, migration +/// bookkeeping, fork snapshots — needs those privileges. Callers MUST have already +/// authorized the access; anything running on behalf of a user should go through +/// [`get_datatable_resource_from_db`] instead. pub async fn get_datatable_resource_from_db_unchecked( db: &DB, w_id: &str, name: &str, ) -> Result { - get_datatable_resource_inner(db, w_id, name, false, None, DatatableAccess::Unchecked).await + get_datatable_resource_inner( + db, + w_id, + name, + false, + Some(ROOT_DATATABLE_ROLE), + DatatableAccess::Unchecked, + ) + .await } /// Resolve a data table's connection credentials as `role` (default `root`), @@ -1264,7 +1300,15 @@ pub async fn get_datatable_replication_resource_from_db_unchecked( w_id: &str, name: &str, ) -> Result { - get_datatable_resource_inner(db, w_id, name, true, None, DatatableAccess::Unchecked).await + get_datatable_resource_inner( + db, + w_id, + name, + true, + Some(ROOT_DATATABLE_ROLE), + DatatableAccess::Unchecked, + ) + .await } /// Resolve which postgres login the data table should be reached through, and @@ -2488,6 +2532,12 @@ mod tests { // A tenant list is a whitelist, so an empty one grants nobody... let empty = DataTableRole::default(); assert!(!can_use_datatable_role(&empty, &alice.to_authed_ref())); + // ...while the wildcard grants everyone, whatever they belong to. + let wildcard = DataTableRole { + tenants: vec![DATATABLE_TENANT_WILDCARD.to_string()], + ..Default::default() + }; + assert!(can_use_datatable_role(&wildcard, &stranger.to_authed_ref())); // ...except admins, who reach every role so they cannot lock themselves // out of their own data table. let mut admin = authed("alice", &[], &[]); @@ -2554,7 +2604,11 @@ mod tests { }, forked_from: None, migrations_enabled: None, - permissions: Some(DataTablePermissions { enabled: true, roles: map, default_role: None }), + permissions: Some(DataTablePermissions { + enabled: true, + roles: map, + default_role: None, + }), } } @@ -2594,6 +2648,22 @@ mod tests { assert_eq!(name, ROOT_DATATABLE_ROLE); } + /// The internal machinery — role DDL, migration bookkeeping, fork snapshots — + /// is built on the unchecked resolution and needs root's privileges, so a + /// configured default role must not divert it. + #[test] + fn the_unchecked_resolution_is_root_even_when_another_role_is_default() { + let mut dt = permissioned(&[(ROOT_DATATABLE_ROLE, &[]), ("analyst", &[])]); + dt.permissions.as_mut().unwrap().default_role = Some("analyst".to_string()); + + let (name, entry) = datatable_role_entry(&dt, "main", Some(ROOT_DATATABLE_ROLE)) + .unwrap() + .unwrap(); + assert_eq!(name, ROOT_DATATABLE_ROLE); + // root reuses the data table's own connection rather than a created login. + assert!(entry.pg_rolename.is_none()); + } + #[test] fn naming_a_role_on_an_unpermissioned_datatable_is_refused() { let mut dt = permissioned(&[(ROOT_DATATABLE_ROLE, &[]), ("analyst", &["u/alice"])]); diff --git a/backend/windmill-worker/src/duckdb_executor.rs b/backend/windmill-worker/src/duckdb_executor.rs index 577a53443f..1e328c1f67 100644 --- a/backend/windmill-worker/src/duckdb_executor.rs +++ b/backend/windmill-worker/src/duckdb_executor.rs @@ -13,7 +13,7 @@ use windmill_common::error::{to_anyhow, Error, Result}; use windmill_common::utils::sanitize_string_from_password; use windmill_common::worker::{get_memory, to_raw_value, Connection, SqlResultCollectionStrategy}; use windmill_common::workspaces::{ - get_datatable_resource_from_db, get_ducklake_from_db_unchecked, + get_datatable_resource_from_db, get_ducklake_from_db_unchecked, parse_datatable_ref, strip_fork_reserved_attach_args, DatatableAccess, DucklakeCatalogResourceType, }; use windmill_common::PgDatabase; @@ -2542,18 +2542,6 @@ fn fork_defer_statements( Ok(stmts) } -/// Split a `datatable://?role=` reference. The role rides in the -/// reference rather than in a file-level annotation because one DuckDB script can -/// attach several data tables, each under a different role. -fn parse_datatable_ref(reference: &str) -> (&str, Option<&str>) { - let (name, query) = reference.split_once('?').unwrap_or((reference, "")); - let role = query - .split('&') - .find_map(|param| param.strip_prefix("role=")) - .filter(|role| !role.is_empty()); - (name, role) -} - async fn transform_attach_datatable( query: &str, conn: &Connection, diff --git a/backend/windmill-worker/src/pg_executor.rs b/backend/windmill-worker/src/pg_executor.rs index 6ea86e8aef..a1405176bb 100644 --- a/backend/windmill-worker/src/pg_executor.rs +++ b/backend/windmill-worker/src/pg_executor.rs @@ -27,7 +27,9 @@ use windmill_common::error::{self, Error}; use windmill_common::worker::{ to_raw_value, Connection, SqlAnnotations, SqlResultCollectionStrategy, CLOUD_HOSTED, }; -use windmill_common::workspaces::{get_datatable_resource_from_db, DatatableAccess}; +use windmill_common::workspaces::{ + get_datatable_resource_from_db, parse_datatable_ref, DatatableAccess, +}; use windmill_common::{PgDatabase, PrepareQueryColumnInfo, PrepareQueryResult, DB}; use windmill_parser::{Arg, Typ}; use windmill_parser_sql::{ @@ -592,10 +594,14 @@ pub async fn do_postgresql( } else { match pg_args.get("database").cloned() { Some(Value::String(db_str)) if db_str.starts_with("datatable://") => { - let db_str = db_str.trim_start_matches("datatable://"); + let reference = db_str.trim_start_matches("datatable://"); + let (db_str, uri_role) = parse_datatable_ref(reference); // `-- role ` rather than an argument: an argument named - // `role` would collide with a query parameter of that name. - let role = SqlAnnotations::datatable_role(query); + // `role` would collide with a query parameter of that name. It + // wins over a role carried by the reference, which is how + // generated SQL (the database manager's) selects one. + let role = SqlAnnotations::datatable_role(query) + .or_else(|| uri_role.map(|r| r.to_string())); Some(match conn { Connection::Http(client) => { get_datatable_resource_from_agent_http( diff --git a/frontend/src/lib/components/DBManagerContent.svelte b/frontend/src/lib/components/DBManagerContent.svelte index 9332d7ad26..c1e017dfb9 100644 --- a/frontend/src/lib/components/DBManagerContent.svelte +++ b/frontend/src/lib/components/DBManagerContent.svelte @@ -68,7 +68,10 @@ function getDbSchemasPath(input: DbInput): string { switch (input.type) { case 'database': - return input.resourcePath + // The role is part of the identity: two roles on the same data table + // may see different tables, so their schemas cannot share a cache + // entry — and the same string is what selects the role downstream. + return input.resourcePath + (input.role ? `?role=${input.role}` : '') case 'ducklake': return 'ducklake://' + input.ducklake } @@ -97,7 +100,7 @@ if (input.type == 'database') { $dbSchemas[dbSchemasPath] = await getDbSchemas( input.resourceType, - input.resourcePath, + getDbSchemasPath(input), ws, (message: string) => sendUserToast(message, true) ) diff --git a/frontend/src/lib/components/DBManagerDrawer.svelte b/frontend/src/lib/components/DBManagerDrawer.svelte index 9f60cb5388..76f6528270 100644 --- a/frontend/src/lib/components/DBManagerDrawer.svelte +++ b/frontend/src/lib/components/DBManagerDrawer.svelte @@ -58,6 +58,54 @@ })) ) + // Roles the *caller* may use, so the picker never offers one that would be + // refused. Absent/disabled permissions yield no roles and hide the picker. + const usableRoles = resource( + () => [ws, uriState.selectedDatatable] as const, + async ([workspace, datatable]) => { + if (!workspace || !datatable) return undefined + try { + return await WorkspaceService.listUsableDatatableRoles({ workspace, datatableName: datatable }) + } catch (e) { + // Never leave the drawer waiting on this: fall back to the + // unpermissioned shape so it opens and the server picks the role. + console.error('Failed to load datatable roles:', e) + return { enabled: false, roles: [], default_role: 'root' } + } + } + ) + + // The content must not mount until the role is settled: mounting is what fires + // the schema and metadata queries, and a first round sent without a role would + // run — and cache — as whatever the server defaults to. + const roleSettled = $derived( + !uriState.isDatatableInput || + (usableRoles.current !== undefined && + (!usableRoles.current.enabled || + usableRoles.current.roles.length === 0 || + uriState.selectedRole !== undefined)) + ) + + const roleItems = $derived( + (usableRoles.current?.roles ?? []).map((r) => ({ + value: r, + label: r === usableRoles.current?.default_role ? `${r} (default)` : r + })) + ) + + // Settle the role before anything queries the data table: the schema and + // metadata fetches run as whatever role the input carries, so leaving it unset + // until the user touches the picker would send the first — and cached — round + // of queries as a role they may not be allowed to use. + $effect(() => { + const roles = usableRoles.current + if (!roles?.enabled || uriState.selectedRole !== undefined) return + const effective = roles.roles.includes(roles.default_role) + ? roles.default_role + : roles.roles[0] + if (effective) untrack(() => (uriState.selectedRole = effective)) + }) + // Refetch datatables when switching to a datatable input $effect(() => { if (uriState.isDatatableInput) { @@ -176,8 +224,8 @@ noPadding id="db-manager-drawer" > - {#if uriState.effectiveInput && ws} - {#key uriState.selectedDatatable} + {#if uriState.effectiveInput && ws && roleSettled} + {#key `${uriState.selectedDatatable}~${uriState.selectedRole ?? ''}`} {/if} + {#if usableRoles.current?.enabled && roleItems.length > 0} +