From 19eaf3e2cf98a165e98b0580ce9fec4200a0dce3 Mon Sep 17 00:00:00 2001 From: hugocasa Date: Tue, 14 Jul 2026 16:40:10 +0200 Subject: [PATCH] fix: cap job token authority at workspace admin A job token ($WM_TOKEN) derives its identity from an app/flow/schedule on_behalf_of, which a wm_deployers member controls. Ensure such a token can never act above workspace admin, regardless of the identity it runs as. - ApiAuthed gains token_is_job (set from the JWT job_id claim). - require_super_admin / is_super_admin / require_devops_role now take &ApiAuthed and deny job tokens for superadmin/devops-gated endpoints (one choke point instead of a per-route denylist); require_super_admin_email kept for internal, non-request callers. - Inline is_super_admin_email(authed.email) capability gates migrated to the cap-aware is_super_admin(db, &authed). - Defense in depth: validate_on_behalf_of rejects reserved internal identities and mismatched-superadmin emails when persisting an app/flow/script/schedule/ trigger on_behalf_of; app execution refuses reserved-identity policies. Co-Authored-By: Claude Opus 4.8 (1M context) --- backend/ee-repo-ref.txt | 2 +- backend/tests/preserve_on_behalf_of.rs | 163 ++++++++++++++++++ backend/tests/trigger_listener_queries.rs | 1 + backend/tests/wm_token_superadmin_guard.rs | 61 +++++++ backend/windmill-api-auth/src/auth.rs | 10 ++ backend/windmill-api-auth/src/lib.rs | 79 +++++++-- backend/windmill-api-configs/src/lib.rs | 14 +- backend/windmill-api-flows/src/flows.rs | 30 ++++ backend/windmill-api-groups/src/groups.rs | 14 +- .../tests/native_triggers.rs | 1 + backend/windmill-api-schedule/src/lib.rs | 22 ++- backend/windmill-api-scripts/src/scripts.rs | 14 ++ backend/windmill-api-settings/src/lib.rs | 87 +++++----- backend/windmill-api-users/src/users.rs | 18 +- backend/windmill-api-workers/src/lib.rs | 14 +- .../src/datatable_migrations.rs | 2 +- .../windmill-api-workspaces/src/workspaces.rs | 42 ++--- .../src/workspaces_extra.rs | 15 +- backend/windmill-api/src/apps.rs | 42 +++++ backend/windmill-api/src/db_health.rs | 16 +- backend/windmill-api/src/jobs.rs | 10 +- backend/windmill-api/src/offboarding.rs | 4 +- backend/windmill-api/src/service_logs.rs | 8 +- backend/windmill-api/src/users.rs | 6 +- backend/windmill-common/src/auth.rs | 104 ++++++++++- backend/windmill-store/src/resources.rs | 4 +- backend/windmill-trigger/src/handler.rs | 23 +++ 27 files changed, 663 insertions(+), 143 deletions(-) diff --git a/backend/ee-repo-ref.txt b/backend/ee-repo-ref.txt index 589a683dba..206c7b8dfd 100644 --- a/backend/ee-repo-ref.txt +++ b/backend/ee-repo-ref.txt @@ -1 +1 @@ -2ba6a2a75b6fc97858b306b2c98ada481e363c10 +39a5c579808a810438be69529714d6a2fc7da9d6 \ No newline at end of file diff --git a/backend/tests/preserve_on_behalf_of.rs b/backend/tests/preserve_on_behalf_of.rs index d808348937..6b27b4300b 100644 --- a/backend/tests/preserve_on_behalf_of.rs +++ b/backend/tests/preserve_on_behalf_of.rs @@ -2810,3 +2810,166 @@ async fn test_schedule_permissions_superadmin_not_in_workspace( Ok(()) } + +// ============================================================================ +// Forged-superadmin on_behalf_of guard (GHSA-hfh4-cx4h-3fcr) +// ============================================================================ + +/// A `wm_deployers` member must not be able to deploy a runnable whose preserved +/// on_behalf_of resolves to a superadmin identity it does not genuinely name: +/// the reserved internal sentinels, or a superadmin's email pinned onto an +/// unrelated principal. Deploying on behalf of a *consistently named* real user — +/// even a real superadmin, e.g. a git-sync round-trip of superadmin-authored +/// content — stays allowed; that is the intended deployer capability. +#[sqlx::test(fixtures("preserve_on_behalf_of"))] +async fn test_reject_forged_superadmin_on_behalf_of(db: Pool) -> anyhow::Result<()> { + initialize_tracing().await; + + let server = ApiServer::start(db.clone()).await?; + let port = server.addr.port(); + let base = format!("http://localhost:{port}/api/w/test-workspace"); + + // Reserved internal sentinel (grants is_super_admin at execution by email). + const SENTINEL: &str = "superadmin_secret@windmill.dev"; + // Reserved sentinel matched on permissioned_as. + const SYNC_SENTINEL: &str = "superadmin_sync@windmill.dev"; + // Real instance superadmin, present only in `password` (not a workspace member). + const REAL_SA: &str = "superadmin-external@windmill.dev"; + + // App: deployer cannot pin the sentinel email. + let resp = authed( + client().post(format!("{base}/apps/create")), + "DEPLOYER_TOKEN", + ) + .json(&new_app_with_on_behalf_of( + "u/deployer-user/app_sentinel", + Some("u/original-user"), + Some(SENTINEL), + true, + )) + .send() + .await?; + assert_eq!( + resp.status(), + 400, + "deployer must not pin the sentinel as an app on_behalf_of_email: {}", + resp.text().await? + ); + + // App: deployer cannot pin a real superadmin's email onto an unrelated principal. + let resp = authed( + client().post(format!("{base}/apps/create")), + "DEPLOYER_TOKEN", + ) + .json(&new_app_with_on_behalf_of( + "u/deployer-user/app_mismatch_sa", + Some("u/original-user"), + Some(REAL_SA), + true, + )) + .send() + .await?; + assert_eq!( + resp.status(), + 400, + "deployer must not pin a superadmin email onto an unrelated principal: {}", + resp.text().await? + ); + + // App: a consistently named real superadmin identity is allowed (deployer feature). + let resp = authed( + client().post(format!("{base}/apps/create")), + "DEPLOYER_TOKEN", + ) + .json(&new_app_with_on_behalf_of( + "u/deployer-user/app_consistent_sa", + Some("u/superadmin-external"), + Some(REAL_SA), + true, + )) + .send() + .await?; + assert_eq!( + resp.status(), + 201, + "deployer may preserve a consistently named superadmin identity: {}", + resp.text().await? + ); + + // Flow: deployer cannot pin the sentinel email. + let resp = authed( + client().post(format!("{base}/flows/create")), + "DEPLOYER_TOKEN", + ) + .json(&new_flow_with_on_behalf_of( + "u/deployer-user/flow_sentinel", + Some(SENTINEL), + true, + )) + .send() + .await?; + assert_eq!( + resp.status(), + 400, + "deployer must not pin the sentinel as a flow on_behalf_of_email: {}", + resp.text().await? + ); + + // Script: deployer cannot pin the sentinel email. + let resp = authed( + client().post(format!("{base}/scripts/create")), + "DEPLOYER_TOKEN", + ) + .json(&new_script_with_on_behalf_of( + "u/deployer-user/script_sentinel", + Some(SENTINEL), + true, + )) + .send() + .await?; + assert_eq!( + resp.status(), + 400, + "deployer must not pin the sentinel as a script on_behalf_of_email: {}", + resp.text().await? + ); + + // Schedule: deployer cannot preserve the sync sentinel as permissioned_as. + let resp = authed( + client().post(format!("{base}/scripts/create")), + "DEPLOYER_TOKEN", + ) + .json(&new_script_with_on_behalf_of( + "u/deployer-user/sched_guard_script", + None, + false, + )) + .send() + .await?; + assert_eq!(resp.status(), 201, "{}", resp.text().await?); + + let resp = authed( + client().post(format!("{base}/schedules/create")), + "DEPLOYER_TOKEN", + ) + .json(&json!({ + "path": "u/deployer-user/schedule_sync_sentinel", + "schedule": "0 0 */6 * * *", + "timezone": "UTC", + "script_path": "u/deployer-user/sched_guard_script", + "is_flow": false, + "enabled": false, + "permissioned_as": SYNC_SENTINEL, + "preserve_permissioned_as": true + })) + .send() + .await?; + assert_eq!( + resp.status(), + 400, + "deployer must not preserve the sync sentinel as a schedule permissioned_as: {}", + resp.text().await? + ); + + Ok(()) +} diff --git a/backend/tests/trigger_listener_queries.rs b/backend/tests/trigger_listener_queries.rs index c3f6357d4c..11ff8fcc8f 100644 --- a/backend/tests/trigger_listener_queries.rs +++ b/backend/tests/trigger_listener_queries.rs @@ -175,6 +175,7 @@ fn make_authed() -> windmill_api_auth::ApiAuthed { username_override: None, token_prefix: None, read_only: false, + token_is_job: false, } } diff --git a/backend/tests/wm_token_superadmin_guard.rs b/backend/tests/wm_token_superadmin_guard.rs index 650cf384d4..a63c97313e 100644 --- a/backend/tests/wm_token_superadmin_guard.rs +++ b/backend/tests/wm_token_superadmin_guard.rs @@ -172,6 +172,67 @@ async fn test_wm_token_cannot_manage_superadmin_users(db: Pool) -> any resp.text().await? ); + // 4f. Cannot read the full user directory. This route is only guarded by + // `require_super_admin` (no per-route job-token denylist), so it proves + // the cap is baked into `require_super_admin` itself — the advisory PoC. + let resp = authed(client().get(format!("{base}/list_as_super_admin")), &sa_wm) + .send() + .await?; + assert_eq!( + resp.status(), + 401, + "superadmin WM_TOKEN must not list all users: {}", + resp.text().await? + ); + + // 4g. Cannot read instance settings (unredacted SMTP/OAuth secrets, license + // key, SSO config). Also guarded only by `require_super_admin`. + let settings_base = format!("http://localhost:{port}/api/settings"); + let resp = authed( + client().get(format!("{settings_base}/global/license_key")), + &sa_wm, + ) + .send() + .await?; + assert_eq!( + resp.status(), + 401, + "superadmin WM_TOKEN must not read instance settings: {}", + resp.text().await? + ); + + // 4h. No false positive: a real superadmin API token still reads the + // directory (the cap keys off the job token, not the identity). + let resp = authed( + client().get(format!("{base}/list_as_super_admin")), + "SECRET_TOKEN", + ) + .send() + .await?; + assert_eq!( + resp.status(), + 200, + "a real superadmin token must still list users: {}", + resp.text().await? + ); + + // 4i. The instance-level `devops` role is also capped: a superadmin (hence + // devops) WM_TOKEN must not reach a require_devops_role route. + let resp = authed( + client().get(format!( + "http://localhost:{port}/api/service_logs/list_files" + )), + &sa_wm, + ) + .send() + .await?; + assert_eq!( + resp.status(), + 401, + "superadmin WM_TOKEN must not reach a devops route: {}", + resp.text().await? + ); + // 5. Escape hatch / no false positive: a real superadmin API token // (SECRET_TOKEN, no job_id) can still create tokens. let resp = authed( diff --git a/backend/windmill-api-auth/src/auth.rs b/backend/windmill-api-auth/src/auth.rs index 59d9cd80b3..ffba697ddd 100644 --- a/backend/windmill-api-auth/src/auth.rs +++ b/backend/windmill-api-auth/src/auth.rs @@ -207,6 +207,9 @@ impl AuthCache { username_override, token_prefix: claims.audit_span, read_only: false, + // A `job_id` claim marks the WM_TOKEN of a running + // job, whose authority is capped at workspace admin. + token_is_job: claims.job_id.is_some(), }; let job_id = claims.job_id.and_then(|j| uuid::Uuid::from_str(&j).ok()); AUTH_CACHE.insert( @@ -304,6 +307,7 @@ impl AuthCache { username_override, token_prefix: Some(safe_token_prefix(token)), read_only, + token_is_job: false, }) } else { tracing::warn!( @@ -354,6 +358,7 @@ impl AuthCache { username_override, token_prefix: Some(safe_token_prefix(token)), read_only, + token_is_job: false, }) } else { tracing::warn!( @@ -425,6 +430,7 @@ impl AuthCache { username_override, token_prefix: Some(safe_token_prefix(token)), read_only, + token_is_job: false, }) } None if super_admin => { @@ -446,6 +452,7 @@ impl AuthCache { username_override, token_prefix: Some(safe_token_prefix(token)), read_only, + token_is_job: false, }), Err(e) => { tracing::error!( @@ -469,6 +476,7 @@ impl AuthCache { username_override, token_prefix: Some(safe_token_prefix(token)), read_only, + token_is_job: false, }) } } @@ -504,6 +512,7 @@ impl AuthCache { username_override: None, token_prefix: Some(safe_token_prefix(token)), read_only: false, + token_is_job: false, }; Some(OptJobAuthed { authed, job_id: None }) } else { @@ -711,6 +720,7 @@ fn no_auth_admin_authed() -> ApiAuthed { username_override: None, token_prefix: None, read_only: false, + token_is_job: false, } } diff --git a/backend/windmill-api-auth/src/lib.rs b/backend/windmill-api-auth/src/lib.rs index 25f572d8ae..4e3cf123b4 100644 --- a/backend/windmill-api-auth/src/lib.rs +++ b/backend/windmill-api-auth/src/lib.rs @@ -56,6 +56,12 @@ pub struct ApiAuthed { pub username_override: Option, pub token_prefix: Option, pub read_only: bool, + /// True when this authed comes from a job token (`$WM_TOKEN`, a JWT carrying + /// a `job_id`). A job token's identity is derived from an app/flow/schedule + /// `on_behalf_of`, which a non-admin `wm_deployers` member can point at a + /// superadmin, so its authority is capped at workspace admin: + /// [`require_super_admin`] denies it even when the identity is a superadmin. + pub token_is_job: bool, } impl ApiAuthed { @@ -105,6 +111,8 @@ impl From for ApiAuthed { username_override: None, token_prefix: value.token_prefix, read_only: false, + // A plain `Authed` (on-behalf/internal) carries no token context. + token_is_job: false, } } } @@ -193,15 +201,46 @@ impl windmill_mcp::server::McpAuth for ApiAuthed { // ------------ Utility functions ------------ -pub async fn require_super_admin(db: &DB, email: &str) -> error::Result<()> { - let is_admin = is_super_admin_email(db, email).await?; +/// Require the caller to be a super admin. A super admin acting through a job +/// token (`$WM_TOKEN`) is capped at workspace admin and rejected here — see +/// [`ApiAuthed::token_is_job`] — so no `require_super_admin` endpoint (instance +/// settings, the user directory, ...) is reachable by a job token whose run-as +/// identity a `wm_deployers` member pointed at a superadmin. A genuine super +/// admin who needs such an operation from a script uses a dedicated superadmin +/// API token (which only a real super admin can create) instead of `$WM_TOKEN`. +pub async fn require_super_admin(db: &DB, authed: &ApiAuthed) -> error::Result<()> { + if authed.token_is_job && is_super_admin_email(db, &authed.email).await? { + return Err(Error::NotAuthorized( + "This endpoint cannot be called with a job token ($WM_TOKEN): a job token runs as its \ + app/flow on_behalf_of identity and is capped at workspace admin. If a script \ + genuinely needs a superadmin operation, create a dedicated superadmin token from the \ + User settings drawer (the 'Tokens' section), store it as a secret, and use that \ + token explicitly instead of $WM_TOKEN." + .to_owned(), + )); + } + require_super_admin_email(db, &authed.email).await +} - if !is_admin { +/// The caller's *effective* super admin status: a real super admin, but not +/// when acting through a job token (`$WM_TOKEN`), which is capped at workspace +/// admin (see [`ApiAuthed::token_is_job`]). Use this for inline capability gates +/// the same way [`require_super_admin`] is used for hard gates, so a job token +/// can never exceed workspace admin on any route. +pub async fn is_super_admin(db: &DB, authed: &ApiAuthed) -> error::Result { + Ok(!authed.token_is_job && is_super_admin_email(db, &authed.email).await?) +} + +/// Email-only super admin check for internal (non-request) callers that have no +/// token context — background tasks, sync jobs, ... A request handler must use +/// [`require_super_admin`] instead so the job-token cap is applied. +pub async fn require_super_admin_email(db: &DB, email: &str) -> error::Result<()> { + if is_super_admin_email(db, email).await? { + Ok(()) + } else { Err(Error::NotAuthorized( "This endpoint requires the caller to be a super admin".to_owned(), )) - } else { - Ok(()) } } @@ -502,16 +541,29 @@ pub fn build_scope_path_predicate( } } -pub async fn require_devops_role(db: &DB, email: &str) -> error::Result<()> { - let is_devops = is_devops_email(db, email).await?; - - if is_devops { - Ok(()) - } else { - Err(Error::NotAuthorized( +/// Require the caller to have the instance-level `devops` role. Like +/// [`require_super_admin`], a caller acting through a job token (`$WM_TOKEN`) is +/// capped at workspace admin and rejected — the `devops` role is instance-level, +/// so it must not be reachable by a job token whose on_behalf_of identity a +/// `wm_deployers` member controls (`is_devops_email` also returns true for +/// superadmins). A genuine devops user who needs this from a script uses a +/// dedicated token instead of `$WM_TOKEN`. +pub async fn require_devops_role(db: &DB, authed: &ApiAuthed) -> error::Result<()> { + if !is_devops_email(db, &authed.email).await? { + return Err(Error::NotAuthorized( "This endpoint requires the caller to have the `devops` role".to_string(), - )) + )); } + if authed.token_is_job { + return Err(Error::NotAuthorized( + "This endpoint cannot be called with a job token ($WM_TOKEN): a job token is capped \ + at workspace admin. If a script genuinely needs a devops operation, create a \ + dedicated token from the User settings drawer, store it as a secret, and use that \ + token explicitly instead of $WM_TOKEN." + .to_owned(), + )); + } + Ok(()) } // ------------ Folder ownership checks ------------ @@ -762,6 +814,7 @@ pub async fn fetch_api_authed_from_permissioned_as( username_override: None, token_prefix: authed.token_prefix, read_only: false, + token_is_job: false, }; API_AUTHED_CACHE.insert( diff --git a/backend/windmill-api-configs/src/lib.rs b/backend/windmill-api-configs/src/lib.rs index f776712063..bf7fb6c540 100644 --- a/backend/windmill-api-configs/src/lib.rs +++ b/backend/windmill-api-configs/src/lib.rs @@ -117,7 +117,7 @@ async fn get_config( Path(name): Path, Extension(db): Extension, ) -> error::JsonResult> { - require_devops_role(&db, &authed.email).await?; + require_devops_role(&db, &authed).await?; let config = sqlx::query_as!(Config, "SELECT name, config FROM config WHERE name = $1", name) .fetch_optional(&db) @@ -133,7 +133,7 @@ async fn update_config( authed: ApiAuthed, Json(config): Json, ) -> error::Result { - require_devops_role(&db, &authed.email).await?; + require_devops_role(&db, &authed).await?; #[cfg(not(feature = "enterprise"))] let config = if name.starts_with("worker__") { @@ -212,7 +212,7 @@ async fn delete_config( Extension(db): Extension, authed: ApiAuthed, ) -> error::Result { - require_devops_role(&db, &authed.email).await?; + require_devops_role(&db, &authed).await?; let mut tx = db.begin().await?; @@ -280,7 +280,7 @@ async fn native_kubernetes_autoscaling_healthcheck( authed: ApiAuthed, Extension(db): Extension, ) -> Result<(), windmill_autoscaling::kubernetes_integration_ee::KubeError> { - require_devops_role(&db, &authed.email).await.map_err(|e| { + require_devops_role(&db, &authed).await.map_err(|e| { windmill_autoscaling::kubernetes_integration_ee::KubeError::Other(e.to_string()) })?; @@ -317,7 +317,7 @@ async fn list_configs( authed: ApiAuthed, Extension(db): Extension, ) -> error::JsonResult> { - require_devops_role(&db, &authed.email).await?; + require_devops_role(&db, &authed).await?; let configs = sqlx::query_as!(Config, "SELECT name, config FROM config") .fetch_all(&db) .await?; @@ -342,7 +342,7 @@ async fn list_all_workspace_dependencies( authed: ApiAuthed, Extension(db): Extension, ) -> error::JsonResult> { - require_devops_role(&db, &authed.email).await?; + require_devops_role(&db, &authed).await?; let deps = sqlx::query!( r#"SELECT workspace_id, name, language AS "language: windmill_common::scripts::ScriptLang" FROM workspace_dependencies @@ -374,7 +374,7 @@ async fn list_all_dedicated_with_deps( authed: ApiAuthed, Extension(db): Extension, ) -> error::JsonResult> { - require_devops_role(&db, &authed.email).await?; + require_devops_role(&db, &authed).await?; let rows = sqlx::query!( r#"SELECT DISTINCT ON (workspace_id, path) diff --git a/backend/windmill-api-flows/src/flows.rs b/backend/windmill-api-flows/src/flows.rs index a9376d02ed..eca878ee8e 100644 --- a/backend/windmill-api-flows/src/flows.rs +++ b/backend/windmill-api-flows/src/flows.rs @@ -591,6 +591,22 @@ async fn create_flow( } } + // Reject a forged superadmin run identity: only a preserved value is + // caller-controlled (otherwise `resolve_on_behalf_of_email` stores the + // deployer's own email below). A flow stores no permissioned_as, so this is + // the sentinel guard. + if nf.preserve_on_behalf_of.unwrap_or(false) + && windmill_common::can_preserve_on_behalf_of(&authed) + { + windmill_common::auth::validate_on_behalf_of( + &db, + &w_id, + None, + nf.on_behalf_of_email.as_deref(), + ) + .await?; + } + let mut tx = user_db.clone().begin(&authed).await?; check_path_conflict(&mut tx, &w_id, &nf.path).await?; @@ -1031,6 +1047,20 @@ async fn update_flow( validate_flow(&nf).await?; + // Reject a forged superadmin run identity in a preserved value (otherwise + // `resolve_on_behalf_of_email` stores the deployer's own email below). + if nf.preserve_on_behalf_of.unwrap_or(false) + && windmill_common::can_preserve_on_behalf_of(&authed) + { + windmill_common::auth::validate_on_behalf_of( + &db, + &w_id, + None, + nf.on_behalf_of_email.as_deref(), + ) + .await?; + } + let authed = maybe_refresh_folders(&flow_path, &w_id, authed, &db).await; let mut tx = user_db.clone().begin(&authed).await?; diff --git a/backend/windmill-api-groups/src/groups.rs b/backend/windmill-api-groups/src/groups.rs index 263daad624..951c9dba3f 100644 --- a/backend/windmill-api-groups/src/groups.rs +++ b/backend/windmill-api-groups/src/groups.rs @@ -298,7 +298,7 @@ async fn create_igroup( ) -> Result { use uuid::Uuid; - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let mut tx = db.begin().await?; let normalized_name = convert_name(&ng.name); @@ -463,7 +463,7 @@ async fn update_igroup( Path(name): Path, Json(igroup_update): Json, ) -> Result { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let mut tx: Transaction<'_, Postgres> = db.begin().await?; let exists_opt = sqlx::query("SELECT 1 FROM instance_group WHERE name = $1") @@ -518,7 +518,7 @@ async fn delete_igroup( Extension(db): Extension, Path(name): Path, ) -> Result { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let mut tx: Transaction<'_, Postgres> = db.begin().await?; // Fetch group's instance_role and members before deletion @@ -818,7 +818,7 @@ async fn add_user_igroup( Path(name): Path, Json(Email { email }): Json, ) -> Result { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let mut tx: Transaction<'_, Postgres> = db.begin().await?; @@ -1096,7 +1096,7 @@ async fn remove_user_igroup( Path(name): Path, Json(Email { email }): Json, ) -> Result { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let mut tx = db.begin().await?; let group_opt = sqlx::query_scalar!("SELECT name FROM instance_group WHERE name = $1", name,) @@ -1225,7 +1225,7 @@ async fn export_igroups( authed: ApiAuthed, Extension(db): Extension, ) -> JsonResult> { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let mut tx = db.begin().await?; let igroups = sqlx::query_as!( ExportedIGroup, @@ -1261,7 +1261,7 @@ async fn overwrite_igroups( Extension(db): Extension, Json(igroups): Json>, ) -> Result { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let mut tx = db.begin().await?; sqlx::query!("DELETE FROM email_to_igroup") diff --git a/backend/windmill-api-integration-tests/tests/native_triggers.rs b/backend/windmill-api-integration-tests/tests/native_triggers.rs index 1d809ae526..d798c01a38 100644 --- a/backend/windmill-api-integration-tests/tests/native_triggers.rs +++ b/backend/windmill-api-integration-tests/tests/native_triggers.rs @@ -52,6 +52,7 @@ fn test_authed() -> ApiAuthed { username_override: None, token_prefix: None, read_only: false, + token_is_job: false, } } diff --git a/backend/windmill-api-schedule/src/lib.rs b/backend/windmill-api-schedule/src/lib.rs index 573c47e355..5b3a8f2308 100644 --- a/backend/windmill-api-schedule/src/lib.rs +++ b/backend/windmill-api-schedule/src/lib.rs @@ -306,6 +306,16 @@ async fn create_schedule( ) .await?; + // Reject a forged superadmin run identity in a preserved permissioned_as + // (the sentinel guard; the email is derived from it so it always belongs). + windmill_common::auth::validate_on_behalf_of( + &db, + &w_id, + Some(&resolved_permissioned_as), + Some(&resolved_email), + ) + .await?; + let schedule = sqlx::query_as!( Schedule, r#" @@ -518,6 +528,16 @@ async fn edit_schedule( authed.email.clone() }; + // Reject a forged superadmin run identity in a preserved permissioned_as + // (the sentinel guard; the email is derived from it so it always belongs). + windmill_common::auth::validate_on_behalf_of( + &db, + &w_id, + Some(&resolved_permissioned_as), + Some(&resolved_email), + ) + .await?; + let schedule = sqlx::query_as!( Schedule, r#" @@ -1310,7 +1330,7 @@ async fn set_default_error_handler( Path(w_id): Path, Json(payload): Json, ) -> Result<()> { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let (key, value) = match payload.handler_type { HandlerType::Error => { let key = format!("default_error_handler_{}", w_id); diff --git a/backend/windmill-api-scripts/src/scripts.rs b/backend/windmill-api-scripts/src/scripts.rs index fd9899ea10..099d799fea 100644 --- a/backend/windmill-api-scripts/src/scripts.rs +++ b/backend/windmill-api-scripts/src/scripts.rs @@ -978,6 +978,20 @@ async fn create_script_internal<'c>( } } } + // Reject a forged superadmin run identity in a preserved value (otherwise + // `resolve_on_behalf_of_email` stores the deployer's own email below). A + // script stores no permissioned_as, so this is the sentinel guard. + if ns.preserve_on_behalf_of.unwrap_or(false) + && windmill_common::can_preserve_on_behalf_of(&authed) + { + windmill_common::auth::validate_on_behalf_of( + &db, + &w_id, + None, + ns.on_behalf_of_email.as_deref(), + ) + .await?; + } if sqlx::query_scalar!( "SELECT 1 FROM script WHERE hash = $1 AND workspace_id = $2", hash.0, diff --git a/backend/windmill-api-settings/src/lib.rs b/backend/windmill-api-settings/src/lib.rs index 9d37233228..43d41e582d 100644 --- a/backend/windmill-api-settings/src/lib.rs +++ b/backend/windmill-api-settings/src/lib.rs @@ -25,7 +25,7 @@ mod log_cleanup; #[cfg(feature = "parquet")] mod storage_usage; -use windmill_api_auth::{require_devops_role, require_super_admin, ApiAuthed}; +use windmill_api_auth::{is_super_admin, require_devops_role, require_super_admin, ApiAuthed}; use windmill_common::utils::HTTP_CLIENT_PERMISSIVE as HTTP_CLIENT; use windmill_common::DB; @@ -52,7 +52,6 @@ use windmill_common::secret_backend::{ AwsSecretsManagerSettings, AzureKeyVaultSettings, SecretMigrationReport, VaultSettings, }; use windmill_common::{ - auth::is_super_admin_email, ee_oss::{get_license_plan, LicensePlan}, email_oss::send_email_plain_text, error::{self, JsonResult, Result}, @@ -232,7 +231,7 @@ pub async fn test_email( authed: ApiAuthed, Json(test_email): Json, ) -> error::Result { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let smtp = test_email.smtp; let to = test_email.to; @@ -269,7 +268,7 @@ pub async fn test_s3_bucket( // local-filesystem surface (see validate_object_storage_test). On self-hosted instances the // object store usually lives on the local/private network and all authenticated users are // trusted, so testing there stays unrestricted. Super admins keep the unrestricted path too. - let is_super_admin = is_super_admin_email(&db, &authed.email).await?; + let is_super_admin = is_super_admin(&db, &authed).await?; let restrict = !is_super_admin && *CLOUD_HOSTED; if restrict { validate_object_storage_test(&test_s3_bucket).await?; @@ -569,7 +568,7 @@ async fn get_object_storage_usage( Extension(db): Extension, authed: ApiAuthed, ) -> error::JsonResult> { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; Ok(Json(storage_usage::get_status(&db).await?)) } @@ -578,7 +577,7 @@ async fn compute_object_storage_usage( Extension(db): Extension, authed: ApiAuthed, ) -> error::Result { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; storage_usage::try_start(&db).await?; storage_usage::spawn_compute(db.clone()); Ok(axum::http::StatusCode::ACCEPTED) @@ -589,7 +588,7 @@ async fn run_log_cleanup( Extension(db): Extension, authed: ApiAuthed, ) -> error::Result { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; log_cleanup::try_start(&db).await?; log_cleanup::spawn_cleanup(db.clone()); Ok(axum::http::StatusCode::ACCEPTED) @@ -600,7 +599,7 @@ async fn log_cleanup_status( Extension(db): Extension, authed: ApiAuthed, ) -> error::JsonResult> { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; Ok(Json(log_cleanup::get_status(&db).await?)) } @@ -609,7 +608,7 @@ async fn audit_logs_s3_status( Extension(db): Extension, authed: ApiAuthed, ) -> error::JsonResult> { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; Ok(Json(audit_logs_s3::get_status(&db).await?)) } @@ -619,7 +618,7 @@ async fn run_audit_logs_s3_backfill( authed: ApiAuthed, Json(req): Json, ) -> error::Result { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; if !matches!(get_license_plan().await, LicensePlan::Enterprise) { return Err(error::Error::BadRequest( "Audit log export to object storage is an Enterprise feature".to_string(), @@ -635,7 +634,7 @@ async fn audit_logs_s3_backfill_status( Extension(db): Extension, authed: ApiAuthed, ) -> error::JsonResult> { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; Ok(Json(audit_logs_s3_backfill::get_status(&db).await?)) } @@ -649,7 +648,7 @@ pub async fn test_license_key( authed: ApiAuthed, Json(TestKey { license_key }): Json, ) -> error::Result { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let (_, expired, _offline_meta) = validate_license_key(license_key, Some(&db)).await?; if expired { @@ -670,7 +669,7 @@ pub async fn get_offline_license_status( Extension(db): Extension, authed: ApiAuthed, ) -> error::JsonResult> { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let offline = (**windmill_common::ee_oss::LICENSE_OFFLINE_METADATA.load()).clone(); let is_offline = matches!(&offline, Some(m) if m.is_offline()); @@ -696,7 +695,7 @@ pub async fn get_instance_hash( Extension(db): Extension, authed: ApiAuthed, ) -> error::JsonResult { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; #[cfg(feature = "enterprise")] let hash = windmill_common::ee_oss::compute_instance_hash(&db) .await @@ -710,7 +709,7 @@ pub async fn get_local_settings( Extension(db): Extension, authed: ApiAuthed, ) -> error::JsonResult { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let mut settings = serde_json::Map::new(); for key in ENV_SETTINGS.iter() { @@ -769,7 +768,7 @@ pub async fn set_global_setting( Path(key): Path, Json(value): Json, ) -> error::Result<()> { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; set_global_setting_internal(&db, key, value.value.unwrap_or(serde_json::Value::Null)).await } @@ -1093,7 +1092,7 @@ async fn get_instance_config( Extension(db): Extension, authed: ApiAuthed, ) -> JsonResult { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let config = InstanceConfig::from_db(&db) .await .map_err(|e| error::Error::internal_err(e.to_string()))?; @@ -1104,7 +1103,7 @@ async fn get_instance_config_yaml( Extension(db): Extension, authed: ApiAuthed, ) -> error::Result { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let config = InstanceConfig::from_db(&db) .await .map_err(|e| error::Error::internal_err(e.to_string()))?; @@ -1122,7 +1121,7 @@ async fn set_instance_config( authed: ApiAuthed, Json(desired): Json, ) -> error::Result<()> { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let current = InstanceConfig::from_db(&db) .await @@ -1221,7 +1220,7 @@ pub async fn get_global_setting( && key != HTTP_ROUTE_WORKSPACED_ROUTE_SETTING && key != WS_BASE_URL_SETTING { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; } let value = sqlx::query!("SELECT value FROM global_settings WHERE name = $1", key) .fetch_optional(&db) @@ -1243,7 +1242,7 @@ async fn list_global_settings( Extension(db): Extension, authed: ApiAuthed, ) -> JsonResult> { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let settings = sqlx::query_as!(GlobalSetting, "SELECT name, value FROM global_settings") .fetch_all(&db) .await?; @@ -1259,7 +1258,7 @@ async fn list_global_settings() -> JsonResult { } pub async fn send_stats(Extension(db): Extension, authed: ApiAuthed) -> Result { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; windmill_common::stats_oss::send_stats( &HTTP_CLIENT, &db, @@ -1276,7 +1275,7 @@ async fn restart_worker_group( authed: ApiAuthed, Path(worker_group): Path, ) -> error::Result { - require_devops_role(&db, &authed.email).await?; + require_devops_role(&db, &authed).await?; sqlx::query!( "INSERT INTO notify_event (channel, payload) VALUES ('restart_worker_group', $1)", @@ -1301,7 +1300,7 @@ pub async fn get_stats( Extension(db): Extension, authed: ApiAuthed, ) -> error::JsonResult { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let stats = windmill_common::stats_oss::get_stats_payload( &db, &windmill_common::stats_oss::SendStatsReason::Manual, @@ -1331,7 +1330,7 @@ pub async fn get_latest_key_renewal_attempt( Extension(db): Extension, authed: ApiAuthed, ) -> JsonResult> { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let last_attempt = sqlx::query!( "SELECT value, created_at FROM metrics WHERE id = $1 ORDER BY created_at DESC LIMIT 1", @@ -1374,7 +1373,7 @@ pub async fn renew_license_key( Query(LicenseQuery { license_key }): Query, authed: ApiAuthed, ) -> Result { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let result = windmill_common::ee_oss::renew_license_key( &HTTP_CLIENT, &db, @@ -1420,7 +1419,7 @@ pub async fn test_critical_channels( authed: ApiAuthed, Json(test_critical_channels): Json>, ) -> Result { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; #[cfg(feature = "enterprise")] send_critical_alert( @@ -1444,7 +1443,7 @@ pub async fn get_critical_alerts( authed: ApiAuthed, Query(params): Query, ) -> JsonResult { - require_devops_role(&db, &authed.email).await?; + require_devops_role(&db, &authed).await?; windmill_alerting::get_critical_alerts(db, params, None).await } @@ -1460,7 +1459,7 @@ pub async fn acknowledge_critical_alert( authed: ApiAuthed, Path(id): Path, ) -> error::Result { - require_devops_role(&db, &authed.email).await?; + require_devops_role(&db, &authed).await?; windmill_alerting::acknowledge_critical_alert(db, None, id).await } @@ -1474,7 +1473,7 @@ pub async fn acknowledge_all_critical_alerts( Extension(db): Extension, authed: ApiAuthed, ) -> error::Result { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; windmill_alerting::acknowledge_all_critical_alerts(db, None).await } @@ -1528,7 +1527,7 @@ async fn list_custom_instance_pg_databases( )) })?; - if is_super_admin_email(&db, &authed.email).await? { + if is_super_admin(&db, &authed).await? { // Enrich each database with the list of workspaces referencing it through // either a ducklake catalog or a datatable database whose resource_type is // 'instance'. Not stored in DB to avoid drift. @@ -1578,7 +1577,7 @@ async fn refresh_custom_instance_user_pwd( authed: ApiAuthed, Extension(db): Extension, ) -> JsonResult<()> { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; windmill_common::utils::refresh_custom_instance_user_pwd(&db).await?; Ok(Json(())) } @@ -1616,7 +1615,7 @@ async fn setup_custom_instance_pg_database_inner( dbname: &str, logs: &mut CustomInstanceDbLogs, ) -> Result<()> { - require_super_admin(db, &authed.email).await?; + require_super_admin(db, &authed).await?; logs.super_admin = "OK".to_string(); let wmill_pg_creds = PgDatabase::parse_uri(&get_database_url().await?.as_str().await)?; logs.database_credentials = "OK".to_string(); @@ -1731,7 +1730,7 @@ async fn drop_custom_instance_pg_database( Extension(db): Extension, Path(dbname): Path, ) -> Result { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; windmill_common::drop_custom_instance_database(&db, &dbname).await?; @@ -1754,7 +1753,7 @@ pub async fn test_secret_backend( authed: ApiAuthed, Json(settings): Json, ) -> Result { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; windmill_common::secret_backend::test_vault_connection(&settings, Some(&db)).await?; @@ -1774,7 +1773,7 @@ pub async fn migrate_secrets_to_vault( authed: ApiAuthed, Json(settings): Json, ) -> JsonResult { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let report = windmill_common::secret_backend::migrate_secrets_to_vault(&db, &settings).await?; @@ -1794,7 +1793,7 @@ pub async fn migrate_secrets_to_database( authed: ApiAuthed, Json(settings): Json, ) -> JsonResult { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let report = windmill_common::secret_backend::migrate_secrets_to_database(&db, &settings).await?; @@ -1811,7 +1810,7 @@ pub async fn test_azure_kv_backend( authed: ApiAuthed, Json(settings): Json, ) -> Result { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; windmill_common::secret_backend::test_azure_kv_connection(&settings).await?; @@ -1827,7 +1826,7 @@ pub async fn migrate_secrets_to_azure_kv( authed: ApiAuthed, Json(settings): Json, ) -> JsonResult { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let report = windmill_common::secret_backend::migrate_secrets_to_azure_kv(&db, &settings).await?; @@ -1844,7 +1843,7 @@ pub async fn migrate_secrets_from_azure_kv( authed: ApiAuthed, Json(settings): Json, ) -> JsonResult { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let report = windmill_common::secret_backend::migrate_secrets_from_azure_kv(&db, &settings).await?; @@ -1859,7 +1858,7 @@ pub async fn test_aws_sm_backend( authed: ApiAuthed, Json(settings): Json, ) -> Result { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; windmill_common::secret_backend::test_aws_sm_connection(&settings).await?; Ok("Successfully connected to AWS Secrets Manager".to_string()) } @@ -1871,7 +1870,7 @@ pub async fn migrate_secrets_to_aws_sm( authed: ApiAuthed, Json(settings): Json, ) -> JsonResult { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let report = windmill_common::secret_backend::migrate_secrets_to_aws_sm(&db, &settings).await?; Ok(Json(report)) } @@ -1883,7 +1882,7 @@ pub async fn migrate_secrets_from_aws_sm( authed: ApiAuthed, Json(settings): Json, ) -> JsonResult { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let report = windmill_common::secret_backend::migrate_secrets_from_aws_sm(&db, &settings).await?; Ok(Json(report)) @@ -1976,7 +1975,7 @@ async fn sync_cached_resource_types( Extension(db): Extension, authed: ApiAuthed, ) -> error::Result { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; use windmill_common::worker::HUB_RT_CACHE_DIR; let cache_path = format!("{}/resource_types.json", *HUB_RT_CACHE_DIR); diff --git a/backend/windmill-api-users/src/users.rs b/backend/windmill-api-users/src/users.rs index ff0e8e754e..37e756ba2a 100644 --- a/backend/windmill-api-users/src/users.rs +++ b/backend/windmill-api-users/src/users.rs @@ -460,7 +460,7 @@ async fn list_users_as_super_admin( Query(pagination): Query, Query(ActiveUsersOnly { active_only }): Query, ) -> JsonResult> { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let per_page = pagination.per_page.unwrap_or(10000).max(1); let offset = (pagination.page.unwrap_or(1).max(1) - 1) * per_page; @@ -1431,7 +1431,7 @@ async fn update_user( Extension(db): Extension, Json(eu): Json, ) -> Result { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; forbid_superadmin_job_token(&db, &authed.email, job_id).await?; let mut tx = db.begin().await?; @@ -1607,7 +1607,7 @@ async fn delete_user( Path(email_to_delete): Path, Extension(db): Extension, ) -> Result { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; forbid_superadmin_job_token(&db, &authed.email, job_id).await?; let mut tx = db.begin().await?; @@ -1904,7 +1904,7 @@ async fn set_login_type( OptJobAuthed { job_id, .. }: OptJobAuthed, Json(et): Json, ) -> Result { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; forbid_superadmin_job_token(&db, &authed.email, job_id).await?; let mut tx = db.begin().await?; @@ -2219,7 +2219,7 @@ async fn impersonate( } else { Some(&token) }; - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; forbid_superadmin_job_token(&db, &authed.email, job_id).await?; if new_token.impersonate_email.is_none() { @@ -2673,11 +2673,11 @@ struct WorkspaceUsernameInfo { username: String, } async fn get_instance_username_info( - ApiAuthed { email, .. }: ApiAuthed, + authed: ApiAuthed, Path(user_email): Path, Extension(db): Extension, ) -> JsonResult { - require_super_admin(&db, &email).await?; + require_super_admin(&db, &authed).await?; let mut tx = db.begin().await?; let instance_username = match sqlx::query_scalar!( "SELECT username FROM password WHERE email = $1", @@ -2747,7 +2747,7 @@ async fn export_global_users( authed: ApiAuthed, OptJobAuthed { job_id, .. }: OptJobAuthed, ) -> JsonResult> { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; forbid_superadmin_job_token(&db, &authed.email, job_id).await?; let mut tx = db.begin().await?; let users = sqlx::query_as!( @@ -2787,7 +2787,7 @@ async fn overwrite_global_users( OptJobAuthed { job_id, .. }: OptJobAuthed, Json(users): Json>, ) -> Result { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; forbid_superadmin_job_token(&db, &authed.email, job_id).await?; let mut tx = db.begin().await?; sqlx::query!("DELETE FROM password") diff --git a/backend/windmill-api-workers/src/lib.rs b/backend/windmill-api-workers/src/lib.rs index d65316555e..a7a9b67358 100644 --- a/backend/windmill-api-workers/src/lib.rs +++ b/backend/windmill-api-workers/src/lib.rs @@ -102,7 +102,7 @@ async fn list_worker_pings( Extension(user_db): Extension, Query(query): Query, ) -> JsonResult> { - let has_devops_role = require_devops_role(&db, &authed.email).await.is_ok(); + let has_devops_role = require_devops_role(&db, &authed).await.is_ok(); if *HIDE_WORKERS_FOR_NON_ADMINS && !has_devops_role { return Ok(Json(vec![])); } @@ -158,7 +158,7 @@ async fn exists_workers_with_tags( // When TAGS_ARE_SENSITIVE is enabled, filter tags based on workspace visibility if *TAGS_ARE_SENSITIVE { - let has_devops_role = require_devops_role(&db, &authed.email).await.is_ok(); + let has_devops_role = require_devops_role(&db, &authed).await.is_ok(); if !has_devops_role { if let Some(ref workspace) = tags_query.workspace { // Filter to only tags visible in this workspace @@ -212,7 +212,7 @@ async fn get_custom_tags( return Ok(Json(all_tags)); } if *TAGS_ARE_SENSITIVE { - let has_devops_role = require_devops_role(&db, &authed.email).await.is_ok(); + let has_devops_role = require_devops_role(&db, &authed).await.is_ok(); if !has_devops_role { return Ok(Json(vec![])); } @@ -249,7 +249,7 @@ async fn get_queue_metrics( authed: ApiAuthed, Extension(db): Extension, ) -> JsonResult> { - require_devops_role(&db, &authed.email).await?; + require_devops_role(&db, &authed).await?; let queue_metrics = sqlx::query_as!( QueueMetric, @@ -274,7 +274,7 @@ async fn get_queue_counts( authed: ApiAuthed, Extension(db): Extension, ) -> JsonResult> { - require_devops_role(&db, &authed.email).await?; + require_devops_role(&db, &authed).await?; let queue_counts = windmill_common::queue::get_queue_counts(&db).await; Ok(Json(queue_counts)) } @@ -283,7 +283,7 @@ async fn get_queue_running_counts( authed: ApiAuthed, Extension(db): Extension, ) -> JsonResult> { - require_devops_role(&db, &authed.email).await?; + require_devops_role(&db, &authed).await?; let queue_running_counts = windmill_common::queue::get_queue_running_counts(&db).await; Ok(Json(queue_running_counts)) } @@ -308,7 +308,7 @@ async fn get_workspace_fairness_events( authed: ApiAuthed, Extension(db): Extension, ) -> JsonResult> { - require_devops_role(&db, &authed.email).await?; + require_devops_role(&db, &authed).await?; // No cloud-host gate — workspace fairness is an Enterprise feature // available on any multi-tenant EE deployment. Non-EE / non-enabled diff --git a/backend/windmill-api-workspaces/src/datatable_migrations.rs b/backend/windmill-api-workspaces/src/datatable_migrations.rs index fef9963933..d7d6c036c4 100644 --- a/backend/windmill-api-workspaces/src/datatable_migrations.rs +++ b/backend/windmill-api-workspaces/src/datatable_migrations.rs @@ -680,7 +680,7 @@ async fn datatable_migrations_status( /// Only workspace admins and super admins may opt a data table in or out of /// migrations. async fn require_datatable_migrations_manager(db: &DB, authed: &ApiAuthed) -> Result<()> { - if authed.is_admin || require_super_admin(db, &authed.email).await.is_ok() { + if authed.is_admin || require_super_admin(db, &authed).await.is_ok() { Ok(()) } else { Err(Error::BadRequest( diff --git a/backend/windmill-api-workspaces/src/workspaces.rs b/backend/windmill-api-workspaces/src/workspaces.rs index ba359cbffb..bf79b03de8 100644 --- a/backend/windmill-api-workspaces/src/workspaces.rs +++ b/backend/windmill-api-workspaces/src/workspaces.rs @@ -7,8 +7,8 @@ */ use windmill_api_auth::{ - build_scope_path_predicate, check_scopes, require_devops_role, require_is_writer, - require_super_admin, ApiAuthed, + build_scope_path_predicate, check_scopes, is_super_admin, require_devops_role, + require_is_writer, require_super_admin, ApiAuthed, }; use windmill_api_users::users::WorkspaceInvite; use windmill_common::email_oss::send_email_if_possible; @@ -2176,7 +2176,7 @@ async fn create_pg_database( windmill_common::validate_dbname(&req.target_dbname)?; // Non-superadmin: restrict dbname to wm_fork_ prefix - if !windmill_common::auth::is_super_admin_email(&db, &authed.email).await? { + if !is_super_admin(&db, &authed).await? { if !req.target_dbname.starts_with("wm_fork_") { return Err(Error::BadRequest( "Non-superadmin users can only create databases with names starting with 'wm_fork_'" @@ -2292,7 +2292,7 @@ async fn import_pg_database( resolve_pg_source_checked(&db, &user_db, &authed, &w_id, &req.target).await?; if let Some(ref override_dbname) = req.target_dbname_override { - if !windmill_common::auth::is_super_admin_email(&db, &authed.email).await? { + if !is_super_admin(&db, &authed).await? { if !override_dbname.starts_with("wm_fork_") { return Err(Error::BadRequest( "Non-superadmin users can only override target dbname with names starting with 'wm_fork_'" @@ -2365,11 +2365,11 @@ async fn edit_ducklake_config( authed: ApiAuthed, Extension(db): Extension, Path(w_id): Path, - ApiAuthed { is_admin, username, email, .. }: ApiAuthed, + ApiAuthed { is_admin, username, .. }: ApiAuthed, Json(new_config): Json, ) -> Result { require_admin(is_admin, &username)?; - let is_superadmin = require_super_admin(&db, &email).await.is_ok(); + let is_superadmin = require_super_admin(&db, &authed).await.is_ok(); // Lake names end up interpolated in `ATTACH 'ducklake://'`, // generated maintenance SQL and the reserved maintenance schedule path @@ -2449,7 +2449,7 @@ async fn edit_ducklake_config( &new_config.settings.ducklakes, &old_ducklakes, &username, - &email, + &authed.email, ) .await?; @@ -2462,11 +2462,11 @@ async fn edit_datatable_config( authed: ApiAuthed, Extension(db): Extension, Path(w_id): Path, - ApiAuthed { is_admin, username, email, .. }: ApiAuthed, + ApiAuthed { is_admin, username, .. }: ApiAuthed, Json(mut new_config): Json, ) -> Result { require_admin(is_admin, &username)?; - let is_superadmin = require_super_admin(&db, &email).await.is_ok(); + let is_superadmin = require_super_admin(&db, &authed).await.is_ok(); let mut tx = db.begin().await?; @@ -3585,7 +3585,7 @@ async fn set_encryption_key( Path(w_id): Path, Json(request): Json, ) -> Result<()> { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; if !WORKSPACE_KEY_REGEXP.is_match(request.new_key.as_str()) { return Err(Error::BadRequest( @@ -3739,7 +3739,7 @@ async fn get_workspace_as_superadmin( Extension(db): Extension, Path(w_id): Path, ) -> JsonResult { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let workspace = sqlx::query_as!( Workspace, "SELECT @@ -3770,7 +3770,7 @@ async fn list_workspaces_as_super_admin( Query(pagination): Query, ApiAuthed { email, .. }: ApiAuthed, ) -> JsonResult> { - require_devops_role(&db, &email).await?; + require_devops_role(&db, &authed).await?; let (per_page, offset) = paginate(pagination); let mut tx = user_db.begin(&authed).await?; @@ -4000,7 +4000,7 @@ async fn create_workspace( Json(nw): Json, ) -> Result { if *CREATE_WORKSPACE_REQUIRE_SUPERADMIN { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; } #[cfg(not(feature = "enterprise"))] @@ -5328,7 +5328,7 @@ async fn create_workspace_fork_branch( } if *DISABLE_WORKSPACE_FORK { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; } if let RuleCheckResult::Blocked(msg) = check_user_against_rule( &w_id, @@ -5723,7 +5723,7 @@ async fn create_workspace_fork( _check_nb_of_workspaces(&db).await?; if *DISABLE_WORKSPACE_FORK { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; } if let RuleCheckResult::Blocked(msg) = check_user_against_rule( &parent_workspace_id, @@ -6040,7 +6040,7 @@ async fn attach_dev_workspace( .fetch_optional(&db) .await? .unwrap_or(false); - if !is_admin_of_dev && !windmill_common::auth::is_super_admin_email(&db, &authed.email).await? { + if !is_admin_of_dev && !is_super_admin(&db, &authed).await? { return Err(Error::PermissionDenied(format!( "Attaching workspace '{dev_w_id}' as a dev requires being an admin of it (or a superadmin)" ))); @@ -6405,9 +6405,7 @@ async fn archive_workspace( .fetch_optional(&db) .await? .unwrap_or(false); - if !is_prod_admin - && !windmill_common::auth::is_super_admin_email(&db, &authed.email).await? - { + if !is_prod_admin && !is_super_admin(&db, &authed).await? { return Err(Error::PermissionDenied(format!( "Archiving dev workspace '{w_id}' requires being an admin of its parent prod workspace '{prod}' (or a superadmin)" ))); @@ -8041,7 +8039,7 @@ async fn compare_workspaces( // source AND the fork (superadmin satisfies both), which guarantees full // visibility of every item on every side. `fork_authed.is_admin` already folds // in superadmin; `authed.is_admin` (source side) does not, so OR it in. - let is_super_admin = windmill_common::auth::is_super_admin_email(&db, &authed.email).await?; + let is_super_admin = is_super_admin(&db, &authed).await?; let sees_all_items = is_super_admin || (authed.is_admin && fork_authed.is_admin); let all_ahead_items_visible = all_ahead_items_visible || sees_all_items; let all_behind_items_visible = all_behind_items_visible || sees_all_items; @@ -8139,6 +8137,8 @@ async fn load_workspace_authed( username_override: base_authed.username_override.clone(), token_prefix: base_authed.token_prefix.clone(), read_only: base_authed.read_only, + // Same token, different workspace: a job token stays capped. + token_is_job: base_authed.token_is_job, }); }; @@ -8168,6 +8168,8 @@ async fn load_workspace_authed( username_override: base_authed.username_override.clone(), token_prefix: base_authed.token_prefix.clone(), read_only: base_authed.read_only, + // Same token, different workspace: a job token stays capped. + token_is_job: base_authed.token_is_job, }) } diff --git a/backend/windmill-api-workspaces/src/workspaces_extra.rs b/backend/windmill-api-workspaces/src/workspaces_extra.rs index 6b4ace5f0a..0edd85e833 100644 --- a/backend/windmill-api-workspaces/src/workspaces_extra.rs +++ b/backend/windmill-api-workspaces/src/workspaces_extra.rs @@ -1,6 +1,6 @@ use std::collections::HashMap; -use windmill_api_auth::{require_super_admin, ApiAuthed}; +use windmill_api_auth::{is_super_admin, require_super_admin, ApiAuthed}; use windmill_common::DB; use crate::workspaces::{ @@ -21,7 +21,6 @@ use windmill_audit::ActionKind; use windmill_common::worker::CLOUD_HOSTED; use windmill_common::{ - auth::is_super_admin_email, db::UserDB, error::{Error, Result}, utils::require_admin, @@ -43,14 +42,14 @@ pub(crate) async fn change_workspace_id( Extension(db): Extension, Json(rw): Json, ) -> Result { - if *CLOUD_HOSTED && !is_super_admin_email(&db, &authed.email).await? { + if *CLOUD_HOSTED && !is_super_admin(&db, &authed).await? { return Err(Error::BadRequest( "This feature is not available on the cloud".to_string(), )); } if *CREATE_WORKSPACE_REQUIRE_SUPERADMIN { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; } else { require_admin(authed.is_admin, &authed.username)?; } @@ -811,7 +810,7 @@ pub(crate) async fn delete_workspace( let mut tx = db.begin().await?; if !(is_fork && is_workspace_owner(&authed, &w_id, &mut tx).await?) - && !is_super_admin_email(&db, &authed.email).await? + && !is_super_admin(&db, &authed).await? { return Err(Error::PermissionDenied( "Deleting this workspace requires being the fork's owner or a superadmin".to_string(), @@ -1157,7 +1156,7 @@ pub async fn drop_forked_datatable_databases( let is_fork = workspace_is_fork(&db, &w_id).await?; let mut tx = db.begin().await?; if !(is_fork && is_workspace_owner(&authed, &w_id, &mut tx).await?) - && !is_super_admin_email(&db, &authed.email).await? + && !is_super_admin(&db, &authed).await? { return Err(Error::PermissionDenied( "Dropping forked datatable databases requires being the fork's owner or a superadmin" @@ -1314,7 +1313,7 @@ pub async fn drop_forked_ducklake_namespaces( let is_fork = workspace_is_fork(&db, &w_id).await?; let mut tx = db.begin().await?; if !(is_fork && is_workspace_owner(&authed, &w_id, &mut tx).await?) - && !is_super_admin_email(&db, &authed.email).await? + && !is_super_admin(&db, &authed).await? { return Err(Error::PermissionDenied( "Dropping forked ducklake namespaces requires being the fork's owner or a superadmin" @@ -1819,7 +1818,7 @@ async fn require_prod_admin_for_dev_workspace( .fetch_optional(db) .await? .unwrap_or(false); - if !is_prod_admin && !is_super_admin_email(db, &authed.email).await? { + if !is_prod_admin && !is_super_admin(db, &authed).await? { return Err(Error::PermissionDenied(format!( "Destroying dev workspace '{w_id}' or its data requires being an admin of its parent prod workspace '{prod}' (or a superadmin)" ))); diff --git a/backend/windmill-api/src/apps.rs b/backend/windmill-api/src/apps.rs index 596d28ed64..ae3a4a52c7 100644 --- a/backend/windmill-api/src/apps.rs +++ b/backend/windmill-api/src/apps.rs @@ -1934,6 +1934,17 @@ async fn create_app_internal<'a>( } } + // Reject a forged superadmin run identity in the (possibly preserved) policy. + // Done on the non-RLS pool before the transaction below, like the resolution + // above, to avoid holding a second connection while `tx` is checked out. + windmill_common::auth::validate_on_behalf_of( + &db, + w_id, + app.policy.on_behalf_of.as_deref(), + app.policy.on_behalf_of_email.as_deref(), + ) + .await?; + let mut tx = user_db.clone().begin(&authed).await?; let path = app.path.clone(); if &app.path == "" { @@ -2445,6 +2456,25 @@ async fn update_app_internal<'a>( check_scopes(&authed, || format!("apps:write:{}", npath))?; } + // Reject a forged superadmin run identity in a preserved policy. Mirror the + // `should_preserve` gate below (only a preserved value is caller-controlled; + // otherwise the policy is rewritten to the deployer's own identity) and run + // it on the non-RLS pool before the transaction to avoid a second connection. + if let Some(npolicy) = ns.policy.as_ref() { + let should_preserve = ns.preserve_on_behalf_of.unwrap_or(false) + && windmill_common::can_preserve_on_behalf_of(&authed) + && npolicy.on_behalf_of.is_some(); + if should_preserve { + windmill_common::auth::validate_on_behalf_of( + &db, + w_id, + npolicy.on_behalf_of.as_deref(), + npolicy.on_behalf_of_email.as_deref(), + ) + .await?; + } + } + let mut tx = user_db.clone().begin(&authed).await?; let mut preserved_on_behalf_of: Option = None; @@ -4278,6 +4308,18 @@ fn get_on_behalf_of(policy: &Policy) -> Result<(String, String)> { ) })? .to_string(); + // Defence in depth against a policy that already carries a forged superadmin + // sentinel (deployed before validation existed, or copied verbatim by a + // workspace fork): the sentinels are internal-only and never a legitimate app + // run identity, so refuse to execute rather than mint a superadmin token. + if windmill_common::auth::is_reserved_on_behalf_of_identity( + Some(&permissioned_as), + Some(&email), + ) { + return Err(Error::BadRequest( + "app on_behalf_of is a reserved internal identity and cannot be executed".to_string(), + )); + } Ok((permissioned_as, email)) } diff --git a/backend/windmill-api/src/db_health.rs b/backend/windmill-api/src/db_health.rs index 0b2e86567b..02b2efd182 100644 --- a/backend/windmill-api/src/db_health.rs +++ b/backend/windmill-api/src/db_health.rs @@ -197,10 +197,10 @@ struct SlowQueriesQuery { } async fn get_db_health( - ApiAuthed { email, .. }: ApiAuthed, + authed: ApiAuthed, Extension(db): Extension, ) -> JsonResult { - require_super_admin(&db, &email).await?; + require_super_admin(&db, &authed).await?; let (database_size, connection_pool, table_maintenance, slow_queries, datatables) = tokio::try_join!( fetch_database_size(&db), @@ -220,11 +220,11 @@ async fn get_db_health( } async fn get_db_health_jobs( - ApiAuthed { email, .. }: ApiAuthed, + authed: ApiAuthed, Extension(db): Extension, Query(query): Query, ) -> JsonResult { - require_super_admin(&db, &email).await?; + require_super_admin(&db, &authed).await?; let scan_limit = query.scan_limit.unwrap_or(10_000).clamp(1_000, 1_000_000); @@ -237,20 +237,20 @@ async fn get_db_health_jobs( } async fn get_slow_queries( - ApiAuthed { email, .. }: ApiAuthed, + authed: ApiAuthed, Extension(db): Extension, Query(query): Query, ) -> JsonResult> { - require_super_admin(&db, &email).await?; + require_super_admin(&db, &authed).await?; let sort = query.sort.unwrap_or(SlowQuerySort::Total); Ok(Json(fetch_slow_queries(&db, sort).await?)) } async fn reset_slow_queries( - ApiAuthed { email, .. }: ApiAuthed, + authed: ApiAuthed, Extension(db): Extension, ) -> windmill_common::error::Result { - require_super_admin(&db, &email).await?; + require_super_admin(&db, &authed).await?; sqlx::query("SELECT pg_stat_statements_reset()") .execute(&db) .await diff --git a/backend/windmill-api/src/jobs.rs b/backend/windmill-api/src/jobs.rs index 14cbb76c91..1fcb9bbc9e 100644 --- a/backend/windmill-api/src/jobs.rs +++ b/backend/windmill-api/src/jobs.rs @@ -26,7 +26,7 @@ use tokio::io::AsyncReadExt; use tower::ServiceBuilder; use url::Url; #[cfg(all(feature = "enterprise", feature = "smtp"))] -use windmill_common::auth::is_super_admin_email; +use windmill_api_auth::is_super_admin; use windmill_common::auth::TOKEN_PREFIX_LEN; #[cfg(feature = "run_inline")] use windmill_common::client::AuthedClient; @@ -1875,7 +1875,7 @@ async fn send_email_with_instance_smtp( let is_handler_job = authed.email == EMAIL_ERROR_HANDLER_USER_EMAIL || authed.email == SCHEDULE_ERROR_HANDLER_USER_EMAIL; - if !is_handler_job && !is_super_admin_email(&db, &authed.email).await? { + if !is_handler_job && !is_super_admin(&db, &authed).await? { return Err(Error::NotAuthorized( "Only super admin or whitelisted token can access email workspace error handler feature" .to_string(), @@ -7195,7 +7195,7 @@ async fn add_batch_jobs( Path((w_id, n)): Path<(String, i32)>, Json(batch_info): Json, ) -> error::JsonResult> { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let ( hash, @@ -9129,11 +9129,11 @@ struct TagCount { } async fn count_by_tag( - ApiAuthed { email, .. }: ApiAuthed, + authed: ApiAuthed, Extension(db): Extension, Query(query): Query, ) -> JsonResult> { - require_super_admin(&db, &email).await?; + require_super_admin(&db, &authed).await?; let horizon = query.horizon_secs.unwrap_or(3600); // Default to 1 hour if not specified let counts = sqlx::query_as!( diff --git a/backend/windmill-api/src/offboarding.rs b/backend/windmill-api/src/offboarding.rs index 24b6d3f10d..7192576c65 100644 --- a/backend/windmill-api/src/offboarding.rs +++ b/backend/windmill-api/src/offboarding.rs @@ -459,7 +459,7 @@ pub(crate) async fn global_offboard_preview( Extension(db): Extension, Path(email): Path, ) -> JsonResult { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let workspaces = sqlx::query!( "SELECT workspace_id, username FROM usr WHERE email = $1", @@ -488,7 +488,7 @@ pub(crate) async fn offboard_global_user( Path(email): Path, Json(req): Json, ) -> Result> { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; forbid_superadmin_job_token(&db, &authed.email, job_id).await?; let workspaces = sqlx::query!( diff --git a/backend/windmill-api/src/service_logs.rs b/backend/windmill-api/src/service_logs.rs index b1902f7c1a..99dc7c41a8 100644 --- a/backend/windmill-api/src/service_logs.rs +++ b/backend/windmill-api/src/service_logs.rs @@ -43,12 +43,12 @@ pub struct LogFile { pub json_fmt: bool, } async fn list_files( - ApiAuthed { email, .. }: ApiAuthed, + authed: ApiAuthed, Extension(db): Extension, Query(pagination): Query, Query(lq): Query, ) -> JsonResult> { - require_devops_role(&db, &email).await?; + require_devops_role(&db, &authed).await?; let (per_page, offset) = windmill_common::utils::paginate(pagination); let mut sqlb = sql_builder::SqlBuilder::select_from("log_file") @@ -89,13 +89,13 @@ async fn list_files( } async fn get_log_file( - ApiAuthed { email, .. }: ApiAuthed, + authed: ApiAuthed, Extension(db): Extension, Path(path): Path, ) -> windmill_common::error::Result { use windmill_common::tracing_init::TMP_WINDMILL_LOGS_SERVICE; - require_devops_role(&db, &email).await?; + require_devops_role(&db, &authed).await?; let path = path.to_path(); if path.contains("..") { return Err(Error::BadRequest("Invalid path".to_string())); diff --git a/backend/windmill-api/src/users.rs b/backend/windmill-api/src/users.rs index 36b0d6c734..4c3e578307 100644 --- a/backend/windmill-api/src/users.rs +++ b/backend/windmill-api/src/users.rs @@ -115,7 +115,7 @@ async fn list_ext_jwt_tokens( Extension(db): Extension, Query(query): Query, ) -> Result>> { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let (per_page, offset) = windmill_common::utils::paginate(windmill_common::utils::Pagination { page: query.page, @@ -159,7 +159,7 @@ async fn set_password_of_user( OptJobAuthed { job_id, .. }: OptJobAuthed, Json(ep): Json, ) -> Result { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; forbid_superadmin_job_token(&db, &authed.email, job_id).await?; crate::users_oss::set_password(db, argon2, authed, &email, ep).await } @@ -176,7 +176,7 @@ async fn rename_user( Extension(db): Extension, Json(ru): Json, ) -> Result { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; forbid_superadmin_job_token(&db, &authed.email, job_id).await?; let mut tx = db.begin().await?; diff --git a/backend/windmill-common/src/auth.rs b/backend/windmill-common/src/auth.rs index bd6bf1ef9d..8ed8bab848 100644 --- a/backend/windmill-common/src/auth.rs +++ b/backend/windmill-common/src/auth.rs @@ -301,6 +301,75 @@ pub async fn is_super_admin_email<'c>(db: impl sqlx::PgExecutor<'c>, email: &str Ok(is_admin) } +/// The three reserved internal identities that grant instance-superadmin at +/// execution: `superadmin_secret@` / `superadmin_notification@` (matched on the +/// email) and `superadmin_sync@` (matched on `permissioned_as`). They belong to +/// no real user, so a stored `on_behalf_of` (app policy, flow/script, +/// schedule, trigger) must never carry one as either field — it would be a +/// forged superadmin run identity. Mirror of the `is_super_admin` derivation in +/// [`fetch_authed_from_permissioned_as_inner`]. +pub fn is_reserved_on_behalf_of_identity( + permissioned_as: Option<&str>, + on_behalf_of_email: Option<&str>, +) -> bool { + const RESERVED: [&str; 3] = [ + SUPERADMIN_SECRET_EMAIL, + SUPERADMIN_NOTIFICATION_EMAIL, + SUPERADMIN_SYNC_EMAIL, + ]; + [permissioned_as, on_behalf_of_email] + .into_iter() + .flatten() + .any(|v| RESERVED.contains(&v)) +} + +/// Guard a caller-supplied `on_behalf_of` before it is persisted on a deployable +/// object. The stored identity is trusted verbatim at execution and decides +/// `is_super_admin` (see [`fetch_authed_from_permissioned_as_inner`]), so a +/// deploy must not be able to forge a superadmin run identity. +/// +/// Enforces two invariants that no legitimate deploy can violate, so this does +/// *not* restrict the intended `wm_deployers` capability of deploying on behalf +/// of a real user — including a real superadmin, e.g. git-sync of +/// superadmin-authored content where `permissioned_as`/`email` name that user +/// consistently: +/// 1. The identity is not a reserved internal sentinel (above). +/// 2. If `on_behalf_of_email` resolves to a superadmin, it must genuinely belong +/// to `permissioned_as` — a superadmin's email cannot be pinned onto an +/// unrelated principal. Only apps store both fields; flows/scripts store just +/// the email (deriving `permissioned_as` from the deployer at execution) and +/// schedules/triggers store just `permissioned_as` (deriving the email from +/// it), so for those this second check is a no-op and the sentinel guard is +/// what applies. +pub async fn validate_on_behalf_of( + db: &DB, + w_id: &str, + permissioned_as: Option<&str>, + on_behalf_of_email: Option<&str>, +) -> Result<()> { + if is_reserved_on_behalf_of_identity(permissioned_as, on_behalf_of_email) { + return Err(Error::BadRequest( + "on_behalf_of cannot be a reserved internal identity".to_string(), + )); + } + + if let (Some(permissioned_as), Some(email)) = (permissioned_as, on_behalf_of_email) { + // `email` is already non-sentinel here, so this only matches a real + // `password.super_admin` user. + if is_super_admin_email(db, email).await? { + let resolved = + crate::users::get_email_from_permissioned_as(permissioned_as, w_id, db).await?; + if resolved != email { + return Err(Error::BadRequest(format!( + "on_behalf_of_email '{email}' does not belong to on_behalf_of '{permissioned_as}'" + ))); + } + } + } + + Ok(()) +} + pub async fn is_devops_email(db: &DB, email: &str) -> Result { if is_super_admin_email(db, email).await? { return Ok(true); @@ -678,7 +747,40 @@ pub mod aws { #[cfg(test)] mod tests { - use super::is_user_token; + use super::{is_reserved_on_behalf_of_identity, is_user_token}; + use crate::users::{ + SUPERADMIN_NOTIFICATION_EMAIL, SUPERADMIN_SECRET_EMAIL, SUPERADMIN_SYNC_EMAIL, + }; + + #[test] + fn reserved_on_behalf_of_identity_matches_every_sentinel_in_either_field() { + // Matched on the email (secret / notification) or on permissioned_as (sync). + assert!(is_reserved_on_behalf_of_identity( + None, + Some(SUPERADMIN_SECRET_EMAIL) + )); + assert!(is_reserved_on_behalf_of_identity( + None, + Some(SUPERADMIN_NOTIFICATION_EMAIL) + )); + assert!(is_reserved_on_behalf_of_identity( + Some(SUPERADMIN_SYNC_EMAIL), + None + )); + // A sentinel smuggled as a raw-email permissioned_as (schedules/triggers + // derive the email from it) is caught too. + assert!(is_reserved_on_behalf_of_identity( + Some(SUPERADMIN_SECRET_EMAIL), + None + )); + // Ordinary identities pass. + assert!(!is_reserved_on_behalf_of_identity(None, None)); + assert!(!is_reserved_on_behalf_of_identity( + Some("u/alice"), + Some("alice@example.com") + )); + assert!(!is_reserved_on_behalf_of_identity(Some("g/team"), None)); + } #[test] fn user_tokens_are_editable() { diff --git a/backend/windmill-store/src/resources.rs b/backend/windmill-store/src/resources.rs index 98f91065e4..22ae79df2e 100644 --- a/backend/windmill-store/src/resources.rs +++ b/backend/windmill-store/src/resources.rs @@ -11,7 +11,7 @@ use std::net::IpAddr; use windmill_api_auth::{ build_scope_path_predicate, check_scopes, maybe_refresh_folders, require_owner_of_path, - require_super_admin, ApiAuthed, Tokened, + require_super_admin_email, ApiAuthed, Tokened, }; use windmill_common::db::DB; use windmill_common::workspaces::{check_deploy_rules, RuleCheckResult}; @@ -695,7 +695,7 @@ pub async fn get_resource_value_interpolated_internal<'a>( ) -> Result> { // This is a special syntax to help debugging custom instance databases if let Some(dbname) = path.strip_prefix("CUSTOM_INSTANCE_DB/") { - require_super_admin(db_with_opt_authed.db(), &db_with_opt_authed.email()).await?; + require_super_admin_email(db_with_opt_authed.db(), &db_with_opt_authed.email()).await?; let mut pg_creds = PgDatabase::parse_uri(&get_database_url().await?.as_str().await)?; pg_creds.dbname = dbname.to_string(); let pg_creds = serde_json::to_value(&pg_creds) diff --git a/backend/windmill-trigger/src/handler.rs b/backend/windmill-trigger/src/handler.rs index ed3b89fc65..8697c562b1 100644 --- a/backend/windmill-trigger/src/handler.rs +++ b/backend/windmill-trigger/src/handler.rs @@ -520,6 +520,17 @@ async fn create_trigger( } } + // Reject a forged superadmin run identity in a preserved permissioned_as + // (the sentinel guard; a trigger's email is derived from it at execution). + let resolved_permissioned_as = new_trigger.base.resolve_permissioned_as(&authed); + windmill_common::auth::validate_on_behalf_of( + &db, + &workspace_id, + Some(&resolved_permissioned_as), + None, + ) + .await?; + let on_behalf_of_info = windmill_common::check_on_behalf_of_preservation( new_trigger.base.permissioned_as.as_deref(), new_trigger.base.preserve_permissioned_as.unwrap_or(false), @@ -754,6 +765,18 @@ async fn update_trigger( let new_path = edit_trigger.base.path.to_string(); let labels = edit_trigger.base.labels.clone(); + + // Reject a forged superadmin run identity in a preserved permissioned_as + // (the sentinel guard; a trigger's email is derived from it at execution). + let resolved_permissioned_as = edit_trigger.base.resolve_permissioned_as(&authed); + windmill_common::auth::validate_on_behalf_of( + &db, + &workspace_id, + Some(&resolved_permissioned_as), + None, + ) + .await?; + let on_behalf_of_info = windmill_common::check_on_behalf_of_preservation( edit_trigger.base.permissioned_as.as_deref(), edit_trigger.base.preserve_permissioned_as.unwrap_or(false),