Add effective group as the first element in the call to set groups (#937)

Author: squell

src/su/context.rs

@@ -112,7 +112,7 @@ impl SuContext {
 
             // last argument is the primary group
             group = primary_group.clone();
-            user.groups.push(primary_group.gid);
+            user.groups.insert(0, primary_group.gid);
         }
 
         // add additional group if current user is root

src/system/mod.rs

@@ -276,9 +276,16 @@ pub fn set_target_user(
 ) {
     use std::os::unix::process::CommandExt;
 
-    // add target group to list of additional groups if not present
-    if !target_user.groups.contains(&target_group.gid) {
-        target_user.groups.push(target_group.gid);
+    if let Some(index) = target_user
+        .groups
+        .iter()
+        .position(|id| id == &target_group.gid)
+    {
+        // make sure the requested group id is the first in the list (necessary on FreeBSD)
+        target_user.groups.swap(0, index)
+    } else {
+        // add target group to list of additional groups if not present
+        target_user.groups.insert(0, target_group.gid);
     }
 
     // we need to do this in a `pre_exec` call since the `groups` method in `process::Command` is unstable

test-framework/sudo-compliance-tests/src/sudo/flag_group.rs

@@ -77,6 +77,38 @@ fn adds_group_to_groups_output() -> Result<()> {
     Ok(())
 }
 
+#[test]
+fn supplementary_groups_can_be_made_primary() -> Result<()> {
+    let extra_group = "rustaceans";
+    let env = Env(SUDOERS_ALL_ALL_NOPASSWD)
+        .user(User(USERNAME).secondary_group(extra_group))
+        .group(Group(extra_group))
+        .group("secondary-group")
+        .build()?;
+
+    let stdout = Command::new("groups")
+        .as_user(USERNAME)
+        .output(&env)?
+        .stdout()?;
+    let mut groups_without_sudo = stdout.split_ascii_whitespace().collect::<Vec<_>>();
+
+    let stdout = Command::new("sudo")
+        .args(["-g", extra_group, "groups"])
+        .as_user(USERNAME)
+        .output(&env)?
+        .stdout()?;
+
+    let mut groups_with_sudo = stdout.split_ascii_whitespace().collect::<Vec<_>>();
+
+    assert_eq!(groups_with_sudo[0], extra_group);
+
+    groups_without_sudo.sort();
+    groups_with_sudo.sort();
+    assert_eq!(groups_with_sudo, groups_without_sudo);
+
+    Ok(())
+}
+
 #[test]
 fn group_can_be_specified_by_id() -> Result<()> {
     let expected_gid = 1234;