diff --git a/backend/.sqlx/query-07168aaf14cb6beff0ad4274b441f7f387f5055c47f493271d26731336257384.json b/backend/.sqlx/query-07168aaf14cb6beff0ad4274b441f7f387f5055c47f493271d26731336257384.json index d29a18c691..e7ed0aee65 100644 --- a/backend/.sqlx/query-07168aaf14cb6beff0ad4274b441f7f387f5055c47f493271d26731336257384.json +++ b/backend/.sqlx/query-07168aaf14cb6beff0ad4274b441f7f387f5055c47f493271d26731336257384.json @@ -46,11 +46,11 @@ ] }, "nullable": [ - true, - true, - true, - true, - true, + false, + false, + false, + false, + false, true, true ] diff --git a/backend/.sqlx/query-6fbd5826ae1fafd7bbb218324e3e2d85231b889743de637eb35e756da29ad47b.json b/backend/.sqlx/query-4efaa4c3c14987fe95a508cc0cbac91ee793ad6f5ceed5b3eda9b48617c5434d.json similarity index 66% rename from backend/.sqlx/query-6fbd5826ae1fafd7bbb218324e3e2d85231b889743de637eb35e756da29ad47b.json rename to backend/.sqlx/query-4efaa4c3c14987fe95a508cc0cbac91ee793ad6f5ceed5b3eda9b48617c5434d.json index 1e39bfdaab..bdb39d134f 100644 --- a/backend/.sqlx/query-6fbd5826ae1fafd7bbb218324e3e2d85231b889743de637eb35e756da29ad47b.json +++ b/backend/.sqlx/query-4efaa4c3c14987fe95a508cc0cbac91ee793ad6f5ceed5b3eda9b48617c5434d.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "SELECT COALESCE(u.username, CASE WHEN d.workspace_id = 'admins' THEN d.email END) as \"username?\",\n d.created_at as \"draft_saved_at!\"\n FROM draft d\n LEFT JOIN usr u\n ON u.workspace_id = d.workspace_id\n AND u.email = d.email\n WHERE d.workspace_id = $1\n AND d.path = $2\n AND d.typ = $3\n AND (d.email IS NULL OR d.email <> $4)\n ORDER BY d.email NULLS LAST", + "query": "SELECT COALESCE(u.username, p.username, CASE WHEN p.email IS NOT NULL THEN d.email END) as \"username?\",\n d.created_at as \"draft_saved_at!\"\n FROM draft d\n LEFT JOIN usr u\n ON u.workspace_id = d.workspace_id\n AND u.email = d.email\n LEFT JOIN password p\n ON p.email = d.email\n AND p.super_admin = true\n WHERE d.workspace_id = $1\n AND d.path = $2\n AND d.typ = $3\n AND (d.email IS NULL OR d.email <> $4)\n ORDER BY d.email NULLS LAST", "describe": { "columns": [ { @@ -60,5 +60,5 @@ false ] }, - "hash": "6fbd5826ae1fafd7bbb218324e3e2d85231b889743de637eb35e756da29ad47b" + "hash": "4efaa4c3c14987fe95a508cc0cbac91ee793ad6f5ceed5b3eda9b48617c5434d" } diff --git a/backend/.sqlx/query-9ee6263223bd415d7ac21328a8ebc731d43e7331b9fd83282a87f8e2fd8fa770.json b/backend/.sqlx/query-9ee6263223bd415d7ac21328a8ebc731d43e7331b9fd83282a87f8e2fd8fa770.json new file mode 100644 index 0000000000..64dfe64198 --- /dev/null +++ b/backend/.sqlx/query-9ee6263223bd415d7ac21328a8ebc731d43e7331b9fd83282a87f8e2fd8fa770.json @@ -0,0 +1,20 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT EXISTS(SELECT 1 FROM password WHERE username IS NOT NULL)", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "exists", + "type_info": "Bool" + } + ], + "parameters": { + "Left": [] + }, + "nullable": [ + null + ] + }, + "hash": "9ee6263223bd415d7ac21328a8ebc731d43e7331b9fd83282a87f8e2fd8fa770" +} diff --git a/backend/.sqlx/query-ae2293c7d9d2b5dad61b27db3b29d5f87fc6b8d53fe6a073eef488533cbd3ed3.json b/backend/.sqlx/query-ae2293c7d9d2b5dad61b27db3b29d5f87fc6b8d53fe6a073eef488533cbd3ed3.json new file mode 100644 index 0000000000..1b4082b18a --- /dev/null +++ b/backend/.sqlx/query-ae2293c7d9d2b5dad61b27db3b29d5f87fc6b8d53fe6a073eef488533cbd3ed3.json @@ -0,0 +1,23 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT COALESCE(\n (SELECT email FROM usr WHERE workspace_id = $1 AND username = $2),\n (SELECT email FROM password WHERE (username = $2 OR email = $2) AND super_admin = true)\n )", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "coalesce", + "type_info": "Varchar" + } + ], + "parameters": { + "Left": [ + "Text", + "Text" + ] + }, + "nullable": [ + null + ] + }, + "hash": "ae2293c7d9d2b5dad61b27db3b29d5f87fc6b8d53fe6a073eef488533cbd3ed3" +} diff --git a/backend/.sqlx/query-dadf78bae0299b24f6798e7a2cc86f0ecfcf63daa0b185994a410eb2fe41fad9.json b/backend/.sqlx/query-dadf78bae0299b24f6798e7a2cc86f0ecfcf63daa0b185994a410eb2fe41fad9.json deleted file mode 100644 index 6ae3f60e49..0000000000 --- a/backend/.sqlx/query-dadf78bae0299b24f6798e7a2cc86f0ecfcf63daa0b185994a410eb2fe41fad9.json +++ /dev/null @@ -1,23 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "SELECT email FROM usr WHERE workspace_id = $1 AND username = $2", - "describe": { - "columns": [ - { - "ordinal": 0, - "name": "email", - "type_info": "Varchar" - } - ], - "parameters": { - "Left": [ - "Text", - "Text" - ] - }, - "nullable": [ - false - ] - }, - "hash": "dadf78bae0299b24f6798e7a2cc86f0ecfcf63daa0b185994a410eb2fe41fad9" -} diff --git a/backend/.sqlx/query-fc1663ecda7167dc12c0fd5a206cbddf85e55f6c7ef226cdd66e025c002bac98.json b/backend/.sqlx/query-fc1663ecda7167dc12c0fd5a206cbddf85e55f6c7ef226cdd66e025c002bac98.json new file mode 100644 index 0000000000..626c541556 --- /dev/null +++ b/backend/.sqlx/query-fc1663ecda7167dc12c0fd5a206cbddf85e55f6c7ef226cdd66e025c002bac98.json @@ -0,0 +1,20 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT COUNT(*) FROM workspace WHERE id != 'admins' AND deleted = true", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "count", + "type_info": "Int8" + } + ], + "parameters": { + "Left": [] + }, + "nullable": [ + null + ] + }, + "hash": "fc1663ecda7167dc12c0fd5a206cbddf85e55f6c7ef226cdd66e025c002bac98" +} diff --git a/backend/migrations/20260701123436_realign_superadmin_favorites_to_derived_username.down.sql b/backend/migrations/20260701123436_realign_superadmin_favorites_to_derived_username.down.sql new file mode 100644 index 0000000000..2e3e434bcc --- /dev/null +++ b/backend/migrations/20260701123436_realign_superadmin_favorites_to_derived_username.down.sql @@ -0,0 +1,3 @@ +-- Irreversible data backfill: once realigned to the derived username, a favorite +-- is indistinguishable from one legitimately created under that username, so there +-- is nothing safe to revert. No-op. diff --git a/backend/migrations/20260701123436_realign_superadmin_favorites_to_derived_username.up.sql b/backend/migrations/20260701123436_realign_superadmin_favorites_to_derived_username.up.sql new file mode 100644 index 0000000000..b06f63d7af --- /dev/null +++ b/backend/migrations/20260701123436_realign_superadmin_favorites_to_derived_username.up.sql @@ -0,0 +1,19 @@ +-- A superadmin acting in a workspace they are not a member of used to be +-- identified by their raw email (so favorites were stored with usr = email). +-- They are now identified by their instance-derived username (password.username), +-- so realign those pre-existing favorites to keep them visible. Only email-keyed +-- rows are ever a superadmin's (members always store a non-email username), and +-- the anti-join skips rows that would collide with an already-derived favorite. +UPDATE favorite f +SET usr = p.username +FROM password p +WHERE f.usr = p.email + AND p.super_admin = true + AND p.username IS NOT NULL + AND NOT EXISTS ( + SELECT 1 FROM favorite f2 + WHERE f2.workspace_id = f.workspace_id + AND f2.usr = p.username + AND f2.favorite_kind = f.favorite_kind + AND f2.path = f.path + ); diff --git a/backend/tests/preserve_on_behalf_of.rs b/backend/tests/preserve_on_behalf_of.rs index 7ed5fc7697..d808348937 100644 --- a/backend/tests/preserve_on_behalf_of.rs +++ b/backend/tests/preserve_on_behalf_of.rs @@ -2704,7 +2704,9 @@ async fn test_schedule_permissions_workspace_admin(db: Pool) -> anyhow Ok(()) } -/// Superadmin NOT in workspace creates a schedule — uses email as permissioned_as +/// Superadmin NOT in workspace creates a schedule — uses their instance-derived +/// username (`password.username`) as permissioned_as, not the raw email. The +/// email is still stored directly on the schedule for downstream resolution. #[sqlx::test(fixtures("preserve_on_behalf_of"))] async fn test_schedule_permissions_superadmin_not_in_workspace( db: Pool, @@ -2758,16 +2760,16 @@ async fn test_schedule_permissions_superadmin_not_in_workspace( .fetch_one(&db) .await?; - // Superadmin not in workspace: username_to_permissioned_as uses the email directly - // since the authed username for a superadmin not in workspace IS the email + // Superadmin not in workspace: the authed username is now their instance-derived + // username (`password.username` = 'superadmin-external'), so permissioned_as is + // `u/` rather than the raw email. The email is still stored directly. assert_eq!( schedule.email, "superadmin-external@windmill.dev", "schedule email should be superadmin email" ); assert_eq!( - schedule.permissioned_as, - schedule.email.clone(), - "permissioned_as should match email for superadmin not in workspace" + schedule.permissioned_as, "u/superadmin-external", + "permissioned_as should use the instance-derived username, not the email" ); // Update by the same superadmin diff --git a/backend/windmill-api-auth/src/auth.rs b/backend/windmill-api-auth/src/auth.rs index a6a0fc4a8e..dea200e417 100644 --- a/backend/windmill-api-auth/src/auth.rs +++ b/backend/windmill-api-auth/src/auth.rs @@ -30,6 +30,7 @@ use windmill_common::{ }, error::{Error, JsonResult}, jwt, + usernames::get_instance_username_or_fallback_to_email, users::{COOKIE_NAME, SUPERADMIN_SECRET_EMAIL}, }; @@ -419,18 +420,34 @@ impl AuthCache { read_only, }) } - None if super_admin => Some(ApiAuthed { - email: email.clone(), - username: email, - is_admin: super_admin, - is_operator: false, - groups: vec![], - folders: vec![], - scopes, - username_override, - token_prefix: Some(safe_token_prefix(token)), - read_only, - }), + None if super_admin => { + // Fail closed on a DB error rather than + // letting the email leak in as the username. + match get_instance_username_or_fallback_to_email( + &self.db, &email, + ) + .await + { + Ok(username) => Some(ApiAuthed { + email, + username, + is_admin: super_admin, + is_operator: false, + groups: vec![], + folders: vec![], + scopes, + username_override, + token_prefix: Some(safe_token_prefix(token)), + read_only, + }), + Err(e) => { + tracing::error!( + "Failed to resolve instance username for superadmin {email}: {e:#}" + ); + None + } + } + } None => None, } } else { diff --git a/backend/windmill-api-flows/src/flows.rs b/backend/windmill-api-flows/src/flows.rs index b5f8e2f285..a9376d02ed 100644 --- a/backend/windmill-api-flows/src/flows.rs +++ b/backend/windmill-api-flows/src/flows.rs @@ -155,10 +155,11 @@ async fn list_flows( "o.labels", "draft.email IS NOT NULL as is_draft", // Per-path draft owners as a JSON array; see scripts.rs for the rationale - // (admins-workspace identity fallback, legacy NULL-email row). - "(SELECT json_agg(json_build_object('username', COALESCE(u.username, CASE WHEN d.workspace_id = 'admins' THEN d.email END)) ORDER BY COALESCE(u.username, CASE WHEN d.workspace_id = 'admins' THEN d.email END) NULLS LAST) \ + // (non-member superadmin identity fallback via `password`, legacy NULL-email row). + "(SELECT json_agg(json_build_object('username', COALESCE(u.username, p.username, CASE WHEN p.email IS NOT NULL THEN d.email END)) ORDER BY COALESCE(u.username, p.username, CASE WHEN p.email IS NOT NULL THEN d.email END) NULLS LAST) \ FROM draft d \ LEFT JOIN usr u ON u.workspace_id = d.workspace_id AND u.email = d.email \ + LEFT JOIN password p ON p.email = d.email AND p.super_admin = true \ WHERE d.workspace_id = o.workspace_id AND d.path = o.path AND d.typ = 'flow') as draft_users", "folder_labels(o.workspace_id, o.path) as inherited_labels" ]) diff --git a/backend/windmill-api-integration-tests/tests/workspace_comparison.rs b/backend/windmill-api-integration-tests/tests/workspace_comparison.rs index 88ffe6bb26..f0e77d2038 100644 --- a/backend/windmill-api-integration-tests/tests/workspace_comparison.rs +++ b/backend/windmill-api-integration-tests/tests/workspace_comparison.rs @@ -1399,7 +1399,9 @@ async fn test_delete_fork_purges_workspace_diff(db: Pool) -> anyhow::R // Delete the fork through the real handler. let delete_response = client .client() - .delete(&format!("{base_url}/workspaces/delete/wm-fork-test-workspace")) + .delete(&format!( + "{base_url}/workspaces/delete/wm-fork-test-workspace" + )) .send() .await?; assert!( @@ -1521,3 +1523,122 @@ async fn test_create_fork_purges_stale_diff_state(db: Pool) -> anyhow: Ok(()) } + +/// Regression: a stale/phantom trigger diff row must never block a privileged +/// user's deploy. Triggers (unlike scripts/flows) are not re-validated by +/// `compare_workspaces`, so a cached `has_changes=true` row for a trigger that +/// no longer exists in the table is trusted, then dropped by the visibility +/// filter (the row is gone) — flipping `all_ahead_items_visible` to false and +/// hiding the deploy button. This used to happen even for a superadmin, because +/// the item's absence is indistinguishable from a permission-hidden item. +/// +/// The blast-radius guard forces the flag true for anyone who sees the relevant +/// side in full: a target/fork admin (or superadmin) for ahead items. A regular +/// user with no such visibility still gets the (conservative) warning, since we +/// cannot tell a phantom from a genuine permission gap on their behalf. +/// +/// The diff row and visibility query are OSS, so this runs on any build (no +/// tally needed — the row is inserted directly, mimicking a delete that left the +/// diff behind). +#[sqlx::test(migrations = "../migrations", fixtures("base"))] +async fn test_compare_workspaces_phantom_trigger_shortfuse( + db: Pool, +) -> anyhow::Result<()> { + initialize_tracing().await; + + let server = ApiServer::start(db.clone()).await?; + let port = server.addr.port(); + let base_url = format!("http://localhost:{port}/api"); + let superadmin = windmill_api_client::create_client( + &format!("http://localhost:{port}"), + "SECRET_TOKEN".to_string(), + ); + let non_admin = windmill_api_client::create_client( + &format!("http://localhost:{port}"), + "SECRET_TOKEN_2".to_string(), + ); + + // Fork of test-workspace (INSERT directly; we only need the pair to exist). + sqlx::query!( + "INSERT INTO workspace (id, name, owner, parent_workspace_id) + VALUES ('wm-fork-test-workspace', 'Fork', 'test-user', 'test-workspace')" + ) + .execute(&db) + .await?; + sqlx::query!("INSERT INTO workspace_settings (workspace_id) VALUES ('wm-fork-test-workspace')") + .execute(&db) + .await?; + sqlx::query!( + "INSERT INTO workspace_key(workspace_id, kind, key) + VALUES ('wm-fork-test-workspace', 'cloud', 'test-key')" + ) + .execute(&db) + .await?; + + // Phantom rows: cached diffs for http_triggers with no backing row (the exact + // state a trigger delete used to leave behind before it reset the tally). One + // ahead (fork side), one behind (source side) so both guard branches are hit. + sqlx::query!( + "INSERT INTO workspace_diff + (source_workspace_id, fork_workspace_id, path, kind, ahead, behind, has_changes, exists_in_source, exists_in_fork) + VALUES + ('test-workspace', 'wm-fork-test-workspace', 'f/rt/ghost', 'http_trigger', 1, 0, true, false, true), + ('test-workspace', 'wm-fork-test-workspace', 'f/rt/ghost_behind', 'http_trigger', 0, 1, true, true, false)" + ) + .execute(&db) + .await?; + + // Superadmin: the guard forces `all_ahead_items_visible = true`, and the + // non-existent trigger is not surfaced as a diff. + let comparison: serde_json::Value = superadmin + .client() + .get(&format!( + "{base_url}/w/test-workspace/workspaces/compare/wm-fork-test-workspace" + )) + .send() + .await? + .json() + .await?; + assert_eq!( + comparison["all_ahead_items_visible"].as_bool(), + Some(true), + "phantom trigger row must not trip the 'not visible' warning for a superadmin: {comparison}" + ); + assert_eq!( + comparison["all_behind_items_visible"].as_bool(), + Some(true), + "phantom behind trigger row must not trip the warning for a superadmin: {comparison}" + ); + assert!( + !comparison["diffs"] + .as_array() + .unwrap() + .iter() + .any(|d| d["path"] == "f/rt/ghost" || d["path"] == "f/rt/ghost_behind"), + "non-existent triggers must not be surfaced as diffs: {comparison}" + ); + + // Non-superadmin, non-fork-admin member of the source: no full-visibility + // guarantee, so the warning still (conservatively) fires. + let comparison: serde_json::Value = non_admin + .client() + .get(&format!( + "{base_url}/w/test-workspace/workspaces/compare/wm-fork-test-workspace" + )) + .send() + .await? + .json() + .await?; + assert_eq!( + comparison["all_ahead_items_visible"].as_bool(), + Some(false), + "a user without full visibility must not be short-circuited by the guard: {comparison}" + ); + assert_eq!( + comparison["all_behind_items_visible"].as_bool(), + Some(false), + "the behind-side guard must not fire for a non-admin either: {comparison}" + ); + + Ok(()) +} diff --git a/backend/windmill-api-scripts/src/scripts.rs b/backend/windmill-api-scripts/src/scripts.rs index a61977937d..5d5ea010bb 100644 --- a/backend/windmill-api-scripts/src/scripts.rs +++ b/backend/windmill-api-scripts/src/scripts.rs @@ -211,12 +211,15 @@ async fn list_scripts( // Canonical reference for the draft-feature comments; flows/apps point here. // Per-path draft owners as a JSON array (`Json>`); NULL -> None, // never an empty array. LEFT JOIN `usr` keeps orphaned drafts (user left workspace) - // visible with `username = None`. In the `admins` workspace username IS the email, - // so fall back to `d.email` there or the authed user's own draft resolves to a phantom - // "Legacy workspace draft"; the genuine NULL-email legacy row stays None. - "(SELECT json_agg(json_build_object('username', COALESCE(u.username, CASE WHEN d.workspace_id = 'admins' THEN d.email END)) ORDER BY COALESCE(u.username, CASE WHEN d.workspace_id = 'admins' THEN d.email END) NULLS LAST) \ + // visible with `username = None`. A superadmin authoring in a workspace they are not + // a member of has no `usr` row, so fall back to their instance-derived username + // (`password.username`), or their email when derivation is disabled — this keeps the + // raw email out of the payload whenever a derived username exists. The genuine + // NULL-email legacy row stays None (no `usr`/`password` match, `d.email` is NULL). + "(SELECT json_agg(json_build_object('username', COALESCE(u.username, p.username, CASE WHEN p.email IS NOT NULL THEN d.email END)) ORDER BY COALESCE(u.username, p.username, CASE WHEN p.email IS NOT NULL THEN d.email END) NULLS LAST) \ FROM draft d \ LEFT JOIN usr u ON u.workspace_id = d.workspace_id AND u.email = d.email \ + LEFT JOIN password p ON p.email = d.email AND p.super_admin = true \ WHERE d.workspace_id = o.workspace_id AND d.path = o.path AND d.typ = 'script') as draft_users", "folder_labels(o.workspace_id, o.path) as inherited_labels" ]) diff --git a/backend/windmill-api-settings/src/lib.rs b/backend/windmill-api-settings/src/lib.rs index 8339df7b76..0a58f1f4b3 100644 --- a/backend/windmill-api-settings/src/lib.rs +++ b/backend/windmill-api-settings/src/lib.rs @@ -869,6 +869,37 @@ async fn run_setting_pre_write_hook( err )) })?; + } else { + // Disabling is only allowed before any instance-wide username has been + // assigned. Once usernames exist they are globally unique and are baked + // into stored `u/` identities (schedules, triggers, drafts, + // and non-member superadmin ownership). Disabling would drop back to + // workspace-local username uniqueness, letting a member reuse an + // existing instance username and silently take over those identities — + // so the setting is effectively one-way once derivation has taken + // effect. Re-saving `false` on an already-disabled instance is a no-op + // and stays allowed (guarded by the current-value check). + let currently_enabled = sqlx::query_scalar!( + "SELECT value FROM global_settings WHERE name = $1", + AUTOMATE_USERNAME_CREATION_SETTING + ) + .fetch_optional(db) + .await? + .and_then(|v| v.as_bool()) + .unwrap_or(true); + if currently_enabled { + let usernames_exist = sqlx::query_scalar!( + "SELECT EXISTS(SELECT 1 FROM password WHERE username IS NOT NULL)" + ) + .fetch_one(db) + .await? + .unwrap_or(false); + if usernames_exist { + return Err(error::Error::BadRequest( + "automate_username_creation cannot be disabled once instance-wide usernames have been assigned: existing u/ identities (schedules, triggers, drafts, superadmin ownership) rely on those usernames staying stable and globally unique.".to_string(), + )); + } + } } } CRITICAL_ALERT_MUTE_UI_SETTING => { diff --git a/backend/windmill-api-users/src/users.rs b/backend/windmill-api-users/src/users.rs index a0dca7576a..3fcebdc083 100644 --- a/backend/windmill-api-users/src/users.rs +++ b/backend/windmill-api-users/src/users.rs @@ -240,6 +240,11 @@ pub struct UserInfo { pub folders_owners: Vec, pub name: Option, pub is_service_account: bool, + // True when this row is a superadmin viewing a workspace they are not a + // member of (so `is_admin`/`role` reflect the superadmin fallback, not an + // actual membership). Always false for real member rows. + #[serde(default)] + pub non_member: bool, } #[derive(FromRow, Serialize)] @@ -699,13 +704,17 @@ async fn whoami( ) -> JsonResult { let ApiAuthed { username, email, is_admin, groups, folders, .. } = authed; let user = get_user(&w_id, &username, &db).await?; - if let Some(user) = user { + // Only treat the row as "this user is a member" when its email matches; the + // derived username is instance-unique so a match on a different email should + // never happen, but guard against it so a non-member superadmin is never + // shown another member's identity/role. + if let Some(user) = user.filter(|u| u.email == email) { Ok(Json(user)) } else { Ok(Json(UserInfo { workspace_id: w_id, - email: email.clone(), - username: email, + email, + username, name: None, is_admin, is_super_admin: is_admin, @@ -725,6 +734,7 @@ async fn whoami( .filter_map(|x| if x.2 { Some(x.0) } else { None }) .collect(), is_service_account: false, + non_member: true, })) } } @@ -889,6 +899,7 @@ async fn get_user(w_id: &str, username: &str, db: &DB) -> Result, Extension(db): Extension, ) -> Result { + // Members only: this workspace-scoped endpoint has no superadmin/target gate, + // so it must NOT use the `password` superadmin fallback — otherwise any + // workspace-authenticated caller could turn a guessed derived username into a + // non-member superadmin's email. Internal callers that legitimately need the + // fallback (schedule/trigger/draft resolution) use `resolve_username_to_email` + // directly and never return the email to an arbitrary caller. let email = sqlx::query_scalar!( "SELECT email FROM usr WHERE username = $1 AND workspace_id = $2", &username, diff --git a/backend/windmill-api-workspaces/src/workspaces.rs b/backend/windmill-api-workspaces/src/workspaces.rs index b4acb17254..eee4dada50 100644 --- a/backend/windmill-api-workspaces/src/workspaces.rs +++ b/backend/windmill-api-workspaces/src/workspaces.rs @@ -3901,6 +3901,21 @@ async fn _check_nb_of_workspaces(db: &DB) -> Result<()> { return Ok(()); } +async fn _check_nb_of_archived_workspaces(db: &DB) -> Result<()> { + let nb_archived = sqlx::query_scalar!( + "SELECT COUNT(*) FROM workspace WHERE id != 'admins' AND deleted = true", + ) + .fetch_one(db) + .await?; + if nb_archived.unwrap_or(0) >= 1 { + return Err(Error::BadRequest( + "You have reached the maximum number of archived workspaces (1) without an enterprise license. Permanently delete or unarchive the existing archived workspace first" + .to_string(), + )); + } + return Ok(()); +} + async fn create_workspace( authed: ApiAuthed, Extension(db): Extension, @@ -5775,6 +5790,11 @@ async fn archive_workspace( ) -> Result { require_admin(authed.is_admin, &authed.username)?; + // CE caps the number of archived (soft-deleted) workspaces so archiving can't be used to + // stockpile hidden workspaces. Enforced here so a second archive is refused up front. + #[cfg(not(feature = "enterprise"))] + _check_nb_of_archived_workspaces(&db).await?; + // If this is an attached dev workspace, archiving it leaves the prod with no active dev (the // unique index and user_workspaces both ignore deleted=true), so clear the prod's // dev_workspace_lock too. Gate it on prod-admin since it removes prod's protection rule (mirrors @@ -5887,6 +5907,13 @@ async fn unarchive_workspace( authed: ApiAuthed, ) -> Result { require_admin(authed.is_admin, &authed.username)?; + + // Unarchiving re-activates a soft-deleted workspace, so it must respect the + // same CE workspace-count cap as creating one. The archived workspace is + // deleted = true and thus excluded from the count until it is restored. + #[cfg(not(feature = "enterprise"))] + _check_nb_of_workspaces(&db).await?; + let mut tx = db.begin().await?; sqlx::query!("UPDATE workspace SET deleted = false WHERE id = $1", &w_id) .execute(&mut *tx) @@ -7360,6 +7387,24 @@ async fn compare_workspaces( .map(|s| s.behind) .fold(0, |acc, s| acc + s.try_into().unwrap_or(0)); + // Blast-radius guard for the "changes not visible to your user" warning + // (which hides the deploy button entirely). The flag is a pure visibility + // guarantee — the actual deploy/update is separately authorized against the + // target workspace's create/update endpoints — so it must be forced true for + // anyone who, as a rule, sees every item on the relevant side (RLS is bypassed + // for admins); any diff the visibility filter dropped for them is provably a + // stale/phantom row, never a permission gap. Crucially the two sides live in + // different workspaces: ahead items are the fork's own changes (gated by the + // TARGET/fork admin), behind items physically live in the parent (gated by the + // SOURCE/parent admin). A target admin does NOT see the parent side, so it must + // not clear the behind flag, and vice versa. `target_admin` already folds in + // superadmin; `source_admin` (parent side) ORs it in explicitly. + let is_super_admin = windmill_common::auth::is_super_admin_email(&db, &authed.email).await?; + let target_admin = fork_authed.is_admin; + let source_admin = is_super_admin || authed.is_admin; + let all_ahead_items_visible = all_ahead_items_visible || target_admin; + let all_behind_items_visible = all_behind_items_visible || source_admin; + return Ok(Json(WorkspaceComparison { all_ahead_items_visible, all_behind_items_visible, diff --git a/backend/windmill-api/openapi.yaml b/backend/windmill-api/openapi.yaml index 9be2175f53..53df857d8a 100644 --- a/backend/windmill-api/openapi.yaml +++ b/backend/windmill-api/openapi.yaml @@ -23974,6 +23974,9 @@ components: - $ref: "#/components/schemas/UserSource" is_service_account: type: boolean + non_member: + type: boolean + description: "True when this is a superadmin viewing a workspace they are not a member of (is_admin/role reflect the superadmin fallback, not an actual membership)." required: - email - username diff --git a/backend/windmill-api/src/apps.rs b/backend/windmill-api/src/apps.rs index 08a41e2d1f..38fa5ff862 100644 --- a/backend/windmill-api/src/apps.rs +++ b/backend/windmill-api/src/apps.rs @@ -415,13 +415,14 @@ async fn list_apps( "app.labels", "draft.path IS NOT NULL as is_draft", // Per-path draft owners as a JSON array; see scripts.rs for the rationale - // (admins-workspace identity fallback, legacy NULL-email row). + // (non-member superadmin identity fallback via `password`, legacy NULL-email row). // `app`/`raw_app` are separate draft kinds over one `app` table — match // either (like the `is_draft` join below), else a deployed raw app's draft // owners are dropped and the row shows "Draft" with no user badge. - "(SELECT json_agg(json_build_object('username', COALESCE(u.username, CASE WHEN d.workspace_id = 'admins' THEN d.email END)) ORDER BY COALESCE(u.username, CASE WHEN d.workspace_id = 'admins' THEN d.email END) NULLS LAST) \ + "(SELECT json_agg(json_build_object('username', COALESCE(u.username, p.username, CASE WHEN p.email IS NOT NULL THEN d.email END)) ORDER BY COALESCE(u.username, p.username, CASE WHEN p.email IS NOT NULL THEN d.email END) NULLS LAST) \ FROM draft d \ LEFT JOIN usr u ON u.workspace_id = d.workspace_id AND u.email = d.email \ + LEFT JOIN password p ON p.email = d.email AND p.super_admin = true \ WHERE d.workspace_id = app.workspace_id AND d.path = app.path AND d.typ IN ('app', 'raw_app')) as draft_users", "folder_labels(app.workspace_id, app.path) as inherited_labels", ]) diff --git a/backend/windmill-api/src/drafts.rs b/backend/windmill-api/src/drafts.rs index 9d4976489b..72791de78f 100644 --- a/backend/windmill-api/src/drafts.rs +++ b/backend/windmill-api/src/drafts.rs @@ -18,6 +18,7 @@ use windmill_common::{ db::UserDB, error::{Error, Result}, user_drafts::{DraftUserRef, UserDraftItemKind, ENCRYPTED_DRAFT_PREFIX}, + users::resolve_username_to_email, variables::{build_crypt, encrypt}, }; @@ -171,11 +172,16 @@ fn list_drafts_query(all_users: bool) -> String { // draft author at this (path, kind), legacy NULL-email row surfaced as a // null username. Restricted to the shared full-page-editor kinds — drawer // kinds keep their drafts private, so we never reveal their authors. + // A superadmin authoring in a workspace they are not a member of has no `usr` + // row: fall back to their instance-derived username (`password.username`), or + // their email when derivation is disabled (`password.username` is NULL). This + // keeps the raw email out of the payload whenever a derived username exists. let draft_users = r#"CASE WHEN d.typ::text IN ('script', 'flow', 'app', 'raw_app') THEN ( - SELECT json_agg(json_build_object('username', COALESCE(u.username, CASE WHEN du.workspace_id = 'admins' THEN du.email END)) - ORDER BY COALESCE(u.username, CASE WHEN du.workspace_id = 'admins' THEN du.email END) NULLS LAST) + SELECT json_agg(json_build_object('username', COALESCE(u.username, p.username, CASE WHEN p.email IS NOT NULL THEN du.email END)) + ORDER BY COALESCE(u.username, p.username, CASE WHEN p.email IS NOT NULL THEN du.email END) NULLS LAST) FROM draft du LEFT JOIN usr u ON u.workspace_id = du.workspace_id AND u.email = du.email + LEFT JOIN password p ON p.email = du.email AND p.super_admin = true WHERE du.workspace_id = d.workspace_id AND du.path = d.path AND du.typ = d.typ ) ELSE NULL END"#; // Default lists the user's own drafts AND the legacy NULL-email rows; with @@ -578,20 +584,12 @@ async fn get_draft_for_user( // Username -> email, scoped to the workspace. None signals "fetch the // legacy NULL-email row" (distinct from a username with no draft, which - // 404s below). + // 404s below). Resolution falls back to the instance `password` table so a + // superadmin's draft (they are not a `usr` member of the workspace, and + // their username is their instance-derived one) still resolves. let owner_email: Option = if let Some(username) = &query.username { - let email = sqlx::query_scalar!( - r#"SELECT email FROM usr WHERE workspace_id = $1 AND username = $2"#, - &w_id, - username, - ) - .fetch_optional(&db) - .await?; - match email { + match resolve_username_to_email(&w_id, username, &db).await? { Some(e) => Some(e), - // The `admins` workspace has no `usr` rows (username IS the email - // there), so accept it as the owner email directly. - None if w_id == "admins" => Some(username.clone()), None => { return Err(Error::NotFound(format!( "no user with username {username} in workspace" @@ -805,7 +803,6 @@ async fn require_can_read_path( Err(Error::NotFound(format!("no draft visible at {path}"))) } - #[cfg(test)] mod tests { use super::strip_json_nul; @@ -841,7 +838,9 @@ mod tests { // "a" carries a real NUL; "b" carries the literal text backslash-u0000. // The value walk strips the former and leaves the latter intact — the // pathological case that needed a fallback in SQL is trivial in Rust. - let v = parsed(strip_json_nul(r#"{"a":"x\u0000y","b":"p\\u0000q"}"#.to_string())); + let v = parsed(strip_json_nul( + r#"{"a":"x\u0000y","b":"p\\u0000q"}"#.to_string(), + )); assert_eq!(v["a"], "xy"); assert_eq!(v["b"], "p\\u0000q"); } diff --git a/backend/windmill-common/src/auth.rs b/backend/windmill-common/src/auth.rs index 5c1cedc6d8..bd6bf1ef9d 100644 --- a/backend/windmill-common/src/auth.rs +++ b/backend/windmill-common/src/auth.rs @@ -413,9 +413,18 @@ async fn fetch_authed_from_permissioned_as_inner( }) } } else { + // Bare (no `u/`|`g/` prefix) permissioned_as is reached for superadmins + // whose identifier is their email (they are not a workspace member). Use + // the instance-derived username when available so no email leaks + // downstream as the acting username. + let username = if is_super_admin && permissioned_as == email { + crate::usernames::get_instance_username_or_fallback_to_email(&mut *conn, email).await? + } else { + permissioned_as.to_string() + }; Ok(Authed { email: email.to_string(), - username: permissioned_as.to_string(), + username, is_admin: is_super_admin, is_operator: true, groups: vec![], diff --git a/backend/windmill-common/src/user_drafts.rs b/backend/windmill-common/src/user_drafts.rs index 620cbe2a5f..7e0c1f3f55 100644 --- a/backend/windmill-common/src/user_drafts.rs +++ b/backend/windmill-common/src/user_drafts.rs @@ -237,18 +237,22 @@ async fn fetch_other_drafts_users( kind: UserDraftItemKind, path: &str, ) -> Result> { - // The `admins` workspace has no `usr` rows (username IS the email there), - // so fall back to `d.email` when the join misses, else a real teammate's - // draft renders as a phantom "Legacy draft". The genuine NULL-email legacy - // row keeps `username = None` (its `d.email` is NULL, so the CASE yields NULL). + // A superadmin authoring in a workspace they are not a member of has no `usr` + // row: fall back to their instance-derived username (`password.username`), or + // their email when derivation is disabled. Else a real teammate's draft renders + // as a phantom "Legacy draft". The genuine NULL-email legacy row keeps + // `username = None` (no `usr`/`password` match and `d.email` is NULL). let rows = sqlx::query_as!( OtherDraftUser, - r#"SELECT COALESCE(u.username, CASE WHEN d.workspace_id = 'admins' THEN d.email END) as "username?", + r#"SELECT COALESCE(u.username, p.username, CASE WHEN p.email IS NOT NULL THEN d.email END) as "username?", d.created_at as "draft_saved_at!" FROM draft d LEFT JOIN usr u ON u.workspace_id = d.workspace_id AND u.email = d.email + LEFT JOIN password p + ON p.email = d.email + AND p.super_admin = true WHERE d.workspace_id = $1 AND d.path = $2 AND d.typ = $3 diff --git a/backend/windmill-common/src/usernames.rs b/backend/windmill-common/src/usernames.rs index 2501841cac..6fde2a8186 100644 --- a/backend/windmill-common/src/usernames.rs +++ b/backend/windmill-common/src/usernames.rs @@ -94,6 +94,38 @@ pub async fn generate_instance_username_for_all_users(db: &DB) -> error::Result< Ok(()) } +/// Resolve the username to use for a user (typically a superadmin) accessing a +/// workspace they are not a member of. When instance-wide username derivation is +/// enabled (`automate_username_creation`, the default on almost all instances), +/// `password.username` is populated, so we return that derived username instead +/// of leaking the raw email address downstream (as job/script author, audit +/// actor, etc.). Falls back to the email when no derived username exists (the few +/// instances that keep `automate_username_creation` disabled). +/// +/// The hot callers (token auth, `fetch_api_authed`) are already behind their own +/// 120s auth caches, so this deliberately does not add another email->username +/// cache (which would only help the cold JWT-mint / job-perms paths while risking +/// cross-DB contamination in the shared-process integration tests). +/// +/// A DB error is propagated rather than swallowed into the email fallback: falling +/// back to the email on a transient failure would reintroduce the very email leak +/// this path exists to prevent, so callers fail closed instead. +pub async fn get_instance_username_or_fallback_to_email<'e, E>( + db: E, + email: &str, +) -> error::Result +where + E: sqlx::PgExecutor<'e>, +{ + let derived = sqlx::query_scalar!("SELECT username FROM password WHERE email = $1", email) + .fetch_optional(db) + .await? + .flatten(); + // No derived username (automate_username_creation disabled) → fall back to the + // email. This is the only legitimate fallback; a query error propagates above. + Ok(derived.unwrap_or_else(|| email.to_string())) +} + pub async fn get_instance_username_or_create_pending<'c>( tx: &mut Transaction<'c, Postgres>, email: &str, diff --git a/backend/windmill-common/src/users.rs b/backend/windmill-common/src/users.rs index d40b5cd19c..738dc84d14 100644 --- a/backend/windmill-common/src/users.rs +++ b/backend/windmill-common/src/users.rs @@ -46,8 +46,34 @@ lazy_static::lazy_static! { const EMAIL_CACHE_TTL_SECS: u64 = 60; +/// Resolve a workspace-scoped username to its email. +/// +/// Members are found in `usr`. A superadmin acting in a workspace they are *not* +/// a member of has no `usr` row; they carry either their instance-derived +/// username (`password.username`, when `automate_username_creation` is enabled) +/// or their email (when it is disabled), so fall back to `password` on both, +/// gated on `super_admin` since only superadmins can act without membership. +/// Returns `None` when the username resolves to nobody. +pub async fn resolve_username_to_email<'c>( + workspace_id: &str, + username: &str, + db: impl sqlx::PgExecutor<'c>, +) -> crate::error::Result> { + Ok(sqlx::query_scalar!( + "SELECT COALESCE( + (SELECT email FROM usr WHERE workspace_id = $1 AND username = $2), + (SELECT email FROM password WHERE (username = $2 OR email = $2) AND super_admin = true) + )", + workspace_id, + username + ) + .fetch_optional(db) + .await? + .flatten()) +} + /// Get email from permissioned_as string. -/// - "u/{username}" → lookup email from usr table (cached) +/// - "u/{username}" → resolve via [`resolve_username_to_email`] (cached) /// - "g/{group}" → "group-{group}@windmill.dev" /// - raw email → return as-is pub async fn get_email_from_permissioned_as<'c>( @@ -62,14 +88,9 @@ pub async fn get_email_from_permissioned_as<'c>( return Ok(email); } } - let email = sqlx::query_scalar!( - "SELECT email FROM usr WHERE username = $1 AND workspace_id = $2", - username, - workspace_id - ) - .fetch_optional(db) - .await? - .unwrap_or_else(|| format!("{}@unknown.windmill.dev", username)); + let email = resolve_username_to_email(workspace_id, username, db) + .await? + .unwrap_or_else(|| format!("{}@unknown.windmill.dev", username)); let key = (workspace_id.to_string(), username.to_string()); EMAIL_CACHE.insert(key, (email.clone(), std::time::Instant::now())); Ok(email) diff --git a/backend/windmill-trigger/src/handler.rs b/backend/windmill-trigger/src/handler.rs index 65072bc9ab..ed3b89fc65 100644 --- a/backend/windmill-trigger/src/handler.rs +++ b/backend/windmill-trigger/src/handler.rs @@ -905,6 +905,26 @@ async fn delete_trigger( tx.commit().await?; + // Reset the fork/parent workspace_diff tally for this path, exactly as + // create/update and every other kind's delete does. Without this a deleted + // trigger leaves its cached `has_changes=true` diff row behind: the compare + // trusts it (triggers aren't re-validated like scripts/flows), then drops it + // as it no longer exists in the table — a phantom "ahead" item that reads as + // "changes not visible to your user" and hides the deploy button, even for + // superadmins. Re-tallying sets has_changes=NULL so the next compare + // re-evaluates and corrects/removes the row. + handle_deployment_metadata( + &authed.email, + &authed.username, + &db, + &workspace_id, + T::get_deployed_object(path.to_string(), None), + Some(format!("{} '{}' deleted", T::DEPLOYMENT_NAME, path)), + true, + None, + ) + .await?; + // Trigger gone for everyone: wipe ALL users' drafts at this path; see scripts.rs. delete_all_drafts_for_path(&db, &workspace_id, T::user_draft_item_kind(), path).await?; diff --git a/frontend/src/lib/components/GlobalUserOffboardingModal.svelte b/frontend/src/lib/components/GlobalUserOffboardingModal.svelte index 12af797140..bc2cc5b996 100644 --- a/frontend/src/lib/components/GlobalUserOffboardingModal.svelte +++ b/frontend/src/lib/components/GlobalUserOffboardingModal.svelte @@ -34,6 +34,7 @@ let wsConfigs: Record< string, { + reassign: boolean targetKind: 'user' | 'folder' selectedUser: string | undefined selectedFolder: string | undefined @@ -71,16 +72,21 @@ UserService.listUsernames({ workspace: wp.workspace_id }), FolderService.listFolders({ workspace: wp.workspace_id }) ]) + const users = usernamesList + .filter((u: string) => u !== wp.username) + .map((u: string) => ({ label: u, value: u })) return { workspace_id: wp.workspace_id, config: { + // Reassignment requires another workspace user to own items and back + // triggers/runnables; with no other user (e.g. single-member forks) it + // is impossible, so default off and leave items as-is. + reassign: users.length > 0, targetKind: 'user' as const, selectedUser: undefined as string | undefined, selectedFolder: undefined as string | undefined, selectedOperator: undefined as string | undefined, - users: usernamesList - .filter((u: string) => u !== wp.username) - .map((u: string) => ({ label: u, value: u })), + users, folders: foldersList.map((f: { name: string }) => ({ label: f.name, value: f.name @@ -112,15 +118,28 @@ : undefined } + // At least one workspace can be reassigned (has another assignable user). + let anyReassignableWorkspace = $derived( + workspacesWithItems.some((wp) => (wsConfigs[wp.workspace_id]?.users.length ?? 0) > 0) + ) + // At least one workspace is actually selected for reassignment. + let anyWorkspaceReassigned = $derived( + workspacesWithItems.some((wp) => wsConfigs[wp.workspace_id]?.reassign) + ) + let canSubmit = $derived( - !doReassign || + (!doReassign || workspacesWithItems.every((wp) => { - const target = getReassignTo(wp.workspace_id) const cfg = wsConfigs[wp.workspace_id] + if (!cfg?.reassign) return true + const target = getReassignTo(wp.workspace_id) if (!target) return false if (!cfg?.selectedOperator) return false return true - }) + })) && + // Reassign-only runs (no deletion) must reassign at least one workspace, + // otherwise the request is an empty no-op reported as success. + (deleteUser || anyWorkspaceReassigned) ) async function submit() { @@ -129,13 +148,16 @@ try { const reassignments: Record = {} - for (const wp of workspacesWithItems) { - const target = getReassignTo(wp.workspace_id) - const cfg = wsConfigs[wp.workspace_id] - if (target) { - reassignments[wp.workspace_id] = { - reassign_to: target, - new_on_behalf_of_user: cfg?.selectedOperator + if (doReassign) { + for (const wp of workspacesWithItems) { + const cfg = wsConfigs[wp.workspace_id] + if (!cfg?.reassign) continue + const target = getReassignTo(wp.workspace_id) + if (target) { + reassignments[wp.workspace_id] = { + reassign_to: target, + new_on_behalf_of_user: cfg?.selectedOperator + } } } } @@ -222,7 +244,7 @@ {/if} {#if doReassign} - {#each workspacesWithItems as wp} + {#each workspacesWithItems as wp (wp.workspace_id)} {@const cfg = wsConfigs[wp.workspace_id]}
@@ -230,23 +252,39 @@ {wp.workspace_id} ({wp.username}) + {#if cfg} + + {/if}
{#if cfg} - + {#if cfg.users.length === 0} +

+ No other users in this workspace. Items will be left as-is. +

+ {:else if cfg.reassign} + + {:else} +

Items will be left as-is.

+ {/if} {/if}
{/each} @@ -283,7 +321,7 @@ different user/folder.

    - {#each conflicts as conflict} + {#each conflicts as conflict, i (i)}
  • {conflict}
    • {/each}
@@ -293,7 +331,11 @@ {/if}
- {#if workspacesWithItems.length > 0 || deleteUser} + {#if !deleteUser && workspacesWithItems.length > 0 && !anyReassignableWorkspace} + {#if !loading} + + {/if} + {:else if workspacesWithItems.length > 0 || deleteUser}
diff --git a/frontend/src/lib/stores.ts b/frontend/src/lib/stores.ts index feadf3056d..9c1475f3a6 100644 --- a/frontend/src/lib/stores.ts +++ b/frontend/src/lib/stores.ts @@ -31,6 +31,8 @@ export interface UserExt { folders_owners: string[] is_service_account?: boolean impersonating_email?: string + // true when the user is a superadmin viewing a workspace they are not a member of + non_member?: boolean } export interface UserWorkspace {