[pve-devel] [RFC manager] api: replication: allow users to enumerate accessible replication jobs

[Fabian Grünbichler]

On December 1, 2023 2:24 pm, Lukas Wagner wrote:
> Previously, the /cluster/replication API handler would fail completely
> with a HTTP 403 if a user does have VM.Audit permissions for
> a single VM/CT. That was due to the 'noerr' parameter not set for
> $rpcenv->check()
> 
> Signed-off-by: Lukas Wagner <l.wagner at proxmox.com>
> ---
> Not sure if this violates our API stability guarantees, so I'm sending
> this as an RFC in advance. If this change is problematic, we could 
> hide the new behavior behind an optional flag.
> 
> This change is necessary for retrieving a list of known job-ids for
> enhancements to the notification matching rule edit window.

this seems very much in line with how we treat other, similar list calls
for various entities, and was also likely what was originally intended
(the `next if !` doesn't make any sense otherwise, after all).

going from a likely too strict check that is accidentally erroring out,
to the proper check not erroring out is definitely not an API breaking
change - if somebody was relying on this to error out if the calling
user doesn't have access to all replicated VMs, then they are relying on
undocumented behaviour..

consider this

Reviewed-by: Fabian Grünbichler <f.gruenbichler at proxmox.com>

>  PVE/API2/ReplicationConfig.pm | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/PVE/API2/ReplicationConfig.pm b/PVE/API2/ReplicationConfig.pm
> index 8af62621..d0e8a49e 100644
> --- a/PVE/API2/ReplicationConfig.pm
> +++ b/PVE/API2/ReplicationConfig.pm
> @@ -20,7 +20,8 @@ __PACKAGE__->register_method ({
>      method => 'GET',
>      description => "List replication jobs.",
>      permissions => {
> -	description => "Requires the VM.Audit permission on /vms/<vmid>.",
> +	description => "Will only return replication jobs for which the calling user has"
> +	    . " VM.Audit permission on /vms/<vmid>.",
>  	user => 'all',
>      },
>      parameters => {
> @@ -47,7 +48,7 @@ __PACKAGE__->register_method ({
>  	foreach my $id (sort keys %{$cfg->{ids}}) {
>  	    my $d = $cfg->{ids}->{$id};
>  	    my $vmid = $d->{guest};
> -	    next if !$rpcenv->check($authuser, "/vms/$vmid", [ 'VM.Audit' ]);
> +	    next if !$rpcenv->check($authuser, "/vms/$vmid", [ 'VM.Audit' ], 1);
>  	    $d->{id} = $id;
>  	    push @$res, $d;
>  	}
> -- 
> 2.39.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
>