pw: use existing group entry even if it already has members

[naman_freebsdfoundation.org]

Summary

Fixes an issue raised in https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=238995.

Signed-off-by: Naman Sood <[e-mail]>

Test Plan

Try adding a new user with a non-login group such that /etc/group already lists that user as a member of that group

[jrm]

Runtime looks good.

Without patch:

1. Add foo:*:12345:someuser to /etc/group.
2. pw user add someuser -G foo

% tail -n2 /etc/group
foo:*:12345:someuser,someuser
someuser:*:1004:

With patch:

1. Add foo:*:12345:someuser to /etc/group.
2. pw user add someuser -G foo

% tail -n2 /etc/group
foo:*:12345:someuser
someuser:*:1002:

Adding @bapt who has done some work here recently.

usr.sbin/pw/pw_user.c

381–382

I'm not clear why this change was made?

[naman_freebsdfoundation.org]

usr.sbin/pw/pw_user.c

381–382

This fixes a different issue raised in that same bug report. To reproduce:

1. Create a group and add at least one member to it for whom it is not the login group (ie. have a nonempty member list in /etc/group)

2. Add a user with the same name as that group and don't specify a login group.

The new user should get the step 1 group as their login group by default, but for some reason (not documented in the git commit history) that only happens if that group has no members in /etc/group. Failing this check, pw creates a duplicate entry for that group in /etc/group.

[bapt]

LGTM, I would appreciate if the 2 issues are solved in 2 differents commits, also since the rewrite pw(8) has a test suite, it would be nice to add regression tests for the issues fixed here, along with the fixes.

usr.sbin/pw/pw_user.c

381–382

should be imho in 2 differents commits.

[naman_freebsdfoundation.org]

Split the changes into two commits and added tests. Checked that the tests fail before the two commits and pass after.

• pw: use existing group entry even if it already has members
• pw: check for user in group entry before adding them
• pw: add regression tests

[jrm]

Runtime test of the second report in bug 238995.

Without patch:

1. Add aarch64:*:31415:aarch64 to /etc/group.
2. pw user add aarch64 -u 31415

% tail -n2 /etc/group
aarch64:*:31415:aarch64
aarch64:*:31416:

With patch:
Steps 1. and 2. above, then

% grep aarch64 /etc/group
aarch64:*:31415:aarch64

Looks good.

[jrm]

With new tests, but without pw updates:

# kyua report
===> Failed tests
pw_useradd_test:user_add_already_in_group  ->  failed: atf-check failed; see the output of the test for details  [0.090s]
pw_useradd_test:user_add_existing_login_group  ->  failed: atf-check failed; see the output of the test for details  [0.138s]
===> Summary
Results read from /root/.kyua/store/results.usr_tests_usr.sbin_pw.20230718-192827-913107.db
Test cases: 95 total, 0 skipped, 0 expected failures, 0 broken, 2 failed
Total time: 13.330s

With new tests and pw updates:

# kyua report
===> Summary
Results read from /root/.kyua/store/results.usr_tests_usr.sbin_pw.20230718-193628-332707.db
Test cases: 95 total, 0 skipped, 0 expected failures, 0 broken, 0 failed
Total time: 10.816s

[naman_freebsdfoundation.org]

Updated to only include the first commit after discussion with @jrm. Will make separate PRs for the other two commits.

Edit: the other code change is in D41076.