fix #6429: openid: use the updated user config for the login permissions

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 <me@dualfroz.com>

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);

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();

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};

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();