From f97ac52934278ef611520c1f3972d8d226af8f73 Mon Sep 17 00:00:00 2001
From: Simon Rettberg
Date: Tue, 14 Jan 2020 16:50:47 +0100
Subject: [permissionmanager] Also disallow deleting builtin roles

---
 modules-available/permissionmanager/page.inc.php   | 28 +++++++++++++---------
 .../permissionmanager/templates/rolestable.html    |  2 ++
 2 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/modules-available/permissionmanager/page.inc.php b/modules-available/permissionmanager/page.inc.php
index 63cbcb59..b431d9c9 100644
--- a/modules-available/permissionmanager/page.inc.php
+++ b/modules-available/permissionmanager/page.inc.php
@@ -29,21 +29,12 @@ class Page_PermissionManager extends Page
 		} elseif ($action === 'deleteRole') {
 			User::assertPermission('roles.edit');
 			$id = Request::post('deleteId', false, 'int');
+			$this->denyActionIfBuiltin($id);
 			PermissionDbUpdate::deleteRole($id);
 		} elseif ($action === 'saveRole') {
 			User::assertPermission('roles.edit');
 			$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');
-				}
-			}
+			$this->denyActionIfBuiltin($roleID);
 			$roleName = Request::post("rolename", '', 'string');
 			if (empty($roleName)) {
 				Message::addError('main.parameter-empty', 'rolename');
@@ -315,4 +306,19 @@ class Page_PermissionManager extends Page
 		return $result;
 	}
 
+	private function denyActionIfBuiltin($roleID)
+	{
+		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');
+			}
+		}
+	}
+
 }
diff --git a/modules-available/permissionmanager/templates/rolestable.html b/modules-available/permissionmanager/templates/rolestable.html
index f3521964..170dde88 100644
--- a/modules-available/permissionmanager/templates/rolestable.html
+++ b/modules-available/permissionmanager/templates/rolestable.html
@@ -41,6 +41,7 @@
 							</a>
 						</td>
 						<td class="text-center">
+							{{^builtin}}
 							<button type="submit" name="deleteId" value="{{roleid}}" class="btn btn-xs btn-danger" {{perms.roles.edit.disabled}}
 									data-confirm="#confirm-role-{{roleid}}" data-title="{{rolename}}">
 								<span class="glyphicon glyphicon-trash"></span>
@@ -49,6 +50,7 @@
 								<p>{{lang_roleDeleteConfirm}}</p>
 								{{lang_numAssignedUsers}}: {{users}}
 							</div>
+							{{/builtin}}
 						</td>
 					</tr>
 				{{/roles}}
-- 
cgit v1.2.3-55-g7522