From: "Fabian Grünbichler" <f.gruenbichler@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Cc: w.bumiller@proxmox.com
Subject: Re: [pve-devel] [PATCH access-control 1/2] tfa: when modifying others, verify the current user's password
Date: Mon, 06 Dec 2021 15:06:47 +0100 [thread overview]
Message-ID: <1638799288.xt0u3c1b8k.astroid@nora.none> (raw)
In-Reply-To: <20211206133627.101909-1-w.bumiller@proxmox.com>
doesn't work for me
On December 6, 2021 2:36 pm, Wolfgang Bumiller wrote:
> this was wrong as it asked for the password of the
> to-be-edited user instead, which makes no sense
>
> Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
> ---
> src/PVE/API2/TFA.pm | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/src/PVE/API2/TFA.pm b/src/PVE/API2/TFA.pm
> index be696e1..343374e 100644
> --- a/src/PVE/API2/TFA.pm
> +++ b/src/PVE/API2/TFA.pm
> @@ -101,7 +101,7 @@ my $TFA_UPDATE_INFO_SCHEMA = {
> my sub root_permission_check : prototype($$$$) {
> my ($rpcenv, $authuser, $userid, $password) = @_;
>
> - ($userid, my $ruid, my $realm) = PVE::AccessControl::verify_username($userid);
> + ($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';
> @@ -111,11 +111,13 @@ my sub root_permission_check : prototype($$$$) {
> raise_param_exc({ 'password' => 'password is required to modify TFA data' })
> if !defined($password);
>
> + ($authuser, my $ruid, my $auth_realm) = PVE::AccessControl::verify_username($authuser);
> +
> my $domain_cfg = cfs_read_file('domains.cfg');
> - my $cfg = $domain_cfg->{ids}->{$realm};
> - die "auth domain '$realm' does not exist\n" if !$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, $realm, $ruid, $password);
> + $plugin->authenticate_user($cfg, $auth_realm, $authuser, $password);
this should likely still be $ruid? (which is actually just the username
without realm, which is what authenticate_user expects. so maybe
$username might also be a more readable variable name - ruid reads to me
like euid and friends)
> }
>
> return wantarray ? ($userid, $realm) : $userid;
> --
> 2.30.2
>
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
>
next prev parent reply other threads:[~2021-12-06 14:07 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-12-06 13:36 Wolfgang Bumiller
2021-12-06 13:36 ` [pve-devel] [PATCH access-control 2/2] tfa list: account for admin permissions Wolfgang Bumiller
2021-12-06 14:06 ` Fabian Grünbichler [this message]
2021-12-06 14:13 [pve-devel] [PATCH access-control 1/2] tfa: when modifying others, verify the current user's password Wolfgang Bumiller
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1638799288.xt0u3c1b8k.astroid@nora.none \
--to=f.gruenbichler@proxmox.com \
--cc=pve-devel@lists.proxmox.com \
--cc=w.bumiller@proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.