[pve-devel] [PATCH v3 common] CLIHandler: add a default read_password implementation

[Philip Abernethy]

On Tue, Sep 26, 2017 at 02:01:02PM +0200, Wolfgang Bumiller wrote:
> Moved here from pvesh, pveum and pct. They all need the same
> functionality currently. 'pvesh' wants to pass the
> Term::ReadLine instance, and it's possible we might want to
> skip verification in some use cases in the future, in which
> case the function can be replaced by one calling this one
> with $noverify set to true.
> ---
>  debian/control        | 17 ++++++++++++++++-
>  src/PVE/CLIHandler.pm | 27 +++++++++++++++++++++++++++
>  2 files changed, 43 insertions(+), 1 deletion(-)
> 
> diff --git a/debian/control b/debian/control
> index 56aa891..4a16118 100644
> --- a/debian/control
> +++ b/debian/control
> @@ -7,7 +7,22 @@ Standards-Version: 3.8.4
>  
>  Package: libpve-common-perl
>  Architecture: all
> -Depends: ${perl:Depends} ${misc:Depends}, libclone-perl, libdevel-cycle-perl, libwww-perl, libjson-perl, liblinux-inotify2-perl, libio-stringy-perl, liburi-perl, libstring-shellquote-perl, libnet-ip-perl, libfilesys-df-perl, libnet-dbus-perl, libcrypt-openssl-rsa-perl, libcrypt-openssl-random-perl, libmime-base32-perl
> +Depends: ${perl:Depends} ${misc:Depends},
> + libclone-perl,
> + libdevel-cycle-perl,
> + libwww-perl,
> + libjson-perl,
> + liblinux-inotify2-perl,
> + libio-stringy-perl,
> + liburi-perl,
> + libstring-shellquote-perl,
> + libnet-ip-perl,
> + libfilesys-df-perl,
> + libnet-dbus-perl,
> + libcrypt-openssl-rsa-perl,
> + libcrypt-openssl-random-perl,
> + libmime-base32-perl,
> + libterm-readline-gnu-perl
>  Breaks: qemu-server (<< 4.0-109), pve-container (<< 1.0-93)
>  Description: Proxmox VE base library
>   This package contains the base library used by other Proxmox VE components.
> diff --git a/src/PVE/CLIHandler.pm b/src/PVE/CLIHandler.pm
> index 6646dbb..4ee4edd 100644
> --- a/src/PVE/CLIHandler.pm
> +++ b/src/PVE/CLIHandler.pm
> @@ -2,6 +2,7 @@ package PVE::CLIHandler;
>  
>  use strict;
>  use warnings;
> +use Term::ReadLine;
>  
>  use PVE::SafeSyslog;
>  use PVE::Exception qw(raise raise_param_exc);
> @@ -47,6 +48,32 @@ my $complete_command_names = sub {
>      return $res;
>  };
>  
> +sub read_password {
> +    my ($term, $noverify) = @_;
> +
> +    $term = Term::ReadLine->new('pve-cli') if !defined($term);
> +
> +    my $attribs = $term->Attribs;
> +    # Hide text:
> +    my $old = $attribs->{redisplay_function};
> +    $attribs->{redisplay_function} = $attribs->{shadow_redisplay};
> +    # No history access or recording:
> +    my @history = $term->GetHistory();
> +    $term->StifleHistory(0);
> +
> +    my $passwd = $term->readline('Password: ');
> +    my $passwd2 = $term->readline('Retype new password: ') if !$noverify;
> +
> +    # Restore settings
> +    $term->unstifle_history();
> +    $term->SetHistory(@history);
> +    $attribs->{redisplay_function} = $old;
> +
> +    die "Passwords do not match.\n" if !$noverify && $passwd ne $passwd2;
> +
> +    return $passwd;
> +}
> +
While you're at it, would you consider making read_password only require
a single entry and have a separate change_password sub?
As I understand it, this was the situation before and I'm not sure I
understand why it was changed.
I, for one, find having to repeat the password to log in highly
unintuitive and annoying. Changing the password absolutely should
require double entry to avoid typos and perhaps should also require
re-authentication, which could be done by read_password. But I don't see
the use of retyping my password for authentication.
>  __PACKAGE__->register_method ({
>      name => 'help', 
>      path => 'help',
> -- 
> 2.11.0
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel