diff --git a/src/PVE/API2/OpenId.pm b/src/PVE/API2/OpenId.pm index 429cb3a..5b03a8a 100644 --- a/src/PVE/API2/OpenId.pm +++ b/src/PVE/API2/OpenId.pm @@ -206,6 +206,8 @@ __PACKAGE__->register_method({ # first, check if $username respects our naming conventions PVE::Auth::Plugin::verify_username($username); + my $user_cfg_changed; + if ($config->{'autocreate'} && !$rpcenv->check_user_exist($username, 1)) { PVE::AccessControl::lock_user_config( sub { @@ -231,6 +233,7 @@ __PACKAGE__->register_method({ }, "autocreate openid user failed", ); + $user_cfg_changed = 1; } else { # test if user exists and is enabled $rpcenv->check_user_enabled($username); @@ -316,6 +319,7 @@ __PACKAGE__->register_method({ }, "openid group mapping failed", ); + $user_cfg_changed = 1; } else { syslog( 'err', @@ -327,6 +331,9 @@ __PACKAGE__->register_method({ } } + # the cached user config of this request does not know about the changes yet + $rpcenv->reload_user_config() if $user_cfg_changed; + my $ticket = PVE::AccessControl::assemble_ticket($username); my $csrftoken = PVE::AccessControl::assemble_csrf_prevention_token($username); my $cap = $rpcenv->compute_api_permission($username); diff --git a/src/PVE/RPCEnvironment.pm b/src/PVE/RPCEnvironment.pm index 7591aa9..c627ae5 100644 --- a/src/PVE/RPCEnvironment.pm +++ b/src/PVE/RPCEnvironment.pm @@ -635,6 +635,15 @@ sub init_request { } } +# reload the user config, needed if the current request modified it and the permissions should +# already reflect that +sub reload_user_config { + my ($self) = @_; + + $self->{aclcache} = {}; + $self->{user_cfg} = PVE::Cluster::cfs_read_file('user.cfg'); +} + # hacks: to provide better backwards compatibility # old code uses PVE::RPCEnvironment::get(); diff --git a/src/test/api-tests.pl b/src/test/api-tests.pl index a1a987a..f7e202a 100644 --- a/src/test/api-tests.pl +++ b/src/test/api-tests.pl @@ -7,6 +7,6 @@ use TAP::Harness; my $harness = TAP::Harness->new({ verbosity => -1 }); -my $result = $harness->runtests('api-get-permissions-test.pl'); +my $result = $harness->runtests('api-get-permissions-test.pl', 'openid-login-test.pl'); exit -1 if $result->{failed}; diff --git a/src/test/openid-login-test.pl b/src/test/openid-login-test.pl new file mode 100644 index 0000000..e533088 --- /dev/null +++ b/src/test/openid-login-test.pl @@ -0,0 +1,137 @@ +#!/usr/bin/env perl + +use strict; +use warnings; + +use lib qw(..); + +use Test::More; +use Test::MockModule; + +use PVE::AccessControl; +use PVE::RPCEnvironment; +use PVE::API2::OpenId; + +my $realm_config = { + type => 'openid', + 'issuer-url' => 'https://idp.example.com', + 'client-id' => 'pve', + autocreate => 1, + 'groups-claim' => 'groups', +}; + +my $user_cfg_raw; +my $user_cfg_version = 1; + +my $read_file = sub { + my ($filename) = @_; + + return PVE::AccessControl::parse_user_config('/etc/pve/user.cfg', $user_cfg_raw) + if $filename eq 'user.cfg'; + return { ids => { oidc => $realm_config } } if $filename eq 'domains.cfg'; + return {} if $filename eq 'datacenter.cfg'; + + die "unexpected read of '$filename'\n"; +}; + +my $write_file = sub { + my ($filename, $cfg) = @_; + + die "unexpected write of '$filename'\n" if $filename ne 'user.cfg'; + + $user_cfg_raw = PVE::AccessControl::write_user_config('/etc/pve/user.cfg', $cfg); + $user_cfg_version++; +}; + +my $cluster_module = Test::MockModule->new('PVE::Cluster'); +$cluster_module->noop('cfs_update', 'log_msg'); +$cluster_module->mock( + cfs_file_version => sub { return ($user_cfg_version, {}) }, + cfs_read_file => $read_file, + get_clinfo => sub { return {} }, +); + +my $access_control_module = Test::MockModule->new('PVE::AccessControl'); +$access_control_module->mock( + cfs_lock_file => sub { + my ($filename, $timeout, $code) = @_; + my $res = eval { $code->() }; + return $res; + }, + assemble_ticket => sub { return "ticket:$_[0]" }, + assemble_csrf_prevention_token => sub { return "csrf:$_[0]" }, +); + +my $openid_api_module = Test::MockModule->new('PVE::API2::OpenId'); +$openid_api_module->mock( + cfs_read_file => $read_file, + cfs_write_file => $write_file, + syslog => sub { }, +); + +my $claims; +my $openid_module = Test::MockModule->new('PVE::RS::OpenId'); +$openid_module->mock( + verify_public_auth_state => sub { return ('oidc', 'private-state') }, + discover => sub { return bless {}, 'PVE::RS::OpenId' }, + verify_authorization_code => sub { return $claims }, +); + +my $rpcenv = PVE::RPCEnvironment->init('cli'); + +my ($handler, $handler_info) = PVE::API2::OpenId->find_handler('POST', 'login'); + +# each test is comprised of the following keys: +# description => what the test is about +# user_cfg => user.cfg before the login +# claims => claims returned by the OpenID provider +# expected_vms_cap => VM privileges the login result must contain +my $tests = [ + { + description => 'new user added to a group with ACLs', + user_cfg => <<"EOF", +group:tenant-oidc::: + +acl:1:/vms:\@tenant-oidc:PVEVMAdmin: +EOF + claims => { sub => 'alice', groups => ['tenant'] }, + expected_vms_cap => ['VM.Allocate', 'VM.PowerMgmt'], + }, + { + description => 'existing user newly added to a group with ACLs', + user_cfg => <<"EOF", +user:bob\@oidc:1:0::::: + +group:tenant-oidc::: + +acl:1:/vms:\@tenant-oidc:PVEVMAdmin: +EOF + claims => { sub => 'bob', groups => ['tenant'] }, + expected_vms_cap => ['VM.Allocate', 'VM.PowerMgmt'], + }, +]; + +for my $test (@$tests) { + $user_cfg_raw = $test->{user_cfg}; + $claims = $test->{claims}; + + $rpcenv->init_request(); + + my $login = eval { + $handler->handle( + $handler_info, + { + state => 'public-state', + code => 'authorization-code', + 'redirect-url' => 'https://pve.example.com:8006', + }, + ); + }; + is($@, '', "$test->{description}: login succeeds"); + + my $vms_cap = $login->{cap}->{vms} // {}; + my $missing = [grep { !$vms_cap->{$_} } $test->{expected_vms_cap}->@*]; + is_deeply($missing, [], "$test->{description}: permissions are active right away"); +} + +done_testing();