diff --git a/lib/perl/OVH/Bastion/Plugin/groupSetRole.pm b/lib/perl/OVH/Bastion/Plugin/groupSetRole.pm index 43919a3..10d2e1e 100644 --- a/lib/perl/OVH/Bastion/Plugin/groupSetRole.pm +++ b/lib/perl/OVH/Bastion/Plugin/groupSetRole.pm @@ -163,7 +163,44 @@ sub act { if ($type eq 'member') { - # in that case, we also need to handle the symlink + if ($action eq 'add' && OVH::Bastion::is_group_guest(group => $shortGroup, account => $account)) { + + # if the user is a guest, must remove all his guest accesses first + $fnret = OVH::Bastion::get_acl_way(way => 'groupguest', group => $shortGroup, account => $account); + if ($fnret && $fnret->value) { + osh_warn("This account was previously a guest of this group, with the following accesses:"); + my @acl = @{$fnret->value}; + OVH::Bastion::print_acls(acls => [{type => 'group-guest', group => $shortGroup, acl => \@acl}]); + + osh_info("\nCleaning these guest accesses before granting membership..."); + + # foreach guest access, delete + foreach my $access (@acl) { + my $machine = $access->{'ip'}; + $machine .= ':' . $access->{'port'} if defined $access->{'port'}; + $machine = $access->{'user'} . '@' . $machine if defined $access->{'user'}; + $fnret = OVH::Bastion::Plugin::groupSetRole::act( + account => $account, + group => $shortGroup, + action => 'del', + type => 'guest', + user => $access->{'user'}, + userAny => (defined $access->{'user'} ? 0 : 1), + port => $access->{'port'}, + portAny => (defined $access->{'port'} ? 0 : 1), + host => $access->{'ip'}, + self => $self, + ); + if (!$fnret) { + osh_warn("Failed removing guest access to $machine, proceeding anyway..."); + warn_syslog("Failed removing guest access to $machine in group $shortGroup for $account, before granting this account full membership on behalf of $self: " + . $fnret->msg); + } + } + } + } + + # then, for add and del, we need to handle the symlink @command = qw{ sudo -n -u allowkeeper -- /usr/bin/env perl -T }; push @command, $OVH::Bastion::BASEPATH . '/bin/helper/osh-groupAddSymlinkToAccount'; push @command, '--group', $group; # must be first params, forced in sudoers.d @@ -204,6 +241,11 @@ sub act { } } + # If the account is already a member, can't add/del them as guest + if (OVH::Bastion::is_group_member(group => $shortGroup, account => $account)) { + return R('ERR_MEMBER_CANNOT_BE_GUEST', msg => "Can't $action $account as a guest of group $shortGroup, they're already a member!"); + } + # Add/Del user access to user@host:port with group key @command = qw{ sudo -n -u allowkeeper -- /usr/bin/env perl -T }; push @command, $OVH::Bastion::BASEPATH . '/bin/helper/osh-accountAddGroupServer'; diff --git a/tests/functional/tests.d/350-groups.sh b/tests/functional/tests.d/350-groups.sh index d30bfe7..087c58d 100644 --- a/tests/functional/tests.d/350-groups.sh +++ b/tests/functional/tests.d/350-groups.sh @@ -81,6 +81,8 @@ testsuite_groups() # now create g1 + grant groupCreate + run groupCreate a2_fail_to_create_g1_with_a1_as_owner_because_not_granted $a2 --osh groupCreate --group $group1 --algo rsa --size 2048 --owner $account1 retvalshouldbe 106 contain "you to be specifically granted" @@ -127,6 +129,8 @@ EOS #g3_pubkey=$(get_json | $jq .value.public_key.line) #g3_fp=$( get_json | $jq .value.public_key.fingerprint) + revoke groupCreate + success groupInfo a0_info_on_g3_after_create $a0 --osh groupInfo --group $group3 json .error_code OK .command groupInfo .value.group $group3 json --arg want "$account0" '.value.owners|sort == ($want|split(" ")|sort)' true @@ -527,11 +531,32 @@ EOS retvalshouldbe 107 json .error_code KO_ACCESS_DENIED + # try to add a3 as a guest of g1, should not work because already a member + plgfail groupAddGuestAccess a1_add_a3_guest_of_g1_fail_already_member $a1 --osh groupAddGuestAccess --group $group1 --account $account3 --user g2 --host 127.0.0.2 --port 22 + json .command groupAddGuestAccess .error_code ERR_MEMBER_CANNOT_BE_GUEST + # now remove membership of a3 success groupDelMember a2_del_a3_as_g1_member $a2 --osh groupDelMember --group $group1 --account $account3 json .command groupDelMember .error_code OK .value null + # add a guest access to a3... + success groupAddGuestAccess a1_add_a3_guest_of_g1 $a1 --osh groupAddGuestAccess --group $group1 --account $account3 --user g2 --host 127.0.0.2 --port 22 + json .command groupAddGuestAccess .error_code OK + + # ... then add it as member again: it should remove the guest access we've added just above... + success groupAddMember a1_add_a3_member_of_g1 $a1 --osh groupAddMember --group $group1 --account $account3 + contain "Cleaning these guest accesses" + json .command groupAddMember .error_code OK + + # ... then remove its membership + success groupDelMember a2_del_a3_as_g1_member_2 $a2 --osh groupDelMember --group $group1 --account $account3 + json .command groupDelMember .error_code OK .value null + + # ... and verify there's no ghost guest access remaining + success groupListGuestAccesses a2_list_a3_guest_access_g1_empty $a2 --osh groupListGuestAccesses --group $group1 --account $account3 + json .command groupListGuestAccesses .error_code OK_EMPTY .value null + # new state: g1[a1(ow,gk,acl,member) a2(gk) acl(g1@127.0.0.1:22,g2@127.0.0.2:22)] g3[a0,a2,a3(ow,gk,acl,member)] # check that a3 can no longer access the ips @@ -580,6 +605,8 @@ EOS success groupAddServer a3_add_server_to_g3 $a3 --osh groupAddServer --group $group3 --host 10.20.0.0/17 --port-any --user-any + grant accountAddPersonalAccess + run accountAddPersonalAccess a0_add_personal_access_to_a3_works_slash $a0 --osh accountAddPersonalAccess --account $account3 --host 77.66.55.0/24 json .command accountAddPersonalAccess .error_code OK .value null