From 4842b6510546f76906b216cb05d98ec9768f9e8e Mon Sep 17 00:00:00 2001 From: Dominik Csapak Date: Thu, 21 Jun 2018 14:36:52 +0200 Subject: [PATCH] remove read_password_func from cli handler we have a param_mapping now, with which we can impement that e.g. instead of using: sub read_password { # read password } we can do: sub param_mapping { my ($name) = @_; my $password_map = ['password', sub{ # read password }, ' [$password_map], 'apicall2' => [$password_map], ... }; return $mapping->{$name}; } this has the advantage that we can use different subs for different api calls (e.g. pveum passwd vs pveum ticket) if a CLIHandler implemenation still has a read_password sub and no param_mapping, we generate one for them with read_password as the sub for the standard mapping 'pve-password' Signed-off-by: Dominik Csapak --- src/PVE/CLIHandler.pm | 50 ++++++++++++++++++++++++++++++------------ src/PVE/JSONSchema.pm | 15 +------------ src/PVE/RESTHandler.pm | 36 +++++++++++++++--------------- 3 files changed, 55 insertions(+), 46 deletions(-) diff --git a/src/PVE/CLIHandler.pm b/src/PVE/CLIHandler.pm index a0d5b0e..8753e73 100644 --- a/src/PVE/CLIHandler.pm +++ b/src/PVE/CLIHandler.pm @@ -51,6 +51,7 @@ my $standard_mappings = { }, }, }; + sub get_standard_mapping { my ($name, $base) = @_; @@ -67,6 +68,31 @@ sub get_standard_mapping { return $res; } +my $gen_param_mapping_func = sub { + my ($cli_handler_class) = @_; + + my $param_mapping = $cli_handler_class->can('param_mapping'); + + if (!$param_mapping) { + my $read_password = $cli_handler_class->can('read_password'); + my $string_param_mapping = $cli_handler_class->can('string_param_file_mapping'); + + return $string_param_mapping if !$read_password; + + $param_mapping = sub { + my ($name) = @_; + + my $password_map = get_standard_mapping('pve-password', { + func => $read_password + }); + my $map = $string_param_mapping ? $string_param_mapping->($name) : []; + return [@$map, $password_map]; + }; + } + + return $param_mapping; +}; + my $assert_initialized = sub { my @caller = caller; die "$caller[0]:$caller[2] - not initialized\n" @@ -159,9 +185,7 @@ sub generate_usage_str { $separator //= ''; $indent //= ''; - my $read_password_func = $cli_handler_class->can('read_password'); - my $param_mapping_func = $cli_handler_class->can('param_mapping') || - $cli_handler_class->can('string_param_file_mapping'); + my $param_mapping_func = $gen_param_mapping_func->($cli_handler_class); my ($subcmd, $def, undef, undef, $cmdstr) = resolve_cmd($cmd); $abort->("unknown command '$cmdstr'") if !defined($def) && ref($cmd) eq 'ARRAY'; @@ -182,7 +206,7 @@ sub generate_usage_str { $str .= $indent; $str .= $class->usage_str($name, "$prefix $cmd", $arg_param, $fixed_param, $format, - $read_password_func, $param_mapping_func); + $param_mapping_func); $oldclass = $class; } elsif (defined($def->{$cmd}->{alias}) && ($format eq 'asciidoc')) { @@ -206,7 +230,7 @@ sub generate_usage_str { $str .= $indent; $str .= $class->usage_str($name, $prefix, $arg_param, $fixed_param, $format, - $read_password_func, $param_mapping_func); + $param_mapping_func); } return $str; }; @@ -642,7 +666,7 @@ sub setup_environment { } my $handle_cmd = sub { - my ($args, $read_password_func, $preparefunc, $param_mapping_func) = @_; + my ($args, $preparefunc, $param_mapping_func) = @_; $cmddef->{help} = [ __PACKAGE__, 'help', ['extra-args'] ]; @@ -672,7 +696,7 @@ my $handle_cmd = sub { my ($class, $name, $arg_param, $uri_param, $outsub) = @{$def || []}; $abort->("unknown command '$cmd_str'") if !$class; - my $res = $class->cli_handler($cmd_str, $name, $cmd_args, $arg_param, $uri_param, $read_password_func, $param_mapping_func); + my $res = $class->cli_handler($cmd_str, $name, $cmd_args, $arg_param, $uri_param, $param_mapping_func); if (defined $outsub) { my $result_schema = $class->map_method_by_name($name)->{returns}; @@ -681,7 +705,7 @@ my $handle_cmd = sub { }; my $handle_simple_cmd = sub { - my ($args, $read_password_func, $preparefunc, $param_mapping_func) = @_; + my ($args, $preparefunc, $param_mapping_func) = @_; my ($class, $name, $arg_param, $uri_param, $outsub) = @{$cmddef}; die "no class specified" if !$class; @@ -710,7 +734,7 @@ my $handle_simple_cmd = sub { &$preparefunc() if $preparefunc; - my $res = $class->cli_handler($name, $name, \@ARGV, $arg_param, $uri_param, $read_password_func, $param_mapping_func); + my $res = $class->cli_handler($name, $name, \@ARGV, $arg_param, $uri_param, $param_mapping_func); if (defined $outsub) { my $result_schema = $class->map_method_by_name($name)->{returns}; @@ -734,9 +758,7 @@ sub run_cli_handler { my $preparefunc = $params{prepare}; - my $read_password_func = $class->can('read_password'); - my $param_mapping_func = $cli_handler_class->can('param_mapping') || - $class->can('string_param_file_mapping'); + my $param_mapping_func = $gen_param_mapping_func->($cli_handler_class); $exename = &$get_exe_name($class); @@ -746,9 +768,9 @@ sub run_cli_handler { $cmddef = ${"${class}::cmddef"}; if (ref($cmddef) eq 'ARRAY') { - &$handle_simple_cmd(\@ARGV, $read_password_func, $preparefunc, $param_mapping_func); + $handle_simple_cmd->(\@ARGV, $preparefunc, $param_mapping_func); } else { - &$handle_cmd(\@ARGV, $read_password_func, $preparefunc, $param_mapping_func); + $handle_cmd->(\@ARGV, $preparefunc, $param_mapping_func); } exit 0; diff --git a/src/PVE/JSONSchema.pm b/src/PVE/JSONSchema.pm index 2865d1a..41a6652 100644 --- a/src/PVE/JSONSchema.pm +++ b/src/PVE/JSONSchema.pm @@ -1338,7 +1338,7 @@ sub method_get_child_link { # a way to parse command line parameters, using a # schema to configure Getopt::Long sub get_options { - my ($schema, $args, $arg_param, $fixed_param, $pwcallback, $param_mapping_hash) = @_; + my ($schema, $args, $arg_param, $fixed_param, $param_mapping_hash) = @_; if (!$schema || !$schema->{properties}) { raise("too many arguments\n", code => HTTP_BAD_REQUEST) @@ -1367,11 +1367,6 @@ sub get_options { # optional and call the mapping function afterwards. push @getopt, "$prop:s"; push @interactive, [$prop, $mapping->{func}]; - } elsif ($prop eq 'password' && $pwcallback) { - # we do not accept plain password on input line, instead - # we turn this into a boolean option and ask for password below - # using $pwcallback() (for security reasons). - push @getopt, "$prop"; } elsif ($pd->{type} eq 'boolean') { push @getopt, "$prop:s"; } else { @@ -1423,14 +1418,6 @@ sub get_options { } } - if (my $pd = $schema->{properties}->{password}) { - if ($pd->{type} ne 'boolean' && $pwcallback) { - if ($opts->{password} || !$pd->{optional}) { - $opts->{password} = &$pwcallback(); - } - } - } - foreach my $entry (@interactive) { my ($opt, $func) = @$entry; my $pd = $schema->{properties}->{$opt}; diff --git a/src/PVE/RESTHandler.pm b/src/PVE/RESTHandler.pm index 61dee20..c315813 100644 --- a/src/PVE/RESTHandler.pm +++ b/src/PVE/RESTHandler.pm @@ -432,7 +432,7 @@ sub handle { # $style: 'config', 'config-sub', 'arg' or 'fixed' # $mapdef: parameter mapping ({ desc => XXX, func => sub {...} }) my $get_property_description = sub { - my ($name, $style, $phash, $format, $hidepw, $mapdef) = @_; + my ($name, $style, $phash, $format, $mapdef) = @_; my $res = ''; @@ -449,10 +449,6 @@ my $get_property_description = sub { my $type_text = PVE::JSONSchema::schema_get_type_text($phash, $style); - if ($hidepw && $name eq 'password') { - $type_text = ''; - } - if ($mapdef && $phash->{type} eq 'string') { $type_text = $mapdef->{desc}; } @@ -580,10 +576,9 @@ my $compute_param_mapping_hash = sub { # 'short' ... command line only (text, one line) # 'full' ... text, include description # 'asciidoc' ... generate asciidoc for man pages (like 'full') -# $hidepw ... hide password option (use this if you provide a read passwork callback) # $param_mapping_func ... mapping for string parameters to file path parameters sub usage_str { - my ($self, $name, $prefix, $arg_param, $fixed_param, $format, $hidepw, $param_mapping_func) = @_; + my ($self, $name, $prefix, $arg_param, $fixed_param, $format, $param_mapping_func) = @_; $format = 'long' if !$format; @@ -616,7 +611,7 @@ sub usage_str { foreach my $k (@$arg_param) { next if defined($fixed_param->{$k}); # just to be sure next if !$prop->{$k}; # just to be sure - $argdescr .= &$get_property_description($k, 'fixed', $prop->{$k}, $format, 0); + $argdescr .= $get_property_description->($k, 'fixed', $prop->{$k}, $format); } my $idx_param = {}; # -vlan\d+ -scsi\d+ @@ -628,7 +623,13 @@ sub usage_str { my $type_text = $prop->{$k}->{type} || 'string'; - next if $hidepw && ($k eq 'password') && !$prop->{$k}->{optional}; + my $param_mapping_hash = {}; + + if (defined($param_mapping_func)) { + my $mapping = $param_mapping_func->($name); + $param_mapping_hash = $compute_param_mapping_hash->($mapping); + next if $k eq 'password' && $param_mapping_hash->{$k} && !$prop->{$k}->{optional}; + } my $base = $k; if ($k =~ m/^([a-z]+)(\d+)$/) { @@ -640,11 +641,9 @@ sub usage_str { } } - my $param_mapping_hash = $compute_param_mapping_hash->(&$param_mapping_func($name)) - if $param_mapping_func; - $opts .= &$get_property_description($base, 'arg', $prop->{$k}, $format, - $hidepw, $param_mapping_hash->{$k}); + $opts .= $get_property_description->($base, 'arg', $prop->{$k}, $format, + $param_mapping_hash->{$k}); if (!$prop->{$k}->{optional}) { $args .= " " if $args; @@ -707,7 +706,7 @@ sub dump_properties { } } - $raw .= &$get_property_description($base, $style, $phash, $format, 0); + $raw .= $get_property_description->($base, $style, $phash, $format); next if $style ne 'config'; @@ -740,14 +739,15 @@ my $replace_file_names_with_contents = sub { }; sub cli_handler { - my ($self, $prefix, $name, $args, $arg_param, $fixed_param, $read_password_func, $param_mapping_func) = @_; + my ($self, $prefix, $name, $args, $arg_param, $fixed_param, $param_mapping_func) = @_; my $info = $self->map_method_by_name($name); my $res; eval { - my $param_mapping_hash = $compute_param_mapping_hash->($param_mapping_func->($name)) if $param_mapping_func; - my $param = PVE::JSONSchema::get_options($info->{parameters}, $args, $arg_param, $fixed_param, $read_password_func, $param_mapping_hash); + my $param_mapping_hash = {}; + $param_mapping_hash = $compute_param_mapping_hash->($param_mapping_func->($name)) if $param_mapping_func; + my $param = PVE::JSONSchema::get_options($info->{parameters}, $args, $arg_param, $fixed_param, $param_mapping_hash); if (defined($param_mapping_hash)) { &$replace_file_names_with_contents($param, $param_mapping_hash); @@ -760,7 +760,7 @@ sub cli_handler { die $err if !$ec || $ec ne "PVE::Exception" || !$err->is_param_exc(); - $err->{usage} = $self->usage_str($name, $prefix, $arg_param, $fixed_param, 'short', $read_password_func, $param_mapping_func); + $err->{usage} = $self->usage_str($name, $prefix, $arg_param, $fixed_param, 'short', $param_mapping_func); die $err; } -- 2.39.2