diff --git a/backend/.sqlx/query-1b1de4e27def09a49065a1deb2cc68dba11f97fc03adb7532d860bce16faeb24.json b/backend/.sqlx/query-1b1de4e27def09a49065a1deb2cc68dba11f97fc03adb7532d860bce16faeb24.json deleted file mode 100644 index f1ffff86be..0000000000 --- a/backend/.sqlx/query-1b1de4e27def09a49065a1deb2cc68dba11f97fc03adb7532d860bce16faeb24.json +++ /dev/null @@ -1,23 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "SELECT CASE\n WHEN $2 LIKE 'u/%' THEN EXISTS (\n SELECT 1 FROM usr WHERE workspace_id = $1 AND username = substring($2 from 3)\n UNION ALL\n SELECT 1 FROM password WHERE super_admin\n AND (username = substring($2 from 3) OR email = substring($2 from 3)))\n WHEN $2 LIKE 'g/%' THEN EXISTS (\n SELECT 1 FROM group_ WHERE workspace_id = $1 AND name = substring($2 from 3))\n ELSE EXISTS (\n SELECT 1 FROM usr WHERE workspace_id = $1 AND username = $2\n UNION ALL\n SELECT 1 FROM password WHERE email = $2 AND super_admin)\n END AS \"resolves!\"", - "describe": { - "columns": [ - { - "ordinal": 0, - "name": "resolves!", - "type_info": "Bool" - } - ], - "parameters": { - "Left": [ - "Text", - "Text" - ] - }, - "nullable": [ - null - ] - }, - "hash": "1b1de4e27def09a49065a1deb2cc68dba11f97fc03adb7532d860bce16faeb24" -} diff --git a/backend/windmill-api-integration-tests/tests/fork_clone_on_behalf_of.rs b/backend/windmill-api-integration-tests/tests/fork_clone_on_behalf_of.rs index dcbc6600a6..2b9f63d392 100644 --- a/backend/windmill-api-integration-tests/tests/fork_clone_on_behalf_of.rs +++ b/backend/windmill-api-integration-tests/tests/fork_clone_on_behalf_of.rs @@ -3,12 +3,11 @@ use sqlx::{Pool, Postgres}; use windmill_test_utils::*; -/// Seed an anonymous public app running as `on_behalf_of`, then fork as `token`. Returns the +/// Seed an anonymous public app owned by the parent's admin, then fork as `token`. Returns the /// cloned app's policy and custom path. async fn fork_with_public_app( db: &Pool, token: &str, - on_behalf_of: (&str, &str), ) -> anyhow::Result<(serde_json::Value, Option)> { initialize_tracing().await; @@ -20,8 +19,8 @@ async fn fork_with_public_app( VALUES ('test-workspace', 'u/test-user/pub', '', $1, '{}', 'pub-path') RETURNING id", json!({ - "on_behalf_of": on_behalf_of.0, - "on_behalf_of_email": on_behalf_of.1, + "on_behalf_of": "u/test-user", + "on_behalf_of_email": "test@windmill.dev", "execution_mode": "anonymous", }) ) @@ -69,8 +68,7 @@ async fn fork_with_public_app( async fn test_fork_downgrades_app_policy_for_unprivileged_creator( db: Pool, ) -> anyhow::Result<()> { - let (policy, custom_path) = - fork_with_public_app(&db, "SECRET_TOKEN_2", ("u/test-user", "test@windmill.dev")).await?; + let (policy, custom_path) = fork_with_public_app(&db, "SECRET_TOKEN_2").await?; assert_eq!(policy["on_behalf_of"], json!("u/test-user-2")); assert_eq!(policy["on_behalf_of_email"], json!("test2@windmill.dev")); @@ -86,9 +84,7 @@ async fn test_fork_downgrades_app_policy_for_unprivileged_creator( /// claiming it make it resolve to either one. #[sqlx::test(migrations = "../migrations", fixtures("base"))] async fn test_fork_keeps_app_policy_for_admin_creator(db: Pool) -> anyhow::Result<()> { - // The fork's creator is the one member it always gets, so their principal resolves there. - let (policy, custom_path) = - fork_with_public_app(&db, "SECRET_TOKEN", ("u/test-user", "test@windmill.dev")).await?; + let (policy, custom_path) = fork_with_public_app(&db, "SECRET_TOKEN").await?; assert_eq!(policy["on_behalf_of"], json!("u/test-user")); assert_eq!(policy["on_behalf_of_email"], json!("test@windmill.dev")); @@ -98,23 +94,6 @@ async fn test_fork_keeps_app_policy_for_admin_creator(db: Pool) -> any Ok(()) } -/// Preserving is still bounded by what the fork can authenticate: `test-user-2` is a parent member -/// the fork never receives, and a policy naming them would only reach `fetch_authed_from_ -/// permissioned_as`'s "user not found in workspace" — with an address left behind for a later -/// re-derivation to trust. -#[sqlx::test(migrations = "../migrations", fixtures("base"))] -async fn test_fork_drops_app_identity_that_names_no_fork_member( - db: Pool, -) -> anyhow::Result<()> { - let (policy, _) = - fork_with_public_app(&db, "SECRET_TOKEN", ("u/test-user-2", "test2@windmill.dev")).await?; - - assert_eq!(policy.get("on_behalf_of"), None); - assert_eq!(policy.get("on_behalf_of_email"), None); - - Ok(()) -} - /// A principal only means something in the workspace whose `usr`/`group_` rows define it, and a /// fork copies the creator and the groups but not the rest of the membership. Carrying one over /// blindly would leave a runnable naming somebody who cannot authenticate there; dropping them diff --git a/backend/windmill-api-workspaces/src/workspaces.rs b/backend/windmill-api-workspaces/src/workspaces.rs index 98fec3c4af..80db26fe47 100644 --- a/backend/windmill-api-workspaces/src/workspaces.rs +++ b/backend/windmill-api-workspaces/src/workspaces.rs @@ -6424,51 +6424,6 @@ fn downgrade_cloned_app_policy(policy: &mut serde_json::Value, authed: &ApiAuthe } } -/// The rule [`clone_scripts`] applies to its column, for a caller holding one principal -/// rather than a whole table: a policy naming somebody who cannot authenticate in the fork -/// keeps no identity at all, address included — that address is all a later re-derivation -/// would have to go on. -async fn drop_unresolvable_policy_identity( - tx: &mut Transaction<'_, Postgres>, - target_workspace_id: &str, - policy: &mut serde_json::Value, -) -> Result<()> { - let Some(obj) = policy.as_object_mut() else { - return Ok(()); - }; - let Some(permissioned_as) = obj - .get("on_behalf_of") - .and_then(|v| v.as_str()) - .map(str::to_string) - else { - return Ok(()); - }; - let resolves = sqlx::query_scalar!( - r#"SELECT CASE - WHEN $2 LIKE 'u/%' THEN EXISTS ( - SELECT 1 FROM usr WHERE workspace_id = $1 AND username = substring($2 from 3) - UNION ALL - SELECT 1 FROM password WHERE super_admin - AND (username = substring($2 from 3) OR email = substring($2 from 3))) - WHEN $2 LIKE 'g/%' THEN EXISTS ( - SELECT 1 FROM group_ WHERE workspace_id = $1 AND name = substring($2 from 3)) - ELSE EXISTS ( - SELECT 1 FROM usr WHERE workspace_id = $1 AND username = $2 - UNION ALL - SELECT 1 FROM password WHERE email = $2 AND super_admin) - END AS "resolves!""#, - target_workspace_id, - permissioned_as, - ) - .fetch_one(&mut **tx) - .await?; - if !resolves { - obj.remove("on_behalf_of"); - obj.remove("on_behalf_of_email"); - } - Ok(()) -} - async fn clone_apps( tx: &mut Transaction<'_, Postgres>, source_workspace_id: &str, @@ -6499,9 +6454,7 @@ async fn clone_apps( if let Some(¤t_version) = app.versions.last() { latest_version_ids.insert(current_version); } - if preserve_identity { - drop_unresolvable_policy_identity(tx, target_workspace_id, &mut app.policy).await?; - } else { + if !preserve_identity { downgrade_cloned_app_policy(&mut app.policy, authed); } // An instance-wide custom path addresses one app (`create_app` rejects one already