From 86aaf27659211735a5f0979ddaa6c4e2819544e6 Mon Sep 17 00:00:00 2001 From: Mathijs van Veluw Date: Wed, 8 Jan 2025 18:13:45 +0100 Subject: [PATCH] Prevent new users/members to be stored in db when invite fails (#5350) * Prevent new users/members when invite fails Currently when a (new) user gets invited as a member to an org, and SMTP is enabled, but sending the invite fails, the user is still created. They will only not have received a mail, and admins/owners need to re-invite the member again. Since the dialog window still keeps on-top when this fails, it kinda invites to click try again, but that will fail in mentioning the user is already a member. To prevent this weird flow, this commit will delete the user, invite and member if sending the mail failed. This allows the inviter to try again if there was a temporary hiccup for example, or contact the server admin and does not leave stray users/members around. Fixes #5349 Signed-off-by: BlackDex * Adjust deleting records Signed-off-by: BlackDex --------- Signed-off-by: BlackDex --- src/api/core/organizations.rs | 131 +++++++++++++++++++--------------- src/api/core/public.rs | 37 +++++++--- 2 files changed, 100 insertions(+), 68 deletions(-) diff --git a/src/api/core/organizations.rs b/src/api/core/organizations.rs index 902ab25a12..6f404b5604 100644 --- a/src/api/core/organizations.rs +++ b/src/api/core/organizations.rs @@ -895,6 +895,7 @@ async fn send_invite(org_id: &str, data: Json, headers: AdminHeaders data.access_all = true; } + let mut user_created: bool = false; for email in data.emails.iter() { let mut user_org_status = UserOrgStatus::Invited as i32; let user = match User::find_by_mail(email, &mut conn).await { @@ -908,13 +909,13 @@ async fn send_invite(org_id: &str, data: Json, headers: AdminHeaders } if !CONFIG.mail_enabled() { - let invitation = Invitation::new(email); - invitation.save(&mut conn).await?; + Invitation::new(email).save(&mut conn).await?; } - let mut user = User::new(email.clone()); - user.save(&mut conn).await?; - user + let mut new_user = User::new(email.clone()); + new_user.save(&mut conn).await?; + user_created = true; + new_user } Some(user) => { if UserOrganization::find_by_user_and_org(&user.uuid, org_id, &mut conn).await.is_some() { @@ -929,11 +930,49 @@ async fn send_invite(org_id: &str, data: Json, headers: AdminHeaders } }; - let mut new_user = UserOrganization::new(user.uuid.clone(), String::from(org_id)); + let mut new_member = UserOrganization::new(user.uuid.clone(), String::from(org_id)); let access_all = data.access_all; - new_user.access_all = access_all; - new_user.atype = new_type; - new_user.status = user_org_status; + new_member.access_all = access_all; + new_member.atype = new_type; + new_member.status = user_org_status; + new_member.save(&mut conn).await?; + + if CONFIG.mail_enabled() { + let org_name = match Organization::find_by_uuid(org_id, &mut conn).await { + Some(org) => org.name, + None => err!("Error looking up organization"), + }; + + if let Err(e) = mail::send_invite( + &user, + Some(String::from(org_id)), + Some(new_member.uuid.clone()), + &org_name, + Some(headers.user.email.clone()), + ) + .await + { + // Upon error delete the user, invite and org member records when needed + if user_created { + user.delete(&mut conn).await?; + } else { + new_member.delete(&mut conn).await?; + } + + err!(format!("Error sending invite: {e:?} ")); + }; + } + + log_event( + EventType::OrganizationUserInvited as i32, + &new_member.uuid.clone(), + org_id, + &headers.user.uuid, + headers.device.atype, + &headers.ip.ip, + &mut conn, + ) + .await; // If no accessAll, add the collections received if !access_all { @@ -954,39 +993,10 @@ async fn send_invite(org_id: &str, data: Json, headers: AdminHeaders } } - new_user.save(&mut conn).await?; - for group in data.groups.iter() { - let mut group_entry = GroupUser::new(String::from(group), new_user.uuid.clone()); + let mut group_entry = GroupUser::new(String::from(group), new_member.uuid.clone()); group_entry.save(&mut conn).await?; } - - log_event( - EventType::OrganizationUserInvited as i32, - &new_user.uuid, - org_id, - &headers.user.uuid, - headers.device.atype, - &headers.ip.ip, - &mut conn, - ) - .await; - - if CONFIG.mail_enabled() { - let org_name = match Organization::find_by_uuid(org_id, &mut conn).await { - Some(org) => org.name, - None => err!("Error looking up organization"), - }; - - mail::send_invite( - &user, - Some(String::from(org_id)), - Some(new_user.uuid), - &org_name, - Some(headers.user.email.clone()), - ) - .await?; - } } Ok(()) @@ -1064,7 +1074,7 @@ async fn _reinvite_user(org_id: &str, user_org: &str, invited_by_email: &str, co let invitation = Invitation::new(&user.email); invitation.save(conn).await?; } else { - let _ = Invitation::take(&user.email, conn).await; + Invitation::take(&user.email, conn).await; let mut user_org = user_org; user_org.status = UserOrgStatus::Accepted as i32; user_org.save(conn).await?; @@ -2026,6 +2036,9 @@ struct OrgImportData { users: Vec, } +/// This function seems to be deprected +/// It is only used with older directory connectors +/// TODO: Cleanup Tech debt #[post("/organizations//import", data = "")] async fn import(org_id: &str, data: Json, headers: Headers, mut conn: DbConn) -> EmptyResult { let data = data.into_inner(); @@ -2069,23 +2082,10 @@ async fn import(org_id: &str, data: Json, headers: Headers, mut c UserOrgStatus::Accepted as i32 // Automatically mark user as accepted if no email invites }; - let mut new_org_user = UserOrganization::new(user.uuid.clone(), String::from(org_id)); - new_org_user.access_all = false; - new_org_user.atype = UserOrgType::User as i32; - new_org_user.status = user_org_status; - - new_org_user.save(&mut conn).await?; - - log_event( - EventType::OrganizationUserInvited as i32, - &new_org_user.uuid, - org_id, - &headers.user.uuid, - headers.device.atype, - &headers.ip.ip, - &mut conn, - ) - .await; + let mut new_member = UserOrganization::new(user.uuid.clone(), String::from(org_id)); + new_member.access_all = false; + new_member.atype = UserOrgType::User as i32; + new_member.status = user_org_status; if CONFIG.mail_enabled() { let org_name = match Organization::find_by_uuid(org_id, &mut conn).await { @@ -2096,12 +2096,27 @@ async fn import(org_id: &str, data: Json, headers: Headers, mut c mail::send_invite( &user, Some(String::from(org_id)), - Some(new_org_user.uuid), + Some(new_member.uuid.clone()), &org_name, Some(headers.user.email.clone()), ) .await?; } + + // Save the member after sending an email + // If sending fails the member will not be saved to the database, and will not result in the admin needing to reinvite the users manually + new_member.save(&mut conn).await?; + + log_event( + EventType::OrganizationUserInvited as i32, + &new_member.uuid, + org_id, + &headers.user.uuid, + headers.device.atype, + &headers.ip.ip, + &mut conn, + ) + .await; } } } diff --git a/src/api/core/public.rs b/src/api/core/public.rs index 3b3e74cb94..f5f92e6211 100644 --- a/src/api/core/public.rs +++ b/src/api/core/public.rs @@ -52,6 +52,7 @@ async fn ldap_import(data: Json, token: PublicToken, mut conn: Db let data = data.into_inner(); for user_data in &data.members { + let mut user_created: bool = false; if user_data.deleted { // If user is marked for deletion and it exists, revoke it if let Some(mut user_org) = @@ -97,9 +98,9 @@ async fn ldap_import(data: Json, token: PublicToken, mut conn: Db new_user.save(&mut conn).await?; if !CONFIG.mail_enabled() { - let invitation = Invitation::new(&new_user.email); - invitation.save(&mut conn).await?; + Invitation::new(&new_user.email).save(&mut conn).await?; } + user_created = true; new_user } }; @@ -109,13 +110,13 @@ async fn ldap_import(data: Json, token: PublicToken, mut conn: Db UserOrgStatus::Accepted as i32 // Automatically mark user as accepted if no email invites }; - let mut new_org_user = UserOrganization::new(user.uuid.clone(), org_id.clone()); - new_org_user.set_external_id(Some(user_data.external_id.clone())); - new_org_user.access_all = false; - new_org_user.atype = UserOrgType::User as i32; - new_org_user.status = user_org_status; + let mut new_member = UserOrganization::new(user.uuid.clone(), org_id.clone()); + new_member.set_external_id(Some(user_data.external_id.clone())); + new_member.access_all = false; + new_member.atype = UserOrgType::User as i32; + new_member.status = user_org_status; - new_org_user.save(&mut conn).await?; + new_member.save(&mut conn).await?; if CONFIG.mail_enabled() { let (org_name, org_email) = match Organization::find_by_uuid(&org_id, &mut conn).await { @@ -123,8 +124,24 @@ async fn ldap_import(data: Json, token: PublicToken, mut conn: Db None => err!("Error looking up organization"), }; - mail::send_invite(&user, Some(org_id.clone()), Some(new_org_user.uuid), &org_name, Some(org_email)) - .await?; + if let Err(e) = mail::send_invite( + &user, + Some(org_id.clone()), + Some(new_member.uuid.clone()), + &org_name, + Some(org_email), + ) + .await + { + // Upon error delete the user, invite and org member records when needed + if user_created { + user.delete(&mut conn).await?; + } else { + new_member.delete(&mut conn).await?; + } + + err!(format!("Error sending invite: {e:?} ")); + } } } }