diff --git a/filemin/filemin-lib.pl b/filemin/filemin-lib.pl index 4ebb299e7..923a46a09 100644 --- a/filemin/filemin-lib.pl +++ b/filemin/filemin-lib.pl @@ -71,24 +71,9 @@ return get_selinux_command_type() ? sub can_write { my ($file) = @_; -# No restrictions for root -if (&webmin_user_is_admin()) { - return 1; - } -# If strict check is enabled or if safe user check for write -# access explicitly +# Enforce strict and safe-user ACLs regardless of the Webmin login role if ($access{'work_as_user_strict'} || $access{'_safe'}) { - # Check if the file is a symbolic link - if (-l $file) { - # Resolve symbolic link - my $resolved_file = readlink($file); - # If the link is broken, allow writing - # to the link itself - return -w $file if (!$resolved_file); - # Otherwise, check the resolved file - $file = $resolved_file; - } - # Check if the file itself is writable + # The file test follows symlinks relative to the link's directory return -w $file; } # Otherwise, allow writing depending on Unix permissions diff --git a/t/web-lib-funcs-login-mode.t b/t/web-lib-funcs-login-mode.t new file mode 100644 index 000000000..d228e4837 --- /dev/null +++ b/t/web-lib-funcs-login-mode.t @@ -0,0 +1,92 @@ +#!/usr/bin/perl +# Tests for login privileges and RPC authorization in web-lib-funcs.pl. + +use strict; +use warnings; +use Test::More; +use File::Basename qw(dirname); +use File::Spec; + +my $script = File::Spec->rel2abs( + File::Spec->catfile(dirname(__FILE__), '..', 'web-lib-funcs.pl')); +require $script; + +our ($remote_user, $base_remote_user); +my (%test_acls, %test_modules); +my $test_product = 'webmin'; +my ($test_master, $test_reseller) = (1, 0); + +# Supply login context without loading modules or workstation configuration. +{ + no warnings qw(redefine once); + *main::get_module_acl = sub { return %{$test_acls{$_[0]} || { }}; }; + *main::foreign_available = sub { return $test_modules{$_[0]}; }; + *main::foreign_require = sub { }; + *main::get_product_name = sub { return $test_product; }; + *virtual_server::master_admin = sub { return $test_master; }; + *virtual_server::reseller_admin = sub { return $test_reseller; }; +} + +# Check the public helpers used by themes, not just the mode string. +sub check_mode +{ +my ($expected) = @_; +is(main::webmin_user_login_mode(), $expected, "login mode is $expected"); +is(!!main::webmin_user_is_admin(), !!($expected eq 'root'), + 'admin status matches the login role'); +ok(main::webmin_user_is($expected), 'role predicate matches'); +} + +$remote_user = $base_remote_user = 'webminadmin'; + +# New and existing administrators keep their UI privileges for every RPC mode. +foreach my $rpc (0 .. 3) { + subtest "administrator with rpc=$rpc" => sub { + %test_acls = (webminadmin => { rpc => $rpc }); + check_mode('root'); + is(0 + main::webmin_user_can_rpc(), (0, 1, 0, 2)[$rpc], + 'RPC authorization still follows the RPC setting'); + }; +} + +# Either login identity can require safe mode, regardless of RPC permission. +foreach my $safe_user ('webminadmin', 'unixlogin') { + subtest "safe restriction on $safe_user" => sub { + local $remote_user = 'unixlogin'; + foreach my $rpc (0 .. 3) { + %test_acls = (webminadmin => { rpc => $rpc }, + unixlogin => { rpc => $rpc }); + $test_acls{$safe_user}->{'_safe'} = 1; + check_mode('safe-user'); + } + }; +} + +subtest 'different login identities with RPC disabled' => sub { + local $remote_user = 'unixlogin'; + %test_acls = (webminadmin => { rpc => 0 }, unixlogin => { rpc => 0 }); + check_mode('root'); +}; + +# Product-specific roles must not gain admin privileges when RPC is disabled. +subtest 'product roles with RPC disabled' => sub { + %test_acls = (webminadmin => { rpc => 0 }); + $test_product = 'usermin'; + check_mode('mail-user'); + $test_product = 'webmin'; + + %test_modules = ('virtual-server' => 1); + check_mode('root'); + $test_master = 0; + check_mode('virtual-owner'); + $test_reseller = 1; + check_mode('virtual-reseller'); + + %test_modules = ('server-manager' => 1); + local $server_manager::access{'owner'} = 1; + check_mode('cloud-owner'); + $server_manager::access{'owner'} = 0; + check_mode('root'); +}; + +done_testing(); diff --git a/web-lib-funcs.pl b/web-lib-funcs.pl index 8c754760f..2b3eff992 100755 --- a/web-lib-funcs.pl +++ b/web-lib-funcs.pl @@ -14338,10 +14338,9 @@ my $foreign_server_manager my %uaccess = &get_module_acl($remote_user, ""); my %access = &get_module_acl($base_remote_user, ""); -# Check if mode must be restricted +# Safe mode depends on explicit restrictions, not RPC permission if ($base_remote_user !~ /^(root|admin|sysadm)$/) { - if ($uaccess{'_safe'} == 1 || $access{'_safe'} == 1 || - $uaccess{'rpc'} == 0 || $access{'rpc'} == 0) { + if ($uaccess{'_safe'} == 1 || $access{'_safe'} == 1) { # Safe Webmin user $mode = 'safe-user'; }