code rabbit suggestions

Author: nikhilsinhaparseable

src/handlers/http/modal/ingest/ingestor_rbac.rs

@@ -58,7 +58,7 @@ pub async fn post_user(
 // Handler for DELETE /api/v1/user/delete/{username}
 pub async fn delete_user(username: web::Path<String>) -> Result<impl Responder, RBACError> {
     let username = username.into_inner();
-    let _ = UPDATE_LOCK.lock().await;
+    let _guard = UPDATE_LOCK.lock().await;
     // fail this request if the user does not exists
     if !Users.contains(&username) {
         return Err(RBACError::UserDoesNotExist);

src/handlers/http/modal/ingest/ingestor_role.rs

@@ -50,7 +50,7 @@ pub async fn put(
     let mut session_refresh_users: HashSet<String> = HashSet::new();
     for user_group in read_user_groups().values() {
         if user_group.roles.contains(&name) {
-            session_refresh_users.extend(user_group.get_usernames());
+            session_refresh_users.extend(user_group.users.iter().map(|u| u.userid().to_string()));
         }
     }
 

src/handlers/http/modal/query/querier_rbac.rs

@@ -32,8 +32,8 @@ use crate::{
     },
     rbac::{
         Users,
-        map::{roles, write_user_groups},
-        user,
+        map::{roles, users, write_user_groups},
+        user::{self, UserType},
     },
     validator,
 };
@@ -74,7 +74,7 @@ pub async fn post_user(
             return Err(RBACError::RolesDoNotExist(non_existant_roles));
         }
     }
-    let _ = UPDATE_LOCK.lock().await;
+    let _guard = UPDATE_LOCK.lock().await;
     if Users.contains(&username)
         || metadata
             .users
@@ -106,7 +106,7 @@ pub async fn post_user(
 // Handler for DELETE /api/v1/user/{username}
 pub async fn delete_user(username: web::Path<String>) -> Result<impl Responder, RBACError> {
     let username = username.into_inner();
-    let _ = UPDATE_LOCK.lock().await;
+    let _guard = UPDATE_LOCK.lock().await;
     // fail this request if the user does not exists
     if !Users.contains(&username) {
         return Err(RBACError::UserDoesNotExist);
@@ -120,11 +120,21 @@ pub async fn delete_user(username: web::Path<String>) -> Result<impl Responder,
     let mut groups_to_update = Vec::new();
     for user_group in user_groups {
         if let Some(ug) = write_user_groups().get_mut(&user_group) {
-            ug.remove_users_by_user_ids(HashSet::from_iter([username.clone()]))?;
-            groups_to_update.push(ug.clone());
+            // Look up the user by display name to get the correct userid
+            if let Some(user) = users().get(&username) {
+                let user_id = match &user.ty {
+                    UserType::Native(basic) => basic.username.clone(),
+                    UserType::OAuth(oauth) => oauth.userid.clone(),
+                };
+                ug.remove_users_by_user_ids(HashSet::from_iter([user_id]))?;
+                groups_to_update.push(ug.clone());
+            } else {
+                // User not found, skip or log as needed
+                continue;
+            }
         } else {
             continue;
-        };
+        }
     }
 
     // For each updated group, replace in place if found; otherwise push
@@ -134,7 +144,7 @@ pub async fn delete_user(username: web::Path<String>) -> Result<impl Responder,
             .iter_mut()
             .find(|ug| ug.name == updated_group.name)
         {
-            *existing = updated_group.clone();
+            existing.clone_from(updated_group);
         } else {
             metadata.user_groups.push(updated_group.clone());
         }
@@ -262,7 +272,7 @@ pub async fn post_gen_password(username: web::Path<String>) -> Result<impl Respo
     let mut new_hash = String::default();
     let mut metadata = get_metadata().await?;
 
-    let _ = UPDATE_LOCK.lock().await;
+    let _guard = UPDATE_LOCK.lock().await;
     let user::PassCode { password, hash } = user::Basic::gen_new_password();
     new_password.clone_from(&password);
     new_hash.clone_from(&hash);

src/handlers/http/modal/query/querier_role.rs

@@ -53,7 +53,7 @@ pub async fn put(
     let mut session_refresh_users: HashSet<String> = HashSet::new();
     for user_group in read_user_groups().values() {
         if user_group.roles.contains(&name) {
-            session_refresh_users.extend(user_group.get_usernames());
+            session_refresh_users.extend(user_group.users.iter().map(|u| u.userid().to_string()));
         }
     }
 

src/handlers/http/rbac.rs

@@ -124,7 +124,7 @@ pub async fn post_user(
             return Err(RBACError::RolesDoNotExist(non_existent_roles));
         }
     }
-    let _ = UPDATE_LOCK.lock().await;
+    let _guard = UPDATE_LOCK.lock().await;
     if Users.contains(&username)
         || metadata
             .users
@@ -159,7 +159,7 @@ pub async fn post_gen_password(username: web::Path<String>) -> Result<impl Respo
     let mut new_hash = String::default();
     let mut metadata = get_metadata().await?;
 
-    let _ = UPDATE_LOCK.lock().await;
+    let _guard = UPDATE_LOCK.lock().await;
     let user::PassCode { password, hash } = user::Basic::gen_new_password();
     new_password.clone_from(&password);
     new_hash.clone_from(&hash);
@@ -235,7 +235,7 @@ pub async fn delete_user(username: web::Path<String>) -> Result<impl Responder,
     if !Users.contains(&username) {
         return Err(RBACError::UserDoesNotExist);
     };
-    let _ = UPDATE_LOCK.lock().await;
+    let _guard = UPDATE_LOCK.lock().await;
 
     // delete from parseable.json first
     let mut metadata = get_metadata().await?;

src/handlers/http/role.rs

@@ -52,7 +52,7 @@ pub async fn put(
     let mut session_refresh_users: HashSet<String> = HashSet::new();
     for user_group in read_user_groups().values() {
         if user_group.roles.contains(&name) {
-            session_refresh_users.extend(user_group.get_usernames());
+            session_refresh_users.extend(user_group.users.iter().map(|u| u.userid().to_string()));
         }
     }
 

src/rbac/user.rs

@@ -204,13 +204,25 @@ impl From<openid::Userinfo> for UserInfo {
 }
 
 /// Represents a user in a UserGroup - simplified structure for both user types
-#[derive(Debug, Clone, PartialEq, Eq, Hash, serde::Serialize, serde::Deserialize)]
+#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)]
 pub struct GroupUser {
     pub userid: String,
     pub username: String,
     pub method: String,
 }
 
+impl PartialEq for GroupUser {
+    fn eq(&self, other: &Self) -> bool {
+        self.userid == other.userid
+    }
+}
+impl Eq for GroupUser {}
+impl std::hash::Hash for GroupUser {
+    fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
+        self.userid.hash(state)
+    }
+}
+
 impl GroupUser {
     pub fn username(&self) -> &str {
         &self.username