refactor: drop the app address from the identity sweeps

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EYWxvwmteVyzcWenJfbnzs
This commit is contained in:
Ruben Fiszel
2026-09-12 11:46:27 +02:00
co-authored by Claude Opus 5
parent 673e3f26e7
commit 8deab82f56
10 changed files with 91 additions and 81 deletions
@@ -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"
}
@@ -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"
}
@@ -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"
}
@@ -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"
}
@@ -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<Postgres>) -> 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<Postgres>) -> 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<Postgres>) -> 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?;
@@ -622,7 +622,7 @@ async fn test_change_user_email(db: Pool<Postgres>) -> 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<Postgres>) -> 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<Postgres>) -> 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<_>>(),
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"
)
+4 -29
View File
@@ -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.
@@ -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(
+6 -10
View File
@@ -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?;
+14 -8
View File
@@ -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?;