do not modify ACLs/Groups for missing users

instead of dropping ACLs and group membership for missing users,
simply warn and leave it in the config

for users that get removed via the api this happens explicitely

this is to prevent that a 'faulty' ldapsync removes users temporarily
and with it all acls that the admin created

we still have a 'purge' flag for the sync where ACLs get removed
explicitly for users removed from ldap

also adapt the tests

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>

diff --git a/PVE/AccessControl.pm b/PVE/AccessControl.pm
index 5e1185f9909efdd7d796c5f8f012c3d8abc0552b..f50a51000e5506b0c7ae20eba4b1ccc565eb841f 100644 (file)
--- a/PVE/AccessControl.pm
+++ b/PVE/AccessControl.pm
@@ -1041,10 +1041,10 @@ sub parse_user_config {
 
                if ($cfg->{users}->{$user}) { # user exists
                    $cfg->{users}->{$user}->{groups}->{$group} = 1;
-                   $cfg->{groups}->{$group}->{users}->{$user} = 1;
                } else {
                    warn "user config - ignore invalid group member '$user'\n";
                }
+               $cfg->{groups}->{$group}->{users}->{$user} = 1;
            }
 
        } elsif ($et eq 'role') {
@@ -1088,17 +1088,15 @@ sub parse_user_config {
                        my ($group) = $ug =~ m/^@(\S+)$/;
 
                        if ($group && verify_groupname($group, 1)) {
-                           if ($cfg->{groups}->{$group}) { # group exists
-                               $cfg->{acl}->{$path}->{groups}->{$group}->{$role} = $propagate;
-                           } else {
+                           if (!$cfg->{groups}->{$group}) { # group does not exist
                                warn "user config - ignore invalid acl group '$group'\n";
                            }
+                           $cfg->{acl}->{$path}->{groups}->{$group}->{$role} = $propagate;
                        } elsif (PVE::Auth::Plugin::verify_username($ug, 1)) {
-                           if ($cfg->{users}->{$ug}) { # user exists
-                               $cfg->{acl}->{$path}->{users}->{$ug}->{$role} = $propagate;
-                           } else {
+                           if (!$cfg->{users}->{$ug}) { # user does not exist
                                warn "user config - ignore invalid acl member '$ug'\n";
                            }
+                           $cfg->{acl}->{$path}->{users}->{$ug}->{$role} = $propagate;
                        } elsif (my ($user, $token) = split_tokenid($ug, 1)) {
                            if (check_token_exist($cfg, $user, $token, 1)) {
                                $cfg->{acl}->{$path}->{tokens}->{$ug}->{$role} = $propagate;

diff --git a/test/parser_writer.pl b/test/parser_writer.pl
index 0aa01b71571c30b6c5e85d443da155af9866905d..2fef7db7711f70c2bfd556d943248dcb555b54f5 100755 (executable)
--- a/test/parser_writer.pl
+++ b/test/parser_writer.pl
@@ -269,6 +269,9 @@ my $default_cfg = {
            'test2@pam' => {
                'PVEDatastoreUser' => 1,
            },
+           'test@pam' => {
+               'PVEDatastoreAdmin' => 1,
+           },
        },
     },
     acl_simple_token => {
@@ -345,6 +348,9 @@ my $default_cfg = {
     acl_complex_missing_group => {
        'path' => '/storage',
        groups => {
+           'testgroup' => {
+               'PVEDatastoreAdmin' => 1,
+           },
            'another' => {
                'PVEDatastoreUser' => 1,
            },
@@ -686,7 +692,7 @@ my $tests = [
        config => {
            users => default_users_with([$default_cfg->{test2_pam}]),
            roles => default_roles(),
-           acl => default_acls_with([$default_cfg->{acl_complex_missing_user}]),
+           acl => default_acls_with([$default_cfg->{acl_simple_user}, $default_cfg->{acl_complex_missing_user}]),
        },
        raw => "".
               $default_raw->{users}->{'root@pam'}."\n".
@@ -694,10 +700,6 @@ my $tests = [
               $default_raw->{acl}->{'acl_simple_user'}."\n".
               $default_raw->{acl}->{'acl_complex_users_1'}."\n".
               $default_raw->{acl}->{'acl_complex_users_2'}."\n",
-       expected_raw => "".
-              $default_raw->{users}->{'root@pam'}."\n".
-              $default_raw->{users}->{'test2_pam'}."\n\n\n\n\n".
-              $default_raw->{acl}->{'acl_complex_users_2'}."\n",
     },
     {
        name => "acl_simple_group",
@@ -738,7 +740,7 @@ my $tests = [
            users => default_users_with([$default_cfg->{test_pam}, $default_cfg->{'test2_pam'}, $default_cfg->{'test3_pam'}]),
            groups => default_groups_with([$default_cfg->{'test_group_second'}]),
            roles => default_roles(),
-           acl => default_acls_with([$default_cfg->{acl_complex_missing_group}]),
+           acl => default_acls_with([$default_cfg->{acl_simple_group}, $default_cfg->{acl_complex_missing_group}]),
        },
        raw => "".
               $default_raw->{users}->{'root@pam'}."\n".
@@ -755,6 +757,8 @@ my $tests = [
               $default_raw->{users}->{'test3_pam'}."\n".
               $default_raw->{users}->{'test_pam'}."\n\n".
               $default_raw->{groups}->{'test_group_second'}."\n\n\n\n".
+              $default_raw->{acl}->{'acl_simple_group'}."\n".
+              $default_raw->{acl}->{'acl_complex_groups_1'}."\n".
               $default_raw->{acl}->{'acl_complex_groups_2'}."\n",
     },
     {