From 2e91ef0a7d6a7af91bc3ba92eec3cef1297d9fb0 Mon Sep 17 00:00:00 2001 From: Aleksander <170264518+t-aleksander@users.noreply.github.com> Date: Wed, 28 Aug 2024 16:25:16 +0200 Subject: [PATCH 1/4] sync network state when deleting a user --- src/handlers/user.rs | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/src/handlers/user.rs b/src/handlers/user.rs index 17b98d70cd..b8848f4e2c 100644 --- a/src/handlers/user.rs +++ b/src/handlers/user.rs @@ -15,9 +15,7 @@ use crate::{ appstate::AppState, auth::{SessionInfo, UserAdminRole}, db::{ - models::enrollment::{Token, PASSWORD_RESET_TOKEN_TYPE}, - AppEvent, MFAMethod, OAuth2AuthorizedApp, Settings, User, UserDetails, UserInfo, Wallet, - WebAuthn, WireguardNetwork, + models::{device::DeviceInfo, enrollment::{Token, PASSWORD_RESET_TOKEN_TYPE}}, AppEvent, GatewayEvent, MFAMethod, OAuth2AuthorizedApp, Settings, User, UserDetails, UserInfo, Wallet, WebAuthn, WireguardNetwork }, error::WebError, ldap::utils::{ldap_add_user, ldap_change_password, ldap_delete_user, ldap_modify_user}, @@ -710,9 +708,22 @@ pub async fn delete_user( }); } if let Some(user) = User::find_by_username(&appstate.pool, &username).await? { - user.delete(&appstate.pool).await?; - let _result = ldap_delete_user(&appstate.pool, &username).await; + // Get rid of all devices of the deleted user from networks first + debug!("User {} deleted user {username}, purging their network devices across all networks.", session.user.username); + let devices = user.devices(&appstate.pool).await?; + let mut transaction = appstate.pool.begin().await?; + let mut events = Vec::new(); + for device in devices { + events.push(GatewayEvent::DeviceDeleted(DeviceInfo::from_device(&mut *transaction, device.device).await?)); + } + appstate.send_multiple_wireguard_events(events); + debug!("Devices of user {username} purged from networks."); + + user.delete(&mut *transaction).await?; + let _result = ldap_delete_user(&mut *transaction, &username).await; appstate.trigger_action(AppEvent::UserDeleted(username.clone())); + transaction.commit().await?; + info!("User {} deleted user {}", session.user.username, &username); Ok(ApiResponse::default()) } else { From b92964f10117dcbea0706f875a3bc708976390da Mon Sep 17 00:00:00 2001 From: Aleksander <170264518+t-aleksander@users.noreply.github.com> Date: Thu, 29 Aug 2024 10:47:32 +0200 Subject: [PATCH 2/4] cleanup, optimization --- src/db/models/mod.rs | 2 +- src/db/models/user.rs | 32 +++++++++++++++++++------------- src/grpc/enrollment.rs | 2 +- src/handlers/user.rs | 20 +++++++++++++++----- 4 files changed, 36 insertions(+), 20 deletions(-) diff --git a/src/db/models/mod.rs b/src/db/models/mod.rs index 73722c1026..e52eb7f1ac 100644 --- a/src/db/models/mod.rs +++ b/src/db/models/mod.rs @@ -198,7 +198,7 @@ pub struct UserDetails { impl UserDetails { pub async fn from_user(pool: &DbPool, user: &User) -> Result { - let devices = user.devices(pool).await?; + let devices = user.user_devices(pool).await?; let wallets = user.wallets(pool).await?; let security_keys = user.security_keys(pool).await?; diff --git a/src/db/models/user.rs b/src/db/models/user.rs index b9b5948652..4a00f309a0 100644 --- a/src/db/models/user.rs +++ b/src/db/models/user.rs @@ -683,24 +683,30 @@ impl User { } } - pub async fn devices(&self, pool: &DbPool) -> Result, SqlxError> { + pub async fn user_devices(&self, pool: &DbPool) -> Result, SqlxError> { + let devices = self.devices(pool).await?; + let mut user_devices = Vec::new(); + for device in devices { + if let Some(user_device) = UserDevice::from_device(pool, device).await? { + user_devices.push(user_device); + } + } + Ok(user_devices) + } + + pub async fn devices<'e, E>(&self, executor: E) -> Result, SqlxError> + where + E: PgExecutor<'e>, + { if let Some(id) = self.id { - let devices = query_as!( + query_as!( Device, - "SELECT device.id \"id?\", name, wireguard_pubkey, user_id, created \ + "SELECT id \"id!\", name, wireguard_pubkey, user_id, created \ FROM device WHERE user_id = $1", id ) - .fetch_all(pool) - .await?; - - let mut user_devices = Vec::new(); - for device in devices { - if let Some(user_device) = UserDevice::from_device(pool, device).await? { - user_devices.push(user_device); - } - } - Ok(user_devices) + .fetch_all(executor) + .await } else { Ok(Vec::new()) } diff --git a/src/grpc/enrollment.rs b/src/grpc/enrollment.rs index 0307b250aa..59e9027506 100644 --- a/src/grpc/enrollment.rs +++ b/src/grpc/enrollment.rs @@ -573,7 +573,7 @@ impl From for AdminInfo { impl InitialUserInfo { async fn from_user(pool: &DbPool, user: User) -> Result { let enrolled = user.is_enrolled(); - let devices = user.devices(pool).await?; + let devices = user.user_devices(pool).await?; let device_names = devices.into_iter().map(|dev| dev.device.name).collect(); Ok(Self { first_name: user.first_name, diff --git a/src/handlers/user.rs b/src/handlers/user.rs index b8848f4e2c..a9411e94f2 100644 --- a/src/handlers/user.rs +++ b/src/handlers/user.rs @@ -15,7 +15,12 @@ use crate::{ appstate::AppState, auth::{SessionInfo, UserAdminRole}, db::{ - models::{device::DeviceInfo, enrollment::{Token, PASSWORD_RESET_TOKEN_TYPE}}, AppEvent, GatewayEvent, MFAMethod, OAuth2AuthorizedApp, Settings, User, UserDetails, UserInfo, Wallet, WebAuthn, WireguardNetwork + models::{ + device::DeviceInfo, + enrollment::{Token, PASSWORD_RESET_TOKEN_TYPE}, + }, + AppEvent, GatewayEvent, MFAMethod, OAuth2AuthorizedApp, Settings, User, UserDetails, + UserInfo, Wallet, WebAuthn, WireguardNetwork, }, error::WebError, ldap::utils::{ldap_add_user, ldap_change_password, ldap_delete_user, ldap_modify_user}, @@ -709,16 +714,21 @@ pub async fn delete_user( } if let Some(user) = User::find_by_username(&appstate.pool, &username).await? { // Get rid of all devices of the deleted user from networks first - debug!("User {} deleted user {username}, purging their network devices across all networks.", session.user.username); - let devices = user.devices(&appstate.pool).await?; + debug!( + "User {} deleted user {username}, purging their network devices across all networks.", + session.user.username + ); let mut transaction = appstate.pool.begin().await?; + let devices = user.devices(&mut *transaction).await?; let mut events = Vec::new(); for device in devices { - events.push(GatewayEvent::DeviceDeleted(DeviceInfo::from_device(&mut *transaction, device.device).await?)); + events.push(GatewayEvent::DeviceDeleted( + DeviceInfo::from_device(&mut *transaction, device).await?, + )); } appstate.send_multiple_wireguard_events(events); debug!("Devices of user {username} purged from networks."); - + user.delete(&mut *transaction).await?; let _result = ldap_delete_user(&mut *transaction, &username).await; appstate.trigger_action(AppEvent::UserDeleted(username.clone())); From 111d673b34f94b92d9bb8d3b21ad990c32006e58 Mon Sep 17 00:00:00 2001 From: Aleksander <170264518+t-aleksander@users.noreply.github.com> Date: Thu, 29 Aug 2024 10:51:04 +0200 Subject: [PATCH 3/4] revert query --- src/db/models/user.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/db/models/user.rs b/src/db/models/user.rs index 4a00f309a0..328f3bfa3f 100644 --- a/src/db/models/user.rs +++ b/src/db/models/user.rs @@ -701,7 +701,7 @@ impl User { if let Some(id) = self.id { query_as!( Device, - "SELECT id \"id!\", name, wireguard_pubkey, user_id, created \ + "SELECT device.id \"id?\", name, wireguard_pubkey, user_id, created \ FROM device WHERE user_id = $1", id ) From bf823b8701b6585100da43a91eccdd90540e2f33 Mon Sep 17 00:00:00 2001 From: Aleksander <170264518+t-aleksander@users.noreply.github.com> Date: Thu, 29 Aug 2024 17:44:06 +0200 Subject: [PATCH 4/4] add comment --- src/db/models/user.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/db/models/user.rs b/src/db/models/user.rs index 328f3bfa3f..4b873e3c96 100644 --- a/src/db/models/user.rs +++ b/src/db/models/user.rs @@ -683,6 +683,9 @@ impl User { } } + /// Returns a vector of [`UserDevice`]s (hence the name). + /// [`UserDevice`] is a struct containing additional network info about a device. + /// If you only need [`Device`]s, use [`User::devices()`] instead. pub async fn user_devices(&self, pool: &DbPool) -> Result, SqlxError> { let devices = self.devices(pool).await?; let mut user_devices = Vec::new(); @@ -694,6 +697,8 @@ impl User { Ok(user_devices) } + /// Returns a vector of [`Device`]s related to a user. If you want to get [`UserDevice`]s (which contain additional network info), + /// use [`User::user_devices()`] instead. pub async fn devices<'e, E>(&self, executor: E) -> Result, SqlxError> where E: PgExecutor<'e>,