diff --git a/src/uu/usermod/src/usermod.rs b/src/uu/usermod/src/usermod.rs index 27fdec8..f2a1280 100644 --- a/src/uu/usermod/src/usermod.rs +++ b/src/uu/usermod/src/usermod.rs @@ -31,6 +31,7 @@ mod options { pub const GID: &str = "gid"; pub const GROUPS: &str = "groups"; pub const APPEND: &str = "append"; + pub const REMOVE: &str = "remove"; pub const LOCK: &str = "lock"; pub const UNLOCK: &str = "unlock"; pub const LOGIN: &str = "login"; @@ -392,17 +393,20 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { wanted.push(name); } - if !append { - for g in group_file.entries_mut() { - g.members.retain(|m| m != login && m != member); + // -r drops the user from the listed groups only, -a adds it to them, + // and plain -G makes them the user's whole list. + let remove = matches.get_flag(options::REMOVE); + let update = |name: &str, members: &mut Vec| { + let listed = wanted.iter().any(|w| w == name); + if (remove && listed) || (!remove && !append) { + members.retain(|m| m != login && m != member); } - } - for gname in &wanted { - if let Some(g) = group_file.find_mut(gname) - && !g.members.iter().any(|m| m == member) - { - g.members.push(member.clone()); + if listed && !remove && !members.iter().any(|m| m == member) { + members.push(member.clone()); } + }; + for g in group_file.entries_mut() { + update(&g.name, &mut g.members); } // /etc/gshadow carries the same membership lists; leaving it @@ -414,17 +418,8 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { LockedFile::::open(&gshadow_path).map_err(|e| { UsermodError::CantUpdateGroup(format!("cannot open gshadow: {e}")) })?; - if !append { - for g in gshadow_file.entries_mut() { - g.members.retain(|m| m != login && m != member); - } - } - for gname in &wanted { - if let Some(g) = gshadow_file.find_mut(gname) - && !g.members.iter().any(|m| m == member) - { - g.members.push(member.clone()); - } + for g in gshadow_file.entries_mut() { + update(&g.name, &mut g.members); } files.push(Box::new(gshadow_file)); } @@ -636,6 +631,15 @@ pub fn uu_app() -> Command { .help("Add to the supplementary groups instead of replacing them (only effective with -G)") .action(ArgAction::SetTrue), ) + .arg( + Arg::new(options::REMOVE) + .short('r') + .long("remove") + .requires(options::GROUPS) + .conflicts_with(options::APPEND) + .help("Remove from the supplementary groups given with -G only") + .action(ArgAction::SetTrue), + ) .arg( Arg::new(options::LOCK) .short('L') @@ -781,6 +785,25 @@ mod tests { ); } + #[test] + fn test_remove_needs_groups_and_excludes_append() { + assert!( + uu_app() + .try_get_matches_from(["usermod", "-r", "-G", "sudo", "u"]) + .is_ok() + ); + assert!( + uu_app() + .try_get_matches_from(["usermod", "-r", "u"]) + .is_err() + ); + assert!( + uu_app() + .try_get_matches_from(["usermod", "-a", "-r", "-G", "sudo", "u"]) + .is_err() + ); + } + fn skip_unless_root() -> bool { !rustix::process::geteuid().is_root() } diff --git a/tests/by-util/test_usermod.rs b/tests/by-util/test_usermod.rs index 1f1257c..4b65a0e 100644 --- a/tests/by-util/test_usermod.rs +++ b/tests/by-util/test_usermod.rs @@ -376,6 +376,26 @@ fn test_add_to_groups() { ); } +#[test] +fn test_remove_from_groups() { + if crate::common::skip_unless_root() { + return; + } + + let dir = setup_prefix( + "testuser:x:1000:1000:Test:/home/testuser:/bin/bash\n", + "testuser:$6$hash:19500:0:99999:7:::\n", + "testuser:x:1000:\ngroup1:x:2001:testuser\ngroup2:x:2002:other,testuser\ngroup3:x:2003:testuser\n", + ); + + let code = run_with_prefix(&dir, &["-r", "-G", "group1,group2", "testuser"]); + assert_eq!(code, 0, "removing from supplementary groups should succeed"); + assert_eq!( + read_group(&dir), + "testuser:x:1000:\ngroup1:x:2001:\ngroup2:x:2002:other\ngroup3:x:2003:testuser\n" + ); +} + #[test] fn test_lock_user() { if crate::common::skip_unless_root() {