mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-06 08:01:35 +00:00
fix(datatables): every route that frees a username takes its tenants with it
Three deletions bypassed the workspace-user cleanup and left `u/<username>` on the roles it named: the superadmin's global delete, which frees the name in every workspace at once, and both leave routes. A member invited into one of those names afterwards inherited the roles. The settings lock says what it hands back — the config as stored, generated role passwords included — so it carries `_unchecked` and the contract that goes with it: callers authorize the read, and redact before passing the value on.
This commit is contained in:
+28
@@ -0,0 +1,28 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "DELETE FROM usr WHERE email = $1 RETURNING workspace_id, username",
|
||||
"describe": {
|
||||
"columns": [
|
||||
{
|
||||
"ordinal": 0,
|
||||
"name": "workspace_id",
|
||||
"type_info": "Varchar"
|
||||
},
|
||||
{
|
||||
"ordinal": 1,
|
||||
"name": "username",
|
||||
"type_info": "Varchar"
|
||||
}
|
||||
],
|
||||
"parameters": {
|
||||
"Left": [
|
||||
"Text"
|
||||
]
|
||||
},
|
||||
"nullable": [
|
||||
false,
|
||||
false
|
||||
]
|
||||
},
|
||||
"hash": "a60524f9342bb0faad27dfc36d4ddae4627509b754ea077c79d7ff9413e60e54"
|
||||
}
|
||||
+23
@@ -0,0 +1,23 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "DELETE FROM usr WHERE workspace_id = $1 AND email = $2 RETURNING username",
|
||||
"describe": {
|
||||
"columns": [
|
||||
{
|
||||
"ordinal": 0,
|
||||
"name": "username",
|
||||
"type_info": "Varchar"
|
||||
}
|
||||
],
|
||||
"parameters": {
|
||||
"Left": [
|
||||
"Text",
|
||||
"Text"
|
||||
]
|
||||
},
|
||||
"nullable": [
|
||||
false
|
||||
]
|
||||
},
|
||||
"hash": "a81e15551d009244525c1b4e6153404e3e2941875e34c02d2ea3f50f3c224a90"
|
||||
}
|
||||
@@ -1662,14 +1662,17 @@ async fn delete_user(
|
||||
.execute(&mut *tx)
|
||||
.await?;
|
||||
|
||||
let usernames = sqlx::query_scalar!(
|
||||
"DELETE FROM usr WHERE email = $1 RETURNING username",
|
||||
// The workspace comes back with the name: a username is scoped to one, and
|
||||
// the data table tenants that name it are stored per workspace.
|
||||
let memberships = sqlx::query!(
|
||||
"DELETE FROM usr WHERE email = $1 RETURNING workspace_id, username",
|
||||
&email_to_delete
|
||||
)
|
||||
.fetch_all(&mut *tx)
|
||||
.await?;
|
||||
|
||||
for username in usernames {
|
||||
for row in memberships {
|
||||
let username = row.username;
|
||||
sqlx::query!("DELETE FROM password WHERE email = $1", &email_to_delete)
|
||||
.execute(&mut *tx)
|
||||
.await?;
|
||||
@@ -1678,6 +1681,15 @@ async fn delete_user(
|
||||
.execute(&mut *tx)
|
||||
.await?;
|
||||
|
||||
// The username is free in that workspace now, so a role still naming it
|
||||
// would hand itself to whoever takes it next.
|
||||
windmill_common::workspaces::remove_datatable_tenant_in_workspace_unchecked(
|
||||
&row.workspace_id,
|
||||
&format!("u/{username}"),
|
||||
&mut tx,
|
||||
)
|
||||
.await?;
|
||||
|
||||
sqlx::query!(
|
||||
"DELETE FROM workspace_invite WHERE email = $1",
|
||||
&email_to_delete
|
||||
@@ -3301,6 +3313,15 @@ async fn leave_workspace(
|
||||
.execute(&mut *tx)
|
||||
.await?;
|
||||
|
||||
// Leaving frees the username here too, and a role that still names it would
|
||||
// be inherited by the next member to take it.
|
||||
windmill_common::workspaces::remove_datatable_tenant_in_workspace_unchecked(
|
||||
&w_id,
|
||||
&format!("u/{}", authed.username),
|
||||
&mut tx,
|
||||
)
|
||||
.await?;
|
||||
|
||||
audit_log(
|
||||
&mut *tx,
|
||||
&authed,
|
||||
|
||||
@@ -1045,7 +1045,7 @@ async fn apply_datatable_acl(
|
||||
// off the catalog and the config, and a role save running at the same time
|
||||
// is what changes both under it. Held to the end of this handler.
|
||||
let mut lock_tx = db.begin().await?;
|
||||
windmill_common::workspaces::lock_workspace_settings(&mut lock_tx, &w_id).await?;
|
||||
windmill_common::workspaces::lock_workspace_settings_unchecked(&mut lock_tx, &w_id).await?;
|
||||
|
||||
// Authorization first: the repair below opens a connection as the instance's
|
||||
// own Postgres user, which is not something a request that is about to be
|
||||
|
||||
@@ -577,7 +577,7 @@ async fn set_datatable_permissions(
|
||||
// computed before the other committed. The settings row is what everything
|
||||
// touching that config takes, so taking it here is what serializes them.
|
||||
let mut tx = db.begin().await?;
|
||||
windmill_common::workspaces::lock_workspace_settings(&mut tx, &w_id).await?;
|
||||
windmill_common::workspaces::lock_workspace_settings_unchecked(&mut tx, &w_id).await?;
|
||||
|
||||
// The roles about to be created are handed privileges by this connection,
|
||||
// which cannot pass on what it holds without the grant option.
|
||||
|
||||
@@ -3710,7 +3710,7 @@ async fn edit_datatable_config(
|
||||
// lock as the role save and the principal cleanups, or it puts back what one
|
||||
// of them just took away.
|
||||
let old_datatables: HashMap<String, DataTable> = serde_json::from_value(
|
||||
windmill_common::workspaces::lock_workspace_settings(&mut tx, &w_id)
|
||||
windmill_common::workspaces::lock_workspace_settings_unchecked(&mut tx, &w_id)
|
||||
.await?
|
||||
.and_then(|d| d.get("datatables").cloned())
|
||||
.unwrap_or(serde_json::Value::Null),
|
||||
@@ -8927,14 +8927,25 @@ async fn leave_workspace(
|
||||
) -> Result<String> {
|
||||
windmill_api_auth::forbid_job_token_account_destruction(&authed)?;
|
||||
let mut tx = db.begin().await?;
|
||||
sqlx::query!(
|
||||
"DELETE FROM usr WHERE workspace_id = $1 AND email = $2",
|
||||
let left = sqlx::query_scalar!(
|
||||
"DELETE FROM usr WHERE workspace_id = $1 AND email = $2 RETURNING username",
|
||||
&w_id,
|
||||
&authed.email
|
||||
)
|
||||
.execute(&mut *tx)
|
||||
.fetch_all(&mut *tx)
|
||||
.await?;
|
||||
|
||||
// Leaving frees the username here, and a role that still names it would be
|
||||
// inherited by the next member to take it.
|
||||
for username in left {
|
||||
windmill_common::workspaces::remove_datatable_tenant_in_workspace_unchecked(
|
||||
&w_id,
|
||||
&format!("u/{username}"),
|
||||
&mut tx,
|
||||
)
|
||||
.await?;
|
||||
}
|
||||
|
||||
audit_log(
|
||||
&mut *tx,
|
||||
&authed,
|
||||
|
||||
@@ -1151,7 +1151,13 @@ pub fn remove_datatable_tenant(datatable: &mut serde_json::Value, tenant: &str)
|
||||
/// principal cleanups below. Without it each of them can persist a block it
|
||||
/// computed before another one committed — a save that planned with `g/devs`
|
||||
/// puts the tenant back after the group's deletion took it away.
|
||||
pub async fn lock_workspace_settings(
|
||||
///
|
||||
/// Authorization: performs none, for any workspace it is handed. What it returns
|
||||
/// is the config as stored, generated role passwords included, so callers MUST
|
||||
/// have authorized the read — every one today is admin-gated or a system path —
|
||||
/// and MUST NOT pass the value outward without
|
||||
/// [`redact_datatable_settings_for_export`].
|
||||
pub async fn lock_workspace_settings_unchecked(
|
||||
tx: &mut sqlx::Transaction<'_, sqlx::Postgres>,
|
||||
w_id: &str,
|
||||
) -> Result<Option<serde_json::Value>> {
|
||||
@@ -1182,7 +1188,7 @@ pub async fn remove_datatable_tenant_in_workspace_unchecked(
|
||||
tenant: &str,
|
||||
tx: &mut sqlx::Transaction<'_, sqlx::Postgres>,
|
||||
) -> Result<()> {
|
||||
let Some(mut settings) = lock_workspace_settings(tx, w_id).await? else {
|
||||
let Some(mut settings) = lock_workspace_settings_unchecked(tx, w_id).await? else {
|
||||
return Ok(());
|
||||
};
|
||||
if remove_datatable_tenant(&mut settings, tenant) {
|
||||
|
||||
Reference in New Issue
Block a user