fix: use userid, username and method in a usergroup

new structure for POST /usergroup

```
{
  "name": "parseable",
  "roles": [],
  "users": [
    {
      "username": "nikhil",
      "method": "native"
    }
  ]
}
```

new structure for add/remove user - PATCH /usergroup/{name}/add(remove)

```
{
  "users": [
    {
      "username": "authentik Default Admin",
      "method": "oauth"
    }
  ]
}
```

Author: nikhilsinhaparseable

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

@@ -48,14 +48,14 @@ pub async fn put(
     // refresh the sessions of all users using this role
     // for this, iterate over all user_groups and users and create a hashset of users
     let mut session_refresh_users: HashSet<String> = HashSet::new();
-    for user_group in read_user_groups().values().cloned() {
+    for user_group in read_user_groups().values() {
         if user_group.roles.contains(&name) {
-            session_refresh_users.extend(user_group.users);
+            session_refresh_users.extend(user_group.get_usernames());
         }
     }
 
     // iterate over all users to see if they have this role
-    for user in users().values().cloned() {
+    for user in users().values() {
         if user.roles.contains(&name) {
             session_refresh_users.insert(user.username().to_string());
         }

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

@@ -103,7 +103,7 @@ pub async fn post_user(
     Ok(password)
 }
 
-// Handler for DELETE /api/v1/user/delete/{username}
+// 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;
@@ -120,19 +120,25 @@ 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(HashSet::from_iter([username.clone()]))?;
+            ug.remove_users_by_user_ids(HashSet::from_iter([username.clone()]))?;
             groups_to_update.push(ug.clone());
-            // ug.update_in_metadata().await?;
         } else {
             continue;
         };
     }
 
-    // update in metadata user group
-    metadata
-        .user_groups
-        .retain(|x| !groups_to_update.contains(x));
-    metadata.user_groups.extend(groups_to_update);
+    // For each updated group, replace in place if found; otherwise push
+    for updated_group in &groups_to_update {
+        if let Some(existing) = metadata
+            .user_groups
+            .iter_mut()
+            .find(|ug| ug.name == updated_group.name)
+        {
+            *existing = updated_group.clone();
+        } else {
+            metadata.user_groups.push(updated_group.clone());
+        }
+    }
     put_metadata(&metadata).await?;
 
     sync_user_deletion_with_ingestors(&username).await?;

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

@@ -51,14 +51,14 @@ pub async fn put(
     // refresh the sessions of all users using this role
     // for this, iterate over all user_groups and users and create a hashset of users
     let mut session_refresh_users: HashSet<String> = HashSet::new();
-    for user_group in read_user_groups().values().cloned() {
+    for user_group in read_user_groups().values() {
         if user_group.roles.contains(&name) {
-            session_refresh_users.extend(user_group.users);
+            session_refresh_users.extend(user_group.get_usernames());
         }
     }
 
     // iterate over all users to see if they have this role
-    for user in users().values().cloned() {
+    for user in users().values() {
         if user.roles.contains(&name) {
             session_refresh_users.insert(user.username().to_string());
         }

src/handlers/http/oidc.rs

@@ -38,7 +38,7 @@ use crate::{
     rbac::{
         self, Users,
         map::{DEFAULT_ROLE, SessionKey},
-        user::{self, User, UserType},
+        user::{self, GroupUser, User, UserType},
     },
     storage::{self, ObjectStorageError, StorageMetadata},
     utils::actix::extract_session_key_from_req,
@@ -432,12 +432,11 @@ pub async fn update_user_if_changed(
         entry.clone_from(&user);
         // migrate user references inside user groups
         for group in metadata.user_groups.iter_mut() {
-            if group.users.remove(&old_username) {
-                group.users.insert(user.username().to_string());
-            }
+            group.users.retain(|u| u.userid() != old_username);
+            group.users.insert(GroupUser::from_user(&user));
         }
-        put_metadata(&metadata).await?;
     }
+    put_metadata(&metadata).await?;
     Users.delete_user(&old_username);
     Users.put_user(user.clone());
     Ok(user)

src/handlers/http/rbac.rs

@@ -42,7 +42,7 @@ use tokio::sync::Mutex;
 
 use super::modal::utils::rbac_utils::{get_metadata, put_metadata};
 
-// async aware lock for updating storage metadata and user map atomicically
+// async aware lock for updating storage metadata and user map atomically
 static UPDATE_LOCK: Mutex<()> = Mutex::const_new(());
 
 #[derive(serde::Serialize)]
@@ -66,13 +66,13 @@ impl From<&user::User> for User {
 }
 
 // Handler for GET /api/v1/user
-// returns list of all registerd users
+// returns list of all registered users
 pub async fn list_users() -> impl Responder {
     web::Json(Users.collect_user::<User>())
 }
 
 /// Handler for GET /api/v1/users
-/// returns list of all registerd users along with their roles and other info
+/// returns list of all registered users along with their roles and other info
 pub async fn list_users_prism() -> impl Responder {
     // get all users
     let prism_users = rbac::map::users().values().map(to_prism_user).collect_vec();
@@ -350,7 +350,7 @@ pub async fn remove_roles_from_user(
 }
 
 #[derive(Debug, Serialize)]
-#[serde(rename = "camelCase")]
+#[serde(rename_all = "camelCase")]
 pub struct InvalidUserGroupError {
     pub valid_name: bool,
     pub non_existent_roles: Vec<String>,
@@ -390,7 +390,7 @@ pub enum RBACError {
     InvalidUserGroupRequest(Box<InvalidUserGroupError>),
     #[error("{0}")]
     InvalidSyncOperation(String),
-    #[error("User group `{0}` is still being used")]
+    #[error("UserGroup `{0}` is still in use")]
     UserGroupNotEmpty(String),
     #[error("Resource `{0}` is still in use")]
     ResourceInUse(String),

src/handlers/http/role.rs

@@ -50,14 +50,14 @@ pub async fn put(
     // refresh the sessions of all users using this role
     // for this, iterate over all user_groups and users and create a hashset of users
     let mut session_refresh_users: HashSet<String> = HashSet::new();
-    for user_group in read_user_groups().values().cloned() {
+    for user_group in read_user_groups().values() {
         if user_group.roles.contains(&name) {
-            session_refresh_users.extend(user_group.users);
+            session_refresh_users.extend(user_group.get_usernames());
         }
     }
 
     // iterate over all users to see if they have this role
-    for user in users().values().cloned() {
+    for user in users().values() {
         if user.roles.contains(&name) {
             session_refresh_users.insert(user.username().to_string());
         }

src/rbac/user.rs

@@ -203,6 +203,56 @@ 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)]
+pub struct GroupUser {
+    pub userid: String,
+    pub username: String,
+    pub method: String,
+}
+
+impl GroupUser {
+    pub fn username(&self) -> &str {
+        &self.username
+    }
+
+    pub fn userid(&self) -> &str {
+        &self.userid
+    }
+
+    pub fn is_oauth(&self) -> bool {
+        self.method == "oauth"
+    }
+
+    pub fn user_type(&self) -> &str {
+        if self.is_oauth() { "oauth" } else { "native" }
+    }
+
+    pub fn from_user(user: &User) -> Self {
+        match &user.ty {
+            UserType::Native(Basic { username, .. }) => GroupUser {
+                userid: username.clone(), // Same value for basic users
+                username: username.clone(),
+                method: "native".to_string(),
+            },
+            UserType::OAuth(OAuth { userid, user_info }) => {
+                // For OAuth users, derive the display username from user_info
+                let display_username = user_info
+                    .name
+                    .clone()
+                    .or_else(|| user_info.email.clone())
+                    .unwrap_or_else(|| userid.clone()); // fallback to userid if nothing else available
+
+                GroupUser {
+                    userid: userid.clone(),
+                    username: display_username,
+                    method: "oauth".to_string(),
+                }
+            }
+        }
+    }
+}
+
 /// Logically speaking, UserGroup is a collection of roles and is applied to a collection of users.
 ///
 /// The users present in a group inherit all the roles present in the group for as long as they are a part of the group.
@@ -212,7 +262,7 @@ pub struct UserGroup {
     // #[serde(default = "crate::utils::uid::gen")]
     // pub id: Ulid,
     pub roles: HashSet<String>,
-    pub users: HashSet<String>,
+    pub users: HashSet<GroupUser>,
 }
 
 fn is_valid_group_name(name: &str) -> bool {
@@ -239,9 +289,9 @@ impl UserGroup {
         let mut non_existent_users = Vec::new();
         if !self.users.is_empty() {
             // validate that the users exist
-            for user in &self.users {
-                if !users().contains_key(user) {
-                    non_existent_users.push(user.clone());
+            for group_user in &self.users {
+                if !users().contains_key(group_user.userid()) {
+                    non_existent_users.push(group_user.username().to_string());
                 }
             }
         }
@@ -266,7 +316,7 @@ impl UserGroup {
             Ok(())
         }
     }
-    pub fn new(name: String, roles: HashSet<String>, users: HashSet<String>) -> Self {
+    pub fn new(name: String, roles: HashSet<String>, users: HashSet<GroupUser>) -> Self {
         UserGroup { name, roles, users }
     }
 
@@ -276,20 +326,20 @@ impl UserGroup {
         }
         self.roles.extend(roles);
         // also refresh all user sessions
-        for username in &self.users {
-            mut_sessions().remove_user(username);
+        for group_user in &self.users {
+            mut_sessions().remove_user(group_user.userid());
         }
         Ok(())
     }
 
-    pub fn add_users(&mut self, users: HashSet<String>) -> Result<(), RBACError> {
+    pub fn add_users(&mut self, users: HashSet<GroupUser>) -> Result<(), RBACError> {
         if users.is_empty() {
             return Ok(());
         }
         self.users.extend(users.clone());
         // also refresh all user sessions
-        for username in &users {
-            mut_sessions().remove_user(username);
+        for group_user in &users {
+            mut_sessions().remove_user(group_user.userid());
         }
         Ok(())
     }
@@ -307,30 +357,61 @@ impl UserGroup {
         self.roles.clone_from(&new_roles);
 
         // also refresh all user sessions
-        for username in &self.users {
-            mut_sessions().remove_user(username);
+        for group_user in &self.users {
+            mut_sessions().remove_user(group_user.userid());
         }
         Ok(())
     }
 
-    pub fn remove_users(&mut self, users: HashSet<String>) -> Result<(), RBACError> {
+    pub fn remove_users(&mut self, users: HashSet<GroupUser>) -> Result<(), RBACError> {
         if users.is_empty() {
             return Ok(());
         }
         let new_users = HashSet::from_iter(self.users.difference(&users).cloned());
-        let removed_users: HashSet<String> = self.users.intersection(&users).cloned().collect();
+        let removed_users: HashSet<GroupUser> = self.users.intersection(&users).cloned().collect();
         if removed_users.is_empty() {
             return Ok(());
         }
         // also refresh all user sessions
-        for username in &removed_users {
-            mut_sessions().remove_user(username);
+        for group_user in &removed_users {
+            mut_sessions().remove_user(group_user.userid());
         }
         self.users.clone_from(&new_users);
 
         Ok(())
     }
 
+    /// Get all usernames in this group
+    pub fn get_usernames(&self) -> Vec<String> {
+        self.users
+            .iter()
+            .map(|u| u.username().to_string())
+            .collect()
+    }
+
+    /// Add users by converting from User references
+    pub fn add_users_from_user_refs(&mut self, user_refs: &[&User]) -> Result<(), RBACError> {
+        let group_users: HashSet<GroupUser> =
+            user_refs.iter().map(|u| GroupUser::from_user(u)).collect();
+        self.add_users(group_users)
+    }
+
+    /// Remove users by user ID
+    pub fn remove_users_by_user_ids(&mut self, user_ids: HashSet<String>) -> Result<(), RBACError> {
+        if user_ids.is_empty() {
+            return Ok(());
+        }
+
+        let users_to_remove: HashSet<GroupUser> = self
+            .users
+            .iter()
+            .filter(|u| user_ids.contains(u.userid()))
+            .cloned()
+            .collect();
+
+        self.remove_users(users_to_remove)
+    }
+
     pub async fn update_in_metadata(&self) -> Result<(), RBACError> {
         let mut metadata = get_metadata().await?;
         metadata.user_groups.retain(|x| x.name != self.name);