diff --git a/backend/.sqlx/query-077467cd813d5af161cb1cc232724f26984822d4c28ba36c0a9331273b10edc0.json b/backend/.sqlx/query-077467cd813d5af161cb1cc232724f26984822d4c28ba36c0a9331273b10edc0.json new file mode 100644 index 0000000000..0f5113bac5 --- /dev/null +++ b/backend/.sqlx/query-077467cd813d5af161cb1cc232724f26984822d4c28ba36c0a9331273b10edc0.json @@ -0,0 +1,12 @@ +{ + "db_name": "PostgreSQL", + "query": "INSERT INTO email_to_igroup (email, igroup) VALUES ('alice@example.com', 'admins') ON CONFLICT DO NOTHING", + "describe": { + "columns": [], + "parameters": { + "Left": [] + }, + "nullable": [] + }, + "hash": "077467cd813d5af161cb1cc232724f26984822d4c28ba36c0a9331273b10edc0" +} diff --git a/backend/.sqlx/query-1721f8b52ea265c0537fd7c742deddf0afbe5cf0d81b15e487c411ae169d3a89.json b/backend/.sqlx/query-1721f8b52ea265c0537fd7c742deddf0afbe5cf0d81b15e487c411ae169d3a89.json new file mode 100644 index 0000000000..ba4ad17175 --- /dev/null +++ b/backend/.sqlx/query-1721f8b52ea265c0537fd7c742deddf0afbe5cf0d81b15e487c411ae169d3a89.json @@ -0,0 +1,20 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT igroup FROM email_to_igroup WHERE email = 'alice@example.com'", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "igroup", + "type_info": "Varchar" + } + ], + "parameters": { + "Left": [] + }, + "nullable": [ + false + ] + }, + "hash": "1721f8b52ea265c0537fd7c742deddf0afbe5cf0d81b15e487c411ae169d3a89" +} diff --git a/backend/.sqlx/query-250a4e3f1a1f95296f7075bf8780e9c7407e89c8f7636484895e99f5a5e71297.json b/backend/.sqlx/query-250a4e3f1a1f95296f7075bf8780e9c7407e89c8f7636484895e99f5a5e71297.json new file mode 100644 index 0000000000..6fd286898a --- /dev/null +++ b/backend/.sqlx/query-250a4e3f1a1f95296f7075bf8780e9c7407e89c8f7636484895e99f5a5e71297.json @@ -0,0 +1,15 @@ +{ + "db_name": "PostgreSQL", + "query": "INSERT INTO usr (workspace_id, username, email, is_admin, operator, added_via)\n VALUES ($1, 'alice', 'alice@example.com', false, true, $2)", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Varchar", + "Jsonb" + ] + }, + "nullable": [] + }, + "hash": "250a4e3f1a1f95296f7075bf8780e9c7407e89c8f7636484895e99f5a5e71297" +} diff --git a/backend/.sqlx/query-2ba03e555d2e09dbd0e2ae5ddfd9a268a675bdb23615c78904cebe7f1e31f400.json b/backend/.sqlx/query-2ba03e555d2e09dbd0e2ae5ddfd9a268a675bdb23615c78904cebe7f1e31f400.json new file mode 100644 index 0000000000..3f4b2f53ab --- /dev/null +++ b/backend/.sqlx/query-2ba03e555d2e09dbd0e2ae5ddfd9a268a675bdb23615c78904cebe7f1e31f400.json @@ -0,0 +1,15 @@ +{ + "db_name": "PostgreSQL", + "query": "INSERT INTO usr (workspace_id, username, email, is_admin, operator, added_via)\n VALUES ($1, 'alice', 'alice@example.com', true, false, $2)", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Varchar", + "Jsonb" + ] + }, + "nullable": [] + }, + "hash": "2ba03e555d2e09dbd0e2ae5ddfd9a268a675bdb23615c78904cebe7f1e31f400" +} diff --git a/backend/.sqlx/query-3bd4f38a1629a69ddda622b6b436198b47c2fe1a507358d49f05031e7beedab6.json b/backend/.sqlx/query-3bd4f38a1629a69ddda622b6b436198b47c2fe1a507358d49f05031e7beedab6.json new file mode 100644 index 0000000000..0c71ee8ad2 --- /dev/null +++ b/backend/.sqlx/query-3bd4f38a1629a69ddda622b6b436198b47c2fe1a507358d49f05031e7beedab6.json @@ -0,0 +1,34 @@ +{ + "db_name": "PostgreSQL", + "query": "\n SELECT workspace_id,\n auto_invite->'instance_groups_roles' as instance_groups_roles,\n auto_invite->'instance_groups' as instance_groups_json\n FROM workspace_settings\n WHERE auto_invite->'instance_groups' ? $1\n ", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "workspace_id", + "type_info": "Varchar" + }, + { + "ordinal": 1, + "name": "instance_groups_roles", + "type_info": "Jsonb" + }, + { + "ordinal": 2, + "name": "instance_groups_json", + "type_info": "Jsonb" + } + ], + "parameters": { + "Left": [ + "Text" + ] + }, + "nullable": [ + false, + null, + null + ] + }, + "hash": "3bd4f38a1629a69ddda622b6b436198b47c2fe1a507358d49f05031e7beedab6" +} diff --git a/backend/.sqlx/query-5d160ba4958583f1ad42de846c544d8d8e81e1b54925a0c5f2cedc1817d99a1b.json b/backend/.sqlx/query-5d160ba4958583f1ad42de846c544d8d8e81e1b54925a0c5f2cedc1817d99a1b.json new file mode 100644 index 0000000000..8fcffe364a --- /dev/null +++ b/backend/.sqlx/query-5d160ba4958583f1ad42de846c544d8d8e81e1b54925a0c5f2cedc1817d99a1b.json @@ -0,0 +1,22 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT igroup FROM email_to_igroup WHERE email = $1", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "igroup", + "type_info": "Varchar" + } + ], + "parameters": { + "Left": [ + "Text" + ] + }, + "nullable": [ + false + ] + }, + "hash": "5d160ba4958583f1ad42de846c544d8d8e81e1b54925a0c5f2cedc1817d99a1b" +} diff --git a/backend/.sqlx/query-62e8e443cf063fcb30799d9c8971c00d761d54811936deb87a0315ca9cdc9769.json b/backend/.sqlx/query-62e8e443cf063fcb30799d9c8971c00d761d54811936deb87a0315ca9cdc9769.json new file mode 100644 index 0000000000..f4a2546900 --- /dev/null +++ b/backend/.sqlx/query-62e8e443cf063fcb30799d9c8971c00d761d54811936deb87a0315ca9cdc9769.json @@ -0,0 +1,32 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT is_admin, operator, added_via FROM usr WHERE workspace_id = 'ws-multi-group' AND email = 'alice@example.com'", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "is_admin", + "type_info": "Bool" + }, + { + "ordinal": 1, + "name": "operator", + "type_info": "Bool" + }, + { + "ordinal": 2, + "name": "added_via", + "type_info": "Jsonb" + } + ], + "parameters": { + "Left": [] + }, + "nullable": [ + false, + false, + true + ] + }, + "hash": "62e8e443cf063fcb30799d9c8971c00d761d54811936deb87a0315ca9cdc9769" +} diff --git a/backend/.sqlx/query-7e01ef5799168c0fc2779d42ce352827e2fda6711c0a1b104ca6435ddb14b47d.json b/backend/.sqlx/query-66e2f8468ba64f22b7a7caa18639d7c833ac2ec573bd89d878b5c8b1afc74d3a.json similarity index 54% rename from backend/.sqlx/query-7e01ef5799168c0fc2779d42ce352827e2fda6711c0a1b104ca6435ddb14b47d.json rename to backend/.sqlx/query-66e2f8468ba64f22b7a7caa18639d7c833ac2ec573bd89d878b5c8b1afc74d3a.json index 7c8a5bc156..15681aab51 100644 --- a/backend/.sqlx/query-7e01ef5799168c0fc2779d42ce352827e2fda6711c0a1b104ca6435ddb14b47d.json +++ b/backend/.sqlx/query-66e2f8468ba64f22b7a7caa18639d7c833ac2ec573bd89d878b5c8b1afc74d3a.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "\n SELECT\n workspace_id,\n auto_invite->'instance_groups_roles' as instance_groups_roles\n FROM workspace_settings\n WHERE\n auto_invite->'instance_groups' IS NOT NULL\n AND auto_invite->'instance_groups' ? $1\n ", + "query": "\n SELECT\n workspace_id,\n auto_invite->'instance_groups_roles' as instance_groups_roles,\n auto_invite->'instance_groups' as instance_groups_json\n FROM workspace_settings\n WHERE\n auto_invite->'instance_groups' IS NOT NULL\n AND auto_invite->'instance_groups' ? $1\n ", "describe": { "columns": [ { @@ -12,6 +12,11 @@ "ordinal": 1, "name": "instance_groups_roles", "type_info": "Jsonb" + }, + { + "ordinal": 2, + "name": "instance_groups_json", + "type_info": "Jsonb" } ], "parameters": { @@ -21,8 +26,9 @@ }, "nullable": [ false, + null, null ] }, - "hash": "7e01ef5799168c0fc2779d42ce352827e2fda6711c0a1b104ca6435ddb14b47d" + "hash": "66e2f8468ba64f22b7a7caa18639d7c833ac2ec573bd89d878b5c8b1afc74d3a" } diff --git a/backend/.sqlx/query-6f941e4454f736b32eaef80cdfb9582d6e75af3dc159e7c5f12497d3957f1eef.json b/backend/.sqlx/query-6f941e4454f736b32eaef80cdfb9582d6e75af3dc159e7c5f12497d3957f1eef.json new file mode 100644 index 0000000000..8446e14aa4 --- /dev/null +++ b/backend/.sqlx/query-6f941e4454f736b32eaef80cdfb9582d6e75af3dc159e7c5f12497d3957f1eef.json @@ -0,0 +1,12 @@ +{ + "db_name": "PostgreSQL", + "query": "INSERT INTO usr (workspace_id, username, email, is_admin, operator)\n VALUES ('ws-multi-group', 'alice', 'alice@example.com', true, false)", + "describe": { + "columns": [], + "parameters": { + "Left": [] + }, + "nullable": [] + }, + "hash": "6f941e4454f736b32eaef80cdfb9582d6e75af3dc159e7c5f12497d3957f1eef" +} diff --git a/backend/.sqlx/query-88a467f3c943b134a81ac69c3c6686d1ce1ff2f5aafc15ff1b63cfa86c09c4f0.json b/backend/.sqlx/query-88a467f3c943b134a81ac69c3c6686d1ce1ff2f5aafc15ff1b63cfa86c09c4f0.json new file mode 100644 index 0000000000..6ee3c89e40 --- /dev/null +++ b/backend/.sqlx/query-88a467f3c943b134a81ac69c3c6686d1ce1ff2f5aafc15ff1b63cfa86c09c4f0.json @@ -0,0 +1,26 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT is_admin, operator FROM usr WHERE workspace_id = 'ws-multi-group' AND email = 'alice@example.com'", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "is_admin", + "type_info": "Bool" + }, + { + "ordinal": 1, + "name": "operator", + "type_info": "Bool" + } + ], + "parameters": { + "Left": [] + }, + "nullable": [ + false, + false + ] + }, + "hash": "88a467f3c943b134a81ac69c3c6686d1ce1ff2f5aafc15ff1b63cfa86c09c4f0" +} diff --git a/backend/.sqlx/query-b38bd869477a729279cac3ccd4825191fb49e17e5f7e7297c0c819f52b486f49.json b/backend/.sqlx/query-b38bd869477a729279cac3ccd4825191fb49e17e5f7e7297c0c819f52b486f49.json new file mode 100644 index 0000000000..955fee77ad --- /dev/null +++ b/backend/.sqlx/query-b38bd869477a729279cac3ccd4825191fb49e17e5f7e7297c0c819f52b486f49.json @@ -0,0 +1,18 @@ +{ + "db_name": "PostgreSQL", + "query": "UPDATE usr SET is_admin = $1, operator = $2, added_via = $3 WHERE workspace_id = $4 AND email = $5 AND added_via->>'source' = 'instance_group'", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Bool", + "Bool", + "Jsonb", + "Text", + "Text" + ] + }, + "nullable": [] + }, + "hash": "b38bd869477a729279cac3ccd4825191fb49e17e5f7e7297c0c819f52b486f49" +} diff --git a/backend/.sqlx/query-e58ef252b0d2b81e9cd76f394a396abefd791906ada29dd5a7a9148157635ca5.json b/backend/.sqlx/query-e58ef252b0d2b81e9cd76f394a396abefd791906ada29dd5a7a9148157635ca5.json new file mode 100644 index 0000000000..855c628c1f --- /dev/null +++ b/backend/.sqlx/query-e58ef252b0d2b81e9cd76f394a396abefd791906ada29dd5a7a9148157635ca5.json @@ -0,0 +1,26 @@ +{ + "db_name": "PostgreSQL", + "query": "\n SELECT auto_invite->'instance_groups_roles' as instance_groups_roles,\n auto_invite->'instance_groups' as instance_groups_json\n FROM workspace_settings WHERE workspace_id = 'ws-multi-group'\n ", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "instance_groups_roles", + "type_info": "Jsonb" + }, + { + "ordinal": 1, + "name": "instance_groups_json", + "type_info": "Jsonb" + } + ], + "parameters": { + "Left": [] + }, + "nullable": [ + null, + null + ] + }, + "hash": "e58ef252b0d2b81e9cd76f394a396abefd791906ada29dd5a7a9148157635ca5" +} diff --git a/backend/ee-repo-ref.txt b/backend/ee-repo-ref.txt index 3fa40f90dc..07aa65d1f5 100644 --- a/backend/ee-repo-ref.txt +++ b/backend/ee-repo-ref.txt @@ -1 +1 @@ -5ee1382dfb23b6a1516e3c7586058cec8240fdf2 +e08a87450627bef9013498e40ee93a47bedda7ee diff --git a/backend/windmill-api-groups/src/groups.rs b/backend/windmill-api-groups/src/groups.rs index 9acc840832..af67f884c9 100644 --- a/backend/windmill-api-groups/src/groups.rs +++ b/backend/windmill-api-groups/src/groups.rs @@ -851,9 +851,21 @@ async fn add_user_igroup( #[cfg(all(feature = "private", feature = "enterprise"))] { use windmill_api_workspaces::workspaces_ee::auto_add_user; + use windmill_common::users::compute_highest_workspace_role; + + // Find all instance groups this user belongs to (includes the newly added group) + let user_igroups: Vec = sqlx::query_scalar!( + "SELECT igroup FROM email_to_igroup WHERE email = $1", + &email + ) + .fetch_all(&mut *tx) + .await?; + let workspaces = sqlx::query!( r#" - SELECT workspace_id, auto_invite->'instance_groups_roles' as instance_groups_roles + SELECT workspace_id, + auto_invite->'instance_groups_roles' as instance_groups_roles, + auto_invite->'instance_groups' as instance_groups_json FROM workspace_settings WHERE auto_invite->'instance_groups' ? $1 "#, @@ -861,34 +873,53 @@ async fn add_user_igroup( ) .fetch_all(&mut *tx) .await?; + for ws in workspaces { - let role = ws + let roles: std::collections::HashMap = ws .instance_groups_roles - .and_then(|r| r.get(&name).and_then(|v| v.as_str().map(String::from))) - .unwrap_or_else(|| "developer".to_string()); - let (is_admin, is_operator) = match role.as_str() { - "admin" => (true, false), - "operator" => (false, true), - _ => (false, false), - }; + .and_then(|r| serde_json::from_value(r).ok()) + .unwrap_or_default(); + + let ws_configured_groups: Vec = ws + .instance_groups_json + .and_then(|ig| serde_json::from_value(ig).ok()) + .unwrap_or_default(); + + let (best_group, is_admin, is_operator) = + compute_highest_workspace_role(&user_igroups, &ws_configured_groups, &roles); + + let instance_group_source = serde_json::json!({ + "source": "instance_group", + "group": &best_group + }); + + // auto_add_user creates the user if they don't exist (ON CONFLICT DO NOTHING). + // The operator flag here doesn't matter for the final state — the UPDATE below + // always sets the correct is_admin/operator based on the highest-precedence role. auto_add_user( &email, &ws.workspace_id, - &is_operator, + &false, &mut tx, &authed, - Some(serde_json::json!({"source": "instance_group", "group": &name})), + Some(instance_group_source.clone()), ) .await?; - if is_admin { - sqlx::query!( - "UPDATE usr SET is_admin = true WHERE workspace_id = $1 AND email = $2", - &ws.workspace_id, - &email - ) - .execute(&mut *tx) - .await?; - } + + // Set the correct role based on highest precedence across all groups. + // For new users, auto_add_user already stored added_via with source=instance_group, + // so this UPDATE will match. For existing instance_group users, it upgrades/corrects + // the role. Manually-added users (added_via is NULL or non-instance_group) are not affected. + sqlx::query!( + "UPDATE usr SET is_admin = $1, operator = $2, added_via = $3 WHERE workspace_id = $4 AND email = $5 AND added_via->>'source' = 'instance_group'", + is_admin, + is_operator, + &instance_group_source, + &ws.workspace_id, + &email + ) + .execute(&mut *tx) + .await?; } } diff --git a/backend/windmill-common/src/users.rs b/backend/windmill-common/src/users.rs index 453466ac7d..f91c21701a 100644 --- a/backend/windmill-common/src/users.rs +++ b/backend/windmill-common/src/users.rs @@ -81,6 +81,46 @@ pub async fn get_email_from_permissioned_as( } } +/// Compute the highest-precedence workspace role for a user across all their instance groups. +/// +/// Precedence: admin (3) > developer (2) > operator (1). +/// Returns `(best_group_name, is_admin, is_operator)`. +pub fn compute_highest_workspace_role( + user_igroups: &[String], + ws_configured_groups: &[String], + ws_roles: &std::collections::HashMap, +) -> (String, bool, bool) { + let mut best_group = String::new(); + let mut best_precedence = 0u8; + + for group in user_igroups { + if !ws_configured_groups.contains(group) { + continue; + } + let default_role = "developer".to_string(); + let role = ws_roles.get(group).unwrap_or(&default_role); + let precedence = match role.as_str() { + "admin" => 3u8, + "operator" => 1, + _ => 2, + }; + if precedence > best_precedence { + best_precedence = precedence; + best_group = group.clone(); + } + } + + let default_role = "developer".to_string(); + let best_role_str = ws_roles.get(&best_group).unwrap_or(&default_role); + let (is_admin, is_operator) = match best_role_str.as_str() { + "admin" => (true, false), + "operator" => (false, true), + _ => (false, false), + }; + + (best_group, is_admin, is_operator) +} + pub fn truncate_token(token: &str) -> String { if token.len() > 10 { let mut s = token[..10].to_owned(); @@ -105,4 +145,63 @@ mod tests { assert_eq!(username_to_permissioned_as("group-all"), "g/all"); assert_eq!(username_to_permissioned_as("group-my-team"), "g/my-team"); } + + #[test] + fn test_compute_highest_workspace_role_admin_wins() { + let user_groups = vec!["ops".to_string(), "admins".to_string()]; + let ws_groups = vec!["ops".to_string(), "admins".to_string()]; + let mut roles = std::collections::HashMap::new(); + roles.insert("ops".to_string(), "operator".to_string()); + roles.insert("admins".to_string(), "admin".to_string()); + + let (group, is_admin, is_operator) = + compute_highest_workspace_role(&user_groups, &ws_groups, &roles); + assert_eq!(group, "admins"); + assert!(is_admin); + assert!(!is_operator); + } + + #[test] + fn test_compute_highest_workspace_role_developer_over_operator() { + let user_groups = vec!["devs".to_string(), "ops".to_string()]; + let ws_groups = vec!["devs".to_string(), "ops".to_string()]; + let mut roles = std::collections::HashMap::new(); + roles.insert("devs".to_string(), "developer".to_string()); + roles.insert("ops".to_string(), "operator".to_string()); + + let (group, is_admin, is_operator) = + compute_highest_workspace_role(&user_groups, &ws_groups, &roles); + assert_eq!(group, "devs"); + assert!(!is_admin); + assert!(!is_operator); + } + + #[test] + fn test_compute_highest_workspace_role_skips_unconfigured_groups() { + let user_groups = vec!["admins".to_string(), "other".to_string()]; + let ws_groups = vec!["ops".to_string()]; // admins not configured for this workspace + let mut roles = std::collections::HashMap::new(); + roles.insert("admins".to_string(), "admin".to_string()); + roles.insert("ops".to_string(), "operator".to_string()); + + let (group, is_admin, is_operator) = + compute_highest_workspace_role(&user_groups, &ws_groups, &roles); + // No user groups match ws_configured_groups, so best_group stays empty + assert_eq!(group, ""); + assert!(!is_admin); + assert!(!is_operator); + } + + #[test] + fn test_compute_highest_workspace_role_defaults_to_developer() { + let user_groups = vec!["team".to_string()]; + let ws_groups = vec!["team".to_string()]; + let roles = std::collections::HashMap::new(); // no role configured → developer + + let (group, is_admin, is_operator) = + compute_highest_workspace_role(&user_groups, &ws_groups, &roles); + assert_eq!(group, "team"); + assert!(!is_admin); + assert!(!is_operator); + } } diff --git a/backend/windmill-common/tests/instance_group_auto_add.rs b/backend/windmill-common/tests/instance_group_auto_add.rs index e8a6c841c6..95b20cc842 100644 --- a/backend/windmill-common/tests/instance_group_auto_add.rs +++ b/backend/windmill-common/tests/instance_group_auto_add.rs @@ -28,10 +28,14 @@ mod tests { use serde_json::json; use sqlx::{Pool, Postgres}; + use windmill_common::users::compute_highest_workspace_role; /// Test that configuring instance groups for a workspace auto-adds existing group members #[ignore = "requires database setup - run with --ignored flag"] - #[sqlx::test(migrations = "../migrations", fixtures("base", "instance_group_auto_add"))] + #[sqlx::test( + migrations = "../migrations", + fixtures("base", "instance_group_auto_add") + )] async fn test_configure_instance_groups_adds_existing_members(db: Pool) { // Configure workspace to auto-add users from 'engineering' group with 'developer' role let groups = vec!["engineering".to_string()]; @@ -112,8 +116,14 @@ mod tests { "Alice should be in the workspace" ); let alice = alice_in_workspace.unwrap(); - assert!(!alice.is_admin, "Alice should not be admin (developer role)"); - assert!(!alice.operator, "Alice should not be operator (developer role)"); + assert!( + !alice.is_admin, + "Alice should not be admin (developer role)" + ); + assert!( + !alice.operator, + "Alice should not be operator (developer role)" + ); // Check added_via field let added_via = alice.added_via.expect("added_via should be set"); @@ -158,7 +168,10 @@ mod tests { /// Test role assignment based on instance group configuration #[ignore = "requires database setup - run with --ignored flag"] - #[sqlx::test(migrations = "../migrations", fixtures("base", "instance_group_auto_add"))] + #[sqlx::test( + migrations = "../migrations", + fixtures("base", "instance_group_auto_add") + )] async fn test_role_assignment_admin(db: Pool) { // Configure workspace with admins group having admin role let groups = vec!["admins".to_string()]; @@ -209,7 +222,10 @@ mod tests { /// Test role assignment for operator #[ignore = "requires database setup - run with --ignored flag"] - #[sqlx::test(migrations = "../migrations", fixtures("base", "instance_group_auto_add"))] + #[sqlx::test( + migrations = "../migrations", + fixtures("base", "instance_group_auto_add") + )] async fn test_role_assignment_operator(db: Pool) { // Configure workspace with sales group having operator role let groups = vec!["sales".to_string()]; @@ -260,7 +276,10 @@ mod tests { /// Test role precedence when user is in multiple instance groups #[ignore = "requires database setup - run with --ignored flag"] - #[sqlx::test(migrations = "../migrations", fixtures("base", "instance_group_auto_add"))] + #[sqlx::test( + migrations = "../migrations", + fixtures("base", "instance_group_auto_add") + )] async fn test_role_precedence_multiple_groups(db: Pool) { // Configure workspace with multiple groups: engineering (admin), sales (operator) // Bob is in both groups, should get admin role (highest precedence) @@ -306,7 +325,10 @@ mod tests { .await .expect("Failed to query user"); - assert!(bob.is_admin, "Bob should be admin (highest precedence role)"); + assert!( + bob.is_admin, + "Bob should be admin (highest precedence role)" + ); assert!(!bob.operator, "Bob should not be operator"); // Verify added_via tracks the primary group (engineering, the one with highest precedence) @@ -320,9 +342,305 @@ mod tests { println!("✓ Role precedence works correctly for users in multiple groups"); } + /// Test that adding a user to a second instance group upgrades their workspace role + /// if the new group has a higher-precedence role. + /// This is a regression test for the bug where only the newly-added group's role was used. + #[ignore = "requires database setup - run with --ignored flag"] + #[sqlx::test( + migrations = "../migrations", + fixtures("base", "instance_group_auto_add") + )] + async fn test_role_upgrade_when_added_to_higher_group(db: Pool) { + // Configure workspace: engineering=operator, admins=admin + let groups = vec!["engineering".to_string(), "admins".to_string()]; + let roles = json!({"engineering": "operator", "admins": "admin"}); + + sqlx::query!( + r#" + UPDATE workspace_settings + SET auto_invite = jsonb_build_object( + 'instance_groups', $2::jsonb, + 'instance_groups_roles', $3::jsonb + ) + WHERE workspace_id = $1 + "#, + "ws-multi-group", + serde_json::to_value(&groups).unwrap(), + &roles, + ) + .execute(&db) + .await + .expect("Failed to update workspace settings"); + + // Step 1: Alice is added via engineering group (operator) + // (alice is already in engineering from fixture) + let added_via = json!({"source": "instance_group", "group": "engineering"}); + sqlx::query!( + "INSERT INTO usr (workspace_id, username, email, is_admin, operator, added_via) + VALUES ($1, 'alice', 'alice@example.com', false, true, $2)", + "ws-multi-group", + &added_via, + ) + .execute(&db) + .await + .expect("Failed to add user"); + + sqlx::query!( + "INSERT INTO usr_to_group (workspace_id, usr, group_) VALUES ($1, 'alice', 'all')", + "ws-multi-group", + ) + .execute(&db) + .await + .expect("Failed to add user to all group"); + + // Verify initial state: alice is operator + let alice = sqlx::query!( + "SELECT is_admin, operator FROM usr WHERE workspace_id = 'ws-multi-group' AND email = 'alice@example.com'" + ) + .fetch_one(&db) + .await + .expect("Failed to query user"); + assert!(!alice.is_admin, "Alice should start as non-admin"); + assert!(alice.operator, "Alice should start as operator"); + + // Step 2: Alice is added to admins group + sqlx::query!( + "INSERT INTO email_to_igroup (email, igroup) VALUES ('alice@example.com', 'admins') ON CONFLICT DO NOTHING" + ) + .execute(&db) + .await + .expect("Failed to add to admins group"); + + // Step 3: Simulate the fixed logic — find all user's groups, compute highest role, update + let user_igroups: Vec = sqlx::query_scalar!( + "SELECT igroup FROM email_to_igroup WHERE email = 'alice@example.com'" + ) + .fetch_all(&db) + .await + .expect("Failed to fetch user groups"); + + let ws = sqlx::query!( + r#" + SELECT auto_invite->'instance_groups_roles' as instance_groups_roles, + auto_invite->'instance_groups' as instance_groups_json + FROM workspace_settings WHERE workspace_id = 'ws-multi-group' + "#, + ) + .fetch_one(&db) + .await + .expect("Failed to fetch workspace settings"); + + let ws_roles: std::collections::HashMap = ws + .instance_groups_roles + .and_then(|r| serde_json::from_value(r).ok()) + .unwrap_or_default(); + + let ws_configured_groups: Vec = ws + .instance_groups_json + .and_then(|ig| serde_json::from_value(ig).ok()) + .unwrap_or_default(); + + let (best_group, is_admin, is_operator) = + compute_highest_workspace_role(&user_igroups, &ws_configured_groups, &ws_roles); + + let instance_group_source = json!({ + "source": "instance_group", + "group": &best_group + }); + + sqlx::query!( + "UPDATE usr SET is_admin = $1, operator = $2, added_via = $3 WHERE workspace_id = $4 AND email = $5 AND added_via->>'source' = 'instance_group'", + is_admin, + is_operator, + &instance_group_source, + "ws-multi-group", + "alice@example.com" + ) + .execute(&db) + .await + .expect("Failed to update user role"); + + // Verify: alice should now be admin (highest precedence) + let alice = sqlx::query!( + "SELECT is_admin, operator, added_via FROM usr WHERE workspace_id = 'ws-multi-group' AND email = 'alice@example.com'" + ) + .fetch_one(&db) + .await + .expect("Failed to query user"); + + assert!(alice.is_admin, "Alice should be upgraded to admin"); + assert!(!alice.operator, "Alice should no longer be operator"); + + let added_via = alice.added_via.expect("added_via should be set"); + assert_eq!( + added_via.get("group").and_then(|v| v.as_str()), + Some("admins"), + "added_via should track the admin group (highest precedence)" + ); + + println!("✓ Role is upgraded when user is added to a higher-precedence group"); + } + + /// Test that adding a user to a lower-precedence group does NOT downgrade their role + #[ignore = "requires database setup - run with --ignored flag"] + #[sqlx::test( + migrations = "../migrations", + fixtures("base", "instance_group_auto_add") + )] + async fn test_no_role_downgrade_when_added_to_lower_group(db: Pool) { + // Configure workspace: engineering=admin, sales=operator + let groups = vec!["engineering".to_string(), "sales".to_string()]; + let roles = json!({"engineering": "admin", "sales": "operator"}); + + sqlx::query!( + r#" + UPDATE workspace_settings + SET auto_invite = jsonb_build_object( + 'instance_groups', $2::jsonb, + 'instance_groups_roles', $3::jsonb + ) + WHERE workspace_id = $1 + "#, + "ws-multi-group", + serde_json::to_value(&groups).unwrap(), + &roles, + ) + .execute(&db) + .await + .expect("Failed to update workspace settings"); + + // Alice starts as admin from engineering + let added_via = json!({"source": "instance_group", "group": "engineering"}); + sqlx::query!( + "INSERT INTO usr (workspace_id, username, email, is_admin, operator, added_via) + VALUES ($1, 'alice', 'alice@example.com', true, false, $2)", + "ws-multi-group", + &added_via, + ) + .execute(&db) + .await + .expect("Failed to add user"); + + // Now simulate adding alice to sales group (operator — lower precedence) + // The fixed code should keep her as admin + let user_igroups = vec!["engineering".to_string(), "sales".to_string()]; + let ws_configured_groups = vec!["engineering".to_string(), "sales".to_string()]; + let ws_roles: std::collections::HashMap = + serde_json::from_value(roles).unwrap(); + + let (best_group, is_admin, is_operator) = + compute_highest_workspace_role(&user_igroups, &ws_configured_groups, &ws_roles); + + let instance_group_source = json!({"source": "instance_group", "group": &best_group}); + sqlx::query!( + "UPDATE usr SET is_admin = $1, operator = $2, added_via = $3 WHERE workspace_id = $4 AND email = $5 AND added_via->>'source' = 'instance_group'", + is_admin, is_operator, &instance_group_source, "ws-multi-group", "alice@example.com" + ) + .execute(&db) + .await + .expect("Failed to update user role"); + + let alice = sqlx::query!( + "SELECT is_admin, operator, added_via FROM usr WHERE workspace_id = 'ws-multi-group' AND email = 'alice@example.com'" + ) + .fetch_one(&db) + .await + .expect("Failed to query user"); + + assert!(alice.is_admin, "Alice should remain admin (not downgraded)"); + assert!(!alice.operator, "Alice should not become operator"); + assert_eq!( + alice + .added_via + .unwrap() + .get("group") + .and_then(|v| v.as_str()), + Some("engineering"), + "added_via should still track engineering (highest precedence)" + ); + + println!("✓ Role is NOT downgraded when user is added to a lower-precedence group"); + } + + /// Test that manually-added users are not affected by instance group role updates + #[ignore = "requires database setup - run with --ignored flag"] + #[sqlx::test( + migrations = "../migrations", + fixtures("base", "instance_group_auto_add") + )] + async fn test_manual_users_not_affected_by_group_role_update(db: Pool) { + // Configure workspace + let groups = vec!["engineering".to_string()]; + let roles = json!({"engineering": "operator"}); + + sqlx::query!( + r#" + UPDATE workspace_settings + SET auto_invite = jsonb_build_object( + 'instance_groups', $2::jsonb, + 'instance_groups_roles', $3::jsonb + ) + WHERE workspace_id = $1 + "#, + "ws-multi-group", + serde_json::to_value(&groups).unwrap(), + &roles, + ) + .execute(&db) + .await + .expect("Failed to update workspace settings"); + + // Alice was manually added as admin (no added_via) + sqlx::query!( + "INSERT INTO usr (workspace_id, username, email, is_admin, operator) + VALUES ('ws-multi-group', 'alice', 'alice@example.com', true, false)", + ) + .execute(&db) + .await + .expect("Failed to add user"); + + // The UPDATE with added_via->>'source' = 'instance_group' filter should NOT match + let instance_group_source = json!({"source": "instance_group", "group": "engineering"}); + let result = sqlx::query!( + "UPDATE usr SET is_admin = $1, operator = $2, added_via = $3 WHERE workspace_id = $4 AND email = $5 AND added_via->>'source' = 'instance_group'", + false, true, &instance_group_source, "ws-multi-group", "alice@example.com" + ) + .execute(&db) + .await + .expect("Failed to execute update"); + + assert_eq!( + result.rows_affected(), + 0, + "UPDATE should not affect manually-added users" + ); + + let alice = sqlx::query!( + "SELECT is_admin, operator, added_via FROM usr WHERE workspace_id = 'ws-multi-group' AND email = 'alice@example.com'" + ) + .fetch_one(&db) + .await + .expect("Failed to query user"); + + assert!(alice.is_admin, "Manually-added admin should remain admin"); + assert!( + !alice.operator, + "Manually-added admin should not become operator" + ); + assert!( + alice.added_via.is_none(), + "added_via should remain NULL for manual users" + ); + + println!("✓ Manually-added users are not affected by instance group role updates"); + } + /// Test removing user from instance group removes them from workspace #[ignore = "requires database setup - run with --ignored flag"] - #[sqlx::test(migrations = "../migrations", fixtures("base", "instance_group_auto_add"))] + #[sqlx::test( + migrations = "../migrations", + fixtures("base", "instance_group_auto_add") + )] async fn test_remove_user_from_instance_group(db: Pool) { // First, add alice to the workspace via engineering group let added_via = json!({"source": "instance_group", "group": "engineering"}); @@ -416,7 +734,10 @@ mod tests { /// Test that users added via domain are not affected by instance group removal #[ignore = "requires database setup - run with --ignored flag"] - #[sqlx::test(migrations = "../migrations", fixtures("base", "instance_group_auto_add"))] + #[sqlx::test( + migrations = "../migrations", + fixtures("base", "instance_group_auto_add") + )] async fn test_domain_added_users_not_affected_by_group_removal(db: Pool) { // Add alice via domain (not instance group) let added_via = json!({"source": "domain", "domain": "example.com"}); @@ -469,7 +790,10 @@ mod tests { /// Test cleanup when instance group is removed from workspace configuration #[ignore = "requires database setup - run with --ignored flag"] - #[sqlx::test(migrations = "../migrations", fixtures("base", "instance_group_auto_add"))] + #[sqlx::test( + migrations = "../migrations", + fixtures("base", "instance_group_auto_add") + )] async fn test_cleanup_removed_instance_groups(db: Pool) { // First, add users via engineering group for (username, email) in &[("alice", "alice@example.com"), ("bob", "bob@example.com")] { @@ -513,12 +837,11 @@ mod tests { // This should trigger cleanup of users added via that group // Get all users in the engineering group - let group_users = sqlx::query_scalar!( - "SELECT email FROM email_to_igroup WHERE igroup = 'engineering'" - ) - .fetch_all(&db) - .await - .expect("Failed to get group users"); + let group_users = + sqlx::query_scalar!("SELECT email FROM email_to_igroup WHERE igroup = 'engineering'") + .fetch_all(&db) + .await + .expect("Failed to get group users"); // Remove users who were added via engineering group for email in group_users { @@ -588,7 +911,10 @@ mod tests { /// Test that users are not duplicated if already in workspace #[ignore = "requires database setup - run with --ignored flag"] - #[sqlx::test(migrations = "../migrations", fixtures("base", "instance_group_auto_add"))] + #[sqlx::test( + migrations = "../migrations", + fixtures("base", "instance_group_auto_add") + )] async fn test_no_duplicate_users(db: Pool) { // Add alice to workspace first (without instance group tracking) sqlx::query!( @@ -636,7 +962,10 @@ mod tests { /// Test workspace without auto-add configured is not affected #[ignore = "requires database setup - run with --ignored flag"] - #[sqlx::test(migrations = "../migrations", fixtures("base", "instance_group_auto_add"))] + #[sqlx::test( + migrations = "../migrations", + fixtures("base", "instance_group_auto_add") + )] async fn test_workspace_without_auto_add_not_affected(db: Pool) { // ws-no-auto-add has no instance_groups configured @@ -672,7 +1001,10 @@ mod tests { /// Test querying workspaces configured with a specific instance group #[ignore = "requires database setup - run with --ignored flag"] - #[sqlx::test(migrations = "../migrations", fixtures("base", "instance_group_auto_add"))] + #[sqlx::test( + migrations = "../migrations", + fixtures("base", "instance_group_auto_add") + )] async fn test_query_workspaces_with_instance_group(db: Pool) { // Configure ws-with-auto-add to use engineering group let groups = vec!["engineering".to_string()]; @@ -708,7 +1040,11 @@ mod tests { .await .expect("Failed to query workspaces"); - assert_eq!(workspaces.len(), 1, "Should find 1 workspace with engineering group"); + assert_eq!( + workspaces.len(), + 1, + "Should find 1 workspace with engineering group" + ); assert_eq!(workspaces[0].workspace_id, "ws-with-auto-add"); // Verify the role configuration is returned correctly