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 041B51FF130 for ; Mon, 20 Jul 2026 15:46:21 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 1E2B4214F8; Mon, 20 Jul 2026 15:46:17 +0200 (CEST) From: Elias Huhsovitz To: pve-devel@lists.proxmox.com Subject: [PATCH access-control 2/2] test: api: add tests for ACL modification endpoint Date: Mon, 20 Jul 2026 15:45:35 +0200 Message-ID: <20260720134535.136172-3-e.huhsovitz@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260720134535.136172-1-e.huhsovitz@proxmox.com> References: <20260720134535.136172-1-e.huhsovitz@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1784555117646 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.049 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) POISEN_SPAM_PILL 0.1 Meta: its spam POISEN_SPAM_PILL_1 0.1 random spam to be learned in bayes POISEN_SPAM_PILL_3 0.1 random spam to be learned in bayes RCVD_IN_DNSWL_LOW -0.7 Sender listed at https://www.dnswl.org/, low trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: N6OXOVPJBDPFG2MLTKBEAWH6GJKCOLBA X-Message-ID-Hash: N6OXOVPJBDPFG2MLTKBEAWH6GJKCOLBA X-MailFrom: e.huhsovitz@proxmox.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 CC: Elias Huhsovitz X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: The `PUT /access/acl` endpoint previously lacked automated test coverage. Introduce `api-update-acl-test.pl` to verify the ACL modification logic. The tests cover the following scenarios: - Adding, modifying, and deleting ACLs for existing users, groups, and tokens. - Rejecting requests to add ACLs for non-existent entities. - Enforcing privilege boundaries by rejecting calls lacking the `Permissions.Modify` privilege. - Deleting ACLs for non-existent entities. Signed-off-by: Elias Huhsovitz --- src/test/api-tests.pl | 2 +- src/test/api-update-acl-test.pl | 301 ++++++++++++++++++++++++++++++++ 2 files changed, 302 insertions(+), 1 deletion(-) create mode 100644 src/test/api-update-acl-test.pl diff --git a/src/test/api-tests.pl b/src/test/api-tests.pl index a1a987a..e9c400a 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', 'api-update-acl-test.pl'); exit -1 if $result->{failed}; diff --git a/src/test/api-update-acl-test.pl b/src/test/api-update-acl-test.pl new file mode 100644 index 0000000..0157533 --- /dev/null +++ b/src/test/api-update-acl-test.pl @@ -0,0 +1,301 @@ +#!/usr/bin/env perl +use v5.36; + +use lib qw(..); + +use Test::More; +use Test::MockModule; + +use PVE::AccessControl; +use PVE::RPCEnvironment; +use PVE::API2::ACL; + +# This test suite verifies the `PUT /access/acl` API endpoint. +# +# It mocks the cluster filesystem and the RPC environment to isolate +# the API logic from external dependencies. The tests pass mock +# configuration hashes directly to the API handler and verify the +# resulting state changes and error messages. +# +# The `get_base_cfg` function generates a clean, default configuration +# hash. The `before_each` helper resets the configuration and all mock +# behaviors before each test to isolate the test cases. + +my $current_cfg; + +# --- MOCKS --- + +my $cluster_module = Test::MockModule->new('PVE::Cluster'); +$cluster_module->noop('cfs_update'); +$cluster_module->mock('cfs_read_file', sub ($filename) { + die "unknown file '$filename'\n" if $filename ne 'user.cfg'; + return $current_cfg; +}); +$cluster_module->mock('cfs_write_file', sub ($filename, $data, $force_utf8 = undef) { + die "unknown file '$filename'\n" if $filename ne 'user.cfg'; + $current_cfg = $data; +}); + +# Re-assign sub-routines to our mocked versions if imported into PVE::API2::ACL. +no warnings 'redefine'; +*PVE::API2::ACL::cfs_read_file = \&PVE::Cluster::cfs_read_file; +*PVE::API2::ACL::cfs_write_file = \&PVE::Cluster::cfs_write_file; + +# Mock lock_user_config to execute the callback immediately. +my $acl_module = Test::MockModule->new('PVE::AccessControl'); +$acl_module->mock('lock_user_config', sub ($code, @args) { + return $code->(@args); +}); + +my $rpcenv_module = Test::MockModule->new('PVE::RPCEnvironment'); + +my $rpcenv = PVE::RPCEnvironment->init('cli'); + +my ($handler, $handler_info) = PVE::API2::ACL->find_handler('PUT', ''); + +# --- HELPER FUNCTIONS --- + +sub get_base_cfg () { + return { + users => { + 'admin@pve' => { + enable => 1, + tokens => { + 'mytoken' => { privsep => 1 }, + }, + }, + }, + groups => { 'admin_group' => {} }, + roles => { + 'PVEAdmin' => { 'Permissions.Modify' => 1 }, + 'NoAccess' => {}, + }, + acl_root => {}, + }; +} + +sub before_each () { + $current_cfg = get_base_cfg(); + + $rpcenv_module->mock('permissions', sub { + return { 'Permissions.Modify' => 1 }; + }); + $rpcenv_module->mock('get_user', sub { + return 'admin@pve'; + }); +} + +sub run_update_acl ($params) { + my $result = eval { $handler->handle($handler_info, $params) }; + return ($result, $@); +} + +# --- TESTS --- + +subtest 'Add ACL for existing user succeeds' => sub { + before_each(); + + my ($res, $err) = run_update_acl({ + path => '/', + users => 'admin@pve', + roles => 'PVEAdmin', + }); + + is($err, '', 'API call succeeds without throwing an error'); + ok(exists($current_cfg->{acl_root}->{users}->{'admin@pve'}->{PVEAdmin}), + 'admin@pve ACL was added to config'); +}; + +subtest 'Delete ACL for existing user succeeds' => sub { + before_each(); + $current_cfg->{acl_root}->{users} = { 'admin@pve' => { PVEAdmin => 1 } }; + + my ($res, $err) = run_update_acl({ + path => '/', + users => 'admin@pve', + roles => 'PVEAdmin', + delete => 1, + }); + + is($err, '', 'API call succeeds without throwing an error'); + ok(!exists($current_cfg->{acl_root}->{users}->{'admin@pve'}->{PVEAdmin}), + 'admin@pve ACL was removed from config'); +}; + +subtest 'Add ACL without Permissions.Modify fails' => sub { + before_each(); + + $rpcenv_module->mock('permissions', sub { return {}; }); + + my ($res, $err) = run_update_acl({ + path => '/', + users => 'admin@pve', + roles => 'PVEAdmin', + }); + + like($err, qr/requires 'Permissions.Modify'/, 'API call fails due to missing privileges'); + is($res, undef, 'API call returns undef on failure'); +}; + +subtest 'Modify existing ACL to add a second role' => sub { + before_each(); + $current_cfg->{acl_root}->{users} = { 'admin@pve' => { PVEAdmin => 1 } }; + + my ($res, $err) = run_update_acl({ + path => '/', + users => 'admin@pve', + roles => 'NoAccess', + }); + + is($err, '', 'API call succeeds without throwing an error'); + ok(exists($current_cfg->{acl_root}->{users}->{'admin@pve'}->{PVEAdmin}), + 'Original PVEAdmin role is preserved'); + ok(exists($current_cfg->{acl_root}->{users}->{'admin@pve'}->{NoAccess}), + 'New NoAccess role was added successfully'); +}; + +subtest 'Modify existing ACL to change propagation flag' => sub { + before_each(); + $current_cfg->{acl_root}->{users} = { 'admin@pve' => { PVEAdmin => 1 } }; + + my ($res, $err) = run_update_acl({ + path => '/', + users => 'admin@pve', + roles => 'PVEAdmin', + propagate => 0, + }); + + is($err, '', 'API call succeeds without throwing an error'); + is($current_cfg->{acl_root}->{users}->{'admin@pve'}->{PVEAdmin}, 0, + 'Propagation flag was updated from 1 to 0'); +}; + +subtest 'Modify existing ACL for group and token' => sub { + before_each(); + $current_cfg->{acl_root}->{groups} = { 'admin_group' => { PVEAdmin => 1 } }; + $current_cfg->{acl_root}->{tokens} = { 'admin@pve!mytoken' => { PVEAdmin => 1 } }; + + my ($res, $err) = run_update_acl({ + path => '/', + groups => 'admin_group', + tokens => 'admin@pve!mytoken', + roles => 'NoAccess', + propagate => 0, + }); + + is($err, '', 'API call succeeds without throwing an error'); + + is($current_cfg->{acl_root}->{groups}->{'admin_group'}->{PVEAdmin}, 1, + 'Group PVEAdmin role is preserved'); + ok(exists($current_cfg->{acl_root}->{groups}->{'admin_group'}->{NoAccess}), + 'Group NoAccess role was added'); + + is($current_cfg->{acl_root}->{tokens}->{'admin@pve!mytoken'}->{PVEAdmin}, 1, + 'Token PVEAdmin role is preserved'); + ok(exists($current_cfg->{acl_root}->{tokens}->{'admin@pve!mytoken'}->{NoAccess}), + 'Token NoAccess role was added'); +}; + +subtest 'Delete ACL for non-existing user' => sub { + before_each(); + $current_cfg->{acl_root}->{users} = { 'ghost@pve' => { PVEAdmin => 1 } }; + + my ($res, $err) = run_update_acl({ + path => '/', + users => 'ghost@pve', + roles => 'PVEAdmin', + delete => 1, + }); + + is($err, '', 'API call succeeds without throwing an error'); + ok(!exists($current_cfg->{acl_root}->{users}->{'ghost@pve'}->{PVEAdmin}), + 'ghost@pve ACL was removed from config'); +}; + +subtest 'Add ACL for non-existing user fails' => sub { + before_each(); + + my ($res, $err) = run_update_acl({ + path => '/', + users => 'ghost@pve', + roles => 'PVEAdmin', + }); + + like($err, qr/user 'ghost\@pve' does not exist/, 'API call fails with correct error message'); + is($res, undef, 'API call returns undef on failure'); +}; + +subtest 'Delete ACL for non-existing group' => sub { + before_each(); + $current_cfg->{acl_root}->{groups} = { 'ghost_group' => { PVEAdmin => 1 } }; + + my ($res, $err) = run_update_acl({ + path => '/', + groups => 'ghost_group', + roles => 'PVEAdmin', + delete => 1, + }); + + is($err, '', 'API call succeeds without throwing an error'); + ok(!exists($current_cfg->{acl_root}->{groups}->{'ghost_group'}->{PVEAdmin}), + 'ghost_group ACL was removed from config'); +}; + +subtest 'Add ACL for non-existing group fails' => sub { + before_each(); + + my ($res, $err) = run_update_acl({ + path => '/', + groups => 'ghost_group', + roles => 'PVEAdmin', + }); + + like($err, qr/group 'ghost_group' does not exist/, 'API call fails with correct error message'); + is($res, undef, 'API call returns undef on failure'); +}; + +subtest 'Delete ACL for non-existing token' => sub { + before_each(); + my $token_id = 'ghost@pve!ghost_token'; + $current_cfg->{acl_root}->{tokens} = { $token_id => { PVEAdmin => 1 } }; + + my ($res, $err) = run_update_acl({ + path => '/', + tokens => $token_id, + roles => 'PVEAdmin', + delete => 1, + }); + + is($err, '', 'API call succeeds without throwing an error'); + ok(!exists($current_cfg->{acl_root}->{tokens}->{$token_id}->{PVEAdmin}), + 'ghost token ACL was removed from config'); +}; + +subtest 'Add ACL for non-existing token fails' => sub { + before_each(); + + my ($res, $err) = run_update_acl({ + path => '/', + tokens => 'admin@pve!ghost_token', + roles => 'PVEAdmin', + }); + + like($err, qr/no such token/, 'API call fails with correct error message'); + is($res, undef, 'API call returns undef on failure'); +}; + +subtest 'Add ACL with non-existing role fails' => sub { + before_each(); + + my ($res, $err) = run_update_acl({ + path => '/', + users => 'admin@pve', + roles => 'InvalidRole', + }); + + like($err, qr/role 'InvalidRole' does not exist/, + 'API call fails with correct error message for invalid role'); + is($res, undef, 'API call returns undef on failure'); +}; + +done_testing(); -- 2.47.3