From: Dietmar Maurer Date: Fri, 27 Jan 2012 07:32:41 +0000 (+0100) Subject: cleanup permission checks X-Git-Url: https://git.proxmox.com/?p=pve-access-control.git;a=commitdiff_plain;h=82b63965ebbeb41d16a76051018992144eda0a6a cleanup permission checks Added new Real.AllocateUser priviledge --- diff --git a/PVE/API2/AccessControl.pm b/PVE/API2/AccessControl.pm index 913bdd8..58d93dd 100644 --- a/PVE/API2/AccessControl.pm +++ b/PVE/API2/AccessControl.pm @@ -54,6 +54,9 @@ __PACKAGE__->register_method ({ path => '', method => 'GET', description => "Directory index.", + permissions => { + user => 'all', + }, parameters => { additionalProperties => 0, properties => {}, @@ -220,10 +223,13 @@ __PACKAGE__->register_method ({ path => 'password', method => 'PUT', permissions => { - description => "Each user is allowed to change his own password. A user can change the password of another user if he has modify permission on /access/groups/ on a group where user is member of.", + description => "Each user is allowed to change his own password. A user can change the password of another user if he has 'Realm.AllocateUser' (on the realm of user ) and 'User.Modify' permission on /access/groups/ on a group where user is member of.", check => [ 'or', ['userid-param', 'self'], - ['userid-group', ['User.Modify']], + [ 'and', + [ 'userid-param', 'Realm.AllocateUser'], + [ 'userid-group', ['User.Modify']] + ] ], }, protected => 1, # else we can't access shadow files diff --git a/PVE/API2/Domains.pm b/PVE/API2/Domains.pm index 8ffd217..27a1c89 100644 --- a/PVE/API2/Domains.pm +++ b/PVE/API2/Domains.pm @@ -18,7 +18,10 @@ __PACKAGE__->register_method ({ path => '', method => 'GET', description => "Authentication domain index.", - permissions => { user => 'world' }, + permissions => { + description => "Anyone can access that, because we need that list for the login box (before the user is authenticated).", + user => 'world', + }, parameters => { additionalProperties => 0, properties => {}, @@ -58,7 +61,7 @@ __PACKAGE__->register_method ({ path => '', method => 'POST', permissions => { - check => ['perm', '/access', ['Sys.Modify']], + check => ['perm', '/access/realm', ['Realm.Allocate']], }, description => "Add an authentication server.", parameters => { @@ -168,7 +171,7 @@ __PACKAGE__->register_method ({ path => '{realm}', method => 'PUT', permissions => { - check => ['perm', '/access', ['Sys.Modify']], + check => ['perm', '/access/realm', ['Realm.Allocate']], }, description => "Update authentication server settings.", protected => 1, @@ -273,7 +276,7 @@ __PACKAGE__->register_method ({ method => 'GET', description => "Get auth server configuration.", permissions => { - check => ['perm', '/access', ['Sys.Audit']], + check => ['perm', '/access/realm', ['Realm.Allocate', 'Sys.Audit'], any => 1], }, parameters => { additionalProperties => 0, @@ -301,7 +304,7 @@ __PACKAGE__->register_method ({ path => '{realm}', method => 'DELETE', permissions => { - check => ['perm', '/access', ['Sys.Modify']], + check => ['perm', '/access/realm', ['Realm.Allocate']], }, description => "Delete an authentication server.", protected => 1, diff --git a/PVE/API2/Group.pm b/PVE/API2/Group.pm index eaee48d..27051a4 100644 --- a/PVE/API2/Group.pm +++ b/PVE/API2/Group.pm @@ -15,7 +15,7 @@ __PACKAGE__->register_method ({ method => 'GET', description => "Group index.", permissions => { - description => "The returned list is restricted to groups where you have 'User.Allocate' or 'Sys.Audit' permissions on '/access', or 'User.Allocate' on /access/groups/.", + description => "The returned list is restricted to groups where you have 'User.Modify', 'Sys.Audit' or 'Group.Allocate' permissions on /access/groups/.", user => 'all', }, parameters => { @@ -41,12 +41,10 @@ __PACKAGE__->register_method ({ my $usercfg = cfs_read_file("user.cfg"); my $authuser = $rpcenv->get_user(); - my $privs = [ 'User.Allocate', 'Sys.Audit' ]; - my $allow = $rpcenv->check_any($authuser, "/access", $privs, 1); - my $allowed_groups = $rpcenv->filter_groups($authuser, $privs, 1); - + my $privs = [ 'User.Modify', 'Sys.Audit', 'Group.Allocate']; + foreach my $group (keys %{$usercfg->{groups}}) { - next if !($allow || $allowed_groups->{$group}); + next if !$rpcenv->check_any($authuser, "/access/groups/$group", $privs, 1); my $data = $usercfg->{groups}->{$group}; my $entry = { groupid => $group }; $entry->{comment} = $data->{comment} if defined($data->{comment}); @@ -62,7 +60,7 @@ __PACKAGE__->register_method ({ path => '', method => 'POST', permissions => { - check => ['perm', '/access', ['Sys.Modify']], + check => ['perm', '/access/groups', ['Group.Allocate']], }, description => "Create new group.", parameters => { @@ -103,7 +101,7 @@ __PACKAGE__->register_method ({ path => '{groupid}', method => 'PUT', permissions => { - check => ['perm', '/access', ['Sys.Modify']], + check => ['perm', '/access/groups', ['Group.Allocate']], }, description => "Update group data.", parameters => { @@ -142,8 +140,8 @@ __PACKAGE__->register_method ({ path => '{groupid}', method => 'GET', permissions => { - check => ['perm', '/access', ['Sys.Audit']], - }, + check => ['perm', '/access/groups', ['Sys.Audit', 'Group.Allocate'], any => 1], + }, description => "Get group configuration.", parameters => { additionalProperties => 0, @@ -191,7 +189,7 @@ __PACKAGE__->register_method ({ path => '{groupid}', method => 'DELETE', permissions => { - check => ['perm', '/access', ['Sys.Modify']], + check => ['perm', '/access/groups', ['Group.Allocate']], }, description => "Delete group.", parameters => { diff --git a/PVE/API2/Role.pm b/PVE/API2/Role.pm index 509938c..6392e13 100644 --- a/PVE/API2/Role.pm +++ b/PVE/API2/Role.pm @@ -19,7 +19,7 @@ __PACKAGE__->register_method ({ method => 'GET', description => "Role index.", permissions => { - check => ['perm', '/access', ['Sys.Audit']], + user => 'all', }, parameters => { additionalProperties => 0, @@ -141,7 +141,7 @@ __PACKAGE__->register_method ({ path => '{roleid}', method => 'GET', permissions => { - check => ['perm', '/access', ['Sys.Audit']], + user => 'all', }, description => "Get role configuration.", parameters => { diff --git a/PVE/API2/User.pm b/PVE/API2/User.pm index 6f4aff1..d09ec40 100644 --- a/PVE/API2/User.pm +++ b/PVE/API2/User.pm @@ -38,7 +38,7 @@ __PACKAGE__->register_method ({ method => 'GET', description => "User index.", permissions => { - description => "The returned list is restricted to users where you have 'User.Modify' or 'User.Allocate' permissions on '/access' or on a group the user belongs too. But it always includes the current (authenticated) user.", + description => "The returned list is restricted to users where you have 'User.Modify' or 'Sys.Audit' permissions on '/access/groups' or on a group the user belongs too. But it always includes the current (authenticated) user.", user => 'all', }, parameters => { @@ -70,9 +70,8 @@ __PACKAGE__->register_method ({ my $res = []; - my $privs = [ 'User.Modify', 'User.Allocate' ]; - - my $canUserMod = $rpcenv->check_any($authuser, "/access", $privs, 1); + my $privs = [ 'User.Modify', 'Sys.Audit' ]; + my $canUserMod = $rpcenv->check_any($authuser, "/access/groups", $privs, 1); my $groups = $rpcenv->filter_groups($authuser, $privs, 1); my $allowed_users = $rpcenv->group_member_join([keys %$groups]); @@ -102,8 +101,11 @@ __PACKAGE__->register_method ({ path => '', method => 'POST', permissions => { - description => "You need 'User.Allocate' permissions to '/access/groups/' for any group specified, or 'User.Allocate' on '/access' if you pass no groups.", - check => ['userid-group', ['User.Allocate'], groups_param => 1], + description => "You need 'Realm.AllocateUser' on '/access/realm/' on the realm of user , and 'User.Modify' permissions to '/access/groups/' for any group specified (or 'User.Modify' on '/access/groups' if you pass no groups.", + check => [ 'and', + [ 'userid-param', 'Realm.AllocateUser'], + [ 'userid-group', ['User.Modify'], groups_param => 1], + ], }, description => "Create new user.", parameters => { @@ -184,7 +186,7 @@ __PACKAGE__->register_method ({ method => 'GET', description => "Get user configuration.", permissions => { - check => ['userid-group', ['User.Modify']], + check => ['userid-group', ['User.Modify', 'Sys.Audit']], }, parameters => { additionalProperties => 0, @@ -302,7 +304,10 @@ __PACKAGE__->register_method ({ method => 'DELETE', description => "Delete user.", permissions => { - check => ['userid-group', ['User.Allocate']], + check => [ 'and', + [ 'userid-param', 'Realm.AllocateUser'], + [ 'userid-group', ['User.Modify']], + ], }, parameters => { additionalProperties => 0, diff --git a/PVE/AccessControl.pm b/PVE/AccessControl.pm index 0dce3e6..ddad2c7 100644 --- a/PVE/AccessControl.pm +++ b/PVE/AccessControl.pm @@ -557,7 +557,7 @@ my $privgroups = { 'VM.PowerMgmt', ], audit => [ - 'VM.Audit' + 'VM.Audit', ], }, Sys => { @@ -588,10 +588,13 @@ my $privgroups = { ], }, User => { - root => [], + root => [ + 'Realm.Allocate', + ], admin => [ 'User.Modify', - 'User.Allocate', + 'Group.Allocate', # edit/change group settings + 'Realm.AllocateUser', ], user => [], audit => [], diff --git a/PVE/RPCEnvironment.pm b/PVE/RPCEnvironment.pm index e6651d0..31692ce 100644 --- a/PVE/RPCEnvironment.pm +++ b/PVE/RPCEnvironment.pm @@ -348,12 +348,12 @@ sub exec_api2_perm_check { } elsif ($test eq 'userid-group') { my $userid = $param->{userid}; my ($t, $privs, %options) = @$check; - return if !$options{groups_param} && !$self->check_user_exist($userid, $noerr); - if (!$self->check_any($username, "/access", $privs, 1)) { + return 0 if !$options{groups_param} && !$self->check_user_exist($userid, $noerr); + if (!$self->check_any($username, "/access/groups", $privs, 1)) { my $groups = $self->filter_groups($username, $privs, 1); if ($options{groups_param}) { my @group_param = PVE::Tools::split_list($param->{groups}); - raise_perm_exc("/access, " . join("|", @$privs)) if !scalar(@group_param); + raise_perm_exc("/access/groups, " . join("|", @$privs)) if !scalar(@group_param); foreach my $pg (@group_param) { raise_perm_exc("/access/groups/$pg, " . join("|", @$privs)) if !$groups->{$pg}; @@ -368,7 +368,7 @@ sub exec_api2_perm_check { } return 1; } elsif ($test eq 'userid-param') { - my $userid = $param->{userid}; + my ($userid, undef, $realm) = verify_username($param->{userid}); return if !$self->check_user_exist($userid, $noerr); my ($t, $subtest) = @$check; die "missing parameters" if !$subtest; @@ -376,10 +376,15 @@ sub exec_api2_perm_check { return 1 if $username eq 'userid'; return 0 if $noerr; raise_perm_exc(); + } elsif ($subtest eq 'Realm.AllocateUser') { + my $path = "/access/realm/$realm"; + return $self->check($username, $path, ['Realm.AllocateUser'], $noerr); + return 0 if $noerr; + raise_perm_exc("$path, 'Realm.AllocateUser'"); } else { die "unknown userid-param test"; } - } elsif ($test eq 'perm-modify') { + } elsif ($test eq 'perm-modify') { my ($t, $tmplpath) = @$check; my $path = PVE::Tools::template_replace($tmplpath, $param); $path = PVE::AccessControl::normalize_path($path);