fix #2302: allow deletion of users when realm enforces TFA

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>

diff --git a/src/PVE/API2/User.pm b/src/PVE/API2/User.pm
index 0a38e38d4d64ea9587bf9b1e07a203fe03c5367d..3eb4038f77f2e10a2cbb8d4864d2c4c0e80907a4 100644 (file)
--- a/src/PVE/API2/User.pm
+++ b/src/PVE/API2/User.pm
@@ -442,12 +442,12 @@ __PACKAGE__->register_method ({
                $plugin->delete_user($cfg, $realm, $ruid);
            }
 
                $plugin->delete_user($cfg, $realm, $ruid);
            }
 

-           # Remove TFA data before removing the user entry as the user entry tells us whether
-           # we need ot update priv/tfa.cfg.
-           PVE::AccessControl::user_set_tfa($userid, $realm, undef, undef, $usercfg, $domain_cfg);
-
+           # Remove user from cache before removing the TFA entry so realms with TFA-enforcement
+           # know that it's OK to drop any TFA entry in that case.

            delete $usercfg->{users}->{$userid};
 
            delete $usercfg->{users}->{$userid};
 

+           PVE::AccessControl::user_set_tfa($userid, $realm, undef, undef, $usercfg, $domain_cfg);
+

            PVE::AccessControl::delete_user_group($userid, $usercfg);
            PVE::AccessControl::delete_user_acl($userid, $usercfg);
            cfs_write_file("user.cfg", $usercfg);
            PVE::AccessControl::delete_user_group($userid, $usercfg);
            PVE::AccessControl::delete_user_acl($userid, $usercfg);
            cfs_write_file("user.cfg", $usercfg);

diff --git a/src/PVE/AccessControl.pm b/src/PVE/AccessControl.pm
index a9d97e857f150053efba737845e2e27b392ad8a3..fcb16bd2dbeceeab5268aa8c9cf7c931d7779ac7 100644 (file)
--- a/src/PVE/AccessControl.pm
+++ b/src/PVE/AccessControl.pm
@@ -1600,8 +1600,7 @@ sub user_set_tfa {
     }
 
     my $user_cfg = $cached_usercfg || cfs_read_file('user.cfg');
     }
 
     my $user_cfg = $cached_usercfg || cfs_read_file('user.cfg');

-    my $user = $user_cfg->{users}->{$userid}
-       or die "user '$userid' not found\n";
+    my $user = $user_cfg->{users}->{$userid};

 
     my $domain_cfg = $cached_domaincfg || cfs_read_file('domains.cfg');
     my $realm_cfg = $domain_cfg->{ids}->{$realm};
 
     my $domain_cfg = $cached_domaincfg || cfs_read_file('domains.cfg');
     my $realm_cfg = $domain_cfg->{ids}->{$realm};


@@ -1612,6 +1611,7 @@ sub user_set_tfa {
        $realm_tfa = PVE::Auth::Plugin::parse_tfa_config($realm_tfa);
        # If the realm has a TFA setting, we're only allowed to use that.
        if (defined($data)) {
        $realm_tfa = PVE::Auth::Plugin::parse_tfa_config($realm_tfa);
        # If the realm has a TFA setting, we're only allowed to use that.
        if (defined($data)) {

+           die "user '$userid' not found\n" if !defined($user);

            my $required_type = $realm_tfa->{type};
            if ($required_type ne $type) {
                die "realm '$realm' only allows TFA of type '$required_type\n";
            my $required_type = $realm_tfa->{type};
            if ($required_type ne $type) {
                die "realm '$realm' only allows TFA of type '$required_type\n";


@@ -1624,9 +1624,11 @@ sub user_set_tfa {
            # realm-configured tfa always uses a simple key list, so use the user.cfg
            $user->{keys} = $data->{keys};
        } else {
            # realm-configured tfa always uses a simple key list, so use the user.cfg
            $user->{keys} = $data->{keys};
        } else {

-           die "realm '$realm' does not allow removing the 2nd factor\n";
+           # TFA is enforce by realm, only allow deletion if the whole user gets delete
+           die "realm '$realm' does not allow removing the 2nd factor\n" if defined($user);

        }
     } else {
        }
     } else {

+       die "user '$userid' not found\n" if !defined($user);

        # Without a realm-enforced TFA setting the user can add a u2f or totp entry by themselves.
        # The 'yubico' type requires yubico server settings, which have to be configured on the
        # realm, so this is not supported here:
        # Without a realm-enforced TFA setting the user can add a u2f or totp entry by themselves.
        # The 'yubico' type requires yubico server settings, which have to be configured on the
        # realm, so this is not supported here:


@@ -1650,10 +1652,10 @@ sub user_set_tfa {
        delete $tfa_cfg->{users}->{$userid};
        cfs_write_file('priv/tfa.cfg', $tfa_cfg);
 
        delete $tfa_cfg->{users}->{$userid};
        cfs_write_file('priv/tfa.cfg', $tfa_cfg);
 

-       delete $user->{keys};
+       delete $user->{keys} if defined($user);

     }
 
     }
 

-    cfs_write_file('user.cfg', $user_cfg);
+    cfs_write_file('user.cfg', $user_cfg) if defined($user);

 }
 
 sub user_get_tfa {
 }
 
 sub user_get_tfa {