Some code changes and optimizations

Some cleanups and optimizations done on the code generated by @Kurnihil

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<PasswordData>,
     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/<org_id>/api-key", data = "<data>")]
-async fn api_key(org_id: String, data: JsonUpcase<PasswordData>, headers: AdminHeaders, conn: DbConn) -> JsonResult {
+async fn api_key(org_id: &str, data: JsonUpcase<PasswordData>, headers: AdminHeaders, conn: DbConn) -> JsonResult {
     _api_key(org_id, data, false, headers, conn).await
 }
 
 #[post("/organizations/<org_id>/rotate-api-key", data = "<data>")]
 async fn rotate_api_key(
-    org_id: String,
+    org_id: &str,
     data: JsonUpcase<PasswordData>,
     headers: AdminHeaders,
     conn: DbConn,

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<Route> {
     routes![ldap_import]
 }
 
-#[derive(Deserialize, Debug)]
+#[derive(Deserialize)]
 #[allow(non_snake_case)]
 struct OrgImportGroupData {
     Name: String,
@@ -23,7 +25,7 @@ struct OrgImportGroupData {
     MemberExternalIds: Vec<String>,
 }
 
-#[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<OrgImportGroupData>,
     Members: Vec<OrgImportUserData>,
     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 = "<data>")]
 async fn ldap_import(data: JsonUpcase<OrgImportData>, 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,6 +117,7 @@ async fn ldap_import(data: JsonUpcase<OrgImportData>, token: PublicToken, mut co
         }
     }
 
+    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,
@@ -129,23 +133,27 @@ async fn ldap_import(data: JsonUpcase<OrgImportData>, token: PublicToken, mut co
 
             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 {
+                    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<String> = 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<OrgImportData>, token: PublicToken, mut co
     Ok(())
 }
 
-#[derive(Debug)]
 pub struct PublicToken(String);
 
 #[rocket::async_trait]

src/db/models/user.rs

@@ -157,16 +157,13 @@ impl User {
     pub fn set_external_id(&mut self, external_id: Option<String>) {
         //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<String> = 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::<UserDb>(conn).ok().from_db()
         }}
     }
+
     pub async fn get_all(conn: &mut DbConn) -> Vec<Self> {
         db_run! {conn: {
             users::table.load::<UserDb>(conn).expect("Error loading users").from_db()