From 83acf4ba56cca400f6845c52b1add258c09836b3 Mon Sep 17 00:00:00 2001 From: Diego Imbert Date: Wed, 2 Sep 2026 00:39:12 +0200 Subject: [PATCH] fix(datatables): follow the resource through variables, and a tenant through a rename MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A linked variable owns the resource at its path: deleting one — singly or in bulk — deletes it, and renaming one moves it, so all three answer to the rule the resource endpoints do. And a data table role's tenants are stored as u/, which the executor compares against the caller's name. A username rename that skipped them took the role away from the user it followed and left it waiting for whoever took the old name next. Also: the ACL endpoints stopped saying 'admins only' — a non-admin may act on what their role owns, which is the contract the handlers implement. --- ...29265289c8d6f625ac272655c88e1ad0b1745.json | 22 ++++++++ backend/windmill-api/openapi.yaml | 6 +-- backend/windmill-api/src/users.rs | 53 +++++++++++++++++++ backend/windmill-store/src/variables.rs | 14 +++++ 4 files changed, 92 insertions(+), 3 deletions(-) create mode 100644 backend/.sqlx/query-e2061df65ffd4a72146c4ca316829265289c8d6f625ac272655c88e1ad0b1745.json diff --git a/backend/.sqlx/query-e2061df65ffd4a72146c4ca316829265289c8d6f625ac272655c88e1ad0b1745.json b/backend/.sqlx/query-e2061df65ffd4a72146c4ca316829265289c8d6f625ac272655c88e1ad0b1745.json new file mode 100644 index 0000000000..0e3c0182a9 --- /dev/null +++ b/backend/.sqlx/query-e2061df65ffd4a72146c4ca316829265289c8d6f625ac272655c88e1ad0b1745.json @@ -0,0 +1,22 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT datatable FROM workspace_settings WHERE workspace_id = $1 FOR UPDATE", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "datatable", + "type_info": "Jsonb" + } + ], + "parameters": { + "Left": [ + "Text" + ] + }, + "nullable": [ + true + ] + }, + "hash": "e2061df65ffd4a72146c4ca316829265289c8d6f625ac272655c88e1ad0b1745" +} diff --git a/backend/windmill-api/openapi.yaml b/backend/windmill-api/openapi.yaml index 3aee369878..fa0e90a3fb 100644 --- a/backend/windmill-api/openapi.yaml +++ b/backend/windmill-api/openapi.yaml @@ -5273,7 +5273,7 @@ paths: /w/{workspace}/workspaces/datatable_acl/{datatable_name}: get: - summary: read the owner and grants of a datatable schema or table (admins only) + summary: read the owner and grants of a datatable schema or table operationId: getDatatableAcl tags: - workspace @@ -5316,7 +5316,7 @@ paths: /w/{workspace}/workspaces/datatable_acl/{datatable_name}/plan: post: - summary: preview the SQL an ownership or grant change would run (admins only) + summary: preview the SQL an ownership or grant change would run (a non-admin may act on what their role owns) operationId: planDatatableAcl tags: - workspace @@ -5353,7 +5353,7 @@ paths: /w/{workspace}/workspaces/datatable_acl/{datatable_name}/apply: post: - summary: run an ownership or grant change (admins only) + summary: run an ownership or grant change (a non-admin may act on what their role owns) operationId: applyDatatableAcl tags: - workspace diff --git a/backend/windmill-api/src/users.rs b/backend/windmill-api/src/users.rs index 8804004f2b..e384ac8103 100644 --- a/backend/windmill-api/src/users.rs +++ b/backend/windmill-api/src/users.rs @@ -868,6 +868,59 @@ async fn update_username_in_workpsace<'c>( .execute(&mut **tx) .await?; + // ---- data table role tenants ---- + + // Who may run as a data table role is stored as `u/`, and the + // executor compares it against the caller's name. Left behind, the rename + // takes the role away from the user it followed and hands it to whoever + // takes the old name next. + let datatable_settings = sqlx::query_scalar!( + "SELECT datatable FROM workspace_settings WHERE workspace_id = $1 FOR UPDATE", + w_id + ) + .fetch_optional(&mut **tx) + .await? + .flatten(); + if let Some(mut settings) = datatable_settings { + let old_tenant = format!("u/{old_username}"); + let new_tenant = serde_json::Value::String(format!("u/{new_username}")); + let mut renamed = false; + for dt in settings + .get_mut("datatables") + .and_then(|d| d.as_object_mut()) + .into_iter() + .flat_map(|datatables| datatables.values_mut()) + { + let Some(roles) = dt + .get_mut("permissions") + .and_then(|p| p.get_mut("roles")) + .and_then(|r| r.as_object_mut()) + else { + continue; + }; + for tenant in roles + .values_mut() + .filter_map(|role| role.get_mut("tenants")) + .filter_map(|t| t.as_array_mut()) + .flatten() + { + if tenant.as_str() == Some(old_tenant.as_str()) { + *tenant = new_tenant.clone(); + renamed = true; + } + } + } + if renamed { + sqlx::query!( + "UPDATE workspace_settings SET datatable = $1 WHERE workspace_id = $2", + settings, + w_id + ) + .execute(&mut **tx) + .await?; + } + } + Ok(()) } diff --git a/backend/windmill-store/src/variables.rs b/backend/windmill-store/src/variables.rs index 8f288dc85e..8ef64e3fd4 100644 --- a/backend/windmill-store/src/variables.rs +++ b/backend/windmill-store/src/variables.rs @@ -744,6 +744,10 @@ async fn delete_variable( return Err(Error::PermissionDenied(msg)); } + // This takes the resource linked at the same path with it, so it answers to + // the same rule as deleting that resource directly. + windmill_common::workspaces::ensure_resource_removal_allowed(&db, &w_id, path).await?; + // Check if variable is a secret before deleting (for Vault cleanup) let is_secret = sqlx::query_scalar!( "SELECT is_secret FROM variable WHERE path = $1 AND workspace_id = $2", @@ -918,6 +922,11 @@ async fn delete_variables_bulk( return Err(Error::PermissionDenied(msg)); } + // Each of these takes the resource linked at the same path with it. + for path in &request.paths { + windmill_common::workspaces::ensure_resource_removal_allowed(&db, &w_id, path).await?; + } + // Query which paths are secrets before deletion (for Vault cleanup) let secret_paths: Vec = sqlx::query_scalar!( "SELECT path FROM variable WHERE path = ANY($1) AND workspace_id = $2 AND is_secret = true", @@ -1099,6 +1108,11 @@ async fn update_variable( // source path. if let Some(npath) = ns.path.as_deref() { check_scopes(&authed, || format!("variables:write:{}", npath))?; + // It moves the resource linked at the same path too, which is how a + // permissioned data table finds its database. + if npath != path { + windmill_common::workspaces::ensure_resource_removal_allowed(&db, &w_id, path).await?; + } check_proper_path(npath)?; } let authed = maybe_refresh_folders(&path, &w_id, authed, &db).await;