From c2b242b4e882697e13436d4776420671f0738540 Mon Sep 17 00:00:00 2001 From: stdweird Date: Sun, 26 Nov 2017 20:03:38 +0100 Subject: [PATCH] process: address remarks, add some refactoring for unittesting (to be squashed) --- src/main/perl/Process.pm | 106 ++++++++------- src/test/perl/process.t | 270 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 329 insertions(+), 47 deletions(-) diff --git a/src/main/perl/Process.pm b/src/main/perl/Process.pm index a9d7aecd..cc39f7f5 100644 --- a/src/main/perl/Process.pm +++ b/src/main/perl/Process.pm @@ -177,25 +177,29 @@ C<< command: [ ]>>. =cut +# if mode is user, return uid and primary gid of +# the user in the user attribute ($self->{user}). +# if mode is group, return gid and undef of +# the group in the group attribute ($self->{group}). sub _get_uid_gid { my ($self, $mode) = @_; my @res; - my $what = $self->{$mode}; - if (defined($what)) { - my $is_id = $what =~ m/^\d+$/ ? 1 : 0; + my $target = $self->{$mode}; + if (defined($target)) { + my $is_id = $target =~ m/^\d+$/ ? 1 : 0; my $is_user = $mode eq 'user' ? 1 : 0; # This is ugly # But you cannot reference the builtin functions, # maybe by using simple wrapper like my $fn = sub { builtin(@_) } (eg sub {getpwname($_[0])}) - # But the getpw / getgr functions are safe to use (they do not die, just return undef) + # But the getpw / getgr functions are safe to use (they do not die, just return empty list) # so no _safe_eval and a funcref required # For the is_id case, strictly not needed to check details, since setuid can change to non-known user # But we don't allow that here. my @info = $is_id ? - ($is_user ? getpwuid($what) : getgrgid($what)) : - ($is_user ? getpwnam($what) : getgrnam($what)); + ($is_user ? getpwuid($target) : getgrgid($target)) : + ($is_user ? getpwnam($target) : getgrnam($target)); # What do we need from info: the IDs, and for users, also the primary groups if (@info) { @@ -203,13 +207,49 @@ sub _get_uid_gid # grnam/uid: gid=2 @res = ($info[2], $is_user ? $info[3] : undef); } else { - $self->error("No such $mode $what (is user $is_user; is id $is_id)"); + $self->error("No such $mode $target (is user $is_user; is id $is_id)"); } } return @res; } +# minimal functions for mocking +sub _uid {return $UID;}; +sub _euid {return $EUID;}; +sub _gid {return "$GID";}; # as string +sub _egid {return "$EGID";}; # as string +sub _set_euid {$EUID = $_[0];}; +sub _set_egid {$EGID = $_[0];}; + +# set euid or egid, with verification and reporting +# return 1 or 0 +# report error on failure +sub _set_uid_gid +{ + my ($self, $target, $set, $get, $name, $suff, $action) = @_; + + return 1 if ! defined($target); + + my $value = &$get; + my $msg = "$name from $value to $suff"; + # stringification to handle numeric case + if ("$value" eq "$target") { + $self->verbose(ucfirst($action)." $msg: no changes required") + } else { + &$set($target); + $value = &$get; + if ("$value" eq "$target") { + $self->verbose(ucfirst($action)." $msg") + } else { + $self->error("Something went wrong $action $msg: new $name $value, reason $!"); + return 0; + } + } + return 1; +}; + + # set euid/egid if user and/or group was set # returns 1 on success. # on failure, report error and return undef @@ -217,63 +257,34 @@ sub _set_eff_user_group { my ($self, $orig) = @_; - my ($uid, $gid, $pri_gid, $gid_full, $oper); + my ($uid, $gid, $pri_gid, $gid_full, $action); my $restore = defined($orig) ? 1 : 0; if ($restore) { - $oper = "restoring"; + $action = "restoring"; ($uid, $gid) = @$orig; # We assume the original gid is the original list of groups $gid_full = "$gid"; } else { - $oper = "changing"; + $action = "changing"; # has to be array context ($uid, $pri_gid) = $self->_get_uid_gid('user'); - ($gid) = $self->_get_uid_gid('group'); + ($gid, undef) = $self->_get_uid_gid('group'); # use user primary group when no group specified $gid = $pri_gid if defined $uid && ! defined $gid; # This is how you set the GID to only the GID (i.e. no other groups) $gid_full = "$gid $gid" if defined $gid; } - # return 1 or 0 my $set_user = sub { - return 1 if ! defined $uid; - - my $msg = "EUID from $EUID to $uid with UID $UID"; - if ($EUID == $uid) { - $self->verbose(ucfirst($oper)." $msg: no changes required") - } else { - $EUID = $uid; - if ($EUID == $uid) { - $self->verbose(ucfirst($oper)." $msg") - } else { - $self->error("Something went wrong $oper $msg: $!"); - return 0; - } - } - return 1; + return $self->_set_uid_gid($uid, \&_set_euid, \&_euid, 'EUID', "$uid with UID "._uid, $action); }; # return 1 or 0 my $set_group = sub { - return 1 if ! defined($gid); - - my $msg = "EGID from $EGID to $gid with GID $GID"; - if ($EGID eq $gid_full) { - $self->verbose(ucfirst($oper)." $msg: no changes required") - } else { - $EGID = $gid_full; - if ($EGID eq $gid_full) { - $self->verbose(ucfirst($oper)." $msg") - } else { - $self->error("Something went wrong $oper $msg: new EGID $EGID, reason $!"); - return 0; - } - } - return 1; + return $self->_set_uid_gid($gid_full, \&_set_egid, \&_egid, 'EGID', "$gid with GID "._gid, $action); }; my $res = 0; @@ -306,10 +317,15 @@ sub _LC_Process $? = 0; $res = $noaction_value; } else { + + my $current_user_group = !(defined $self->{user} or defined $self->{group}); + # The original GID (as list of groups) - my $orig_user_group = [$UID, "$GID"]; + # Only relevant if curent_user_group is false + my $orig_user_group; + $orig_user_group = [_uid, _gid] if !$current_user_group; - if ($self->_set_eff_user_group()) { + if ($current_user_group or $self->_set_eff_user_group()) { my $funcref = $LC_PROCESS_DISPATCH{$function}; if (defined($funcref)) { $res = $funcref->(@$args); @@ -320,7 +336,7 @@ sub _LC_Process } # always try to restore - $self->_set_eff_user_group($orig_user_group); + $self->_set_eff_user_group($orig_user_group) if !$current_user_group; } return $res; diff --git a/src/test/perl/process.t b/src/test/perl/process.t index b993fccc..323e8d8a 100644 --- a/src/test/perl/process.t +++ b/src/test/perl/process.t @@ -1,6 +1,18 @@ use strict; use warnings; +# hello mocking +my $iddata; +my $idcalled; +BEGIN { + # Before CAF::Process + *CORE::GLOBAL::getpwuid = sub {$idcalled->{getpwuid} += 1;return @{$iddata->{getpwuid}};}; + *CORE::GLOBAL::getpwnam = sub {$idcalled->{getpwnam} += 1;return @{$iddata->{getpwnam}};}; + *CORE::GLOBAL::getgrnam = sub {$idcalled->{getgrnam} += 1;return @{$iddata->{getgrnam}};}; + *CORE::GLOBAL::getgrgid = sub {$idcalled->{getgrgid} += 1;return @{$iddata->{getgrgid}};}; +} + + use FindBin qw($Bin); use lib "$Bin/modules"; use testapp; @@ -11,6 +23,27 @@ use Test::Quattor::Object; use Test::MockModule; my $mock = Test::MockModule->new ("CAF::Process"); +# After CAF::Process +no warnings 'redefine'; +*CAF::Process::_uid = sub {$idcalled->{uid} += 1;return $iddata->{uid};}; +*CAF::Process::_euid = sub {$idcalled->{euid} += 1;return $iddata->{euid};}; +*CAF::Process::_gid = sub {$idcalled->{gid} += 1;return $iddata->{gid};}; +*CAF::Process::_egid = sub {$idcalled->{egid} += 1;return $iddata->{egid};}; +*CAF::Process::_set_euid = sub {$idcalled->{seuid} += 1;$iddata->{euid} = $_[0];}; +*CAF::Process::_set_egid = sub {$idcalled->{segid} += 1;$iddata->{egid} = $_[0];}; +use warnings 'redefine'; + +$iddata = { + uid => 122, + euid => 122, + gid => "123 123 10 20 30", + egid => "123 123 10 20 30", + getpwuid => [qw(myself x 122 123)], + getpwnam => [qw(myself x 122 123)], + getgrgid => [qw(mygroup alias 123 myself)], + getgrnam => [qw(mygroup alias 123 myself)], +}; + my $obj = Test::Quattor::Object->new(); my ($p, $this_app, $str, $fh, $out, $out2); @@ -40,7 +73,11 @@ open ($fh, ">", \$str); $this_app = testapp->new ($0, qw (--verbose)); $this_app->config_reporter(logfile => $fh); -$p = CAF::Process->new ($command, log => $obj); +=head2 Test no logging + +=cut + +$p = CAF::Process->new ($command); $p->execute (); is ($execute, 1, "execute called with no logging"); ok (@$cmd == @$command, "Correct command called by execute"); @@ -66,6 +103,11 @@ ok (@$cmd == @$command, "Correct command called by toutput"); ok (!defined ($str), "Nothing logged by toutput"); init_test(); + +=head2 Test with logging + +=cut + # Let's test this with a few options, especially logging. $p = CAF::Process->new ($command, log => $this_app, stdin => "Something"); @@ -124,6 +166,10 @@ like ($str, qr/Running the command: ls /, "run logged with sensitive mode (command not in log)"); +=head2 pushargs / setopts + +=cut + init_test(); # Let's test the rest of the commands $p->pushargs (qw (this does not matter at all)); @@ -136,7 +182,11 @@ ok (@$cmd == @$command, "The command got options appended"); $str = undef; $p->setopts (stdout => \$str); is($str, "", "Stdout is initialized"); -# Test the NoAction flag + +=head2 Test the NoAction flag + +=cut + $CAF::Object::NoAction = 1; init_test(); $p = CAF::Process->new($command); @@ -158,11 +208,19 @@ ok(!$p->{NoAction}, $p = CAF::Process->new($command, keeps_state => 0); ok($p->{NoAction}, "Respect NoAction if the command changes the state"); +=head2 stringification + +=cut + $p = CAF::Process->new($command); my $command_str = join(" ", @$command); is($p->stringify_command, $command_str, "stringify_command returns joined command"); is("$p", $command_str, "overloaded stringification"); +=head2 is_executable / get_executable + +=cut + is(join(" ", @{$p->get_command}), $command_str, "get_command returns ref to command list"); is($p->get_executable, "ls", "get_executable returns executable"); @@ -183,5 +241,213 @@ $p = CAF::Process->new([]); is("$p", "", "Empty command process is empty string"); ok(! $p, "Empty process is logical false (autogeneration of overloaded bool via new stringify)"); +=head2 _set_eff_user_group / _set_uid_gid / _get_uid_gid + +=cut + +$p = CAF::Process->new ($command, log => $obj); +$p->{user} = undef; +$p->{group} = undef; +is_deeply([$p->_get_uid_gid('user')], [], "_get_uid_gid user returns empty array on missing user attr"); +is_deeply([$p->_get_uid_gid('group')], [], "_get_uid_gid group returns empty array on missing group attr"); + +$idcalled = {}; +$p->{user} = 'x'; +is_deeply([$p->_get_uid_gid('user')], [122, 123], "get_uid_gid user returns uid and prim gid"); +is_deeply($idcalled, {getpwnam => 1}, 'get_uid_gid user used pwnam'); + +$idcalled = {}; +$p->{user} = 122; +is_deeply([$p->_get_uid_gid('user')], [122, 123], "get_uid_gid user id returns uid and prim gid"); +is_deeply($idcalled, {getpwuid => 1}, 'get_uid_gid user id used pwuid'); + +$idcalled = {}; +$p->{group} = 'x'; +is_deeply([$p->_get_uid_gid('group')], [123, undef], "get_uid_gid group returns gid and undef"); +is_deeply($idcalled, {getgrnam => 1}, 'get_uid_gid group used grnam'); + +$idcalled = {}; +$p->{group} = 123; +is_deeply([$p->_get_uid_gid('group')], [123, undef], "get_uid_gid group id returns gid and undef"); +is_deeply($idcalled, {getgrgid => 1}, 'get_uid_gid group id used grgid'); + +# unknown userid +$idcalled = {}; +$iddata->{getpwuid} = []; # empty list means unknown user +$p->{user} = 122; +is_deeply([$p->_get_uid_gid('user')], [], "get_uid_gid user id returns empty array with unknown userid"); +is_deeply($idcalled, {getpwuid => 1}, 'get_uid_gid user id used pwuid with unknown userid'); +is($obj->{LOGLATEST}->{ERROR}, 'No such user 122 (is user 1; is id 1)', + 'error reported with with unknown userid'); + + +$idcalled = {}; +$p->{user} = 'x'; +$iddata->{getpwuid} = [qw(myself x 122 123)]; +$p->{group} = undef; + +my $value = []; +my $valueidx = 0; +my $setargs = []; + +my $get = sub { + $valueidx += 1; + # If there's an error, the message will be "No such file or directory" + $! = 2; + return $value->[$valueidx-1]; +}; +my $set = sub {push(@$setargs, \@_)}; + +$valueidx = 0; +$setargs = []; +$value = [123]; +ok($p->_set_uid_gid(123, $set, $get, "something", "suffix", "update"), + "set_uid_gid returns ok if target is current value"); +is_deeply($setargs, [], "set not called, target is current value"); + +$valueidx = 0; +$setargs = []; +$value = [122, 123]; +ok($p->_set_uid_gid(123, $set, $get, "something", "suffix", "update"), + "set_uid_gid returns ok when target set"); +is_deeply($setargs, [[123]], "set called with target success"); + +$valueidx = 0; +$setargs = []; +$value = [122, 122]; +ok(!$p->_set_uid_gid(123, $set, $get, "something", "suffix", "update"), + "set_uid_gid fails when set failed"); +is_deeply($setargs, [[123]], "set called with target failure"); +is($obj->{LOGLATEST}->{ERROR}, + "Something went wrong update something from 122 to suffix: new something 122, reason No such file or directory", + "error reported when failed to set target"); + +$valueidx = 0; +$setargs = []; +$value = [1, 1]; # success +$mock->mock('_set_uid_gid', sub { + # can't compare the methods for some reason + push(@$setargs, [$_[1], $_[4], $_[5], $_[6]]); + $valueidx += 1; + return $value->[$valueidx-1]; +}); + +$valueidx = 0; +$setargs = []; +$value = [1, 1]; # success +ok($p->_set_eff_user_group(), "_set_eff_user_group ok"); +is_deeply($setargs, [ + ['123 123', 'EGID', '123 with GID 123 123 10 20 30', 'changing'], + ['122', 'EUID', '122 with UID 122', 'changing'], +], "_set_uid_gid called as expected (EGID before EUID)"); + +$valueidx = 0; +$setargs = []; +$value = [0, 1]; # one failure +ok(! $p->_set_eff_user_group(), "_set_eff_user_group failed 1st"); + +is_deeply($setargs, [ + ['123 123', 'EGID', '123 with GID 123 123 10 20 30', 'changing'], +], "_set_uid_gid failed, only first called"); + +$valueidx = 0; +$setargs = []; +$value = [1, 0]; # one failure +ok(! $p->_set_eff_user_group(), "_set_eff_user_group failed 2nd"); +is_deeply($setargs, [ + ['123 123', 'EGID', '123 with GID 123 123 10 20 30', 'changing'], + ['122', 'EUID', '122 with UID 122', 'changing'], +], "_set_uid_gid failed, first and second called"); + +# restore original +$valueidx = 0; +$setargs = []; +$value = [1, 1]; # success +my $orig = [124, "124 124 80 90"]; +ok($p->_set_eff_user_group($orig), "_set_eff_user_group ok restore"); +diag explain $setargs; +is_deeply($setargs, [ + ['124', 'EUID', '124 with UID 122', 'restoring'], + ['124 124 80 90', 'EGID', '124 124 80 90 with GID 123 123 10 20 30', 'restoring'], +], "_set_uid_gid called as expected (EUID before EGID) restore"); + +$valueidx = 0; +$setargs = []; +$value = [0, 1]; # one failure +ok(! $p->_set_eff_user_group($orig), "_set_eff_user_group failed 1st restor"); + +is_deeply($setargs, [ + ['124', 'EUID', '124 with UID 122', 'restoring'], +], "_set_uid_gid failed, only first called restore"); + +$valueidx = 0; +$setargs = []; +$value = [1, 0]; # one failure +ok(! $p->_set_eff_user_group($orig), "_set_eff_user_group failed 2nd restore"); +is_deeply($setargs, [ + ['124', 'EUID', '124 with UID 122', 'restoring'], + ['124 124 80 90', 'EGID', '124 124 80 90 with GID 123 123 10 20 30', 'restoring'], +], "_set_uid_gid failed, first and second called restore"); + +=head2 run as user + +=cut + +# insert the mocking here +my $args; +$mock->mock('_set_eff_user_group', sub {shift; push(@$args, \@_); return 1}); +$CAF::Object::NoAction = 1; +$execute = 0; +$idcalled = {}; +$p = CAF::Process->new ($command, log => $obj); +$p->execute (); +is ($execute, 0, "execute called with for user test w NoAction=1"); +is_deeply($idcalled, {}, "none of the user methods called w NoAction=1"); +ok(!defined $args, "_set_eff_user_group not called w NoAction=1"); + +$CAF::Object::NoAction = 0; +$execute = 0; +$idcalled = {}; +$p = CAF::Process->new ($command, log => $obj); +$p->execute (); +is ($execute, 1, "execute called with for user test w/o user"); +is_deeply($idcalled, {}, "none of the user methods called w/o user"); +ok(!defined $args, "_set_eff_user_group not called w/o user"); + +$CAF::Object::NoAction = 1; +$execute = 0; +$idcalled = {}; +$p = CAF::Process->new ($command, log => $obj, user => 'x'); +$p->execute (); +is ($execute, 0, "execute called with for user test w NoAction=1 with user"); +is_deeply($idcalled, {}, "none of the user methods called w NoAction=1 with user"); +ok(!defined $args, "_set_eff_user_group not called wNoAction=1 with user"); +$p = CAF::Process->new ($command, log => $obj, group => 'x'); +$p->execute (); +is ($execute, 0, "execute called with for user test w NoAction=1 with group"); +is_deeply($idcalled, {}, "none of the user methods called w NoAction=1 with group"); +ok(!defined $args, "_set_eff_user_group not called wNoAction=1 with group"); + +$CAF::Object::NoAction = 0; +$execute = 0; +$idcalled = {}; +$p = CAF::Process->new ($command, log => $obj, user => 'x'); +$p->execute (); +is ($execute, 1, "execute called with for user test w user"); +is_deeply($idcalled, {uid => 1, gid => 1}, + "uid/gid called (to determine orig user/group) w user"); +is_deeply($args, [[], [[122, "123 123 10 20 30"]]], + "_set_eff_user_group called w/o args first and then with original uid/gid args w user"); + +$args = undef; +$execute = 0; +$idcalled = {}; +$p = CAF::Process->new ($command, log => $obj, group => 'x'); +$p->execute (); +is ($execute, 1, "execute called with for user test w group"); +is_deeply($idcalled, {uid => 1, gid => 1}, + "uid/gid called (to determine orig user/group) w group"); +is_deeply($args, [[], [[122, "123 123 10 20 30"]]], + "_set_eff_user_group called w/o args first and then with original uid/gid args w group"); done_testing();