From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id BB8121FF0AD for ; Sun, 04 Oct 2026 17:02:47 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 25031215F8; Sun, 04 Oct 2026 17:02:45 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=dualfroz.com; s=dkim; t=1791126155; h=from:subject:date:message-id:to:mime-version: content-transfer-encoding; bh=S2RTlSpiWpWC+7K7aBCZS3loWR4mf5BFOXEpyBdao8c=; b=sJqoAHfioao927NylHii1msNVnvi5YguMUTMLHfKAhQe+L8WMO5kM2TAUgQnypkYXaN/lt goTM0eLFFJkg8tirDoaJj087qzdhQ4egABJUtogVLVt+wfmmJUt54E7JF+7dzS6bgLnctf eICO9a2x9O02r/X9HUtgPvsMuCC5FksclmrLZANlAGJqmNBzm6aLQaXGXP5/KaailuJ33E dy0tGhq0PqKo2BCDZ1Kwqb0FIclvfzeKBd5kOVEIGOGJ/luZxL2G4IZuNBHI5wpiB0x/ak 0aIv3WVb+uDlCoXKbtNlmeKa73PHt8j92kQ2PEiWtoqVjCLv8lRSzSr81OTtuA== From: Michal Fox To: pve-devel@lists.proxmox.com Subject: [PATCH access-control] fix #6429: openid: use the updated user config for the login permissions Date: Sun, 4 Oct 2026 15:02:23 +0000 Message-ID: <20261004150224.7-1-me@dualfroz.com> X-Mailer: git-send-email 2.47.3 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Last-TLS-Session-Version: TLSv1.3 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.183 Adjusted score from AWL reputation of From: address DKIM_SIGNED 0.1 Message has a DKIM or DK signature, not necessarily valid DKIM_VALID -0.1 Message has at least one valid DKIM or DK signature DKIM_VALID_AU -0.1 Message has a valid DKIM or DK signature from author's domain DKIM_VALID_EF -0.1 Message has a valid DKIM or DK signature from envelope-from domain DMARC_PASS -0.1 DMARC pass policy SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record WEIRD_PORT 0.001 Uses non-standard port number for HTTP Message-ID-Hash: 3AIHH6NZLKZQ7AV6P6YOP3VNWXEEKAAR X-Message-ID-Hash: 3AIHH6NZLKZQ7AV6P6YOP3VNWXEEKAAR X-MailFrom: me@dualfroz.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: When the OpenID login creates a user, or adds a user to groups from the groups claim, it writes these changes to user.cfg. The permissions it returns for the web UI however got computed with the copy of user.cfg the RPC environment loaded at the start of the request, which does not know about the new user or group memberships yet. So a user logging in for the first time got no permissions from the ACLs of its groups in this session, and buttons like "Create VM" stayed disabled, until the next login. The same happened to existing users on the login that added them to a new group. Add a helper to reload the user config of the RPC environment, and use it after the login changed user.cfg, before computing the permissions, as suggested in the bug. Add a test for both cases, which mocks the OpenID provider and keeps user.cfg in memory. Signed-off-by: Michal Fox --- Notes: tested with 'make test' in src. without the change to OpenId.pm and RPCEnvironment.pm, the new test fails for both cases, as the returned 'cap' has no VM privileges. src/PVE/API2/OpenId.pm | 7 ++ src/PVE/RPCEnvironment.pm | 9 +++ src/test/api-tests.pl | 2 +- src/test/openid-login-test.pl | 137 ++++++++++++++++++++++++++++++++++ 4 files changed, 154 insertions(+), 1 deletion(-) create mode 100644 src/test/openid-login-test.pl 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 100755 --- 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(); -- 2.43.0