Re: [pve-devel] [PATCH access-control v3 1/1] PVE/AccessControl: add Hardware.* privileges and /hardware/ paths

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

On September 20, 2022 2:50 pm, Dominik Csapak wrote:
> so that we can assign privileges on hardware level
> 
> this will generate a new role (PVEHardwareAdmin)
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  src/PVE/AccessControl.pm  | 13 +++++++++++++
>  src/PVE/RPCEnvironment.pm |  3 ++-
>  2 files changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/src/PVE/AccessControl.pm b/src/PVE/AccessControl.pm
> index c32dcc3..9cbc376 100644
> --- a/src/PVE/AccessControl.pm
> +++ b/src/PVE/AccessControl.pm
> @@ -1080,6 +1080,17 @@ my $privgroups = {
>  	    'Pool.Audit',
>  	],
>      },
> +    Hardware => {
> +	root => [
> +	    'Hardware.Configure', # create/edit mappings
> +	],
> +	admin => [
> +	    'Hardware.Use',
> +	],
> +	audit => [
> +	    'Hardware.Audit',
> +	],
> +    },

I guess the rationale here was that currently hardware is root only, so having

admin => Configure,
user => Use,
audit => Audit,

would mean the existing PVEAdmin roles would gain something that was previously
root only? 

note that the current patch still means that for the "Administrator" role
anyway, since that gets *all* defined privileges.. (which might also be
something worthy of calling out somewhere?)

it still might make sense to put Hardware.Use into the user category for
consistency's sake? also not sure whether it would be worth it to re-think
"Configure" (a bit more explicit) vs. "Modify" (consistent with existing
schema)..

>  };
>  
>  my $valid_privs = {};
> @@ -1209,6 +1220,8 @@ sub check_path {
>  	|/storage/[[:alnum:]\.\-\_]+
>  	|/vms
>  	|/vms/[1-9][0-9]{2,}
> +	|/hardware
> +	|/hardware/[[:alnum:]\.\-\_]+
>      )$!xs;
>  }
>  
> diff --git a/src/PVE/RPCEnvironment.pm b/src/PVE/RPCEnvironment.pm
> index 0ee2346..bcf911b 100644
> --- a/src/PVE/RPCEnvironment.pm
> +++ b/src/PVE/RPCEnvironment.pm
> @@ -187,10 +187,11 @@ sub compute_api_permission {
>  	nodes => qr/Sys\.|Permissions\.Modify/,
>  	sdn => qr/SDN\.|Permissions\.Modify/,
>  	dc => qr/Sys\.Audit|SDN\./,
> +	hardware => qr/Hardware\.|Permissiions\.Modify/,

typo "Permissiions"

>      };
>      map { $res->{$_} = {} } keys %$priv_re_map;
>  
> -    my $required_paths = ['/', '/nodes', '/access/groups', '/vms', '/storage', '/sdn'];
> +    my $required_paths = ['/', '/nodes', '/access/groups', '/vms', '/storage', '/sdn', '/hardware'];
>  
>      my $checked_paths = {};
>      foreach my $path (@$required_paths, keys %{$usercfg->{acl}}) {
> -- 
> 2.30.2