[pve-devel] [PATCH v3 apiclient] fix #2227: enable totp codes to be passed in cli

[Thomas Lamprecht]

Am 7/18/19 um 4:44 PM schrieb Oguz Bektas:
> this patch enables to pass totp codes during cluster join if tfa has been
> enabled for root at pam (or any other user actually, but root seems to cause the
> most problems).
> 
> u2f support is still not implemented.
> 
> Co-developed-by: Thomas Lamprecht <t.lamprecht at proxmox.com>
> Co-developed-by: Wolfgang Bumiller <w.bumiller at proxmox.com>
> Signed-off-by: Oguz Bektas <o.bektas at proxmox.com>
> ---
> 
> v2 -> v3:
> implement thomas' suggestions, namely:
> * factor out code and handle it outside of the login func
> * pass $type and $challenge after detecting a two-factor-auth ticket
> (the detection regex can be improved i think, but i'll leave it like
> this for now)
> * also if $type isn't detected, abort after doing a raise
> 
>  PVE/APIClient/LWP.pm | 30 ++++++++++++++++++++++++------
>  1 file changed, 24 insertions(+), 6 deletions(-)
> 
> diff --git a/PVE/APIClient/LWP.pm b/PVE/APIClient/LWP.pm
> index c0e30ff..2cf7ac7 100755
> --- a/PVE/APIClient/LWP.pm
> +++ b/PVE/APIClient/LWP.pm
> @@ -92,6 +92,23 @@ sub update_ticket {
>      $agent->default_header('Cookie', $cookie);
>  }
>  
> +sub two_factor_auth_login {
> +    my ($self, $type, $challenge) = @_;
> +
> +    if ($type eq 'PVE:tfa') {
> +	raise("TFA-enabled join currently works only with a TTY.") if !-t STDIN;

This has nothing to do with a PVE node join, it hits us there, yes,
but that's only the case because we currently only use the apiclient
there.
But we may ecxtend it's use in the future, and also other, external
people and projects may already use it for different cases.
So please adapt the error message(s) to something more general and
fitting, i.e., you're doing a login which failed.

> +	print "\nEnter OTP code for user $self->{username}: ";
> +	my $tfa_response = <STDIN>;
> +	chomp $tfa_response;
> +	return $self->post('/api2/json/access/tfa', {response => $tfa_response});
> +    } elsif ($type eq 'PVE:u2f') {
> +	# TODO: implement u2f-enabled join
> +	raise("U2F-enabled join is currently not implemented.");

same here

> +    } else {
> +	raise("Authentication type not recognized, aborting!");

maybe include the unrecognised type in the error to help us understand
why someone ran into this, if it happens.

> +    }
> +}
> +
>  sub login {
>      my ($self) = @_;
>  
> @@ -129,15 +146,16 @@ sub login {
>      my $res = from_json($response->decoded_content, {utf8 => 1, allow_nonref => 1});
>  
>      my $data = $extract_data->($res);
> -
> -    # TODO: make it possible to use tfa
> -    if ($data->{ticket} =~ m/^PVE:tfa!/) {
> -	raise("Two Factor Auth is not yet implemented! Try disabling TFA for the user '$username'.\n");
> -    }
> -
>      $self->update_ticket($data->{ticket});
>      $self->update_csrftoken($data->{CSRFPreventionToken});
>  
> +    # handle two-factor login
> +    if ($data->{ticket} =~ m{^([^\s!]+)![^!]*(!([0-9a-zA-Z/.=_\-+]+))?$}) {

maybe a

my $tfa_ticket_re = qr/^([^\s!]+)![^!]*(!([0-9a-zA-Z/.=_\-+]+))?$/;

if ($data->{ticket} =~ m/$tfa_ticket_re/) ...

to make it a a little bit clearer what this regex is for (I mean it's not
really confusing but could still help to understand faster what's going
on here).

> +	my ($type, $challenge) = ($1, $2);
> +	$data = $self->two_factor_auth_login($type, $challenge);
> +	$self->update_ticket($data->{ticket});
> +    }
> +
>      return $data;
>  }
>  
>