From 206d0b94f4010e8a5cbce74c5afbae46adf03d74 Mon Sep 17 00:00:00 2001
From: Simon Rettberg
Date: Thu, 9 Jan 2020 13:22:29 +0100
Subject: [permissionmanager] Make default roles "builtin" i.e. not modifiable
---
inc/request.inc.php | 10 ++++++-
.../inc/getpermissiondata.inc.php | 23 ++++++++++------
.../permissionmanager/install.inc.php | 32 ++++++++++++++--------
.../permissionmanager/lang/de/template-tags.json | 1 +
.../permissionmanager/lang/en/template-tags.json | 1 +
modules-available/permissionmanager/page.inc.php | 27 ++++++++++++++----
.../permissionmanager/templates/roleeditor.html | 7 ++++-
.../permissionmanager/templates/rolestable.html | 14 ++++++++--
8 files changed, 87 insertions(+), 28 deletions(-)
diff --git a/inc/request.inc.php b/inc/request.inc.php
index a2dc9711..6104fafd 100644
--- a/inc/request.inc.php
+++ b/inc/request.inc.php
@@ -6,8 +6,16 @@
class Request
{
+ /**
+ * Required and not empty
+ */
const REQUIRED = "\0\1\2REQ\0\1\2";
+ /**
+ * Required, but might be empty
+ */
+ const REQUIRED_EMPTY = "\0\3\4REQ\0\3\4";
+
/**
*
* @param string $key Key of field to get from $_GET
@@ -61,7 +69,7 @@ class Request
private static function handle(&$array, $key, $default, $type)
{
if (!isset($array[$key])) {
- if ($default === self::REQUIRED) {
+ if ($default === self::REQUIRED || $default === self::REQUIRED_EMPTY) {
Message::addError('main.parameter-missing', $key);
Util::redirect('?do=' . $_REQUEST['do']);
}
diff --git a/modules-available/permissionmanager/inc/getpermissiondata.inc.php b/modules-available/permissionmanager/inc/getpermissiondata.inc.php
index 660c94ae..4dfb09ec 100644
--- a/modules-available/permissionmanager/inc/getpermissiondata.inc.php
+++ b/modules-available/permissionmanager/inc/getpermissiondata.inc.php
@@ -84,7 +84,7 @@ class GetPermissionData
if (!empty($joins)) {
$joins .= ' GROUP BY r.roleid';
}
- return Database::queryAll("SELECT r.roleid, r.rolename, r.roledescription $cols FROM role r
+ return Database::queryAll("SELECT r.roleid, r.rolename, r.builtin, r.roledescription $cols FROM role r
$joins
ORDER BY rolename ASC");
}
@@ -93,20 +93,21 @@ class GetPermissionData
* Get permissions and locations for a given role.
*
* @param string $roleid id of the role
- * @return array array containing an array of permissions and an array of locations
+ * @return array|false array containing an array of permissions and an array of locations, false if not found
*/
public static function getRoleData($roleid)
{
- $query = "SELECT roleid, rolename, roledescription FROM role WHERE roleid = :roleid";
- $data = Database::queryFirst($query, array("roleid" => $roleid));
- $query = "SELECT roleid, locationid FROM role_x_location WHERE roleid = :roleid";
- $res = Database::simpleQuery($query, array("roleid" => $roleid));
+ $data = self::getRole($roleid);
+ $res = Database::simpleQuery("SELECT roleid, locationid FROM role_x_location WHERE roleid = :roleid",
+ array("roleid" => $roleid));
+ if ($res === false)
+ return false;
$data["locations"] = array();
while ($row = $res->fetch(PDO::FETCH_ASSOC)) {
$data["locations"][] = $row['locationid'];
}
- $query = "SELECT roleid, permissionid FROM role_x_permission WHERE roleid = :roleid";
- $res = Database::simpleQuery($query, array("roleid" => $roleid));
+ $res = Database::simpleQuery("SELECT roleid, permissionid FROM role_x_permission WHERE roleid = :roleid",
+ array("roleid" => $roleid));
$data["permissions"] = array();
while ($row = $res->fetch(PDO::FETCH_ASSOC)) {
$data["permissions"][] = $row['permissionid'];
@@ -114,4 +115,10 @@ class GetPermissionData
return $data;
}
+ public static function getRole($roleId)
+ {
+ return Database::queryFirst("SELECT roleid, rolename, builtin, roledescription FROM role
+ WHERE roleid = :roleid", ['roleid' => $roleId]);
+ }
+
}
\ No newline at end of file
diff --git a/modules-available/permissionmanager/install.inc.php b/modules-available/permissionmanager/install.inc.php
index 68f01899..5d1f60da 100644
--- a/modules-available/permissionmanager/install.inc.php
+++ b/modules-available/permissionmanager/install.inc.php
@@ -5,6 +5,7 @@ $res = array();
$res[] = tableCreate('role', "
roleid int(10) unsigned NOT NULL AUTO_INCREMENT,
rolename varchar(200) NOT NULL,
+ builtin bool NOT NULL DEFAULT '0',
roledescription TEXT,
PRIMARY KEY (roleid)
");
@@ -100,20 +101,27 @@ if (!tableHasColumn('role', 'roledescription')) {
$res[] = UPDATE_DONE;
}
-if (!tableHasColumn('role', 'roledescription')) {
- finalResponse(UPDATE_RETRY, 'Try again later');
+// 2020-01-09 flag for builtin roles that can't be edited
+if (!tableHasColumn('role', 'builtin')) {
+ $alter = Database::exec("ALTER TABLE role ADD builtin bool NOT NULL DEFAULT '0' AFTER rolename");
+ if ($alter === false)
+ finalResponse(UPDATE_FAILED, 'Cannot add builtin field to table role: ' . Database::lastError());
+ $res[] = UPDATE_DONE;
}
-if (Database::exec("INSERT INTO `role` VALUES
- (1,'Super-Admin', 'Hat keinerlei Zugriffsbeschränkungen'),
- (2,'Admin', 'Alles bis auf Rechte-/Nutzerverwaltung'),
- (3,'Prüfungsadmin', 'Kann E-Prüfungen verwalten, Prüfungsmodus einschalten, etc.'),
- (4,'Lesezugriff', 'Kann auf die meisten Seiten zugreifen, jedoch keine Änderungen vornehmen')") !== false) {
- // Success, there probably were no roles before, keep going
+if (Database::exec("INSERT INTO `role` (roleid, rolename, builtin, roledescription) VALUES
+ (1,'Super-Admin', 1, 'Hat keinerlei Zugriffsbeschränkungen'),
+ (2,'Admin', 1, 'Alles bis auf Rechte-/Nutzerverwaltung'),
+ (3,'Prüfungsadmin', 1, 'Kann E-Prüfungen verwalten, Prüfungsmodus einschalten, etc.'),
+ (4,'Lesezugriff', 1, 'Kann auf die meisten Seiten zugreifen, jedoch keine Änderungen vornehmen')
+ ON DUPLICATE KEY UPDATE rolename = VALUES(rolename), builtin = 1, roledescription = VALUES(roledescription)") !== false) {
+ // Old ruleset accidentally gave write permissions to the read-only role
+ Database::exec("DELETE FROM role_x_permission WHERE roleid = 4 AND permissionid = 'news.*'");
// Assign roles to location (all)
+ Database::exec("DELETE FROM role_x_location WHERE roleid IN (1,2,3,4)");
Database::exec("INSERT INTO `role_x_location` VALUES (1,NULL),(2,NULL),(3,NULL),(4,NULL)");
// Assign permissions to roles
- Database::exec("INSERT INTO `role_x_permission` VALUES
+ Database::exec("INSERT IGNORE INTO `role_x_permission` VALUES
(3,'exams.exams.*'),
(3,'rebootcontrol.action.*'),
(3,'statistics.hardware.projectors.view'),
@@ -138,7 +146,7 @@ if (Database::exec("INSERT INTO `role` VALUES
(4,'locationinfo.panel.list'),
(4,'locations.location.view'),
(4,'minilinux.view'),
- (4,'news.*'),
+ (4,'news.access-page'),
(4,'permissionmanager.locations.view'),
(4,'permissionmanager.roles.view'),
(4,'permissionmanager.users.view'),
@@ -159,6 +167,8 @@ if (Database::exec("INSERT INTO `role` VALUES
(4,'systemstatus.show.overview.*'),
(4,'systemstatus.tab.*'),
(4,'webinterface.access-page'),
+ (4,'rebootcontrol.subnet.view'),
+ (4,'rebootcontrol.jumphost.view'),
(2,'adduser.user.view-list'),
(2,'backup.*'),
@@ -186,7 +196,7 @@ if (Database::exec("INSERT INTO `role` VALUES
(2,'vmstore.edit'),
(2,'webinterface.*')");
// Assign the first user to the superadmin role (if one exists)
- Database::exec("INSERT INTO `role_x_user` VALUES (1,1)");
+ Database::exec("INSERT IGNORE INTO `role_x_user` VALUES (1,1)");
$res[] = UPDATE_DONE;
}
diff --git a/modules-available/permissionmanager/lang/de/template-tags.json b/modules-available/permissionmanager/lang/de/template-tags.json
index 504ef6d2..e8f44c82 100644
--- a/modules-available/permissionmanager/lang/de/template-tags.json
+++ b/modules-available/permissionmanager/lang/de/template-tags.json
@@ -1,6 +1,7 @@
{
"lang_addRole": "Rollen erteilen",
"lang_addRoleHeading": "Neue Rolle hinzuf\u00fcgen",
+ "lang_copyRoleHeading": "Rolle kopieren",
"lang_description": "Beschreibung",
"lang_editRoleHeading": "Rolle bearbeiten",
"lang_locationAwareDesc": "Berechtigungen mit diesem Symbol k\u00f6nnen auf bestimmte R\u00e4ume\/Orte beschr\u00e4nkt werden. Alle anderen Berechtigungen sind unabh\u00e4ngig von den f\u00fcr diese Rolle ausgew\u00e4hlten Orten.",
diff --git a/modules-available/permissionmanager/lang/en/template-tags.json b/modules-available/permissionmanager/lang/en/template-tags.json
index 6f1fa614..d13397e8 100644
--- a/modules-available/permissionmanager/lang/en/template-tags.json
+++ b/modules-available/permissionmanager/lang/en/template-tags.json
@@ -1,6 +1,7 @@
{
"lang_addRole": "Grant Roles",
"lang_addRoleHeading": "Add new role",
+ "lang_copyRoleHeading": "Copy role",
"lang_description": "Description",
"lang_editRoleHeading": "Edit role",
"lang_locationAwareDesc": "Permissions with this symbol can be restricted to certain locations. All other permissions are independent of the locations selected for this role.",
diff --git a/modules-available/permissionmanager/page.inc.php b/modules-available/permissionmanager/page.inc.php
index 462d3163..63cbcb59 100644
--- a/modules-available/permissionmanager/page.inc.php
+++ b/modules-available/permissionmanager/page.inc.php
@@ -32,10 +32,17 @@ class Page_PermissionManager extends Page
PermissionDbUpdate::deleteRole($id);
} elseif ($action === 'saveRole') {
User::assertPermission('roles.edit');
- $roleID = Request::post("roleid", false, 'int');
- if ($roleID === false) {
- Message::addError('main.parameter-missing', 'roleid');
- Util::redirect('?do=permissionmanager');
+ $roleID = Request::post("roleid", Request::REQUIRED_EMPTY, 'int');
+ if ($roleID) {
+ $existing = GetPermissionData::getRole($roleID);
+ if ($existing === false) {
+ Message::addError('invalid-role-id', $roleID);
+ Util::redirect('?do=permissionmanager');
+ }
+ if ($existing['builtin']) {
+ Message::addError('builtin-role', $existing['rolename']);
+ Util::redirect('?do=permissionmanager');
+ }
}
$roleName = Request::post("rolename", '', 'string');
if (empty($roleName)) {
@@ -116,7 +123,17 @@ class Page_PermissionManager extends Page
$selectedLocations = array();
$roleid = Request::get("roleid", false, 'int');
if ($roleid !== false) {
- $data += GetPermissionData::getRoleData($roleid);
+ $role = GetPermissionData::getRoleData($roleid);
+ if ($role === false) {
+ Message::addError('invalid-role-id', $roleid);
+ Util::redirect('?do=permissionmanager');
+ }
+ if ($role['builtin']) {
+ // Copy the role, as it's builtin
+ $role['roleid'] = '';
+ $role['rolename'] .= ' (2)';
+ }
+ $data += $role;
$selectedPermissions = $data["permissions"];
$selectedLocations = $data["locations"];
}
diff --git a/modules-available/permissionmanager/templates/roleeditor.html b/modules-available/permissionmanager/templates/roleeditor.html
index c464c1fc..37d0d5fe 100644
--- a/modules-available/permissionmanager/templates/roleeditor.html
+++ b/modules-available/permissionmanager/templates/roleeditor.html
@@ -3,7 +3,12 @@
{{lang_editRoleHeading}}
{{/roleid}}
{{^roleid}}
- {{lang_addRoleHeading}}
+ {{#builtin}}
+ {{lang_copyRoleHeading}}
+ {{/builtin}}
+ {{^builtin}}
+ {{lang_addRoleHeading}}
+ {{/builtin}}
{{/roleid}}
diff --git a/modules-available/permissionmanager/templates/rolestable.html b/modules-available/permissionmanager/templates/rolestable.html
index e50dffc7..f3521964 100644
--- a/modules-available/permissionmanager/templates/rolestable.html
+++ b/modules-available/permissionmanager/templates/rolestable.html
@@ -31,7 +31,14 @@
| {{rolename}} |
|
-
+
+ {{#builtin}}
+
+ {{/builtin}}
+ {{^builtin}}
+
+ {{/builtin}}
+
|
|