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

[Thomas Lamprecht]

On 7/17/19 5:29 PM, Oguz Bektas wrote:
> 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: Wolfgang Bumiller <w.bumiller at proxmox.com>
> Signed-off-by: Oguz Bektas <o.bektas at proxmox.com>
> ---

looks good regarding semantics now, I'd like to see some code re-organizing,
though. Mostly it boils down to:

* move then all the handling of it to "complete_tfa_challenge" (and maybe
  rename it to "handle_twofactor_auth", or similar?)

* pull out the the TFA type from the Ticket early and bass it to the
  "handle_twofactor_auth" (or whatever it's called now) method, something like[0]
  if ($data->{ticket} =~ m{^([^\s!]+)![^!]*(!([0-9a-zA-Z/.=_\-+]+))?$}) {
       my ($type, $challenge) = ($1, $2); # type = [tfa, u2f, ..]; $challenge = the tfa challenge (if any)

   (see above as an idea, you could also just pass the ticket and do the $type
   exctraction in there
  [0]: https://git.proxmox.com/?p=pve-access-control.git;a=blob;f=PVE/AccessControl.pm;h=44f4a01ad241ee423e27c282c934563dfa567ac5;hb=HEAD#l324

* then use simple `if ($type eq 'tfa') ... elsif ($type eq 'u2f') raise ...

This would have the benefit that all the TFA handling is pulled out from
login, no real need to have it directly there. And it could be now overwritten
by one using this module as base.

>  PVE/APIClient/LWP.pm | 24 ++++++++++++++++++------
>  1 file changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/PVE/APIClient/LWP.pm b/PVE/APIClient/LWP.pm
> index c0e30ff..1a5f986 100755
> --- a/PVE/APIClient/LWP.pm
> +++ b/PVE/APIClient/LWP.pm
> @@ -92,6 +92,11 @@ sub update_ticket {
>      $agent->default_header('Cookie', $cookie);
>  }
>  
> +sub complete_tfa_challenge {
> +    my ($self, $tfa_response) = @_;
> +    return $self->post('/api2/json/access/tfa', {response => $tfa_response});
> +}
> +
>  sub login {
>      my ($self) = @_;
>  
> @@ -129,15 +134,22 @@ 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});
>  
> +    # TODO: handle u2f-enabled join
> +    if ($data->{ticket} =~ m/^PVE:u2f!/) {
> +	raise("U2F-enabled join is currently not implemented");
> +    }
> +    # handle totp-enabled join
> +    if ($data->{ticket} =~ m/^PVE:tfa!/) {
> +	raise("TFA-enabled join currently works only with a TTY") if !-t STDIN;
> +	print "\nEnter TFA code for user $username: ";
> +	my $tfa_code = <STDIN>;
> +	chomp $tfa_code;
> +	$data = $self->complete_tfa_challenge($tfa_code);
> +	$self->update_ticket($data->{ticket});
> +    }
>      return $data;
>  }
>  
>