[pve-devel] [RFC 06/23] rpcenv: drop unused roles()

[Thomas Lamprecht]

On 10/17/19 3:14 PM, Fabian Grünbichler wrote:
> it doesn't really serve a purpose, and it's not called anywhere in the codebase.

besides of tests/perm-test{5..7} line 17:

https://git.proxmox.com/?p=pve-access-control.git;a=blob;f=test/perm-test5.pl;h=697b959376c2c245d96167b79735d8e02aa8e26a;hb=9b3ae1efe38b76f12d04efec8da980eaa8e2b436#l17
https://git.proxmox.com/?p=pve-access-control.git;a=blob;f=test/perm-test6.pl;h=58ced5f7a110da9b96ea923be3d637eda9014ec5;hb=9b3ae1efe38b76f12d04efec8da980eaa8e2b436#l17
https://git.proxmox.com/?p=pve-access-control.git;a=blob;f=test/perm-test7.pl;h=e2b71a3524d7ed7fbe1bf93495107df0a17eb131;hb=9b3ae1efe38b76f12d04efec8da980eaa8e2b436#l17

with this applied the tests fail..

# make check
Can't locate object method "roles" via package "PVE::RPCEnvironment" at perm-test5.pl line 17, <DATA> line 755.

> 
> Signed-off-by: Fabian Grünbichler <f.gruenbichler at proxmox.com>
> ---
> 
> Notes:
>     alternatively, we can give this the same semantics w.r.t. tokens as
>     PVE::AccessControl::roles, but with pool roles mixed in via
>     $compile_acl_path->()
> 
>  PVE/AccessControl.pm  |  2 +-
>  PVE/RPCEnvironment.pm | 23 -----------------------
>  2 files changed, 1 insertion(+), 24 deletions(-)
> 
> diff --git a/PVE/AccessControl.pm b/PVE/AccessControl.pm
> index f246c85..a84173e 100644
> --- a/PVE/AccessControl.pm
> +++ b/PVE/AccessControl.pm
> @@ -1209,7 +1209,7 @@ sub roles {
>      my ($cfg, $user, $path) = @_;
>  
>      # NOTE: we do not consider pools here.
> -    # You need to use $rpcenv->roles() instead if you want that.
> +    # Use $rpcenv->permission() for any actual permission checks!
>  
>      return 'Administrator' if $user eq 'root at pam'; # root can do anything
>  
> diff --git a/PVE/RPCEnvironment.pm b/PVE/RPCEnvironment.pm
> index 95d3029..7e0af70 100644
> --- a/PVE/RPCEnvironment.pm
> +++ b/PVE/RPCEnvironment.pm
> @@ -81,29 +81,6 @@ my $compile_acl_path = sub {
>      return $privs;
>  };
>  
> -sub roles {
> -   my ($self, $user, $path) = @_;
> -
> -   if ($user eq 'root at pam') { # root can do anything
> -       return ('Administrator');
> -   }
> -
> -   $user = PVE::AccessControl::verify_username($user, 1);
> -   return () if !$user;
> -
> -   my $cache = $self->{aclcache};
> -   $cache->{$user} = {} if !$cache->{$user};
> -
> -   my $acl = $cache->{$user};
> -
> -   my $roles = $acl->{roles}->{$path};
> -   return @$roles if $roles;
> -
> -   &$compile_acl_path($self, $user, $path);
> -   $roles = $acl->{roles}->{$path} || [];
> -   return @$roles;
> -}
> -
>  sub permissions {
>      my ($self, $user, $path) = @_;
>  
>