updating group membership on user deletion

until now groups had members without useraccount after the account has been deleted.

fixes: #5

README.md

@@ -6,17 +6,18 @@
 When done this library intends to provide all the functionality needed to manage users on a linux system.
 
 What is working so far:
-  * Parsing:
+  * [x] Parsing:
-    * `/etc/passwd`
+    * [x] `/etc/passwd`
-    * `/etc/shadow` (root permission needed)
+    * [x] `/etc/shadow` (root permission needed)
-    * `/etc/group`
+    * [x] `/etc/group`
+
   * Modifying:
     * delete a user
         * [x] passwd
         * [x] shadow
         * [X] group
-            * [x] own group
+            * [x] own primary group
-            * [ ] member
+            * [x] membership in other groups
         * [x] home dir
             * [x] delete
             * [x] keep
@@ -32,7 +33,6 @@ What is working so far:
         - [x] shadow
         - [ ] group
             - [ ] own group
-            - [ ] member
         - [ ] home dir
             - [ ] create from skeleton
             - [ ] Skip

src/group/mod.rs

@@ -80,6 +80,11 @@ impl Inner {
             },
         })
     }
+
+    pub(super) fn remove_member(&mut self, kind: MembershipKind, username: &str) {
+        self.members
+            .retain(|u| !(u.username.username == username && u.kind == kind))
+    }
 }
 
 use crate::api::GroupRead;

src/userlib/mod.rs

@@ -7,6 +7,7 @@ use crate::{
     api::{
         CreateUserArgs, DeleteHome, DeleteUserArgs, GroupRead, UserDBRead, UserDBWrite, UserRead,
     },
+    group::MembershipKind,
     UserLibError,
 };
 #[allow(unused_imports)]
@@ -132,6 +133,25 @@ impl UserDBLocal {
         }
     }
 
+    fn write_groups(&self, locked_g: &mut files::LockedFileGuard) -> Result<(), UserLibError> {
+        let content = self
+            .groups
+            .iter()
+            .map(|g| (g.borrow().to_string()))
+            .collect::<Vec<String>>()
+            .join("\n");
+        let replace_result = locked_g.replace_contents(content);
+        match replace_result {
+            Ok(_) => Ok(()),
+            Err(e) => Err(format!(
+                "Error during write to the database. \
+            Please doublecheck as the groupdatabase could be corrupted: {}",
+                e,
+            )
+            .into()),
+        }
+    }
+
     fn delete_home(user: &crate::User) -> std::io::Result<()> {
         if let Some(dir) = user.get_home_dir() {
             std::fs::remove_dir_all(dir)
@@ -145,13 +165,9 @@ impl UserDBLocal {
         }
     }
 
-    fn get_group_pos_by_id(&self, id: u32) -> Option<(&crate::Group, usize)> {
+    fn delete_group_by_id(&mut self, gid: u32) {
-        for (i, group) in self.groups.iter().enumerate() {
+        self.groups
-            if group.borrow().get_gid()? == id {
+            .retain(|g| g.borrow().get_gid().expect("groups have to have a gid") != gid);
-                return Some((group, i));
-            }
-        }
-        None
     }
 }
 
@@ -194,28 +210,65 @@ impl UserDBWrite for UserDBLocal {
                 if args.delete_home == DeleteHome::Delete {
                     Self::delete_home(user)?;
                 }
-                let group = self.get_group_pos_by_id(user.get_gid());
+                println!("The users groups: {:#?}", user.get_groups());
-                if let Some((group, id)) = group {
+                // Iterate over the GIDs to avoid borrowing issues
-                    if group
+                let users_groups: Vec<(MembershipKind, u32)> = user
+                    .get_groups()
+                    .iter()
+                    .map(|(k, g)| (*k, g.borrow().get_gid().unwrap()))
+                    .collect();
+                for (kind, group) in users_groups {
+                    println!("Woring on group: {:?} - {}", kind, group);
+                    match kind {
+                        crate::group::MembershipKind::Primary => {
+                            if self
+                                .get_group_by_id(group)
+                                .expect("The group does not exist")
                                 .borrow()
                                 .get_member_names()
-                        .expect("groups have to have members")
+                                .expect("this group allways has a member")
                                 .len()
                                 == 1
                             {
-                        Self::delete_from_group(group, &group_file_content, &mut locked_g)?;
+                                println!(
-                        let _gres = self.groups.remove(id);
+                                    "Deleting group as the user to be deleted is the only member {:?}", self
+                                    .get_group_by_id(group)
+                                    .expect("The group does not exist")
+                                    .borrow()
+                                    .get_member_names()
+                                );
+                                Self::delete_from_group(
+                                    self.get_group_by_id(group)
+                                        .expect("The group does not exist"),
+                                    &group_file_content,
+                                    &mut locked_g,
+                                )?;
+                                let _gres = self.delete_group_by_id(group);
                             } else {
+                                println!("Do not delete the group as the user to be deleted is not the only member");
+                                // remove the from the group instead of deleting the group if he was not the only user in its primary group.
+                                if let Some(group) = self.get_group_by_id(group) {
+                                    group
+                                        .borrow_mut()
+                                        .remove_member(MembershipKind::Primary, args.username)
+                                };
+                                self.write_groups(&mut locked_g)?;
                                 warn!(
-                            "The primary group {} was not empty and is thus not removed.",
+                                    "The primary group (GID: {}) was not empty and is thus not removed. Only the membership has been removed",
-                            group.borrow().get_groupname().unwrap()
+                                    group
                                 );
                             }
-                } else {
+                        }
-                    warn!(
+                        crate::group::MembershipKind::Member => {
-                        "The users primary group could not be found {}",
+                            println!("delete the membership in the group");
-                        user.get_gid()
+                            if let Some(group) = self.get_group_by_id(group) {
-                    )
+                                group
+                                    .borrow_mut()
+                                    .remove_member(MembershipKind::Member, args.username)
+                            };
+                            self.write_groups(&mut locked_g)?;
+                        }
+                    }
                 }
                 // Remove the user from the memory database(HashMap)
                 let res = self.users.remove(args.username);

tests/delete_user_test.rs

@@ -18,7 +18,7 @@ fn test_delete_user_function() {
     let mf = umanux::Files {
         passwd: Some(p.path.clone()),
         shadow: Some(s.path),
-        group: Some(g.path),
+        group: Some(g.path.clone()),
     };
 
     let mut db = umanux::UserDBLocal::load_files(mf).unwrap();
@@ -34,11 +34,20 @@ fn test_delete_user_function() {
     assert_eq!(user_res.unwrap().get_username().unwrap(), "teste");
     let pflines = pf.lines();
     let pflines2 = pf2.lines();
-    for (l1, l2) in pflines.zip(pflines2) {
+    for (l1, l2) in pflines.zip(pflines2.clone()) {
         if l1 != l2 {
             assert!(l1.starts_with("teste"));
             assert!(l2.starts_with("bergfried"));
             break;
         }
     }
+    for line in pflines2 {
+        assert!(!line.starts_with("teste"))
+    }
+    let gf2 = fs::read_to_string(&g.path).unwrap();
+    let gflines2 = gf2.lines();
+    for line in gflines2 {
+        println!("{}", &line);
+        assert!(!line.ends_with("teste"))
+    }
 }