diff --git a/src/PVE/API2/ACL.pm b/src/PVE/API2/ACL.pm index 857c672..f0c9efb 100644 --- a/src/PVE/API2/ACL.pm +++ b/src/PVE/API2/ACL.pm @@ -60,16 +60,17 @@ __PACKAGE__->register_method ({ my $res = []; my $usercfg = $rpcenv->{user_cfg}; - if (!$usercfg || !$usercfg->{acl}) { + if (!$usercfg || !$usercfg->{acl_root}) { return $res; } my $audit = $rpcenv->check($authuser, '/access', ['Sys.Audit'], 1); - my $acl = $usercfg->{acl}; - foreach my $path (keys %$acl) { + my $root = $usercfg->{acl_root}; + PVE::AccessControl::iterate_acl_tree("/", $root, sub { + my ($path, $node) = @_; foreach my $type (qw(user group token)) { - my $d = $acl->{$path}->{"${type}s"}; + my $d = $node->{"${type}s"}; next if !$d; next if !($audit || $rpcenv->check_perm_modify($authuser, $path, 1)); foreach my $id (keys %$d) { @@ -85,7 +86,7 @@ __PACKAGE__->register_method ({ } } } - } + }); return $res; }}); @@ -156,6 +157,8 @@ __PACKAGE__->register_method ({ $propagate = $param->{propagate} ? 1 : 0; } + my $node = PVE::AccessControl::find_acl_tree_node($cfg->{acl_root}, $path); + foreach my $role (split_list($param->{roles})) { die "role '$role' does not exist\n" if !$cfg->{roles}->{$role}; @@ -166,9 +169,9 @@ __PACKAGE__->register_method ({ if !$cfg->{groups}->{$group}; if ($param->{delete}) { - delete($cfg->{acl}->{$path}->{groups}->{$group}->{$role}); + delete($node->{groups}->{$group}->{$role}); } else { - $cfg->{acl}->{$path}->{groups}->{$group}->{$role} = $propagate; + $node->{groups}->{$group}->{$role} = $propagate; } } @@ -179,9 +182,9 @@ __PACKAGE__->register_method ({ if !$cfg->{users}->{$username}; if ($param->{delete}) { - delete($cfg->{acl}->{$path}->{users}->{$username}->{$role}); + delete ($node->{users}->{$username}->{$role}); } else { - $cfg->{acl}->{$path}->{users}->{$username}->{$role} = $propagate; + $node->{users}->{$username}->{$role} = $propagate; } } @@ -190,9 +193,9 @@ __PACKAGE__->register_method ({ PVE::AccessControl::check_token_exist($cfg, $username, $token); if ($param->{delete}) { - delete $cfg->{acl}->{$path}->{tokens}->{$tokenid}->{$role}; + delete $node->{tokens}->{$tokenid}->{$role}; } else { - $cfg->{acl}->{$path}->{tokens}->{$tokenid}->{$role} = $propagate; + $node->{tokens}->{$tokenid}->{$role} = $propagate; } } } diff --git a/src/PVE/AccessControl.pm b/src/PVE/AccessControl.pm index a95d072..5690a1f 100644 --- a/src/PVE/AccessControl.pm +++ b/src/PVE/AccessControl.pm @@ -951,6 +951,43 @@ sub domain_set_password { $plugin->store_password($cfg, $realm, $username, $password); } +sub iterate_acl_tree { + my ($path, $node, $code) = @_; + + $code->($path, $node); + + $path = '' if $path eq '/'; # avoid leading '//' + + my $children = $node->{children}; + + foreach my $child (keys %$children) { + iterate_acl_tree("$path/$child", $children->{$child}, $code); + } +} + +# find ACL node corresponding to normalized $path under $root +sub find_acl_tree_node { + my ($root, $path) = @_; + + my $split_path = [ split("/", $path) ]; + + if (!$split_path) { + return $root; + } + + my $node = $root; + for my $p (@$split_path) { + next if !$p; + + $node->{children} = {} if !$node->{children}; + $node->{children}->{$p} = {} if !$node->{children}->{$p}; + + $node = $node->{children}->{$p}; + } + + return $node; +} + sub add_user_group { my ($username, $usercfg, $group) = @_; @@ -971,29 +1008,33 @@ sub delete_user_group { sub delete_user_acl { my ($username, $usercfg) = @_; - foreach my $acl (keys %{$usercfg->{acl}}) { + my $code = sub { + my ($path, $acl_node) = @_; - delete ($usercfg->{acl}->{$acl}->{users}->{$username}) - if $usercfg->{acl}->{$acl}->{users}->{$username}; - } + delete ($acl_node->{users}->{$username}) + if $acl_node->{users}->{$username}; + }; + + iterate_acl_tree("/", $usercfg->{acl_root}, $code); } sub delete_group_acl { my ($group, $usercfg) = @_; - foreach my $acl (keys %{$usercfg->{acl}}) { + my $code = sub { + my ($path, $acl_node) = @_; - delete ($usercfg->{acl}->{$acl}->{groups}->{$group}) - if $usercfg->{acl}->{$acl}->{groups}->{$group}; - } + delete ($acl_node->{groups}->{$group}) + if $acl_node->{groups}->{$group}; + }; + + iterate_acl_tree("/", $usercfg->{acl_root}, $code); } sub delete_pool_acl { my ($pool, $usercfg) = @_; - my $path = "/pool/$pool"; - - delete ($usercfg->{acl}->{$path}) + delete ($usercfg->{acl_root}->{children}->{pool}->{children}->{$pool}); } # we automatically create some predefined roles by splitting privs @@ -1290,6 +1331,11 @@ sub userconfig_force_defaults { if (!$cfg->{users}->{'root@pam'}) { $cfg->{users}->{'root@pam'}->{enable} = 1; } + + # add (empty) ACL tree root node + if (!$cfg->{acl_root}) { + $cfg->{acl_root} = {}; + } } sub parse_user_config { @@ -1404,6 +1450,7 @@ sub parse_user_config { $propagate = $propagate ? 1 : 0; if (my $path = normalize_path($pathtxt)) { + my $acl_node; foreach my $role (split_list($rolelist)) { if (!verify_rolename($role, 1)) { @@ -1423,15 +1470,18 @@ sub parse_user_config { if (!$cfg->{groups}->{$group}) { # group does not exist warn "user config - ignore invalid acl group '$group'\n"; } - $cfg->{acl}->{$path}->{groups}->{$group}->{$role} = $propagate; + $acl_node = find_acl_tree_node($cfg->{acl_root}, $path) if !$acl_node; + $acl_node->{groups}->{$group}->{$role} = $propagate; } elsif (PVE::Auth::Plugin::verify_username($ug, 1)) { if (!$cfg->{users}->{$ug}) { # user does not exist warn "user config - ignore invalid acl member '$ug'\n"; } - $cfg->{acl}->{$path}->{users}->{$ug}->{$role} = $propagate; + $acl_node = find_acl_tree_node($cfg->{acl_root}, $path) if !$acl_node; + $acl_node->{users}->{$ug}->{$role} = $propagate; } elsif (my ($user, $token) = split_tokenid($ug, 1)) { if (check_token_exist($cfg, $user, $token, 1)) { - $cfg->{acl}->{$path}->{tokens}->{$ug}->{$role} = $propagate; + $acl_node = find_acl_tree_node($cfg->{acl_root}, $path) if !$acl_node; + $acl_node->{tokens}->{$ug}->{$role} = $propagate; } else { warn "user config - ignore invalid acl token '$ug'\n"; } @@ -1600,8 +1650,8 @@ sub write_user_config { } }; - foreach my $path (sort keys %{$cfg->{acl}}) { - my $d = $cfg->{acl}->{$path}; + iterate_acl_tree("/", $cfg->{acl_root}, sub { + my ($path, $d) = @_; my $rolelist_members = {}; @@ -1620,7 +1670,7 @@ sub write_user_config { } } - } + }); return $data; } @@ -1684,12 +1734,20 @@ sub roles { my $roles = {}; - foreach my $p (sort keys %{$cfg->{acl}}) { - my $final = ($path eq $p); + my $split = [ split("/", $path) ]; + if ($path eq '/') { + $split = [ '' ]; + } - next if !(($p eq '/') || $final || ($path =~ m|^$p/|)); + my $acl = $cfg->{acl_root}; + my $i = 0; - my $acl = $cfg->{acl}->{$p}; + while (@$split) { + my $p = shift @$split; + my $final = !@$split; + if ($p ne '') { + $acl = $acl->{children}->{$p}; + } #print "CHECKACL $path $p\n"; #print "ACL $path = " . Dumper ($acl); @@ -1758,20 +1816,20 @@ sub roles { sub remove_vm_access { my ($vmid) = @_; my $delVMaccessFn = sub { - my $usercfg = cfs_read_file("user.cfg"); + my $usercfg = cfs_read_file("user.cfg"); my $modified; - if (my $acl = $usercfg->{acl}->{"/vms/$vmid"}) { - delete $usercfg->{acl}->{"/vms/$vmid"}; + if (my $acl = $usercfg->{acl_root}->{children}->{vms}->{children}->{$vmid}) { + delete $usercfg->{acl_root}->{children}->{vms}->{children}->{$vmid}; $modified = 1; - } - if (my $pool = $usercfg->{vms}->{$vmid}) { - if (my $data = $usercfg->{pools}->{$pool}) { - delete $data->{vms}->{$vmid}; - delete $usercfg->{vms}->{$vmid}; + } + if (my $pool = $usercfg->{vms}->{$vmid}) { + if (my $data = $usercfg->{pools}->{$pool}) { + delete $data->{vms}->{$vmid}; + delete $usercfg->{vms}->{$vmid}; $modified = 1; - } - } + } + } cfs_write_file("user.cfg", $usercfg) if $modified; }; @@ -1782,18 +1840,18 @@ sub remove_storage_access { my ($storeid) = @_; my $deleteStorageAccessFn = sub { - my $usercfg = cfs_read_file("user.cfg"); + my $usercfg = cfs_read_file("user.cfg"); my $modified; - if (my $storage = $usercfg->{acl}->{"/storage/$storeid"}) { - delete $usercfg->{acl}->{"/storage/$storeid"}; - $modified = 1; - } + if (my $acl = $usercfg->{acl_root}->{children}->{storage}->{children}->{$storeid}) { + delete $usercfg->{acl_root}->{children}->{storage}->{children}->{$storeid}; + $modified = 1; + } foreach my $pool (keys %{$usercfg->{pools}}) { delete $usercfg->{pools}->{$pool}->{storage}->{$storeid}; $modified = 1; } - cfs_write_file("user.cfg", $usercfg) if $modified; + cfs_write_file("user.cfg", $usercfg) if $modified; }; lock_user_config($deleteStorageAccessFn, diff --git a/src/PVE/RPCEnvironment.pm b/src/PVE/RPCEnvironment.pm index 0ee2346..8586938 100644 --- a/src/PVE/RPCEnvironment.pm +++ b/src/PVE/RPCEnvironment.pm @@ -191,9 +191,14 @@ sub compute_api_permission { map { $res->{$_} = {} } keys %$priv_re_map; my $required_paths = ['/', '/nodes', '/access/groups', '/vms', '/storage', '/sdn']; + my $defined_paths = []; + PVE::AccessControl::iterate_acl_tree("/", $usercfg->{acl_root}, sub { + my ($path, $node) = @_; + push @$defined_paths, $path; + }); my $checked_paths = {}; - foreach my $path (@$required_paths, keys %{$usercfg->{acl}}) { + foreach my $path (@$required_paths, @$defined_paths) { next if $checked_paths->{$path}; $checked_paths->{$path} = 1; @@ -245,9 +250,10 @@ sub get_effective_permissions { my $cfg = $self->{user_cfg}; # paths explicitly listed in ACLs - foreach my $acl_path (keys %{$cfg->{acl}}) { - $paths->{$acl_path} = 1; - } + PVE::AccessControl::iterate_acl_tree("/", $cfg->{acl_root}, sub { + my ($path, $node) = @_; + $paths->{$path} = 1; + }); # paths referenced by pool definitions foreach my $pool (keys %{$cfg->{pools}}) { diff --git a/src/test/parser_writer.pl b/src/test/parser_writer.pl index 2fef7db..a5c6227 100755 --- a/src/test/parser_writer.pl +++ b/src/test/parser_writer.pl @@ -120,7 +120,15 @@ sub default_acls_with { foreach my $a (@$extra_acls) { my $acl = dclone($a); my $path = delete $acl->{path}; - $acls->{$path} = $acl; + my $split_path = [ split("/", $path) ]; + my $node = $acls; + for my $p (@$split_path) { + next if !$p; + $node->{children} = {} if !$node->{children}; + $node->{children}->{$p} = {} if !$node->{children}->{$p}; + $node = $node->{children}->{$p}; + } + %$node = ( %$acl ); } return $acls; @@ -451,6 +459,7 @@ my $tests = [ name => "empty_config", config => {}, expected_config => { + acl_root => default_acls(), users => { 'root@pam' => { enable => 1 } }, roles => default_roles(), }, @@ -460,6 +469,7 @@ my $tests = [ { name => "default_config", config => { + acl_root => default_acls(), users => default_users(), roles => default_roles(), }, @@ -468,6 +478,7 @@ my $tests = [ { name => "group_empty", config => { + acl_root => default_acls(), users => default_users(), roles => default_roles(), groups => default_groups_with([$default_cfg->{'test_group_empty'}]), @@ -480,6 +491,7 @@ my $tests = [ { name => "group_inexisting_member", config => { + acl_root => default_acls(), users => default_users(), roles => default_roles(), groups => default_groups_with([$default_cfg->{'test_group_empty'}]), @@ -496,6 +508,7 @@ my $tests = [ { name => "group_invalid_member", expected_config => { + acl_root => default_acls(), users => default_users(), roles => default_roles(), }, @@ -507,6 +520,7 @@ my $tests = [ { name => "group_with_one_member", config => { + acl_root => default_acls(), users => default_users_with([$default_cfg->{test_pam_with_group}]), roles => default_roles(), groups => default_groups_with([$default_cfg->{'test_group_single_member'}]), @@ -520,6 +534,7 @@ my $tests = [ { name => "group_with_members", config => { + acl_root => default_acls(), users => default_users_with([$default_cfg->{test_pam_with_group}, $default_cfg->{test2_pam_with_group}]), roles => default_roles(), groups => default_groups_with([$default_cfg->{'test_group_members'}]), @@ -534,6 +549,7 @@ my $tests = [ { name => "token_simple", config => { + acl_root => default_acls(), users => default_users_with([$default_cfg->{test_pam_with_token}]), roles => default_roles(), }, @@ -545,6 +561,7 @@ my $tests = [ { name => "token_multi", config => { + acl_root => default_acls(), users => default_users_with([$default_cfg->{test_pam_with_token}, $default_cfg->{test_pam2_with_token}]), roles => default_roles(), }, @@ -561,6 +578,7 @@ my $tests = [ { name => "custom_role_with_single_priv", config => { + acl_root => default_acls(), users => default_users(), roles => default_roles_with([$default_cfg->{test_role_single_priv}]), }, @@ -571,6 +589,7 @@ my $tests = [ { name => "custom_role_with_privs", config => { + acl_root => default_acls(), users => default_users(), roles => default_roles_with([$default_cfg->{test_role_privs}]), }, @@ -581,6 +600,7 @@ my $tests = [ { name => "custom_role_with_duplicate_privs", config => { + acl_root => default_acls(), users => default_users(), roles => default_roles_with([$default_cfg->{test_role_privs}]), }, @@ -594,6 +614,7 @@ my $tests = [ { name => "custom_role_with_invalid_priv", config => { + acl_root => default_acls(), users => default_users(), roles => default_roles_with([$default_cfg->{test_role_privs}]), }, @@ -607,6 +628,7 @@ my $tests = [ { name => "pool_empty", config => { + acl_root => default_acls(), users => default_users(), roles => default_roles(), pools => default_pools_with([$default_cfg->{test_pool_empty}]), @@ -618,6 +640,7 @@ my $tests = [ { name => "pool_invalid", config => { + acl_root => default_acls(), users => default_users(), roles => default_roles(), pools => default_pools_with([$default_cfg->{test_pool_empty}]), @@ -632,6 +655,7 @@ my $tests = [ { name => "pool_members", config => { + acl_root => default_acls(), users => default_users(), roles => default_roles(), pools => default_pools_with([$default_cfg->{test_pool_members}]), @@ -644,6 +668,7 @@ my $tests = [ { name => "pool_duplicate_members", config => { + acl_root => default_acls(), users => default_users(), roles => default_roles(), pools => default_pools_with([$default_cfg->{test_pool_members}, $default_cfg->{test_pool_duplicate_vms}, $default_cfg->{test_pool_duplicate_storages}]), @@ -665,7 +690,7 @@ my $tests = [ config => { users => default_users_with([$default_cfg->{test_pam}]), roles => default_roles(), - acl => default_acls_with([$default_cfg->{acl_simple_user}]), + acl_root => default_acls_with([$default_cfg->{acl_simple_user}]), }, raw => "". $default_raw->{users}->{'root@pam'}."\n". @@ -677,7 +702,7 @@ my $tests = [ config => { users => default_users_with([$default_cfg->{test_pam}, $default_cfg->{'test2_pam'}]), roles => default_roles(), - acl => default_acls_with([$default_cfg->{acl_simple_user}, $default_cfg->{acl_complex_users}]), + acl_root => default_acls_with([$default_cfg->{acl_simple_user}, $default_cfg->{acl_complex_users}]), }, raw => "". $default_raw->{users}->{'root@pam'}."\n". @@ -692,7 +717,7 @@ my $tests = [ config => { users => default_users_with([$default_cfg->{test2_pam}]), roles => default_roles(), - acl => default_acls_with([$default_cfg->{acl_simple_user}, $default_cfg->{acl_complex_missing_user}]), + acl_root => default_acls_with([$default_cfg->{acl_simple_user}, $default_cfg->{acl_complex_missing_user}]), }, raw => "". $default_raw->{users}->{'root@pam'}."\n". @@ -707,7 +732,7 @@ my $tests = [ users => default_users_with([$default_cfg->{test_pam_with_group}]), groups => default_groups_with([$default_cfg->{'test_group_single_member'}]), roles => default_roles(), - acl => default_acls_with([$default_cfg->{acl_simple_group}]), + acl_root => default_acls_with([$default_cfg->{acl_simple_group}]), }, raw => "". $default_raw->{users}->{'root@pam'}."\n". @@ -721,7 +746,7 @@ my $tests = [ users => default_users_with([$default_cfg->{test_pam_with_group}, $default_cfg->{'test2_pam_with_group'}, $default_cfg->{'test3_pam'}]), groups => default_groups_with([$default_cfg->{'test_group_members'}, $default_cfg->{'test_group_second'}]), roles => default_roles(), - acl => default_acls_with([$default_cfg->{acl_simple_group}, $default_cfg->{acl_complex_groups}]), + acl_root => default_acls_with([$default_cfg->{acl_simple_group}, $default_cfg->{acl_complex_groups}]), }, raw => "". $default_raw->{users}->{'root@pam'}."\n". @@ -740,7 +765,7 @@ my $tests = [ users => default_users_with([$default_cfg->{test_pam}, $default_cfg->{'test2_pam'}, $default_cfg->{'test3_pam'}]), groups => default_groups_with([$default_cfg->{'test_group_second'}]), roles => default_roles(), - acl => default_acls_with([$default_cfg->{acl_simple_group}, $default_cfg->{acl_complex_missing_group}]), + acl_root => default_acls_with([$default_cfg->{acl_simple_group}, $default_cfg->{acl_complex_missing_group}]), }, raw => "". $default_raw->{users}->{'root@pam'}."\n". @@ -766,7 +791,7 @@ my $tests = [ config => { users => default_users_with([$default_cfg->{test_pam_with_token}]), roles => default_roles(), - acl => default_acls_with([$default_cfg->{acl_simple_token}]), + acl_root => default_acls_with([$default_cfg->{acl_simple_token}]), }, raw => "". $default_raw->{users}->{'root@pam'}."\n". @@ -779,7 +804,7 @@ my $tests = [ config => { users => default_users_with([$default_cfg->{test_pam_with_token}, $default_cfg->{'test_pam2_with_token'}]), roles => default_roles(), - acl => default_acls_with([$default_cfg->{acl_simple_token}, $default_cfg->{acl_complex_tokens}]), + acl_root => default_acls_with([$default_cfg->{acl_simple_token}, $default_cfg->{acl_complex_tokens}]), }, raw => "". $default_raw->{users}->{'root@pam'}."\n". @@ -798,7 +823,7 @@ my $tests = [ config => { users => default_users_with([$default_cfg->{test_pam}, $default_cfg->{test_pam2_with_token}]), roles => default_roles(), - acl => default_acls_with([$default_cfg->{acl_complex_missing_token}]), + acl_root => default_acls_with([$default_cfg->{acl_complex_missing_token}]), }, raw => "". $default_raw->{users}->{'root@pam'}."\n". @@ -825,7 +850,7 @@ my $tests = [ config => { users => default_users_with([$default_cfg->{test_pam}]), roles => default_roles(), - acl => default_acls_with([$default_cfg->{acl_simple_user}]), + acl_root => default_acls_with([$default_cfg->{acl_simple_user}]), }, raw => "". $default_raw->{users}->{'root@pam'}."\n". @@ -843,7 +868,7 @@ my $tests = [ users => default_users_with([$default_cfg->{test_pam_with_group}, $default_cfg->{'test2_pam_with_group'}, $default_cfg->{'test3_pam'}]), groups => default_groups_with([$default_cfg->{'test_group_members'}, $default_cfg->{'test_group_second'}]), roles => default_roles(), - acl => default_acls_with([ + acl_root => default_acls_with([ $default_cfg->{acl_complex_mixed_root}, $default_cfg->{acl_complex_mixed_storage}, ]), @@ -878,7 +903,7 @@ my $tests = [ users => default_users_with([$default_cfg->{test_pam_with_group}, $default_cfg->{'test2_pam_with_group'}, $default_cfg->{'test3_pam'}]), groups => default_groups_with([$default_cfg->{'test_group_members'}, $default_cfg->{'test_group_second'}]), roles => default_roles(), - acl => default_acls_with([ + acl_root => default_acls_with([ $default_cfg->{acl_complex_mixed_root_noprop}, $default_cfg->{acl_complex_mixed_storage_noprop}, ]), @@ -973,6 +998,7 @@ my $tests = [ roles => default_roles_with([{ id => 'testrole' }]), groups => default_groups_with([$default_cfg->{test_group_empty}]), pools => default_pools_with([$default_cfg->{test_pool_empty}]), + acl_root => {}, }, raw => "". 'user:root@pam'."\n". diff --git a/src/test/realm_sync_test.pl b/src/test/realm_sync_test.pl index ea083f3..3281315 100755 --- a/src/test/realm_sync_test.pl +++ b/src/test/realm_sync_test.pl @@ -39,13 +39,11 @@ my $initialusercfg = { 'group1-syncedrealm' => { users => {}, }, 'group2-syncedrealm' => { users => {}, }, }, - acl => { - '/' => { - users => { - 'user3@syncedrealm' => {}, - }, - groups => {}, + acl_root => { + users => { + 'user3@syncedrealm' => {}, }, + groups => {}, }, }; @@ -182,13 +180,11 @@ my $tests = [ 'group2-syncedrealm' => { users => {}, }, 'group3-syncedrealm' => { users => {}, }, }, - acl => { - '/' => { - users => { - 'user3@syncedrealm' => {}, - }, - groups => {}, + acl_root => { + users => { + 'user3@syncedrealm' => {}, }, + groups => {}, }, }, ], @@ -223,13 +219,11 @@ my $tests = [ }, 'group3-syncedrealm' => { users => {}, } }, - acl => { - '/' => { - users => { - 'user3@syncedrealm' => {}, - }, - groups => {}, + acl_root => { + users => { + 'user3@syncedrealm' => {}, }, + groups => {}, }, }, ], @@ -270,11 +264,9 @@ my $tests = [ 'group2-syncedrealm' => { users => {}, }, 'group3-syncedrealm' => { users => {}, }, }, - acl => { - '/' => { - users => {}, - groups => {}, - }, + acl_root => { + users => {}, + groups => {}, }, }, ], @@ -309,11 +301,9 @@ my $tests = [ }, 'group3-syncedrealm' => { users => {}, }, }, - acl => { - '/' => { - users => {}, - groups => {}, - }, + acl_root => { + users => {}, + groups => {}, }, }, ], @@ -349,11 +339,9 @@ my $tests = [ }, 'group3-syncedrealm' => { users => {}, }, }, - acl => { - '/' => { - users => {}, - groups => {}, - }, + acl_root => { + users => {}, + groups => {}, }, }, ],