fix(datatables): follow the resource through variables, and a tenant through a rename

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/<username>, 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.
This commit is contained in:
Diego Imbert
2026-09-02 00:39:12 +02:00
parent 65401f39fe
commit 83acf4ba56
4 changed files with 92 additions and 3 deletions
@@ -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"
}
+3 -3
View File
@@ -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
+53
View File
@@ -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/<username>`, 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(())
}
+14
View File
@@ -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<String> = 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;