From 060941d467693a97f6b005c387957d2c3196b046 Mon Sep 17 00:00:00 2001 From: Wolfgang Bumiller Date: Fri, 15 Mar 2024 13:40:48 +0100 Subject: [PATCH] move/rename root_permission_check to RPCEnvironment now called "reauth_user_for_user_modification" since we re-authenticate the current user while they are trying to modify their (or others') password/tfa settings Signed-off-by: Wolfgang Bumiller --- src/PVE/API2/TFA.pm | 51 ++++++++++++--------------------------- src/PVE/AccessControl.pm | 1 + src/PVE/RPCEnvironment.pm | 32 +++++++++++++++++++++++- 3 files changed, 47 insertions(+), 37 deletions(-) diff --git a/src/PVE/API2/TFA.pm b/src/PVE/API2/TFA.pm index 9fc6bcb..e178e97 100644 --- a/src/PVE/API2/TFA.pm +++ b/src/PVE/API2/TFA.pm @@ -95,36 +95,6 @@ my $TFA_UPDATE_INFO_SCHEMA = { }, }; -# Only root may modify root, regular users need to specify their password. -# -# Returns the userid returned from `verify_username`. -# Or ($userid, $realm) in list context. -my sub root_permission_check : prototype($$$$) { - my ($rpcenv, $authuser, $userid, $password) = @_; - - ($userid, undef, my $realm) = PVE::AccessControl::verify_username($userid); - $rpcenv->check_user_exist($userid); - - raise_perm_exc() if $userid eq 'root@pam' && $authuser ne 'root@pam'; - - # Regular users need to confirm their password to change TFA settings. - if ($authuser ne 'root@pam') { - raise_param_exc({ 'password' => 'password is required to modify TFA data' }) - if !defined($password); - - ($authuser, my $auth_username, my $auth_realm) = - PVE::AccessControl::verify_username($authuser); - - my $domain_cfg = cfs_read_file('domains.cfg'); - my $cfg = $domain_cfg->{ids}->{$auth_realm}; - die "auth domain '$auth_realm' does not exist\n" if !$cfg; - my $plugin = PVE::Auth::Plugin->lookup($cfg->{type}); - $plugin->authenticate_user($cfg, $auth_realm, $auth_username, $password); - } - - return wantarray ? ($userid, $realm) : $userid; -} - # Set TFA to enabled if $tfa_cfg is passed, or to disabled if $tfa_cfg is undef, # When enabling we also merge the old user.cfg keys into the $tfa_cfg. my sub set_user_tfa_enabled : prototype($$$) { @@ -244,8 +214,11 @@ __PACKAGE__->register_method ({ my $rpcenv = PVE::RPCEnvironment::get(); my $authuser = $rpcenv->get_user(); - my $userid = - root_permission_check($rpcenv, $authuser, $param->{userid}, $param->{password}); + my $userid = $rpcenv->reauth_user_for_user_modification( + $authuser, + $param->{userid}, + $param->{password}, + ); my $has_entries_left = PVE::AccessControl::lock_tfa_config(sub { my $tfa_cfg = cfs_read_file('priv/tfa.cfg'); @@ -378,8 +351,11 @@ __PACKAGE__->register_method ({ my $rpcenv = PVE::RPCEnvironment::get(); my $authuser = $rpcenv->get_user(); - my ($userid, $realm) = - root_permission_check($rpcenv, $authuser, $param->{userid}, $param->{password}); + my ($userid, $realm) = $rpcenv->reauth_user_for_user_modification( + $authuser, + $param->{userid}, + $param->{password}, + ); my $type = delete $param->{type}; my $value = delete $param->{value}; @@ -471,8 +447,11 @@ __PACKAGE__->register_method ({ my $rpcenv = PVE::RPCEnvironment::get(); my $authuser = $rpcenv->get_user(); - my $userid = - root_permission_check($rpcenv, $authuser, $param->{userid}, $param->{password}); + my $userid = $rpcenv->reauth_user_for_user_modification( + $authuser, + $param->{userid}, + $param->{password}, + ); PVE::AccessControl::lock_tfa_config(sub { my $tfa_cfg = cfs_read_file('priv/tfa.cfg'); diff --git a/src/PVE/AccessControl.pm b/src/PVE/AccessControl.pm index faea70d..21f93ff 100644 --- a/src/PVE/AccessControl.pm +++ b/src/PVE/AccessControl.pm @@ -16,6 +16,7 @@ use JSON; use Scalar::Util 'weaken'; use URI::Escape; +use PVE::Exception qw(raise_perm_exc raise_param_exc); use PVE::OTP; use PVE::Ticket; use PVE::Tools qw(run_command lock_file file_get_contents split_list safe_print); diff --git a/src/PVE/RPCEnvironment.pm b/src/PVE/RPCEnvironment.pm index 646a9b9..db33cbb 100644 --- a/src/PVE/RPCEnvironment.pm +++ b/src/PVE/RPCEnvironment.pm @@ -5,7 +5,7 @@ use warnings; use PVE::AccessControl; use PVE::Cluster; -use PVE::Exception qw(raise raise_perm_exc); +use PVE::Exception qw(raise raise_param_exc raise_perm_exc); use PVE::INotify; use PVE::ProcFSTools; use PVE::RESTEnvironment; @@ -637,4 +637,34 @@ sub is_worker { return PVE::RESTEnvironment->is_worker(); } +# Only root may modify root, regular users need to specify their password. +# +# Returns the userid returned from `verify_username`. +# Or ($userid, $realm) in list context. +sub reauth_user_for_user_modification : prototype($$$$) { + my ($rpcenv, $authuser, $userid, $password) = @_; + + ($userid, undef, my $realm) = PVE::AccessControl::verify_username($userid); + $rpcenv->check_user_exist($userid); + + raise_perm_exc() if $userid eq 'root@pam' && $authuser ne 'root@pam'; + + # Regular users need to confirm their password to change TFA settings. + if ($authuser ne 'root@pam') { + raise_param_exc({ 'password' => 'password is required to modify TFA data' }) + if !defined($password); + + ($authuser, my $auth_username, my $auth_realm) = + PVE::AccessControl::verify_username($authuser); + + my $domain_cfg = PVE::Cluster::cfs_read_file('domains.cfg'); + my $cfg = $domain_cfg->{ids}->{$auth_realm}; + die "auth domain '$auth_realm' does not exist\n" if !$cfg; + my $plugin = PVE::Auth::Plugin->lookup($cfg->{type}); + $plugin->authenticate_user($cfg, $auth_realm, $auth_username, $password); + } + + return wantarray ? ($userid, $realm) : $userid; +} + 1; -- 2.39.2