From a31f1d85f9dec83a1fda7e6f6727a1db45fc68c5 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Fabian=20Gr=C3=BCnbichler?= Date: Thu, 21 Nov 2019 15:43:23 +0100 Subject: [PATCH] rpcenv: drop unused roles() MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit it was useful for test-cases to verify the behaviour when pools where introduced, but it is not used anywhere else in the code base and those tests can also just check on permission-level. Signed-off-by: Fabian Grünbichler --- PVE/AccessControl.pm | 2 +- PVE/RPCEnvironment.pm | 23 ------------------ test/perm-test5.pl | 2 +- test/perm-test6.pl | 54 +++++++++++++++++++++++++++++++------------ test/perm-test7.pl | 24 +++++++++++++++++-- 5 files changed, 63 insertions(+), 42 deletions(-) diff --git a/PVE/AccessControl.pm b/PVE/AccessControl.pm index 823d1c5..e0cb75d 100644 --- a/PVE/AccessControl.pm +++ b/PVE/AccessControl.pm @@ -1221,7 +1221,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@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@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) = @_; diff --git a/test/perm-test5.pl b/test/perm-test5.pl index 697b959..1da9896 100755 --- a/test/perm-test5.pl +++ b/test/perm-test5.pl @@ -14,7 +14,7 @@ $rpcenv->init_request(userconfig => $cfgfn); sub check_roles { my ($user, $path, $expected_result) = @_; - my @ra = $rpcenv->roles($user, $path); + my @ra = PVE::AccessControl::roles($rpcenv->{user_cfg}, $user, $path); my $res = join(',', sort @ra); die "unexpected result\nneed '${expected_result}'\ngot '$res'\n" diff --git a/test/perm-test6.pl b/test/perm-test6.pl index 58ced5f..71e2a70 100755 --- a/test/perm-test6.pl +++ b/test/perm-test6.pl @@ -14,7 +14,7 @@ $rpcenv->init_request(userconfig => $cfgfn); sub check_roles { my ($user, $path, $expected_result) = @_; - my @ra = $rpcenv->roles($user, $path); + my @ra = PVE::AccessControl::roles($rpcenv->{user_cfg}, $user, $path); my $res = join(',', sort @ra); die "unexpected result\nneed '${expected_result}'\ngot '$res'\n" @@ -23,6 +23,23 @@ sub check_roles { print "ROLES:$path:$user:$res\n"; } +sub check_permissions { + my ($user, $path, $expected_result) = @_; + + my $perm = $rpcenv->permissions($user, $path); + my $res = join(',', sort keys %$perm); + + die "unexpected result\nneed '${expected_result}'\ngot '$res'\n" + if $res ne $expected_result; + + $perm = $rpcenv->permissions($user, $path); + $res = join(',', sort keys %$perm); + die "unexpected result (compiled)\nneed '${expected_result}'\ngot '$res'\n" + if $res ne $expected_result; + + print "PERM:$path:$user:$res\n"; +} + check_roles('User1@pve', '', ''); check_roles('User2@pve', '', ''); check_roles('User3@pve', '', ''); @@ -43,25 +60,32 @@ check_roles('User2@pve', '/vms/300', 'RoleTEST1'); check_roles('User3@pve', '/vms/300', 'NoAccess'); check_roles('User4@pve', '/vms/300', 'Role1'); -check_roles('User1@pve', '/vms/500', 'RoleDEVEL,RoleTEST1'); -check_roles('User2@pve', '/vms/500', 'RoleDEVEL,RoleTEST1'); +check_permissions('User1@pve', '/vms/500', 'VM.Console,VM.PowerMgmt'); +check_permissions('User2@pve', '/vms/500', 'VM.Console,VM.PowerMgmt'); +# without pool check_roles('User3@pve', '/vms/500', 'NoAccess'); +# with pool +check_permissions('User3@pve', '/vms/500', ''); +# without pool check_roles('User4@pve', '/vms/500', ''); +# with pool +check_permissions('User4@pve', '/vms/500', ''); + -check_roles('User1@pve', '/vms/600', 'RoleMARKETING,RoleTEST1'); -check_roles('User2@pve', '/vms/600', 'RoleTEST1'); -check_roles('User3@pve', '/vms/600', 'NoAccess'); -check_roles('User4@pve', '/vms/600', 'RoleMARKETING'); +check_permissions('User1@pve', '/vms/600', 'VM.Console'); +check_permissions('User2@pve', '/vms/600', 'VM.Console'); +check_permissions('User3@pve', '/vms/600', ''); +check_permissions('User4@pve', '/vms/600', 'VM.Console'); -check_roles('User1@pve', '/storage/store1', 'RoleDEVEL,RoleMARKETING'); -check_roles('User2@pve', '/storage/store1', 'RoleDEVEL'); -check_roles('User3@pve', '/storage/store1', 'RoleDEVEL'); -check_roles('User4@pve', '/storage/store1', 'RoleMARKETING'); +check_permissions('User1@pve', '/storage/store1', 'VM.Console,VM.PowerMgmt'); +check_permissions('User2@pve', '/storage/store1', 'VM.PowerMgmt'); +check_permissions('User3@pve', '/storage/store1', 'VM.PowerMgmt'); +check_permissions('User4@pve', '/storage/store1', 'VM.Console'); -check_roles('User1@pve', '/storage/store2', 'RoleDEVEL'); -check_roles('User2@pve', '/storage/store2', 'RoleDEVEL'); -check_roles('User3@pve', '/storage/store2', 'RoleDEVEL'); -check_roles('User4@pve', '/storage/store2', ''); +check_permissions('User1@pve', '/storage/store2', 'VM.PowerMgmt'); +check_permissions('User2@pve', '/storage/store2', 'VM.PowerMgmt'); +check_permissions('User3@pve', '/storage/store2', 'VM.PowerMgmt'); +check_permissions('User4@pve', '/storage/store2', ''); print "all tests passed\n"; diff --git a/test/perm-test7.pl b/test/perm-test7.pl index e2b71a3..2add067 100755 --- a/test/perm-test7.pl +++ b/test/perm-test7.pl @@ -14,7 +14,7 @@ $rpcenv->init_request(userconfig => $cfgfn); sub check_roles { my ($user, $path, $expected_result) = @_; - my @ra = $rpcenv->roles($user, $path); + my @ra = PVE::AccessControl::roles($rpcenv->{user_cfg}, $user, $path); my $res = join(',', sort @ra); die "unexpected result\nneed '${expected_result}'\ngot '$res'\n" @@ -23,10 +23,30 @@ sub check_roles { print "ROLES:$path:$user:$res\n"; } +sub check_permissions { + my ($user, $path, $expected_result) = @_; + + my $perm = $rpcenv->permissions($user, $path); + my $res = join(',', sort keys %$perm); + + die "unexpected result\nneed '${expected_result}'\ngot '$res'\n" + if $res ne $expected_result; + + $perm = $rpcenv->permissions($user, $path); + $res = join(',', sort keys %$perm); + die "unexpected result (compiled)\nneed '${expected_result}'\ngot '$res'\n" + if $res ne $expected_result; + + print "PERM:$path:$user:$res\n"; +} check_roles('User1@pve', '/vms', 'Role1'); check_roles('User1@pve', '/vms/200', 'Role1'); -check_roles('User1@pve', '/vms/100', 'NoAccess'); + +# no pool +check_roles('User1@pve', '/vms/100', 'Role1'); +# with pool +check_permissions('User1@pve', '/vms/100', ''); print "all tests passed\n"; -- 2.39.2