From a05187c0ff8e38b5828c19906a649798b4b650af Mon Sep 17 00:00:00 2001 From: BlackDex Date: Fri, 9 Jun 2023 22:50:44 +0200 Subject: [PATCH] Some code changes and optimizations Some cleanups and optimizations done on the code generated by @Kurnihil --- src/api/core/organizations.rs | 10 +++--- src/api/core/public.rs | 59 ++++++++++++++++++++--------------- src/db/models/user.rs | 14 ++++----- 3 files changed, 44 insertions(+), 39 deletions(-) diff --git a/src/api/core/organizations.rs b/src/api/core/organizations.rs index a71eb641..c404fbf0 100644 --- a/src/api/core/organizations.rs +++ b/src/api/core/organizations.rs @@ -2895,7 +2895,7 @@ async fn get_org_export(org_id: &str, headers: AdminHeaders, mut conn: DbConn) - } async fn _api_key( - org_id: String, + org_id: &str, data: JsonUpcase, rotate: bool, headers: AdminHeaders, @@ -2909,7 +2909,7 @@ async fn _api_key( err!("Invalid password") } - let org_api_key = match OrganizationApiKey::find_by_org_uuid(&org_id, &conn).await { + let org_api_key = match OrganizationApiKey::find_by_org_uuid(org_id, &conn).await { Some(mut org_api_key) => { if rotate { org_api_key.api_key = crate::crypto::generate_api_key(); @@ -2920,7 +2920,7 @@ async fn _api_key( } None => { let api_key = crate::crypto::generate_api_key(); - let new_org_api_key = OrganizationApiKey::new(org_id, api_key); + let new_org_api_key = OrganizationApiKey::new(String::from(org_id), api_key); new_org_api_key.save(&conn).await.expect("Error creating organization API Key"); new_org_api_key } @@ -2934,13 +2934,13 @@ async fn _api_key( } #[post("/organizations//api-key", data = "")] -async fn api_key(org_id: String, data: JsonUpcase, headers: AdminHeaders, conn: DbConn) -> JsonResult { +async fn api_key(org_id: &str, data: JsonUpcase, headers: AdminHeaders, conn: DbConn) -> JsonResult { _api_key(org_id, data, false, headers, conn).await } #[post("/organizations//rotate-api-key", data = "")] async fn rotate_api_key( - org_id: String, + org_id: &str, data: JsonUpcase, headers: AdminHeaders, conn: DbConn, diff --git a/src/api/core/public.rs b/src/api/core/public.rs index c8689222..b2945918 100644 --- a/src/api/core/public.rs +++ b/src/api/core/public.rs @@ -4,6 +4,8 @@ use rocket::{ Request, Route, }; +use std::collections::HashSet; + use crate::{ api::{EmptyResult, JsonUpcase}, auth, @@ -15,7 +17,7 @@ pub fn routes() -> Vec { routes![ldap_import] } -#[derive(Deserialize, Debug)] +#[derive(Deserialize)] #[allow(non_snake_case)] struct OrgImportGroupData { Name: String, @@ -23,7 +25,7 @@ struct OrgImportGroupData { MemberExternalIds: Vec, } -#[derive(Deserialize, Debug)] +#[derive(Deserialize)] #[allow(non_snake_case)] struct OrgImportUserData { Email: String, @@ -31,19 +33,20 @@ struct OrgImportUserData { Deleted: bool, } -#[derive(Deserialize, Debug)] +#[derive(Deserialize)] #[allow(non_snake_case)] struct OrgImportData { Groups: Vec, Members: Vec, OverwriteExisting: bool, - #[allow(dead_code)] - LargeImport: bool, + // LargeImport: bool, // For now this will not be used, upstream uses this to prevent syncs of more then 2000 users or groups without the flag set. } #[post("/public/organization/import", data = "")] async fn ldap_import(data: JsonUpcase, token: PublicToken, mut conn: DbConn) -> EmptyResult { - let _ = &conn; + // Most of the logic for this function can be found here + // https://github.com/bitwarden/server/blob/fd892b2ff4547648a276734fb2b14a8abae2c6f5/src/Core/Services/Implementations/OrganizationService.cs#L1797 + let org_id = token.0; let data = data.into_inner().data; @@ -114,38 +117,43 @@ async fn ldap_import(data: JsonUpcase, token: PublicToken, mut co } } - for group_data in &data.Groups { - let group_uuid = match Group::find_by_external_id(&group_data.ExternalId, &mut conn).await { - Some(group) => group.uuid, - None => { - let mut group = - Group::new(org_id.clone(), group_data.Name.clone(), false, Some(group_data.ExternalId.clone())); - group.save(&mut conn).await?; - group.uuid - } - }; + if CONFIG.org_groups_enabled() { + for group_data in &data.Groups { + let group_uuid = match Group::find_by_external_id(&group_data.ExternalId, &mut conn).await { + Some(group) => group.uuid, + None => { + let mut group = + Group::new(org_id.clone(), group_data.Name.clone(), false, Some(group_data.ExternalId.clone())); + group.save(&mut conn).await?; + group.uuid + } + }; - GroupUser::delete_all_by_group(&group_uuid, &mut conn).await?; + GroupUser::delete_all_by_group(&group_uuid, &mut conn).await?; - for ext_id in &group_data.MemberExternalIds { - if let Some(user) = User::find_by_external_id(ext_id, &mut conn).await { - if let Some(user_org) = UserOrganization::find_by_user_and_org(&user.uuid, &org_id, &mut conn).await { - let mut group_user = GroupUser::new(group_uuid.clone(), user_org.uuid.clone()); - group_user.save(&mut conn).await?; + for ext_id in &group_data.MemberExternalIds { + if let Some(user) = User::find_by_external_id(ext_id, &mut conn).await { + if let Some(user_org) = UserOrganization::find_by_user_and_org(&user.uuid, &org_id, &mut conn).await + { + let mut group_user = GroupUser::new(group_uuid.clone(), user_org.uuid.clone()); + group_user.save(&mut conn).await?; + } } } } + } else { + warn!("Group support is disabled, groups will not be imported!"); } // If this flag is enabled, any user that isn't provided in the Users list will be removed (by default they will be kept unless they have Deleted == true) if data.OverwriteExisting { + // Generate a HashSet to quickly verify if a member is listed or not. + let sync_members: HashSet = data.Members.into_iter().map(|m| m.ExternalId).collect(); for user_org in UserOrganization::find_by_org(&org_id, &mut conn).await { if let Some(user_external_id) = User::find_by_uuid(&user_org.user_uuid, &mut conn).await.map(|u| u.external_id) { - if user_external_id.is_some() - && !data.Members.iter().any(|u| u.ExternalId == *user_external_id.as_ref().unwrap()) - { + if user_external_id.is_some() && !sync_members.contains(&user_external_id.unwrap()) { if user_org.atype == UserOrgType::Owner && user_org.status == UserOrgStatus::Confirmed as i32 { // Removing owner, check that there is at least one other confirmed owner if UserOrganization::count_confirmed_by_org_and_type(&org_id, UserOrgType::Owner, &mut conn) @@ -165,7 +173,6 @@ async fn ldap_import(data: JsonUpcase, token: PublicToken, mut co Ok(()) } -#[derive(Debug)] pub struct PublicToken(String); #[rocket::async_trait] diff --git a/src/db/models/user.rs b/src/db/models/user.rs index a4764ada..596d2d75 100644 --- a/src/db/models/user.rs +++ b/src/db/models/user.rs @@ -157,16 +157,13 @@ impl User { pub fn set_external_id(&mut self, external_id: Option) { //Check if external id is empty. We don't want to have //empty strings in the database - match external_id { - Some(external_id) => { - if external_id.is_empty() { - self.external_id = None; - } else { - self.external_id = Some(external_id) - } + let mut ext_id: Option = None; + if let Some(external_id) = external_id { + if !external_id.is_empty() { + ext_id = Some(external_id); } - None => self.external_id = None, } + self.external_id = ext_id; } /// Set the password hash generated @@ -400,6 +397,7 @@ impl User { users::table.filter(users::external_id.eq(id)).first::(conn).ok().from_db() }} } + pub async fn get_all(conn: &mut DbConn) -> Vec { db_run! {conn: { users::table.load::(conn).expect("Error loading users").from_db()