[Pkg-shadow-devel] [Git][debian/adduser][master] 16 commits: rename test user foo to aust in adduser_system.t
Marc Haber (@zugschlus)
gitlab at salsa.debian.org
Tue Mar 4 19:12:39 GMT 2025
Marc Haber pushed to branch master at Debian / adduser
Commits:
da620467 by Marc Haber at 2025-03-04T08:13:18+01:00
rename test user foo to aust in adduser_system.t
Git-Dch: ignore
- - - - -
eeaff10e by Marc Haber at 2025-03-04T08:13:47+01:00
adduser_system.t: clean up after test
Git-Dch: ignore
- - - - -
f9305d67 by Marc Haber at 2025-03-04T08:15:08+01:00
adduser_system.t: delete user before trying to create it
Git-Dch: ignore
- - - - -
008f1dbc by Marc Haber at 2025-03-04T08:15:57+01:00
adduser_system.t: create user a second time in a row
Git-Dch: ignore
- - - - -
1dafe6c9 by Marc Haber at 2025-03-04T08:16:35+01:00
adduser_system.t: formatting
Git-Dch: ignore
- - - - -
2866a931 by Marc Haber at 2025-03-04T08:17:00+01:00
reduce priority of "system user already exists" message
- - - - -
6ef9cefc by Marc Haber at 2025-03-04T09:15:57+01:00
lower message severity for "skipping crontab removal"
Otherwise, deluser --sytem on a cronless system might not be silent
- - - - -
f775ad84 by Marc Haber at 2025-03-04T09:15:57+01:00
get rid of the string foo in tests
Git-Dch: ignore
- - - - -
e9290ba4 by Marc Haber at 2025-03-04T09:15:57+01:00
move home directory warnings to add(sys)user with appropriate severity
- - - - -
54dae790 by Marc Haber at 2025-03-04T09:15:57+01:00
streamline debugging in existing_*_status
Git-Dch: ignore
- - - - -
fda16705 by Marc Haber at 2025-03-04T09:15:57+01:00
reminder comment for necessary change
Git-Dch: ignore
- - - - -
60bdeb1d by Marc Haber at 2025-03-04T09:15:57+01:00
add more tests to catch the bug under #1099470
Git-Dch: ignore
- - - - -
230cf545 by Matt Barry at 2025-03-04T11:20:32-05:00
cleanup check_user_group, mismatch logic
- - - - -
a63119fc by Marc Haber at 2025-03-04T17:52:14+01:00
do also a test with * as password
Git-Dch: ignore
- - - - -
9bb8e6c9 by Matt Barry at 2025-03-04T13:03:07-05:00
clean up logic for existing system users
- - - - -
d8581ffe by Matt Barry at 2025-03-04T13:19:01-05:00
changelog
- - - - -
7 changed files:
- adduser
- debian/changelog
- debian/tests/control
- debian/tests/f/adduser_system.t
- debian/tests/lib/AdduserTestsCommon.pm
- deluser
- log_test
Changes:
=====================================
adduser
=====================================
@@ -396,15 +396,6 @@ if ((defined($special_home)) && ($special_home !~ m+^/+ )) {
exit( RET_INVALID_HOME_DIRECTORY );
}
-if (defined($special_home) && $special_home ne "/nonexistent" ) {
- if (!defined($no_create_home) && -d $special_home) {
- log_warn( mtx("The home dir %s you specified already exists.\n"), $special_home );
- }
- if (defined($no_create_home) && ! -d $special_home) {
- log_warn( mtx("The home dir %s you specified can't be accessed: %s\n"), $special_home, $! );
- }
-}
-
$SIG{'INT'} = $SIG{'QUIT'} = $SIG{'HUP'} = 'handler';
@@ -567,7 +558,7 @@ if ($action eq "addsysuser") {
acquire_lock();
my $ret = existing_user_status($new_name, $new_uid);
- if ($ret == (EXISTING_FOUND|EXISTING_SYSTEM)) {
+ if (($ret & EXISTING_FOUND) && !($ret & EXISTING_SYSTEM)) {
# a user with this name already exists; it's a problem when it's not a system user
log_fatal( mtx("The user `%s' already exists, but is not a system user. Exiting."), $new_name );
exit( RET_WRONG_OBJECT_PROPERTIES );
@@ -577,7 +568,7 @@ if ($action eq "addsysuser") {
exit( RET_WRONG_OBJECT_PROPERTIES );
}
if ($ret & EXISTING_FOUND) {
- log_fatal( mtx("The system user `%s' already exists. Exiting.\n"), $new_name );
+ log_info( mtx("The system user `%s' already exists. Exiting.\n"), $new_name );
exit( RET_OK );
}
@@ -647,6 +638,15 @@ if ($action eq "addsysuser") {
}
}
+ if (defined($special_home) && $special_home ne "/nonexistent" ) {
+ if (!defined($no_create_home) && -d $special_home) {
+ log_info( mtx("The home dir %s you specified already exists.\n"), $special_home );
+ }
+ if (defined($no_create_home) && ! -d $special_home) {
+ log_info( mtx("The home dir %s you specified can't be accessed: %s\n"), $special_home, $! );
+ }
+ }
+
log_info( mtx("Adding new user `%s' (UID %d) with group `%s' ..."),
$new_name, $new_uid, $ingroup_name );
$home_dir = $special_home || $uid_pool{$new_name}{'home'} || '/nonexistent';
@@ -884,6 +884,15 @@ if ($action eq "adduser") {
}
}
+ if (defined($special_home) && $special_home ne "/nonexistent" ) {
+ if (!defined($no_create_home) && -d $special_home) {
+ log_warn( mtx("The home dir %s you specified already exists.\n"), $special_home );
+ }
+ if (defined($no_create_home) && ! -d $special_home) {
+ log_warn( mtx("The home dir %s you specified can't be accessed: %s\n"), $special_home, $! );
+ }
+ }
+
{
my @grgid=getgrgid($primary_gid);
my $grname=$grgid[0];
@@ -1135,16 +1144,18 @@ sub mktree {
# return 8|2|1 == 11
sub existing_user_status {
my ($new_name,$new_uid) = @_;
- my ($dummy1,$pw,$uid);
+ my ($pw,$uid);
my $ret = EXISTING_NOT_FOUND;
log_trace( "existing_user_status called with new_name %s, new_uid %s", $new_name, $new_uid );
- if (($dummy1,$pw,$uid) = egetpwnam($new_name)) {
+ if ((undef,$pw,$uid) = egetpwnam($new_name)) {
+ log_trace("egetpwnam %s returned successfully, uid = %s", $new_name, $uid);
$ret |= EXISTING_FOUND;
$ret |= EXISTING_ID_MISMATCH if (defined($new_uid) && $uid != $new_uid);
$ret |= EXISTING_SYSTEM if \
($uid >= $config{"first_system_uid"} && $uid <= $config{"last_system_uid"});
- $ret |= EXISTING_LOCKED if (substr($pw,0,1) eq "!"); # TODO: also check expiry?
- }
+ $ret |= EXISTING_ID_MISMATCH if ($new_uid && getpwuid($new_uid));
+ }
+ log_trace( "existing_user_status( %s, %s ) returns %s", $new_name, $new_uid, $ret );
return $ret;
}
@@ -1160,15 +1171,19 @@ sub existing_user_status {
# EXISTING_ID_MISMATCH => 4
sub existing_group_status {
my ($new_name,$new_gid) = @_;
- my ($dummy1,$dummy2,$gid);
+ my $gid;
my $ret = EXISTING_NOT_FOUND;
- if (($dummy1,$dummy2,$gid) = egetgrnam($new_name)) {
- log_trace("egetgrnam %s returned successfully, $gid = %s", $new_name, $gid);
+ log_trace( "existing_group_status called with new_name %s, new_gid %s", $new_name, $new_gid );
+ if ((undef,undef,$gid) = egetgrnam($new_name)) {
+ log_trace("egetgrnam %s returned successfully, gid = %s", $new_name, $gid);
$ret |= EXISTING_FOUND;
$ret |= EXISTING_ID_MISMATCH if (defined($new_gid) && $gid != $new_gid);
$ret |= EXISTING_SYSTEM if \
($gid >= $config{"first_system_gid"} && $gid <= $config{"last_system_gid"});
- }
+ } elsif ($new_gid && getgrgid($new_gid)) {
+ $ret |= EXISTING_ID_MISMATCH;
+ }
+ log_trace( "existing_group_status( %s, %s ) returns %s", $new_name, $new_gid, $ret );
return $ret;
}
@@ -1181,21 +1196,24 @@ sub existing_group_status {
sub check_user_group {
my ($system) = @_;
log_debug( "check_user_group %s called, make_group_also %s", $system, $make_group_also );
- if( !$system || !existing_user_status($new_name, $new_uid) ) {
- if( defined egetpwnam($new_name) ) {
- if( $system ) {
- log_fatal( mtx("The user `%s' already exists, and is not a system user."), $new_name);
- exit( RET_WRONG_OBJECT_PROPERTIES );
- } else {
- log_fatal( mtx("The user `%s' already exists."), $new_name);
- exit( RET_OBJECT_EXISTS );
- }
+
+ my $ustat = existing_user_status($new_name, $new_uid);
+ if ($system) {
+ if (($ustat & EXISTING_FOUND) && !($ustat & EXISTING_SYSTEM)) {
+ log_fatal( mtx("The user `%s' already exists, and is not a system user."), $new_name);
+ exit( RET_WRONG_OBJECT_PROPERTIES );
}
- if (defined($new_uid) && getpwuid($new_uid)) {
- log_fatal( mtx("The UID %d is already in use."), $new_uid);
- exit( RET_ID_IN_USE );
+ # if ($new_uid && !($ustat & EXISTING_SYSTEM)) {
+ # log_fatal( mtx("The uid `%s' is invalid for system users."), $new_name);
+ # exit( RET_OBJECT_EXISTS );
+ # }
+ } else {
+ if ($ustat & EXISTING_FOUND) {
+ log_fatal( mtx("The user `%s' already exists."), $new_name);
+ exit( RET_OBJECT_EXISTS );
}
}
+
if ($make_group_also) {
log_trace( "make_group_also 1, new_name %s, new_uid %s", $new_name, $new_uid );
if( !$system || !existing_group_status($new_name, $new_uid) ) {
=====================================
debian/changelog
=====================================
@@ -1,6 +1,7 @@
adduser (3.144~) UNRELEASED; urgency=medium
- *
+ [ Matt Barry ]
+ * Clean up system account logic
-- Marc Haber <mh+debian-packages at zugschlus.de> Mon, 03 Mar 2025 19:57:46 +0100
=====================================
debian/tests/control
=====================================
@@ -6,6 +6,6 @@ Test-Command: cd testsuite/ && ./runsuite.sh
Depends: adduser, cron, perl, login
Restrictions: allow-stderr breaks-testbed needs-root
-Test-Command: /usr/sbin/adduser --system foo
+Test-Command: /usr/sbin/adduser --system austc3
Depends: adduser, login
Restrictions: needs-root
=====================================
debian/tests/f/adduser_system.t
=====================================
@@ -4,6 +4,7 @@
# default behavior one can expect when creating a new system user. It should
# match the behavior as specified in the adduser(8) man page and vice versa.
+# user name aust stands for "adduser test"
use diagnostics;
use strict;
@@ -13,8 +14,8 @@ use AdduserTestsCommon;
END {
- remove_tree('/home/foo');
- remove_tree('/var/mail/foo');
+ remove_tree('/home/aust');
+ remove_tree('/var/mail/aust');
}
my $uid;
@@ -27,56 +28,158 @@ for (100..999) {
last;
}
-assert_user_does_not_exist('foo');
+assert_command_success('/usr/sbin/deluser',
+ '--stdoutmsglevel=error', '--stderrmsglevel=error',
+ '--system',
+ 'aust');
+assert_user_does_not_exist('aust');
assert_path_does_not_exist('/nonexistent');
assert_command_success('/usr/sbin/adduser',
'--stdoutmsglevel=error', '--stderrmsglevel=error',
'--system',
- 'foo');
-assert_user_exists('foo');
+ 'aust');
+assert_user_exists('aust');
-assert_user_has_uid('foo', $uid);
+assert_command_success('/usr/sbin/adduser',
+ '--stdoutmsglevel=error', '--stderrmsglevel=error',
+ '--system',
+ 'aust');
+assert_user_exists('aust');
+assert_user_has_uid('aust', $uid);
-assert_group_does_not_exist('foo');
-assert_primary_group_membership_exists('foo', 'nogroup');
+assert_group_does_not_exist('aust');
+assert_primary_group_membership_exists('aust', 'nogroup');
-assert_user_has_home_directory('foo', '/nonexistent');
+assert_user_has_home_directory('aust', '/nonexistent');
assert_path_does_not_exist('/nonexistent');
-assert_user_has_login_shell('foo', '/usr/sbin/nologin');
+assert_user_has_login_shell('aust', '/usr/sbin/nologin');
-assert_user_has_disabled_password('foo');
+assert_user_has_disabled_password('aust');
# Ref: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1004710
-assert_path_does_not_exist('/var/mail/foo');
+assert_path_does_not_exist('/var/mail/aust');
while (defined(getpwuid($uid))) {
$uid++;
}
-assert_user_does_not_exist('foo2');
+assert_user_does_not_exist('aust2');
assert_path_does_not_exist('/nonexistent');
assert_command_success('/usr/sbin/adduser',
'--stdoutmsglevel=error', '--stderrmsglevel=error',
'--system',
'--shell', '/bin/sh',
- 'foo2');
-assert_user_exists('foo2');
-
-assert_user_has_uid('foo2', $uid);
+ 'aust2');
+assert_user_exists('aust2');
+assert_user_has_uid('aust2', $uid);
-assert_group_does_not_exist('foo2');
-assert_primary_group_membership_exists('foo2', 'nogroup');
+assert_group_does_not_exist('aust2');
+assert_primary_group_membership_exists('aust2', 'nogroup');
-assert_user_has_home_directory('foo2', '/nonexistent');
+assert_user_has_home_directory('aust2', '/nonexistent');
assert_path_does_not_exist('/nonexistent');
-assert_user_has_login_shell('foo2', '/bin/sh');
+assert_user_has_login_shell('aust2', '/bin/sh');
-assert_user_has_disabled_password('foo2');
+assert_user_has_disabled_password('aust2');
# Ref: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1004710
-assert_path_does_not_exist('/var/mail/foo2');
+assert_path_does_not_exist('/var/mail/aust2');
+
+# Ref: bug #1099470, create and recreate a locked account
+# This might cause some grief when we address #1008082 - #1008084
+assert_command_success('/usr/sbin/deluser',
+ '--stdoutmsglevel=error', '--stderrmsglevel=error',
+ '--system',
+ 'aust');
+assert_user_does_not_exist('aust');
+assert_command_success('/usr/sbin/adduser',
+ '--stdoutmsglevel=error', '--stderrmsglevel=error',
+ '--system',
+ '--disabled-password',
+ 'aust');
+assert_user_exists('aust');
+
+assert_command_success('/usr/sbin/adduser',
+ '--stdoutmsglevel=error', '--stderrmsglevel=error',
+ '--system',
+ '--disabled-password',
+ 'aust');
+assert_user_exists('aust');
+
+assert_command_success('/usr/sbin/deluser',
+ '--stdoutmsglevel=error', '--stderrmsglevel=error',
+ '--system',
+ 'aust');
+assert_user_does_not_exist('aust');
+assert_command_success('/usr/sbin/adduser',
+ '--stdoutmsglevel=error', '--stderrmsglevel=error',
+ '--system',
+ '--disabled-login',
+ 'aust');
+assert_user_exists('aust');
+
+assert_command_success('/usr/sbin/adduser',
+ '--stdoutmsglevel=error', '--stderrmsglevel=error',
+ '--system',
+ '--disabled-login',
+ 'aust');
+assert_user_exists('aust');
+
+assert_command_success('/usr/sbin/deluser',
+ '--stdoutmsglevel=error', '--stderrmsglevel=error',
+ '--system',
+ 'aust');
+assert_user_does_not_exist('aust');
+assert_command_success('/usr/sbin/adduser',
+ '--stdoutmsglevel=error', '--stderrmsglevel=error',
+ '--system',
+ '--disabled-login',
+ 'aust');
+assert_user_exists('aust');
+
+system('echo "aust:*" | chpasswd --encrypted');
+assert_command_success('/usr/sbin/adduser',
+ '--stdoutmsglevel=error', '--stderrmsglevel=error',
+ '--system',
+ '--disabled-login',
+ 'aust');
+assert_user_exists('aust');
+
+system('echo "aust:!foobar" | chpasswd --encrypted');
+assert_command_success('/usr/sbin/adduser',
+ '--stdoutmsglevel=error', '--stderrmsglevel=error',
+ '--system',
+ '--disabled-login',
+ 'aust');
+assert_user_exists('aust');
+
+system('echo "aust:*foobar" | chpasswd --encrypted');
+assert_command_success('/usr/sbin/adduser',
+ '--stdoutmsglevel=error', '--stderrmsglevel=error',
+ '--system',
+ '--disabled-login',
+ 'aust');
+assert_user_exists('aust');
+
+# clean up
+assert_command_success('/usr/sbin/deluser',
+ '--stdoutmsglevel=error', '--stderrmsglevel=error',
+ '--system',
+ 'aust');
+assert_command_success('/usr/sbin/deluser',
+ '--stdoutmsglevel=error', '--stderrmsglevel=error',
+ '--system',
+ 'aust2');
+assert_command_success('/usr/sbin/delgroup',
+ '--stdoutmsglevel=error', '--stderrmsglevel=error',
+ '--system',
+ 'aust');
+assert_command_success('/usr/sbin/delgroup',
+ '--stdoutmsglevel=error', '--stderrmsglevel=error',
+ '--system',
+ 'aust2');
# vim: tabstop=4 shiftwidth=4 expandtab
=====================================
debian/tests/lib/AdduserTestsCommon.pm
=====================================
@@ -294,6 +294,9 @@ sub user_uid_exists {
}
sub assert_user_has_disabled_password {
+ # this should be changed to check against the varios ways of
+ # checking an account: !, * in shadow, and expiry. It should change
+ # to a two-argument variant that checks for certain kind of lock
my $user = shift;
my $name = "user has disabled password: $user";
=====================================
deluser
=====================================
@@ -420,8 +420,13 @@ if($action eq "deluser") {
systemcall_or_warn('/usr/bin/crontab', '-u', $user, '-r');
}
} else {
- log_warn( mtx("`%s' not executed. Skipping crontab removal. Package `cron' required."),
+ if ($config{'system'}) {
+ log_info( mtx("`%s' not executed. Skipping crontab removal. Package `cron' required."),
'/usr/bin/crontab' );
+ } else {
+ log_warn( mtx("`%s' not executed. Skipping crontab removal. Package `cron' required."),
+ '/usr/bin/crontab' );
+ }
}
log_info( mtx("Removing user `%s' ..."), $user);
=====================================
log_test
=====================================
@@ -7,7 +7,7 @@ use Debian::AdduserLogging;
set_msglevel( "error", "warn", "info");
my $message = "this is the message";
-log_trace("mtest trace message: %s", "foo");
+log_trace("mtest trace message: %s", "test string");
log_debug("mtest debug message");
log_info("mtest info message");
log_warn("mtest warn message");
View it on GitLab: https://salsa.debian.org/debian/adduser/-/compare/e940103ab1480e400bbad2b2ae705932ea3142a1...d8581ffe4a9b63056099436ff5421d8220efef97
--
View it on GitLab: https://salsa.debian.org/debian/adduser/-/compare/e940103ab1480e400bbad2b2ae705932ea3142a1...d8581ffe4a9b63056099436ff5421d8220efef97
You're receiving this email because of your account on salsa.debian.org.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://alioth-lists.debian.net/pipermail/pkg-shadow-devel/attachments/20250304/2d0ec3ed/attachment-0001.htm>
More information about the Pkg-shadow-devel
mailing list