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

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
This commit is contained in:
Thomas Lamprecht 2021-09-27 15:31:31 +02:00
parent 3e5b237feb
commit 8ecf1a490d
2 changed files with 11 additions and 9 deletions

View File

@ -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 # Remove user from cache before removing the TFA entry so realms with TFA-enforcement
# we need ot update priv/tfa.cfg. # know that it's OK to drop any TFA entry in that case.
PVE::AccessControl::user_set_tfa($userid, $realm, undef, undef, $usercfg, $domain_cfg);
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_group($userid, $usercfg);
PVE::AccessControl::delete_user_acl($userid, $usercfg); PVE::AccessControl::delete_user_acl($userid, $usercfg);
cfs_write_file("user.cfg", $usercfg); cfs_write_file("user.cfg", $usercfg);

View File

@ -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} my $user = $user_cfg->{users}->{$userid};
or die "user '$userid' not found\n";
my $domain_cfg = $cached_domaincfg || cfs_read_file('domains.cfg'); my $domain_cfg = $cached_domaincfg || cfs_read_file('domains.cfg');
my $realm_cfg = $domain_cfg->{ids}->{$realm}; 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); $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 the realm has a TFA setting, we're only allowed to use that.
if (defined($data)) { if (defined($data)) {
die "user '$userid' not found\n" if !defined($user);
my $required_type = $realm_tfa->{type}; my $required_type = $realm_tfa->{type};
if ($required_type ne $type) { if ($required_type ne $type) {
die "realm '$realm' only allows TFA of type '$required_type\n"; 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 # realm-configured tfa always uses a simple key list, so use the user.cfg
$user->{keys} = $data->{keys}; $user->{keys} = $data->{keys};
} else { } 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. # 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 # The 'yubico' type requires yubico server settings, which have to be configured on the
# realm, so this is not supported here: # realm, so this is not supported here:
@ -1650,10 +1652,10 @@ sub user_set_tfa {
delete $tfa_cfg->{users}->{$userid}; delete $tfa_cfg->{users}->{$userid};
cfs_write_file('priv/tfa.cfg', $tfa_cfg); 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 {