[Pkg-shadow-devel] [Git][debian/adduser][debian-bug-1099470] 6 commits: lower message severity for "skipping crontab removal"

Marc Haber (@zugschlus) gitlab at salsa.debian.org
Tue Mar 4 08:39:32 GMT 2025



Marc Haber pushed to branch debian-bug-1099470 at Debian / adduser


Commits:
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

- - - - -


6 changed files:

- adduser
- 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';
 
@@ -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];
@@ -1139,12 +1148,14 @@ sub existing_user_status {
     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)) {
+        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?
     } 
+    log_trace( "existing_user_status( %s, %s ) returns %s", $new_name, $new_uid, $ret );
     return $ret;
 }
 
@@ -1162,13 +1173,15 @@ sub existing_group_status {
     my ($new_name,$new_gid) = @_;
     my ($dummy1,$dummy2,$gid);
     my $ret = EXISTING_NOT_FOUND;
+    log_trace( "existing_group_status called with new_name %s, new_gid %s", $new_name, $new_gid );
     if (($dummy1,$dummy2,$gid) = egetgrnam($new_name)) {
-        log_trace("egetgrnam %s returned successfully, $gid = %s", $new_name, $gid);
+        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"});
     } 
+    log_trace( "existing_group_status( %s, %s ) returns %s", $new_name, $new_gid, $ret );
     return $ret;
 }
 


=====================================
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
=====================================
@@ -88,6 +88,66 @@ assert_user_has_disabled_password('aust2');
 # Ref: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1004710
 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');
+
 # clean up
 assert_command_success('/usr/sbin/deluser',
 	'--stdoutmsglevel=error', '--stderrmsglevel=error',


=====================================
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/2866a9312159f44039e02466500c0ce9bd35a909...60bdeb1dcc86be7f35453100500f4266c4415ab1

-- 
View it on GitLab: https://salsa.debian.org/debian/adduser/-/compare/2866a9312159f44039e02466500c0ce9bd35a909...60bdeb1dcc86be7f35453100500f4266c4415ab1
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/77d43112/attachment-0001.htm>


More information about the Pkg-shadow-devel mailing list