From aee06a2a8be305d409bf442bf6f43b8b126b2744 Mon Sep 17 00:00:00 2001
From: Jan-Hendrik Willms <tleilax+studip@gmail.com>
Date: Tue, 14 Feb 2023 13:30:15 +0000
Subject: [PATCH] fix permission handling, fixes #2160

Closes #2160

Merge request studip/studip!1395
---
 app/controllers/room_management/overview.php | 44 +++++++++-----------
 lib/resources/ResourceManager.class.php      | 22 ++++++----
 2 files changed, 33 insertions(+), 33 deletions(-)

diff --git a/app/controllers/room_management/overview.php b/app/controllers/room_management/overview.php
index f41cd1945a7..e9bae539636 100644
--- a/app/controllers/room_management/overview.php
+++ b/app/controllers/room_management/overview.php
@@ -28,7 +28,7 @@ class RoomManagement_OverviewController extends AuthenticatedController
 {
     public function before_filter(&$action, &$args)
     {
-        if ($action == 'public_booking_plans') {
+        if ($action === 'public_booking_plans') {
             if (Config::get()->RESOURCES_SHOW_PUBLIC_ROOM_PLANS) {
                 $this->allow_nobody = true;
             } else {
@@ -36,6 +36,7 @@ class RoomManagement_OverviewController extends AuthenticatedController
             }
         }
         parent::before_filter($action, $args);
+
         $this->user = User::findCurrent();
         $this->user_is_root = $GLOBALS['perm']->have_perm('root');
         $this->user_is_global_resource_user = ResourceManager::userHasGlobalPermission($this->user);
@@ -43,16 +44,13 @@ class RoomManagement_OverviewController extends AuthenticatedController
 
         $this->show_resource_actions = (
             ResourceManager::userHasGlobalPermission($this->user, 'autor')
-            ||
-            ResourceManager::userHasResourcePermissions($this->user, 'autor')
+            || ResourceManager::userHasResourcePermissions($this->user, 'autor')
         );
         $this->show_admin_actions = (
             $this->user_is_global_resource_admin
-            ||
-            ResourceManager::userHasResourcePermissions($this->user)
+            || ResourceManager::userHasResourcePermissions($this->user)
         );
         $this->show_global_admin_actions = $this->user_is_global_resource_admin;
-
     }
 
     public function index_action()
@@ -61,10 +59,10 @@ class RoomManagement_OverviewController extends AuthenticatedController
             Navigation::activateItem('/resources/overview');
         }
 
-        $sufficient_permissions =
-            ResourceManager::userHasGlobalPermission($this->user)
-            ||
-            ResourceManager::userHasResourcePermissions($this->user, 'user');
+        $sufficient_permissions = (
+            $this->user_is_global_resource_user
+            || ResourceManager::userHasResourcePermissions($this->user, 'user')
+        );
         if (!$sufficient_permissions) {
             throw new AccessDeniedException();
         }
@@ -97,13 +95,11 @@ class RoomManagement_OverviewController extends AuthenticatedController
         $tree_selected_resource = null;
         if ($this->user_is_global_resource_admin) {
 
-            if (ResourceManager::userHasGlobalPermission($this->user, 'admin')){
-                $locations = Location::findAll();
-                if($locations) {
-                    $sidebar->addWidget(
-                        new ResourceTreeWidget($locations)
-                    );
-                }
+            $locations = Location::findAll();
+            if ($locations) {
+                $sidebar->addWidget(
+                    new ResourceTreeWidget($locations)
+                );
             }
 
             $tree_selected_resource = Request::get('tree_selected_resource');
@@ -128,13 +124,12 @@ class RoomManagement_OverviewController extends AuthenticatedController
         }
         $this->room_requests_activated = Config::get()->RESOURCES_ALLOW_ROOM_REQUESTS;
         $this->display_current_requests = false;
-        if (!$tree_selected_resource && $this->room_requests_activated ) {
+        if (!$tree_selected_resource && $this->room_requests_activated) {
             if (Config::get()->RESOURCES_DISPLAY_CURRENT_REQUESTS_IN_OVERVIEW) {
                 $this->display_current_requests = true;
-                $this->current_user = User::findCurrent();
 
                 //Load a list with the current room requests:
-                if (ResourceManager::userHasGlobalPermission($this->current_user, 'admin')) {
+                if ($this->user_is_global_resource_admin) {
                     //Global resource admins can see all room requests.
                     //Get the 10 latest requests:
                     $room_requests = RoomRequest::findBySql(
@@ -146,7 +141,7 @@ class RoomManagement_OverviewController extends AuthenticatedController
                     //Users who aren't global resource admins see only the requests
                     //of the rooms where they have at least 'autor' permissions.
                     $rooms = RoomManager::getUserRooms(
-                        $this->current_user,
+                        $this->user,
                         'autor'
                     );
                     $room_ids = [];
@@ -329,7 +324,7 @@ class RoomManagement_OverviewController extends AuthenticatedController
 
     public function rooms_action()
     {
-        if (ResourceManager::userHasGlobalPermission($this->user)) {
+        if ($this->user_is_global_resource_user) {
             PageLayout::setTitle(_('Übersicht über alle Räume'));
         } else {
             PageLayout::setTitle(_('Meine Räume'));
@@ -345,8 +340,7 @@ class RoomManagement_OverviewController extends AuthenticatedController
         //Check permissions:
         $sufficient_permissions = (
             $this->user_is_global_resource_user
-            ||
-            ResourceManager::userHasResourcePermissions($this->user, 'user')
+            || ResourceManager::userHasResourcePermissions($this->user, 'user')
         );
         if (!$sufficient_permissions) {
             throw new AccessDeniedException();
@@ -393,7 +387,7 @@ class RoomManagement_OverviewController extends AuthenticatedController
         $rooms_parameter['building_name'] = Request::get('building_room_name');
 
         if ($this->user_is_global_resource_user) {
-            if(Request::get('building_room_name')) {
+            if (Request::get('building_room_name')) {
                 $rooms_sql_with_request .= " ORDER BY sort_position DESC, name ASC, mkdate ASC";
                 $this->rooms = Room::findBySQL($rooms_sql_with_request, $rooms_parameter);
             } else {
diff --git a/lib/resources/ResourceManager.class.php b/lib/resources/ResourceManager.class.php
index 2c4e6ce5918..a30c4702b6f 100644
--- a/lib/resources/ResourceManager.class.php
+++ b/lib/resources/ResourceManager.class.php
@@ -845,12 +845,16 @@ class ResourceManager
      * This method does the mapping from the old resource management permissions
      * to the new resource management permissions.
      */
-    public static function getGlobalResourcePermission(User $user)
+    public static function getGlobalResourcePermission(User $user = null)
     {
+        if (!$user) {
+            return '';
+        }
+
         global $perm;
         //First we check if the user is a root user:
 
-        if ($perm->get_perm($user->id) == 'root') {
+        if ($perm->get_perm($user->id) === 'root') {
             return 'admin';
         }
 
@@ -886,11 +890,15 @@ class ResourceManager
      *     If this is not set the current timestamp will be used.
      */
     public static function userHasResourcePermissions(
-        User $user,
+        User $user = null,
         $level = 'admin',
         $time = null
     )
     {
+        if (!$user) {
+            return false;
+        }
+
         //Get all permissions and temporary permissions of the user:
 
         $permissions = ResourcePermission::findBySQL(
@@ -1062,16 +1070,14 @@ class ResourceManager
      * Checks if the specified user has the specified permission level
      * for the resource management system.
      *
-     * @param User $user The user whose global resource permissions
-     *     shall be checked.
-     * @param string $requested_permission The required permission level
-     *     for the user.
+     * @param User|null $user The user whose global resource permissions shall be checked.
+     * @param string $requested_permission The required permission level for the user.
      *
      * @returns bool True, if the user has the required permission level,
      *     false otherwise.
      */
     public static function userHasGlobalPermission(
-        User $user,
+        User $user = null,
         $requested_permission = 'user'
     )
     {
-- 
GitLab