fix: compute highest workspace role across all instance groups (#8633)
* fix: compute highest workspace role across all instance groups when adding user to group Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * chore: update ee-repo-ref.txt Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test: add regression tests for instance group role precedence Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * chore: update ee-repo-ref to 15dd361793564ab50bc485b71b042da4cfc1d32c This commit updates the EE repository reference after PR #501 was merged in windmill-ee-private. Previous ee-repo-ref: 7e9d0e7ec0b52585cab368b7351e3a5654da3589 New ee-repo-ref: 15dd361793564ab50bc485b71b042da4cfc1d32c Automated by sync-ee-ref workflow. * refactor: extract compute_highest_workspace_role helper and clean up role logic Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * chore: update ee-repo-ref.txt Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * chore: update ee-repo-ref to e08a87450627bef9013498e40ee93a47bedda7ee This commit updates the EE repository reference after PR #502 was merged in windmill-ee-private. Previous ee-repo-ref: a4ba20109813332320839488da5ecf83ca8f70b4 New ee-repo-ref: e08a87450627bef9013498e40ee93a47bedda7ee Automated by sync-ee-ref workflow. --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: windmill-internal-app[bot] <windmill-internal-app[bot]@users.noreply.github.com>
This commit is contained in:
12
backend/.sqlx/query-077467cd813d5af161cb1cc232724f26984822d4c28ba36c0a9331273b10edc0.json
generated
Normal file
12
backend/.sqlx/query-077467cd813d5af161cb1cc232724f26984822d4c28ba36c0a9331273b10edc0.json
generated
Normal file
@@ -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"
|
||||
}
|
||||
20
backend/.sqlx/query-1721f8b52ea265c0537fd7c742deddf0afbe5cf0d81b15e487c411ae169d3a89.json
generated
Normal file
20
backend/.sqlx/query-1721f8b52ea265c0537fd7c742deddf0afbe5cf0d81b15e487c411ae169d3a89.json
generated
Normal file
@@ -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"
|
||||
}
|
||||
15
backend/.sqlx/query-250a4e3f1a1f95296f7075bf8780e9c7407e89c8f7636484895e99f5a5e71297.json
generated
Normal file
15
backend/.sqlx/query-250a4e3f1a1f95296f7075bf8780e9c7407e89c8f7636484895e99f5a5e71297.json
generated
Normal file
@@ -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"
|
||||
}
|
||||
15
backend/.sqlx/query-2ba03e555d2e09dbd0e2ae5ddfd9a268a675bdb23615c78904cebe7f1e31f400.json
generated
Normal file
15
backend/.sqlx/query-2ba03e555d2e09dbd0e2ae5ddfd9a268a675bdb23615c78904cebe7f1e31f400.json
generated
Normal file
@@ -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"
|
||||
}
|
||||
34
backend/.sqlx/query-3bd4f38a1629a69ddda622b6b436198b47c2fe1a507358d49f05031e7beedab6.json
generated
Normal file
34
backend/.sqlx/query-3bd4f38a1629a69ddda622b6b436198b47c2fe1a507358d49f05031e7beedab6.json
generated
Normal file
@@ -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"
|
||||
}
|
||||
22
backend/.sqlx/query-5d160ba4958583f1ad42de846c544d8d8e81e1b54925a0c5f2cedc1817d99a1b.json
generated
Normal file
22
backend/.sqlx/query-5d160ba4958583f1ad42de846c544d8d8e81e1b54925a0c5f2cedc1817d99a1b.json
generated
Normal file
@@ -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"
|
||||
}
|
||||
32
backend/.sqlx/query-62e8e443cf063fcb30799d9c8971c00d761d54811936deb87a0315ca9cdc9769.json
generated
Normal file
32
backend/.sqlx/query-62e8e443cf063fcb30799d9c8971c00d761d54811936deb87a0315ca9cdc9769.json
generated
Normal file
@@ -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"
|
||||
}
|
||||
@@ -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"
|
||||
}
|
||||
12
backend/.sqlx/query-6f941e4454f736b32eaef80cdfb9582d6e75af3dc159e7c5f12497d3957f1eef.json
generated
Normal file
12
backend/.sqlx/query-6f941e4454f736b32eaef80cdfb9582d6e75af3dc159e7c5f12497d3957f1eef.json
generated
Normal file
@@ -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"
|
||||
}
|
||||
26
backend/.sqlx/query-88a467f3c943b134a81ac69c3c6686d1ce1ff2f5aafc15ff1b63cfa86c09c4f0.json
generated
Normal file
26
backend/.sqlx/query-88a467f3c943b134a81ac69c3c6686d1ce1ff2f5aafc15ff1b63cfa86c09c4f0.json
generated
Normal file
@@ -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"
|
||||
}
|
||||
18
backend/.sqlx/query-b38bd869477a729279cac3ccd4825191fb49e17e5f7e7297c0c819f52b486f49.json
generated
Normal file
18
backend/.sqlx/query-b38bd869477a729279cac3ccd4825191fb49e17e5f7e7297c0c819f52b486f49.json
generated
Normal file
@@ -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"
|
||||
}
|
||||
26
backend/.sqlx/query-e58ef252b0d2b81e9cd76f394a396abefd791906ada29dd5a7a9148157635ca5.json
generated
Normal file
26
backend/.sqlx/query-e58ef252b0d2b81e9cd76f394a396abefd791906ada29dd5a7a9148157635ca5.json
generated
Normal file
@@ -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"
|
||||
}
|
||||
@@ -1 +1 @@
|
||||
5ee1382dfb23b6a1516e3c7586058cec8240fdf2
|
||||
e08a87450627bef9013498e40ee93a47bedda7ee
|
||||
|
||||
@@ -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<String> = 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<String, String> = 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<String> = 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?;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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, String>,
|
||||
) -> (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);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<Postgres>) {
|
||||
// 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<Postgres>) {
|
||||
// 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<Postgres>) {
|
||||
// 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<Postgres>) {
|
||||
// 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<Postgres>) {
|
||||
// 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<String> = 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<String, String> = ws
|
||||
.instance_groups_roles
|
||||
.and_then(|r| serde_json::from_value(r).ok())
|
||||
.unwrap_or_default();
|
||||
|
||||
let ws_configured_groups: Vec<String> = 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<Postgres>) {
|
||||
// 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<String, String> =
|
||||
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<Postgres>) {
|
||||
// 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<Postgres>) {
|
||||
// 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<Postgres>) {
|
||||
// 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<Postgres>) {
|
||||
// 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<Postgres>) {
|
||||
// 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<Postgres>) {
|
||||
// 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<Postgres>) {
|
||||
// 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
|
||||
|
||||
Reference in New Issue
Block a user