bugfix

- user sessions get removed upon modifying group's roles

Author: parmesant

src/handlers/http/rbac.rs

@@ -21,7 +21,7 @@ use std::collections::{HashMap, HashSet};
 use crate::{
     rbac::{
         self,
-        map::{read_user_groups, roles, write_user_groups},
+        map::{read_user_groups, roles},
         role::model::DefaultPrivilege,
         user,
         utils::to_prism_user,
@@ -116,14 +116,11 @@ pub async fn post_user(
         return Err(RBACError::RoleValidationError);
     } else {
         let mut non_existent_roles = Vec::new();
-        user_roles
-            .iter()
-            .map(|r| {
-                if !roles().contains_key(r) {
-                    non_existent_roles.push(r.clone());
-                }
-            })
-            .for_each(drop);
+        for role in &user_roles {
+            if !roles().contains_key(role) {
+                non_existent_roles.push(role.clone());
+            }
+        }
         if !non_existent_roles.is_empty() {
             return Err(RBACError::RolesDoNotExist(non_existent_roles));
         }
@@ -244,24 +241,6 @@ pub async fn delete_user(username: web::Path<String>) -> Result<impl Responder,
     // delete from parseable.json first
     let mut metadata = get_metadata().await?;
     metadata.users.retain(|user| user.username() != username);
-
-    // Remove user from all groups
-    let user_groups = Users.get_user_groups(&username);
-    {
-        let mut groups = write_user_groups();
-        for group_name in &user_groups {
-            if let Some(group) = groups.get_mut(group_name) {
-                group.remove_users(HashSet::from_iter([username.clone()]))?;
-            }
-        }
-    }
-
-    // Update metadata with modified groups
-    for group in metadata.user_groups.iter_mut() {
-        if user_groups.contains(&group.name) {
-            group.users.retain(|u| u != &username);
-        }
-    }
     put_metadata(&metadata).await?;
 
     // update in mem table
@@ -284,11 +263,11 @@ pub async fn add_roles_to_user(
     let mut non_existent_roles = Vec::new();
 
     // check if the role exists
-    roles_to_add.iter().for_each(|r| {
-        if roles().get(r).is_none() {
-            non_existent_roles.push(r.clone());
+    for role in &roles_to_add {
+        if !roles().contains_key(role) {
+            non_existent_roles.push(role.clone());
         }
-    });
+    }
 
     if !non_existent_roles.is_empty() {
         return Err(RBACError::RolesDoNotExist(non_existent_roles));
@@ -329,11 +308,11 @@ pub async fn remove_roles_from_user(
     let mut non_existent_roles = Vec::new();
 
     // check if the role exists
-    roles_to_remove.iter().for_each(|r| {
-        if roles().get(r).is_none() {
-            non_existent_roles.push(r.clone());
+    for role in &roles_to_remove {
+        if !roles().contains_key(role) {
+            non_existent_roles.push(role.clone());
         }
-    });
+    }
 
     if !non_existent_roles.is_empty() {
         return Err(RBACError::RolesDoNotExist(non_existent_roles));

src/rbac/user.rs

@@ -31,7 +31,7 @@ use crate::{
         rbac::{InvalidUserGroupError, RBACError},
     },
     parseable::PARSEABLE,
-    rbac::map::{read_user_groups, roles, users},
+    rbac::map::{mut_sessions, read_user_groups, roles, users},
 };
 
 #[derive(Debug, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)]
@@ -266,11 +266,19 @@ impl UserGroup {
 
     pub fn add_roles(&mut self, roles: HashSet<String>) -> Result<(), RBACError> {
         self.roles.extend(roles);
+        // also refresh all user sessions
+        for username in &self.users {
+            mut_sessions().remove_user(username);
+        }
         Ok(())
     }
 
     pub fn add_users(&mut self, users: HashSet<String>) -> Result<(), RBACError> {
-        self.users.extend(users);
+        self.users.extend(users.clone());
+        // also refresh all user sessions
+        for username in &users {
+            mut_sessions().remove_user(username);
+        }
         Ok(())
     }
 
@@ -283,6 +291,10 @@ impl UserGroup {
         }
         self.roles.clone_from(&new_roles);
 
+        // also refresh all user sessions
+        for username in &self.users {
+            mut_sessions().remove_user(username);
+        }
         Ok(())
     }
 
@@ -293,6 +305,10 @@ impl UserGroup {
         if old_users.eq(&new_users) {
             return Ok(());
         }
+        // also refresh all user sessions
+        for username in &users {
+            mut_sessions().remove_user(username);
+        }
         self.users.clone_from(&new_users);
 
         Ok(())
@@ -305,21 +321,4 @@ impl UserGroup {
         put_metadata(&metadata).await?;
         Ok(())
     }
-
-    // // are these methods even needed??
-    // pub fn group_name(&self) -> String {
-    //     self.name.clone()
-    // }
-
-    // pub fn group_id(&self) -> Ulid {
-    //     self.id
-    // }
-
-    // pub fn group_roles(&self) -> HashSet<String> {
-    //     self.roles.clone()
-    // }
-
-    // pub fn group_users(&self) -> HashSet<String> {
-    //     self.users.clone()
-    // }
 }