[pve-devel] [PATCH manager 01/17] copy api permission calculation to /version api call

[Thomas Lamprecht]

On 07/19/2017 03:45 PM, Dominik Csapak wrote:
> the code is copied from the /access/ticket api call
> from the pve-access-control package
> 

I see why you did this. But am really not happy with it...

I'd try to remove the PVE:::Storage usage in AccessControl completely,
the compute_api_permission could not check all possible paths if an user
has access there and if yes add it to the list but go directly through the
actual ACLs and return the ones from this user (and its group, if set) directly.

This needs a bit of work and thought. AFAIK, a general rework of the a bit
rusty permission handling/interface was planed for 5.0, but the replication
and other stuff where more important.
So, I would not move this at all for now, it is not really relevant for the
core of your series, i.e. the DiskStorageSelector. It needs more thought,
not that we have to move it back again then.
And I'd refer from returning the "cap"s in the version call, the ticket call
is a good fit, IMO. The cyclic problems can be hopefully solved at the source
not the usage.

> this will be used instead of the /access/ticket call,
> because we will be using the /version api call to get info from the
> backend to the frontend (e.g. storage type capabilities)
> 
> i do not want to remove the code from the /access/ticket call yet,
> as this would change the api (and someone could rely on that)
> 
> Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
> ---
>   PVE/API2.pm | 82 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 82 insertions(+)
> 
> diff --git a/PVE/API2.pm b/PVE/API2.pm
> index 38baf133..f2663956 100644
> --- a/PVE/API2.pm
> +++ b/PVE/API2.pm
> @@ -6,6 +6,8 @@ use warnings;
>   use PVE::pvecfg;
>   use PVE::RESTHandler;
>   use PVE::JSONSchema;
> +use PVE::Storage;
> +use PVE::Cluster;
>   
>   use base qw(PVE::RESTHandler);
>   
> @@ -79,6 +81,81 @@ __PACKAGE__->register_method ({
>   	return $res;
>       }});
>   
> +my $compute_api_permission = sub {
> +    my ($rpcenv) = @_;
> +
> +    my $authuser = $rpcenv->get_user();
> +    my $usercfg = $rpcenv->{user_cfg};
> +
> +    my $nodelist = PVE::Cluster::get_nodelist();
> +    my $vmlist = PVE::Cluster::get_vmlist() || {};
> +    my $idlist = $vmlist->{ids} || {};
> +
> +    my $cfg = PVE::Storage::config();
> +    my @sids =  PVE::Storage::storage_ids ($cfg);
> +
> +    my $res = {
> +	vms => {},
> +	storage => {},
> +	access => {},
> +	nodes => {},
> +	dc => {},
> +    };
> +
> +    my $extract_vm_caps = sub {
> +	my ($path) = @_;
> +
> +	my $perm = $rpcenv->permissions($authuser, $path);
> +	foreach my $priv (keys %$perm) {
> +	    next if !($priv eq 'Permissions.Modify' || $priv =~ m/^VM\./);
> +	    $res->{vms}->{$priv} = 1;
> +	}
> +    };
> +
> +    foreach my $pool (keys %{$usercfg->{pools}}) {
> +	&$extract_vm_caps("/pool/$pool");
> +    }
> +
> +    foreach my $vmid (keys %$idlist, '__phantom__') {
> +	&$extract_vm_caps("/vms/$vmid");
> +    }
> +
> +    foreach my $storeid (@sids, '__phantom__') {
> +	my $perm = $rpcenv->permissions($authuser, "/storage/$storeid"); foreach my $priv (keys %$perm) {
> +	    next if !($priv eq 'Permissions.Modify' || $priv =~ m/^Datastore\./);
> +	    $res->{storage}->{$priv} = 1;
> +	}
> +    }
> +
> +    foreach my $path (('/access/groups')) {
> +	my $perm = $rpcenv->permissions($authuser, $path);
> +	foreach my $priv (keys %$perm) {
> +	    next if $priv !~ m/^(User|Group)\./;
> +	    $res->{access}->{$priv} = 1;
> +	}
> +    }
> +
> +    foreach my $group (keys %{$usercfg->{users}->{$authuser}->{groups}}, '__phantom__') {
> +	my $perm = $rpcenv->permissions($authuser, "/access/groups/$group");
> +	if ($perm->{'User.Modify'}) {
> +	    $res->{access}->{'User.Modify'} = 1;
> +	}
> +    }
> +
> +    foreach my $node (@$nodelist) {
> +	my $perm = $rpcenv->permissions($authuser, "/nodes/$node");
> +	foreach my $priv (keys %$perm) {
> +	    next if $priv !~ m/^Sys\./;
> +	    $res->{nodes}->{$priv} = 1;
> +	}
> +    }
> +
> +    my $perm = $rpcenv->permissions($authuser, "/");
> +    $res->{dc}->{'Sys.Audit'} = 1 if $perm->{'Sys.Audit'};
> +
> +    return $res;
> +};
> +
>   __PACKAGE__->register_method ({
>       name => 'version',
>       path => 'version',
> @@ -101,12 +178,17 @@ __PACKAGE__->register_method ({
>   	my ($resp, $param) = @_;
>       
>   	my $res = PVE::Cluster::cfs_read_file('datacenter.cfg');
> +	my $rpcenv = PVE::RPCEnvironment::get();
>   
>   	my $vi = PVE::pvecfg::version_info();
>   	foreach my $k (qw(version release repoid)) {
>   	    $res->{$k} = $vi->{$k};
>   	}
>   	
> +	# get api permissions
> +
> +	$res->{cap} = $compute_api_permission->($rpcenv);
> +
>   	return $res;
>       }});
>   
>