mirrors/pfsense
2
0
Fork 0
mirror of https://github.com/pfsense/pfsense.git synced 2025-10-26 11:38:35 +00:00
Code Issues Packages Projects Releases Wiki Activity

User & Group Manager: Improve Deny Config Write Handling. Fixes #9259

Browse Source 
* Denies all changes if a user has the Deny Config Write privilege.
Previously it only denied the config write but some OS operations were
performed.
* Sets an input error so the user is notified that their attempt failed.
* Hides the add and delete buttons so read only users don't see the
option to perform those actions (but are still denied if they submit the
form through other means)

(cherry picked from commit acd7e5601a)
This commit is contained in:
jim-p 2019-08-27 13:19:30 -04:00
parent 0caa971ed9
commit 585bbbd33e
4 changed files with 94 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

36
src/usr/local/www/system_groupmanager.php
View File

@ -60,7 +60,21 @@ function admin_groups_sort() {
usort($a_group, "cpusercmp");
}
if ($_POST['act'] == "delgroup") {
/*
* Check user privileges to test if the user is allowed to make changes.
* Otherwise users can end up in an inconsistent state where some changes are
* performed and others denied. See https://redmine.pfsense.org/issues/9259
*/
phpsession_begin();
$guiuser = getUserEntry($_SESSION['Username']);
$read_only = (is_array($guiuser) && userHasPrivilege($guiuser, "user-config-readonly"));
phpsession_end();
if (!empty($_POST) && $read_only) {
$input_errors = array(gettext("Insufficient privileges to make the requested change (read only)."));
}
if (($_POST['act'] == "delgroup") && !$read_only) {
if (!isset($id) || !isset($_REQUEST['groupname']) ||
!isset($a_group[$id]) ||
@ -84,7 +98,7 @@ if ($_POST['act'] == "delgroup") {
syslog($logging_level, "{$logging_prefix}: {$savemsg}");
}
if ($_POST['act'] == "delpriv") {
if (($_POST['act'] == "delpriv") && !$read_only) {
if (!isset($id) || !isset($a_group[$id])) {
pfSenseHeader("system_groupmanager.php");
@ -124,7 +138,7 @@ if ($act == "edit") {
}
}
if (isset($_POST['dellall_x'])) {
if (isset($_POST['dellall_x']) && !$read_only) {
$del_groups = $_POST['delete_check'];
$deleted_groups = array();
@ -153,7 +167,7 @@ if (isset($_POST['dellall_x'])) {
}
}
if (isset($_POST['save'])) {
if (isset($_POST['save']) && !$read_only) {
unset($input_errors);
$pconfig = $_POST;
@ -264,7 +278,7 @@ if (isset($_POST['save'])) {
}
function build_priv_table() {
global $a_group, $id;
global $a_group, $id, $read_only;
$privhtml = '<div class="table-responsive">';
$privhtml .= '<table class="table table-striped table-hover table-condensed">';
@ -288,7 +302,9 @@ function build_priv_table() {
$user_has_root_priv = true;
}
$privhtml .= '</td>';
$privhtml .= '<td><a class="fa fa-trash" title="' . gettext('Delete Privilege') . '" href="system_groupmanager.php?act=delpriv&amp;groupid=' . $id . '&amp;privid=' . $i . '" usepost></a></td>';
if (!$read_only) {
$privhtml .= '<td><a class="fa fa-trash" title="' . gettext('Delete Privilege') . '" href="system_groupmanager.php?act=delpriv&amp;groupid=' . $id . '&amp;privid=' . $i . '" usepost></a></td>';
}
$privhtml .= '</tr>';
}
@ -309,7 +325,9 @@ function build_priv_table() {
$privhtml .= '</div>';
$privhtml .= '<nav class="action-buttons">';
$privhtml .= '<a href="system_groupmanager_addprivs.php?groupid=' . $id . '" class="btn btn-success"><i class="fa fa-plus icon-embed-btn"></i>' . gettext("Add") . '</a>';
if (!$read_only) {
$privhtml .= '<a href="system_groupmanager_addprivs.php?groupid=' . $id . '" class="btn btn-success"><i class="fa fa-plus icon-embed-btn"></i>' . gettext("Add") . '</a>';
}
$privhtml .= '</nav>';
return($privhtml);
@ -378,7 +396,7 @@ if (!($act == "new" || $act == "edit")) {
</td>
<td>
<a class="fa fa-pencil" title="<?=gettext("Edit group"); ?>" href="?act=edit&amp;groupid=<?=$i?>"></a>
<?php if ($group['scope'] != "system"): ?>
<?php if (($group['scope'] != "system") && !$read_only): ?>
<a class="fa fa-trash" title="<?=gettext("Delete group")?>" href="?act=delgroup&amp;groupid=<?=$i?>&amp;groupname=<?=$group['name']?>" usepost></a>
<?php endif;?>
</td>
@ -393,10 +411,12 @@ if (!($act == "new" || $act == "edit")) {
</div>
<nav class="action-buttons">
<?php if (!$read_only): ?>
<a href="?act=new" class="btn btn-success btn-sm">
<i class="fa fa-plus icon-embed-btn"></i>
<?=gettext("Add")?>
</a>
<?php endif; ?>
</nav>
<?php
include('foot.inc');

16
src/usr/local/www/system_groupmanager_addprivs.php
View File

@ -59,7 +59,21 @@ if (!is_array($a_group['priv'])) {
$spriv_list = $priv_list;
uasort($spriv_list, "compare_by_name");
if ($_POST['save']) {
/*
* Check user privileges to test if the user is allowed to make changes.
* Otherwise users can end up in an inconsistent state where some changes are
* performed and others denied. See https://redmine.pfsense.org/issues/9259
*/
phpsession_begin();
$guiuser = getUserEntry($_SESSION['Username']);
$read_only = (is_array($guiuser) && userHasPrivilege($guiuser, "user-config-readonly"));
phpsession_end();
if (!empty($_POST) && $read_only) {
$input_errors = array(gettext("Insufficient privileges to make the requested change (read only)."));
}
if ($_POST['save'] && !$read_only) {
unset($input_errors);
$pconfig = $_POST;

49
src/usr/local/www/system_usermanager.php
View File

@ -81,7 +81,21 @@ if (isset($id) && $a_user[$id]) {
$pconfig['disabled'] = isset($a_user[$id]['disabled']);
}
if ($_POST['act'] == "deluser") {
/*
* Check user privileges to test if the user is allowed to make changes.
* Otherwise users can end up in an inconsistent state where some changes are
* performed and others denied. See https://redmine.pfsense.org/issues/9259
*/
phpsession_begin();
$guiuser = getUserEntry($_SESSION['Username']);
$read_only = (is_array($guiuser) && userHasPrivilege($guiuser, "user-config-readonly"));
phpsession_end();
if (!empty($_POST) && $read_only) {
$input_errors = array(gettext("Insufficient privileges to make the requested change (read only)."));
}
if (($_POST['act'] == "deluser") && !$read_only) {
if (!isset($_POST['username']) || !isset($a_user[$id]) || ($_POST['username'] != $a_user[$id]['name'])) {
pfSenseHeader("system_usermanager.php");
@ -123,7 +137,7 @@ if ($_POST['act'] == "deluser") {
}
if (isset($_POST['dellall'])) {
if (isset($_POST['dellall']) && !$read_only) {
$del_users = $_POST['delete_check'];
$deleted_users = array();
@ -153,7 +167,7 @@ if (isset($_POST['dellall'])) {
}
}
if ($_POST['act'] == "delcert") {
if (($_POST['act'] == "delcert") && !$read_only) {
if (!$a_user[$id]) {
pfSenseHeader("system_usermanager.php");
@ -169,7 +183,7 @@ if ($_POST['act'] == "delcert") {
$_POST['act'] = "edit";
}
if ($_POST['act'] == "delprivid") {
if (($_POST['act'] == "delprivid") && !$read_only) {
$privdeleted = $priv_list[$a_user[$id]['priv'][$_POST['privid']]]['name'];
unset($a_user[$id]['priv'][$_POST['privid']]);
local_user_set($a_user[$id]);
@ -179,7 +193,7 @@ if ($_POST['act'] == "delprivid") {
$_POST['act'] = "edit";
}
if ($_POST['save']) {
if ($_POST['save'] && !$read_only) {
unset($input_errors);
$pconfig = $_POST;
@ -473,7 +487,7 @@ if ($_POST['save']) {
}
function build_priv_table() {
global $a_user, $id;
global $a_user, $id, $read_only;
$privhtml = '<div class="table-responsive">';
$privhtml .= '<table class="table table-striped table-hover table-condensed">';
@ -506,7 +520,7 @@ function build_priv_table() {
}
$privhtml .= '</td>';
$privhtml .= '<td>';
if (!$group) {
if (!$group && !$read_only) {
$privhtml .= '<a class="fa fa-trash no-confirm icon-pointer" title="' . gettext('Delete Privilege') . '" id="delprivid' . $i . '"></a>';
}
@ -534,14 +548,16 @@ function build_priv_table() {
$privhtml .= '</div>';
$privhtml .= '<nav class="action-buttons">';
$privhtml .= '<a href="system_usermanager_addprivs.php?userid=' . $id . '" class="btn btn-success"><i class="fa fa-plus icon-embed-btn"></i>' . gettext("Add") . '</a>';
if (!$read_only) {
$privhtml .= '<a href="system_usermanager_addprivs.php?userid=' . $id . '" class="btn btn-success"><i class="fa fa-plus icon-embed-btn"></i>' . gettext("Add") . '</a>';
}
$privhtml .= '</nav>';
return($privhtml);
}
function build_cert_table() {
global $a_user, $id;
global $a_user, $id, $read_only;
$certhtml = '<div class="table-responsive">';
$certhtml .= '<table class="table table-striped table-hover table-condensed">';
@ -566,8 +582,10 @@ function build_cert_table() {
$certhtml .= '<td>' . htmlspecialchars($cert['descr']) . $revokedstr . '</td>';
$certhtml .= '<td>' . htmlspecialchars($ca['descr']) . '</td>';
$certhtml .= '<td>';
$certhtml .= '<a id="delcert' . $i .'" class="fa fa-trash no-confirm icon-pointer" title="';
$certhtml .= gettext('Remove this certificate association? (Certificate will not be deleted)') . '"></a>';
if (!$read_only) {
$certhtml .= '<a id="delcert' . $i .'" class="fa fa-trash no-confirm icon-pointer" title="';
$certhtml .= gettext('Remove this certificate association? (Certificate will not be deleted)') . '"></a>';
}
$certhtml .= '</td>';
$certhtml .= '</tr>';
$i++;
@ -580,7 +598,9 @@ function build_cert_table() {
$certhtml .= '</div>';
$certhtml .= '<nav class="action-buttons">';
$certhtml .= '<a href="system_certmanager.php?act=new&amp;userid=' . $id . '" class="btn btn-success"><i class="fa fa-plus icon-embed-btn"></i>' . gettext("Add") . '</a>';
if (!$read_only) {
$certhtml .= '<a href="system_certmanager.php?act=new&amp;userid=' . $id . '" class="btn btn-success"><i class="fa fa-plus icon-embed-btn"></i>' . gettext("Add") . '</a>';
}
$certhtml .= '</nav>';
return($certhtml);
@ -657,7 +677,7 @@ foreach ($a_user as $i => $userent):
<td><?=implode(",", local_user_get_groups($userent))?></td>
<td>
<a class="fa fa-pencil" title="<?=gettext("Edit user"); ?>" href="?act=edit&amp;userid=<?=$i?>"></a>
<?php if (($userent['scope'] != "system") && ($userent['name'] != $_SESSION['Username'])): ?>
<?php if (($userent['scope'] != "system") && ($userent['name'] != $_SESSION['Username']) && !$read_only): ?>
<a class="fa fa-trash" title="<?=gettext("Delete user")?>" href="?act=deluser&amp;userid=<?=$i?>&amp;username=<?=$userent['name']?>" usepost></a>
<?php endif; ?>
</td>
@ -669,6 +689,8 @@ foreach ($a_user as $i => $userent):
</div>
</div>
<nav class="action-buttons">
<?php if (!$read_only): ?>
<a href="?act=new" class="btn btn-sm btn-success">
<i class="fa fa-plus icon-embed-btn"></i>
<?=gettext("Add")?>
@ -678,6 +700,7 @@ foreach ($a_user as $i => $userent):
<i class="fa fa-trash icon-embed-btn"></i>
<?=gettext("Delete")?>
</button>
<?php endif; ?>
</nav>
</form>

16
src/usr/local/www/system_usermanager_addprivs.php
View File

@ -56,7 +56,21 @@ if (!is_array($a_user['priv'])) {
$spriv_list = $priv_list;
uasort($spriv_list, "compare_by_name");
if ($_POST['save']) {
/*
* Check user privileges to test if the user is allowed to make changes.
* Otherwise users can end up in an inconsistent state where some changes are
* performed and others denied. See https://redmine.pfsense.org/issues/9259
*/
phpsession_begin();
$guiuser = getUserEntry($_SESSION['Username']);
$read_only = (is_array($guiuser) && userHasPrivilege($guiuser, "user-config-readonly"));
phpsession_end();
if (!empty($_POST) && $read_only) {
$input_errors = array(gettext("Insufficient privileges to make the requested change (read only)."));
}
if ($_POST['save'] && !$read_only) {
unset($input_errors);
$pconfig = $_POST;