fix(datatables): the three transactions that still locked before the settings row

Offboarding reassigns a departing user's scripts, flows, apps and resources —
rows a rename writes while holding the settings row — so it takes that row
first, and offboarding from several workspaces walks them in `workspace_id`
order. Removing a member locked the `usr` row before the removal reached the
settings row, and a rename renamed the `password` row before its first
workspace: both now come after.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S5arH3G2Sa1Qqm32veJQ1n
This commit is contained in:
Diego Imbert
2026-09-04 11:47:52 +02:00
co-authored by Claude Opus 5
parent 8bb2b0d777
commit 2d09d33997
3 changed files with 24 additions and 16 deletions
+5
View File
@@ -2585,6 +2585,11 @@ async fn delete_workspace_user(
) -> Result<String> {
let mut tx = db.begin().await?;
// Before the `usr` row below — see `lock_workspace_settings_unchecked`. The
// removal itself takes this row too; re-acquiring it inside a transaction
// costs nothing.
windmill_common::workspaces::lock_workspace_settings_unchecked(&mut tx, &w_id).await?;
// Locked so that the authorization below and the delete it guards see the same row.
let target = sqlx::query!(
"SELECT email, is_admin FROM usr where username = $1 AND workspace_id = $2 FOR UPDATE",
+9 -8
View File
@@ -488,8 +488,10 @@ pub(crate) async fn global_offboard_preview(
) -> JsonResult<GlobalOffboardPreview> {
require_super_admin(&db, &authed).await?;
// Ordered, so offboarding from several workspaces takes their settings rows
// in the same sequence as every other multi-workspace path.
let workspaces = sqlx::query!(
"SELECT workspace_id, username FROM usr WHERE email = $1",
"SELECT workspace_id, username FROM usr WHERE email = $1 ORDER BY workspace_id",
&email
)
.fetch_all(&db)
@@ -830,6 +832,12 @@ async fn offboard_user_from_workspace<'c>(
reassign_to: &str,
new_permissioned_as: &str,
) -> Result<OffboardSummary> {
// Before this transaction locks anything else — see
// `lock_workspace_settings_unchecked`. Everything below reassigns rows a
// rename or a deletion writes while holding this row.
let datatable_settings =
windmill_common::workspaces::lock_workspace_settings_unchecked(tx, w_id).await?;
let new_prefix = reassign_to.to_string();
let departing = windmill_common::users::username_to_permissioned_as(username);
@@ -989,13 +997,6 @@ async fn offboard_user_from_workspace<'c>(
// A data table names its database by resource path, so one just moved has to
// move in the config too: left behind it stops resolving, and the path it
// named is free for a resource pointing somewhere else entirely.
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 {
if windmill_common::workspaces::move_datatable_resource_paths(
&mut settings,
+10 -8
View File
@@ -208,14 +208,6 @@ async fn rename_user(
)));
}
sqlx::query!(
"UPDATE password SET username = $1 WHERE email = $2",
ru.new_username,
user_email
)
.execute(&mut *tx)
.await?;
// Ordered, so a rename and a deletion that touch the same workspaces take
// their settings rows in the same sequence rather than head-on.
let workspace_usernames = sqlx::query!(
@@ -240,6 +232,16 @@ async fn rename_user(
.await?;
}
// After the settings rows above: the deletion paths take `password` while
// holding one, so taking it first here would be the other order.
sqlx::query!(
"UPDATE password SET username = $1 WHERE email = $2",
ru.new_username,
user_email
)
.execute(&mut *tx)
.await?;
audit_log(
&mut *tx,
&authed,