From 77f7fb2dd35c337a3a79fcf9038c73cf3939f85f Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Fri, 19 Jan 2024 14:02:15 +0100 Subject: [PATCH] feat: improve SCIM support for groups --- ...9344890a8fc50e503b15182ab3d2f90c75c2e.json | 22 ++++ ...226288b76cb1be7d1326a2cd03532eb405fc8.json | 16 +++ ...d0430ae53c739b3ea603f1a9cef1bd64004e1.json | 40 +++++++ ...f9606387d17454c7ef14a628e3bed107a242b.json | 40 +++++++ ...3c8e892ac4fac6d78bc825cd0b0b1099f4e07.json | 15 --- ...8a34a13579ebf69a73ac6897edaa9e87ab9f1.json | 28 ----- ...696b824685978760b308d8b578df95cf8db45.json | 14 --- ...95a78175ac18986d8bc68110d796fb282743c.json | 16 +++ ...40119112456_add_scim_display_name.down.sql | 1 + ...0240119112456_add_scim_display_name.up.sql | 3 + backend/windmill-api/src/scim.rs | 107 ++++++++++++------ .../(logged)/instance_groups/+page.svelte | 13 +++ 12 files changed, 221 insertions(+), 94 deletions(-) create mode 100644 backend/.sqlx/query-1498f1920ae2d47fd7c369285569344890a8fc50e503b15182ab3d2f90c75c2e.json create mode 100644 backend/.sqlx/query-2368de71006526e662b39692f42226288b76cb1be7d1326a2cd03532eb405fc8.json create mode 100644 backend/.sqlx/query-4c1f35d1375e900bfa956c574e1d0430ae53c739b3ea603f1a9cef1bd64004e1.json create mode 100644 backend/.sqlx/query-a002b2f47928f0f235c7d87ebe5f9606387d17454c7ef14a628e3bed107a242b.json delete mode 100644 backend/.sqlx/query-a28a83edc40e32815cb465338b53c8e892ac4fac6d78bc825cd0b0b1099f4e07.json delete mode 100644 backend/.sqlx/query-a355b3e13faa52b12c7d01fd8c78a34a13579ebf69a73ac6897edaa9e87ab9f1.json delete mode 100644 backend/.sqlx/query-cac375edf290d68d487de4273c3696b824685978760b308d8b578df95cf8db45.json create mode 100644 backend/.sqlx/query-e45efe23065e74bcfacc0b52a2995a78175ac18986d8bc68110d796fb282743c.json create mode 100644 backend/migrations/20240119112456_add_scim_display_name.down.sql create mode 100644 backend/migrations/20240119112456_add_scim_display_name.up.sql diff --git a/backend/.sqlx/query-1498f1920ae2d47fd7c369285569344890a8fc50e503b15182ab3d2f90c75c2e.json b/backend/.sqlx/query-1498f1920ae2d47fd7c369285569344890a8fc50e503b15182ab3d2f90c75c2e.json new file mode 100644 index 0000000000..7763dc50d6 --- /dev/null +++ b/backend/.sqlx/query-1498f1920ae2d47fd7c369285569344890a8fc50e503b15182ab3d2f90c75c2e.json @@ -0,0 +1,22 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT name FROM instance_group WHERE external_id = $1", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "name", + "type_info": "Varchar" + } + ], + "parameters": { + "Left": [ + "Text" + ] + }, + "nullable": [ + false + ] + }, + "hash": "1498f1920ae2d47fd7c369285569344890a8fc50e503b15182ab3d2f90c75c2e" +} diff --git a/backend/.sqlx/query-2368de71006526e662b39692f42226288b76cb1be7d1326a2cd03532eb405fc8.json b/backend/.sqlx/query-2368de71006526e662b39692f42226288b76cb1be7d1326a2cd03532eb405fc8.json new file mode 100644 index 0000000000..2dd0035bde --- /dev/null +++ b/backend/.sqlx/query-2368de71006526e662b39692f42226288b76cb1be7d1326a2cd03532eb405fc8.json @@ -0,0 +1,16 @@ +{ + "db_name": "PostgreSQL", + "query": "INSERT INTO instance_group (name, scim_display_name, external_id) VALUES ($1, $2, $3) ON CONFLICT DO NOTHING", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Varchar", + "Varchar", + "Varchar" + ] + }, + "nullable": [] + }, + "hash": "2368de71006526e662b39692f42226288b76cb1be7d1326a2cd03532eb405fc8" +} diff --git a/backend/.sqlx/query-4c1f35d1375e900bfa956c574e1d0430ae53c739b3ea603f1a9cef1bd64004e1.json b/backend/.sqlx/query-4c1f35d1375e900bfa956c574e1d0430ae53c739b3ea603f1a9cef1bd64004e1.json new file mode 100644 index 0000000000..37b61b8b3c --- /dev/null +++ b/backend/.sqlx/query-4c1f35d1375e900bfa956c574e1d0430ae53c739b3ea603f1a9cef1bd64004e1.json @@ -0,0 +1,40 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT name, external_id, scim_display_name, array_remove(array_agg(email_to_igroup.email), null) as emails FROM email_to_igroup RIGHT JOIN instance_group ON instance_group.name = email_to_igroup.igroup WHERE external_id = $1 GROUP BY name", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "name", + "type_info": "Varchar" + }, + { + "ordinal": 1, + "name": "external_id", + "type_info": "Varchar" + }, + { + "ordinal": 2, + "name": "scim_display_name", + "type_info": "Varchar" + }, + { + "ordinal": 3, + "name": "emails", + "type_info": "VarcharArray" + } + ], + "parameters": { + "Left": [ + "Text" + ] + }, + "nullable": [ + false, + true, + true, + null + ] + }, + "hash": "4c1f35d1375e900bfa956c574e1d0430ae53c739b3ea603f1a9cef1bd64004e1" +} diff --git a/backend/.sqlx/query-a002b2f47928f0f235c7d87ebe5f9606387d17454c7ef14a628e3bed107a242b.json b/backend/.sqlx/query-a002b2f47928f0f235c7d87ebe5f9606387d17454c7ef14a628e3bed107a242b.json new file mode 100644 index 0000000000..a23947c985 --- /dev/null +++ b/backend/.sqlx/query-a002b2f47928f0f235c7d87ebe5f9606387d17454c7ef14a628e3bed107a242b.json @@ -0,0 +1,40 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT name, external_id, scim_display_name, array_remove(array_agg(email_to_igroup.email), null) as emails FROM email_to_igroup RIGHT JOIN instance_group ON instance_group.name = email_to_igroup.igroup WHERE external_id = $1 group by name, external_id", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "name", + "type_info": "Varchar" + }, + { + "ordinal": 1, + "name": "external_id", + "type_info": "Varchar" + }, + { + "ordinal": 2, + "name": "scim_display_name", + "type_info": "Varchar" + }, + { + "ordinal": 3, + "name": "emails", + "type_info": "VarcharArray" + } + ], + "parameters": { + "Left": [ + "Text" + ] + }, + "nullable": [ + false, + true, + true, + null + ] + }, + "hash": "a002b2f47928f0f235c7d87ebe5f9606387d17454c7ef14a628e3bed107a242b" +} diff --git a/backend/.sqlx/query-a28a83edc40e32815cb465338b53c8e892ac4fac6d78bc825cd0b0b1099f4e07.json b/backend/.sqlx/query-a28a83edc40e32815cb465338b53c8e892ac4fac6d78bc825cd0b0b1099f4e07.json deleted file mode 100644 index 12581d6679..0000000000 --- a/backend/.sqlx/query-a28a83edc40e32815cb465338b53c8e892ac4fac6d78bc825cd0b0b1099f4e07.json +++ /dev/null @@ -1,15 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "UPDATE instance_group SET name = $1 where name = $2", - "describe": { - "columns": [], - "parameters": { - "Left": [ - "Varchar", - "Text" - ] - }, - "nullable": [] - }, - "hash": "a28a83edc40e32815cb465338b53c8e892ac4fac6d78bc825cd0b0b1099f4e07" -} diff --git a/backend/.sqlx/query-a355b3e13faa52b12c7d01fd8c78a34a13579ebf69a73ac6897edaa9e87ab9f1.json b/backend/.sqlx/query-a355b3e13faa52b12c7d01fd8c78a34a13579ebf69a73ac6897edaa9e87ab9f1.json deleted file mode 100644 index 3abf62e53f..0000000000 --- a/backend/.sqlx/query-a355b3e13faa52b12c7d01fd8c78a34a13579ebf69a73ac6897edaa9e87ab9f1.json +++ /dev/null @@ -1,28 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "SELECT name, array_remove(array_agg(email_to_igroup.email), null) as emails FROM email_to_igroup RIGHT JOIN instance_group ON instance_group.name = email_to_igroup.igroup WHERE name = $1 GROUP BY name", - "describe": { - "columns": [ - { - "ordinal": 0, - "name": "name", - "type_info": "Varchar" - }, - { - "ordinal": 1, - "name": "emails", - "type_info": "VarcharArray" - } - ], - "parameters": { - "Left": [ - "Text" - ] - }, - "nullable": [ - false, - null - ] - }, - "hash": "a355b3e13faa52b12c7d01fd8c78a34a13579ebf69a73ac6897edaa9e87ab9f1" -} diff --git a/backend/.sqlx/query-cac375edf290d68d487de4273c3696b824685978760b308d8b578df95cf8db45.json b/backend/.sqlx/query-cac375edf290d68d487de4273c3696b824685978760b308d8b578df95cf8db45.json deleted file mode 100644 index bd3c911d70..0000000000 --- a/backend/.sqlx/query-cac375edf290d68d487de4273c3696b824685978760b308d8b578df95cf8db45.json +++ /dev/null @@ -1,14 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "INSERT INTO instance_group (name) VALUES ($1) ON CONFLICT DO NOTHING", - "describe": { - "columns": [], - "parameters": { - "Left": [ - "Varchar" - ] - }, - "nullable": [] - }, - "hash": "cac375edf290d68d487de4273c3696b824685978760b308d8b578df95cf8db45" -} diff --git a/backend/.sqlx/query-e45efe23065e74bcfacc0b52a2995a78175ac18986d8bc68110d796fb282743c.json b/backend/.sqlx/query-e45efe23065e74bcfacc0b52a2995a78175ac18986d8bc68110d796fb282743c.json new file mode 100644 index 0000000000..0d86b458ea --- /dev/null +++ b/backend/.sqlx/query-e45efe23065e74bcfacc0b52a2995a78175ac18986d8bc68110d796fb282743c.json @@ -0,0 +1,16 @@ +{ + "db_name": "PostgreSQL", + "query": "UPDATE instance_group SET scim_display_name = $1, name = $2 where external_id = $3", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Varchar", + "Varchar", + "Text" + ] + }, + "nullable": [] + }, + "hash": "e45efe23065e74bcfacc0b52a2995a78175ac18986d8bc68110d796fb282743c" +} diff --git a/backend/migrations/20240119112456_add_scim_display_name.down.sql b/backend/migrations/20240119112456_add_scim_display_name.down.sql new file mode 100644 index 0000000000..d2f607c5b8 --- /dev/null +++ b/backend/migrations/20240119112456_add_scim_display_name.down.sql @@ -0,0 +1 @@ +-- Add down migration script here diff --git a/backend/migrations/20240119112456_add_scim_display_name.up.sql b/backend/migrations/20240119112456_add_scim_display_name.up.sql new file mode 100644 index 0000000000..1c213c9b74 --- /dev/null +++ b/backend/migrations/20240119112456_add_scim_display_name.up.sql @@ -0,0 +1,3 @@ +-- Add up migration script here +ALTER TABLE instance_group ADD COLUMN IF NOT EXISTS scim_display_name VARCHAR(255); +UPDATE instance_group SET external_id = name, scim_display_name = name; \ No newline at end of file diff --git a/backend/windmill-api/src/scim.rs b/backend/windmill-api/src/scim.rs index c567d6405d..1ff32af5b0 100644 --- a/backend/windmill-api/src/scim.rs +++ b/backend/windmill-api/src/scim.rs @@ -200,24 +200,30 @@ pub async fn create_user( pub async fn get_groups( Extension(db): Extension, Query(query): Query, -) -> Result> { +) -> Result>> { let sqlb = SqlBuilder::select_from("instance_group") - .fields(&["name"]) + .fields(&[ + "name", + "external_id", + "scim_display_name", + "array_remove(array_agg(email_to_igroup.email), null) as emails", + ]) + .right() + .join("email_to_igroup") + .on("instance_group.name = email_to_igroup.igroup") + .group_by("name, external_id") .limit(query.count.unwrap_or(100000)) .offset(query.startIndex.map(|x| x - 1).unwrap_or(0)) .clone(); let sql = sqlb.sql().map_err(|e| Error::InternalErr(e.to_string()))?; - let users = sqlx::query_scalar(&sql) + let groups = sqlx::query_as::<_, Group>(&sql) .fetch_all(&db) .await? .into_iter() - .map(|x: String| User { id: x.clone(), userName: x, active: true }) + .map(|x| group_response(x).0) .collect(); - Ok(resource_response( - "urn:ietf:params:scim:api:messages:2.0:ListResponse", - users, - )) + Ok(Json(groups)) } // { @@ -231,17 +237,19 @@ pub async fn get_groups( // } #[cfg(feature = "enterprise")] -#[derive(Serialize)] -struct Group { +#[derive(Serialize, sqlx::FromRow)] +pub struct Group { name: String, emails: Option>, + external_id: Option, + scim_display_name: Option, } #[cfg(feature = "enterprise")] fn group_response(group: Group) -> JsonScim { let json = json!({ "schemas": ["urn:ietf:params:scim:schemas:core:2.0:Group"], - "displayName": group.name, - "id": convert_name(&group.name), + "displayName": group.scim_display_name.unwrap_or_default(), + "id": group.external_id.unwrap_or_else(|| convert_name(&group.name)), "members": group.emails.unwrap_or_default().into_iter().map(|x| json!({"value": x, "display": x})).collect::>(), "meta": { "resourceType": "Group" @@ -258,7 +266,7 @@ pub async fn get_group( ) -> Result> { let group= sqlx::query_as!( Group, - "SELECT name, array_remove(array_agg(email_to_igroup.email), null) as emails FROM email_to_igroup RIGHT JOIN instance_group ON instance_group.name = email_to_igroup.igroup WHERE name = $1 GROUP BY name", + "SELECT name, external_id, scim_display_name, array_remove(array_agg(email_to_igroup.email), null) as emails FROM email_to_igroup RIGHT JOIN instance_group ON instance_group.name = email_to_igroup.igroup WHERE external_id = $1 group by name, external_id", id ) .fetch_optional(&db) @@ -292,26 +300,41 @@ pub async fn create_group( Extension(db): Extension, Json(body): Json, ) -> Result> { + use uuid::Uuid; + tracing::info!("SCIM creating group: {:?}", body); let mut tx: sqlx::Transaction<'_, sqlx::Postgres> = db.begin().await?; + let id = Uuid::new_v4().to_string(); + let scim_display_name = Some(body.displayName.clone()); + let name = convert_name(&body.displayName.clone()); sqlx::query!( - "INSERT INTO instance_group (name) VALUES ($1) ON CONFLICT DO NOTHING", - convert_name(&body.displayName) + "INSERT INTO instance_group (name, scim_display_name, external_id) VALUES ($1, $2, $3) ON CONFLICT DO NOTHING", + name, + body.displayName, + id, ) .execute(&mut *tx) .await?; + tracing::info!( + "SCIM created group: {} with external_id: {} (display name: {})", + name, + id, + body.displayName + ); for member in &body.members { sqlx::query!( "INSERT INTO email_to_igroup (email, igroup) VALUES ($1, $2) ON CONFLICT DO NOTHING", convert_name(&member.display), - body.displayName, + name, ) .execute(&mut *tx) .await?; } tx.commit().await?; Ok(group_response(Group { - name: body.displayName.clone(), + external_id: Some(id), + name, + scim_display_name, emails: Some( body.members .clone() @@ -349,45 +372,50 @@ pub async fn update_group( if body.schemas.len() == 1 { let schema = body.schemas.get(0).unwrap(); if schema == "urn:ietf:params:scim:schemas:core:2.0:Group" { - sqlx::query!("DELETE FROM email_to_igroup WHERE igroup = $1", id) + let group= sqlx::query_as!( + Group, + "SELECT name, external_id, scim_display_name, array_remove(array_agg(email_to_igroup.email), null) as emails FROM email_to_igroup RIGHT JOIN instance_group ON instance_group.name = email_to_igroup.igroup WHERE external_id = $1 GROUP BY name", + id + ) + .fetch_optional(&db) + .await?; + let mut group = not_found_if_none(group, "Group", id.clone())?; + + sqlx::query!("DELETE FROM email_to_igroup WHERE igroup = $1", group.name) .execute(&mut *tx) .await?; - let id = if let Some(name) = body.displayName.clone() { + let new_name = if let Some(name) = body.displayName.clone() { + let new_name = convert_name(name.as_str()); sqlx::query!( - "UPDATE instance_group SET name = $1 where name = $2", - convert_name(&name.clone()), + "UPDATE instance_group SET scim_display_name = $1, name = $2 where external_id = $3", + name, + new_name, id ) .execute(&mut *tx) .await?; - convert_name(&name) + group.scim_display_name = Some(name); + new_name } else { - id + group.name.clone() }; if let Some(members) = body.members.clone() { + let mut emails = vec![]; for m in members { + emails.push(m.display.clone()); sqlx::query!( "INSERT INTO email_to_igroup (email, igroup) VALUES ($1, $2) ON CONFLICT DO NOTHING", m.display, - id + new_name, ) .execute(&mut *tx) .await?; } tx.commit().await?; - Ok(group_response(Group { - name: body.displayName.unwrap_or_default(), - emails: Some( - body.members - .unwrap_or_default() - .clone() - .into_iter() - .map(|x| x.display.clone()) - .collect(), - ), - })) + group.emails = Some(emails); + Ok(group_response(group)) } else { Err(Error::BadRequest("expected members".to_string())) } @@ -402,10 +430,15 @@ pub async fn update_group( #[cfg(feature = "enterprise")] pub async fn delete_group(Extension(db): Extension, Path(id): Path) -> Result<()> { tracing::info!("SCIM delete group: {:?}", id); - sqlx::query!("DELETE FROM email_to_igroup WHERE igroup = $1", id) + let group = sqlx::query_scalar!("SELECT name FROM instance_group WHERE external_id = $1", id) + .fetch_optional(&db) + .await?; + let group = not_found_if_none(group, "Group", id.clone())?; + + sqlx::query!("DELETE FROM email_to_igroup WHERE igroup = $1", group) .execute(&db) .await?; - sqlx::query!("DELETE FROM instance_group WHERE name = $1", id) + sqlx::query!("DELETE FROM instance_group WHERE name = $1", group) .execute(&db) .await?; Ok(()) diff --git a/frontend/src/routes/(root)/(logged)/instance_groups/+page.svelte b/frontend/src/routes/(root)/(logged)/instance_groups/+page.svelte index 55ef7bfe49..162715a43e 100644 --- a/frontend/src/routes/(root)/(logged)/instance_groups/+page.svelte +++ b/frontend/src/routes/(root)/(logged)/instance_groups/+page.svelte @@ -8,6 +8,7 @@ import PageHeader from '$lib/components/PageHeader.svelte' import TableCustom from '$lib/components/TableCustom.svelte' import { Plus } from 'lucide-svelte' + import { sendUserToast } from '$lib/toast' let newGroupName: string = '' let instanceGroups: InstanceGroup[] | undefined = undefined @@ -81,6 +82,7 @@ Name Summary Members + {#each instanceGroups as { name, summary, emails }} @@ -99,6 +101,17 @@ {summary ? summary.slice(0, 50) + (summary.length > 50 ? '...' : '') : '-'} {emails?.length ?? 0} members + {/each}