[pve-devel] [PATCH pve-access-control] api2: ticket: don't require TFA if the only one is disabled

[pve-devel] [PATCH pve-access-control] api2: ticket: don't require TFA if the only one is disabled

[Hannes Laimer]

If TFA was added and disabled afterwards it was not possible to login
again.

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
This was reported on the forum.

 src/PVE/API2/AccessControl.pm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/PVE/API2/AccessControl.pm b/src/PVE/API2/AccessControl.pm
index 5d78c6f..8a272b4 100644
--- a/src/PVE/API2/AccessControl.pm
+++ b/src/PVE/API2/AccessControl.pm
@@ -172,12 +172,12 @@ my sub create_ticket_do : prototype($$$$$$) {
     my $ticket_data = $username;
     my $aad;
     if ($new_format) {
-	if (defined($tfa_info)) {
+	if (defined($tfa_info) && $tfa_info ne '{}') {
 	    $extra{NeedTFA} = 1;
 	    $ticket_data = "!tfa!$tfa_info";
 	    $aad = $username;
 	}
-    } elsif (defined($tfa_info)) {
+    } elsif (defined($tfa_info) && $tfa_info ne '{}') {
 	$extra{NeedTFA} = 1;
 	if ($tfa_info->{type} eq 'u2f') {
 	    my $u2finfo = $tfa_info->{data};
-- 
2.30.2

Re: [pve-devel] [PATCH pve-access-control] api2: ticket: don't require TFA if the only one is disabled

[Wolfgang Bumiller]

On Mon, Apr 11, 2022 at 07:09:09AM +0000, Hannes Laimer wrote:
> If TFA was added and disabled afterwards it was not possible to login
> again.
> 
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
> This was reported on the forum.
> 
>  src/PVE/API2/AccessControl.pm | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/PVE/API2/AccessControl.pm b/src/PVE/API2/AccessControl.pm
> index 5d78c6f..8a272b4 100644
> --- a/src/PVE/API2/AccessControl.pm
> +++ b/src/PVE/API2/AccessControl.pm
> @@ -172,12 +172,12 @@ my sub create_ticket_do : prototype($$$$$$) {
>      my $ticket_data = $username;
>      my $aad;
>      if ($new_format) {
> -	if (defined($tfa_info)) {
> +	if (defined($tfa_info) && $tfa_info ne '{}') {

While this does will work for PVE, this challenge object originates from
the proxmox-tfa crate's `api` submodule and if we update it there it'll
work for all products.

Basically all the `is_empty()` checks in there should be audited and,
where it makes sense, updated to include the 'enabled' states, so that
TfaUserData::challenge returns `Ok(None)` if no *enabled* 2nd factor
exists.