diff --git a/backend/.sqlx/query-08a3f282c1f8bded7991baeb5dc14131e707e4438fd1ddf38112fe66ad625691.json b/backend/.sqlx/query-08a3f282c1f8bded7991baeb5dc14131e707e4438fd1ddf38112fe66ad625691.json new file mode 100644 index 0000000000..607ccbc62a --- /dev/null +++ b/backend/.sqlx/query-08a3f282c1f8bded7991baeb5dc14131e707e4438fd1ddf38112fe66ad625691.json @@ -0,0 +1,16 @@ +{ + "db_name": "PostgreSQL", + "query": "UPDATE app SET policy = jsonb_set(policy, ARRAY['on_behalf_of'], to_jsonb($1::text))\n WHERE policy->>'on_behalf_of' = $2 AND workspace_id = $3", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Text", + "Text", + "Text" + ] + }, + "nullable": [] + }, + "hash": "08a3f282c1f8bded7991baeb5dc14131e707e4438fd1ddf38112fe66ad625691" +} diff --git a/backend/.sqlx/query-7dc111544e885bb1d549b9da2a03c8ad60b62797f52cb5d8f153e19b001a33ec.json b/backend/.sqlx/query-7dc111544e885bb1d549b9da2a03c8ad60b62797f52cb5d8f153e19b001a33ec.json new file mode 100644 index 0000000000..8d660cfc94 --- /dev/null +++ b/backend/.sqlx/query-7dc111544e885bb1d549b9da2a03c8ad60b62797f52cb5d8f153e19b001a33ec.json @@ -0,0 +1,16 @@ +{ + "db_name": "PostgreSQL", + "query": "UPDATE draft SET value = to_json(jsonb_set(to_jsonb(value), ARRAY['policy', 'on_behalf_of'], to_jsonb($1::text)))\n WHERE typ IN ('app', 'raw_app')\n AND value->'policy'->>'on_behalf_of' = $2\n AND NOT EXISTS (\n SELECT 1 FROM usr WHERE usr.workspace_id = draft.workspace_id AND usr.username = $3\n )", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Text", + "Text", + "Text" + ] + }, + "nullable": [] + }, + "hash": "7dc111544e885bb1d549b9da2a03c8ad60b62797f52cb5d8f153e19b001a33ec" +} diff --git a/backend/.sqlx/query-c38271cd19861d39f7070799c8ee5eec2d171431d3ce710263eeb16010becfae.json b/backend/.sqlx/query-c38271cd19861d39f7070799c8ee5eec2d171431d3ce710263eeb16010becfae.json new file mode 100644 index 0000000000..1dab1e5b71 --- /dev/null +++ b/backend/.sqlx/query-c38271cd19861d39f7070799c8ee5eec2d171431d3ce710263eeb16010becfae.json @@ -0,0 +1,16 @@ +{ + "db_name": "PostgreSQL", + "query": "UPDATE draft SET value = to_json(jsonb_set(to_jsonb(value), ARRAY['policy', 'on_behalf_of'], to_jsonb($1::text))) WHERE typ IN ('app', 'raw_app') AND value->'policy'->>'on_behalf_of' = $2 AND workspace_id = $3", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Text", + "Text", + "Text" + ] + }, + "nullable": [] + }, + "hash": "c38271cd19861d39f7070799c8ee5eec2d171431d3ce710263eeb16010becfae" +} diff --git a/backend/.sqlx/query-ce4976f6708001a667b0fe6189e46fdb95f9f1cd41ebe42551149ad26578d4ef.json b/backend/.sqlx/query-ce4976f6708001a667b0fe6189e46fdb95f9f1cd41ebe42551149ad26578d4ef.json new file mode 100644 index 0000000000..0ea9260ce1 --- /dev/null +++ b/backend/.sqlx/query-ce4976f6708001a667b0fe6189e46fdb95f9f1cd41ebe42551149ad26578d4ef.json @@ -0,0 +1,16 @@ +{ + "db_name": "PostgreSQL", + "query": "UPDATE app SET policy = jsonb_set(policy, ARRAY['on_behalf_of'], to_jsonb($1::text))\n WHERE policy->>'on_behalf_of' = $2\n AND NOT EXISTS (\n SELECT 1 FROM usr WHERE usr.workspace_id = app.workspace_id AND usr.username = $3\n )", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Text", + "Text", + "Text" + ] + }, + "nullable": [] + }, + "hash": "ce4976f6708001a667b0fe6189e46fdb95f9f1cd41ebe42551149ad26578d4ef" +} 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 dae02f3e5a..7778069b9b 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 @@ -20,7 +20,6 @@ async fn fork_with_public_app( RETURNING id", json!({ "on_behalf_of": "u/test-user", - "on_behalf_of_email": "test@windmill.dev", "execution_mode": "anonymous", }) ) @@ -71,7 +70,6 @@ async fn test_fork_repoints_app_identity_for_unprivileged_creator( 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")); // `execution_mode` rides along untouched — see `clone_apps`. assert_eq!(policy["execution_mode"], json!("anonymous")); assert_eq!(custom_path, None); @@ -88,7 +86,6 @@ async fn test_fork_keeps_app_policy_for_admin_creator(db: Pool) -> any 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")); assert_eq!(policy["execution_mode"], json!("anonymous")); assert_eq!(custom_path, None); @@ -113,7 +110,7 @@ async fn test_compare_ignores_app_identity(db: Pool) -> anyhow::Result VALUES ('test-workspace', $1, 'original', $2, '{}') RETURNING id", path, - json!({ "on_behalf_of": "u/test-user", "on_behalf_of_email": "test@windmill.dev" }) + json!({ "on_behalf_of": "u/test-user" }) ) .fetch_one(&db) .await?; @@ -147,7 +144,7 @@ async fn test_compare_ignores_app_identity(db: Pool) -> anyhow::Result sqlx::query!( "UPDATE app SET policy = policy || $1::jsonb WHERE workspace_id = 'wm-fork-cmp' AND path = 'u/test-user/identity_only'", - json!({ "on_behalf_of": "u/someone-else", "on_behalf_of_email": "else@windmill.dev" }) + json!({ "on_behalf_of": "u/someone-else" }) ) .execute(&db) .await?; diff --git a/backend/windmill-api-integration-tests/tests/users.rs b/backend/windmill-api-integration-tests/tests/users.rs index 65d44509d7..56ca64c832 100644 --- a/backend/windmill-api-integration-tests/tests/users.rs +++ b/backend/windmill-api-integration-tests/tests/users.rs @@ -622,7 +622,7 @@ async fn test_change_user_email(db: Pool) -> anyhow::Result<()> { .await?; sqlx::query!( "INSERT INTO app(workspace_id, path, summary, policy, versions) - VALUES ('test-workspace', 'u/test-user-2/app', '', '{\"on_behalf_of\": \"test2@windmill.dev\", \"on_behalf_of_email\": \"test2@windmill.dev\"}'::jsonb, '{}')" + VALUES ('test-workspace', 'u/test-user-2/app', '', '{\"on_behalf_of\": \"test2@windmill.dev\"}'::jsonb, '{}')" ) .execute(&db) .await?; @@ -827,14 +827,6 @@ async fn test_change_user_email_leaves_group_identities(db: Pool) -> a // moves is the address beside it — kept for the workers that still read it — and in every // pair a group-owned identity keeps the group's synthetic address even though a real account // now holds it: rewriting one half leaves the pair naming two accounts. - sqlx::query!( - "INSERT INTO app(workspace_id, path, summary, policy, versions) - VALUES ('test-workspace', 'u/test-user/g', '', '{\"on_behalf_of\": \"g/ops\", \"on_behalf_of_email\": \"group-ops@windmill.dev\"}'::jsonb, '{}'), - ('test-workspace', 'u/test-user/u', '', '{\"on_behalf_of\": \"u/test-user-2\", \"on_behalf_of_email\": \"group-ops@windmill.dev\"}'::jsonb, '{}')" - ) - .execute(&db) - .await?; - sqlx::query!( "INSERT INTO script (workspace_id, path, hash, content, summary, description, language, created_by, created_at, on_behalf_of, on_behalf_of_email) VALUES ('test-workspace', 'u/test-user/sg', 95001, 'def main(): pass', '', '', 'python3', 'test-user', NOW(), 'g/ops', 'group-ops@windmill.dev'), @@ -857,22 +849,6 @@ async fn test_change_user_email_leaves_group_identities(db: Pool) -> a .await?; assert_eq!(resp.status(), 200, "change_email: {}", resp.text().await?); - let apps = sqlx::query!( - "SELECT path, policy->>'on_behalf_of_email' AS email FROM app WHERE workspace_id = 'test-workspace' ORDER BY path" - ) - .fetch_all(&db) - .await?; - assert_eq!( - apps.iter() - .map(|r| (r.path.as_str(), r.email.as_deref())) - .collect::>(), - vec![ - ("u/test-user/g", Some("group-ops@windmill.dev")), - ("u/test-user/u", Some("renamed@windmill.dev")), - ], - "the group-owned app keeps the group's address; the user-owned one moves" - ); - let scripts = sqlx::query!( "SELECT path, on_behalf_of_email AS email FROM script WHERE workspace_id = 'test-workspace' AND path LIKE 'u/test-user/s%' ORDER BY path" ) diff --git a/backend/windmill-api-users/src/users.rs b/backend/windmill-api-users/src/users.rs index 260df2f969..8b6371f5b5 100644 --- a/backend/windmill-api-users/src/users.rs +++ b/backend/windmill-api-users/src/users.rs @@ -2062,19 +2062,6 @@ async fn change_user_email( .execute(&mut *tx) .await?; - // Apps store an address next to their principal, and the synthetic - // `group-{name}@windmill.dev` may be a real user's, so a group-owned app has to keep its - // address when a colliding user moves: an app running in Anonymous or Publisher mode takes - // its permissions from that pair rather than from the caller, and a half-rewritten pair - // names two accounts. Drafts below carry the same pair and need the same guard. - sqlx::query!( - "UPDATE app SET policy = jsonb_set(policy, ARRAY['on_behalf_of_email'], to_jsonb($1::text)) WHERE policy->>'on_behalf_of_email' = $2 AND (policy->>'on_behalf_of' IS NULL OR policy->>'on_behalf_of' NOT LIKE 'g/%')", - &new_email, - &old_email - ) - .execute(&mut *tx) - .await?; - // ---- permissioned_as naming the account by its address ---- // `usr.username` is constrained to `[\w-]+`, so a workspace member is always named // `u/{username}` and their principals survive an address change untouched. The address form @@ -2103,8 +2090,9 @@ async fn change_user_email( // The address these two keep beside the principal is what a worker predating // MIN_VERSION_SUPPORTS_ON_BEHALF_OF_PRINCIPAL reads, so it follows the account for as long - // as one may be live. Group-owned rows are held back for the reason given above the app - // sweep: their address is the group's, which a colliding user does not take with them. + // as one may be live. Group-owned rows are held back: the synthetic + // `group-{name}@windmill.dev` may be a real user's, and their address is the group's, which + // a colliding user does not take with them — a half-rewritten pair names two accounts. sqlx::query!( "UPDATE script SET on_behalf_of_email = $1 WHERE on_behalf_of_email = $2 AND (on_behalf_of IS NULL OR on_behalf_of NOT LIKE 'g/%')", &new_email, @@ -2132,7 +2120,7 @@ async fn change_user_email( // Script/flow rows store only the principal, but drafts carry an address beside it and // `deployDraft` sends both — left stale it contradicts the principal, which resolves to the // new address, and the deploy is rejected. Group-owned drafts are held back for the reason - // given above the app sweep. + // given above the script sweep. sqlx::query!( r#"UPDATE draft SET value = to_json(jsonb_set(to_jsonb(value), ARRAY['on_behalf_of_email'], to_jsonb($1::text))) WHERE typ IN ('script', 'flow') AND value->>'on_behalf_of_email' = $2 AND (value->>'on_behalf_of' IS NULL OR value->>'on_behalf_of' NOT LIKE 'g/%')"#, &new_email, @@ -2158,19 +2146,6 @@ async fn change_user_email( .execute(&mut *tx) .await?; - // A raw-app draft persists the address the client read back too. The deploy sends it beside - // the principal, where an address naming somebody else is rejected — and unlike a live read - // it never refreshes on its own. Same group guard as the deployed policy above, plus the - // `IS NULL` arm: without it the predicate is `NULL` for a draft with no principal, which is - // neither true nor false, so those rows would be skipped. - sqlx::query!( - r#"UPDATE draft SET value = to_json(jsonb_set(to_jsonb(value), ARRAY['policy', 'on_behalf_of_email'], to_jsonb($1::text))) WHERE typ IN ('app', 'raw_app') AND value->'policy'->>'on_behalf_of_email' = $2 AND (value->'policy'->>'on_behalf_of' IS NULL OR value->'policy'->>'on_behalf_of' NOT LIKE 'g/%')"#, - &new_email, - &old_email - ) - .execute(&mut *tx) - .await?; - // A folder's default rules are an ordered array, first match wins, so the rewrite has to // preserve their order. A rule left on the old address makes `ensure_permissioned_as_exists` // reject the creation of every runnable the rule matches. diff --git a/backend/windmill-api-workspaces/src/workspaces.rs b/backend/windmill-api-workspaces/src/workspaces.rs index 879e739dc0..f9bf9625f3 100644 --- a/backend/windmill-api-workspaces/src/workspaces.rs +++ b/backend/windmill-api-workspaces/src/workspaces.rs @@ -6995,10 +6995,6 @@ fn repoint_cloned_app_identity(policy: &mut serde_json::Value, authed: &ApiAuthe "on_behalf_of".to_string(), serde_json::Value::String(username_to_permissioned_as(&authed.username)), ); - obj.insert( - "on_behalf_of_email".to_string(), - serde_json::Value::String(authed.email.clone()), - ); } async fn clone_apps( diff --git a/backend/windmill-api/src/offboarding.rs b/backend/windmill-api/src/offboarding.rs index 26eb06c8e8..736b81d28f 100644 --- a/backend/windmill-api/src/offboarding.rs +++ b/backend/windmill-api/src/offboarding.rs @@ -957,26 +957,22 @@ async fn offboard_user_from_workspace<'c>( .unwrap_or(0); sqlx::query!( - "UPDATE app SET policy = jsonb_set( - jsonb_set(policy, ARRAY['on_behalf_of'], to_jsonb($1::text)), - ARRAY['on_behalf_of_email'], to_jsonb($4::text) - ) WHERE policy->>'on_behalf_of' = $2 AND workspace_id = $3", + "UPDATE app SET policy = jsonb_set(policy, ARRAY['on_behalf_of'], to_jsonb($1::text)) + WHERE policy->>'on_behalf_of' = $2 AND workspace_id = $3", &new_permissioned_as, departing.as_deref(), - w_id, - new_on_behalf_of_user_email + w_id ) .execute(&mut **tx) .await?; // An app draft carries a copy of the deployed policy and is deployed from it, so it needs - // the same pair rewritten — the draft sweep above only covers scripts and flows. + // the same principal rewritten — the draft sweep above only covers scripts and flows. sqlx::query!( - r#"UPDATE draft SET value = to_json(jsonb_set(jsonb_set(to_jsonb(value), ARRAY['policy', 'on_behalf_of'], to_jsonb($1::text)), ARRAY['policy', 'on_behalf_of_email'], to_jsonb($4::text))) WHERE typ IN ('app', 'raw_app') AND value->'policy'->>'on_behalf_of' = $2 AND workspace_id = $3"#, + r#"UPDATE draft SET value = to_json(jsonb_set(to_jsonb(value), ARRAY['policy', 'on_behalf_of'], to_jsonb($1::text))) WHERE typ IN ('app', 'raw_app') AND value->'policy'->>'on_behalf_of' = $2 AND workspace_id = $3"#, new_permissioned_as, departing.as_deref(), - w_id, - new_on_behalf_of_user_email + w_id ) .execute(&mut **tx) .await?; diff --git a/backend/windmill-api/src/users.rs b/backend/windmill-api/src/users.rs index 4dd073b3af..1e3d3e73e6 100644 --- a/backend/windmill-api/src/users.rs +++ b/backend/windmill-api/src/users.rs @@ -277,18 +277,22 @@ async fn rename_user( // The per-workspace sweep below only reaches accounts with a `usr` row. A superadmin acting // outside their workspaces has none, yet an app can name them: their principal is - // `u/{password.username}`, which this rename just moved. Matching on the address as well - // keeps a like-named member of some other workspace out of it. + // `u/{password.username}`, which this rename just moved. The `NOT EXISTS` is what keeps a + // like-named member of some other workspace out of it — in a workspace holding that + // username, `resolve_username_to_email` answers from `usr` and never reaches this account, + // and the per-workspace sweep is what moves those rows. if let Some(old_username) = old_instance_username.filter(|u| *u != ru.new_username) { let old_principal = windmill_common::users::username_to_permissioned_as(&old_username); - let new_principal = - windmill_common::users::username_to_permissioned_as(&ru.new_username); + let new_principal = windmill_common::users::username_to_permissioned_as(&ru.new_username); sqlx::query!( "UPDATE app SET policy = jsonb_set(policy, ARRAY['on_behalf_of'], to_jsonb($1::text)) - WHERE policy->>'on_behalf_of' = $2 AND policy->>'on_behalf_of_email' = $3", + WHERE policy->>'on_behalf_of' = $2 + AND NOT EXISTS ( + SELECT 1 FROM usr WHERE usr.workspace_id = app.workspace_id AND usr.username = $3 + )", &new_principal, &old_principal, - user_email + &old_username ) .execute(&mut *tx) .await?; @@ -296,10 +300,12 @@ async fn rename_user( r#"UPDATE draft SET value = to_json(jsonb_set(to_jsonb(value), ARRAY['policy', 'on_behalf_of'], to_jsonb($1::text))) WHERE typ IN ('app', 'raw_app') AND value->'policy'->>'on_behalf_of' = $2 - AND value->'policy'->>'on_behalf_of_email' = $3"#, + AND NOT EXISTS ( + SELECT 1 FROM usr WHERE usr.workspace_id = draft.workspace_id AND usr.username = $3 + )"#, &new_principal, &old_principal, - user_email + &old_username ) .execute(&mut *tx) .await?;