fix: guests: get rid of ghost guest accesses in corner cases

Adding a guest access to a member of a group is now denied, to avoid having
dangling guest accesses when their membership is revoked. In effect, they
could no longer access the group servers, even as guest, because they no longer
had access to the group key, but their previous guest accesses were still
visible in groupListGuestAccesses, causing possible confusion.

We now also revoke all guest accesses of an account to a group, if any,
when it's being set as a member of this group, so that when/if the account
membership is revoked, we don't end up with the same ghost guest accesses as above.

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';

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