From 45ddcf37ccaffd110b9b29c1cf8ac65f26f7d850 Mon Sep 17 00:00:00 2001
From: Moritz Strohm <strohm@data-quest.de>
Date: Fri, 29 Jul 2022 14:29:12 +0000
Subject: [PATCH] make public booking plans only visible when
 RESOURCES_SHOW_PUBLIC_ROOM_PLANS is set, fixes #143

Closes #143

Merge request studip/studip!847
---
 app/controllers/resources/export.php         |  6 ----
 app/controllers/resources/resource.php       | 35 ++++++--------------
 app/controllers/resources/room_planning.php  | 31 ++++++++---------
 app/controllers/room_management/planning.php |  5 +--
 app/routes/Resources.php                     |  2 +-
 app/views/resources/search/rooms.php         |  2 +-
 lib/models/resources/Resource.class.php      | 17 +++++-----
 lib/models/resources/Room.class.php          |  4 +--
 8 files changed, 37 insertions(+), 65 deletions(-)

diff --git a/app/controllers/resources/export.php b/app/controllers/resources/export.php
index 2a94ec4a232..a329b9d5971 100644
--- a/app/controllers/resources/export.php
+++ b/app/controllers/resources/export.php
@@ -410,12 +410,6 @@ class Resources_ExportController extends AuthenticatedController
         if (!$user_has_user_permissions) {
             throw new AccessDeniedException();
         }
-        if ($this->resource instanceof Room) {
-            if (!$this->resource->booking_plan_is_public
-                && !$user_has_user_permissions) {
-                throw new AccessDeniedException();
-            }
-        }
 
         $week_timestamp = Request::get('timestamp', time());
 
diff --git a/app/controllers/resources/resource.php b/app/controllers/resources/resource.php
index d5bb7ac1c97..b2aaa783d71 100644
--- a/app/controllers/resources/resource.php
+++ b/app/controllers/resources/resource.php
@@ -458,20 +458,7 @@ class Resources_ResourceController extends AuthenticatedController
         //and the user has at least 'user' permissions on the resource.
         //The booking plan is also visible when the resource is a room
         //and its booking plan is publicly available.
-        $booking_plan_is_visible = false;
-        if ($GLOBALS['user']->id != 'nobody') {
-            $booking_plan_is_visible = $this->resource->userHasPermission(
-                $current_user,
-                'user'
-            );
-        }
-        if ($booking_plan_is_visible === false) {
-            $booking_plan_is_visible = (
-                ($this->resource instanceof Room)
-                and $this->resource->booking_plan_is_public
-            );
-        }
-        if (!$booking_plan_is_visible) {
+        if (!$this->resource->bookingPlanVisibleForUser($current_user)) {
             throw new AccessDeniedException(
                 _('Der Belegungsplan ist für Sie nicht zugänglich!')
             );
@@ -561,17 +548,15 @@ class Resources_ResourceController extends AuthenticatedController
                 );
             }
         }
-        if (($this->resource instanceof Room) and $this->resource->booking_plan_is_public) {
-            $actions->addLink(
-                _('QR-Code anzeigen'),
-                $this->resource->getActionURL('booking_plan'),
-                Icon::create('code-qr'),
-                [
-                    'data-qr-code' => '',
-                    'data-qr-code-print' => '1'
-                ]
-            );
-        }
+        $actions->addLink(
+            _('QR-Code anzeigen'),
+            $this->resource->getActionURL('booking_plan'),
+            Icon::create('code-qr'),
+            [
+                'data-qr-code' => '',
+                'data-qr-code-print' => '1'
+            ]
+        );
 
         $sidebar->addWidget($actions);
 
diff --git a/app/controllers/resources/room_planning.php b/app/controllers/resources/room_planning.php
index c4bf84aac6b..89a0549c412 100644
--- a/app/controllers/resources/room_planning.php
+++ b/app/controllers/resources/room_planning.php
@@ -160,11 +160,10 @@ class Resources_RoomPlanningController extends AuthenticatedController
             if ($this->resource->userHasPermission($current_user, 'admin')) {
                 $this->booking_types[] = 3;
             }
-        }
-        //If the plan visibility cannot be determined by the user,
-        //we can still check if the plan is visible to the public:
-        if (!$plan_is_visible && ($this->resource instanceof Room)) {
-            $plan_is_visible = $this->resource->booking_plan_is_public;
+        } else {
+            //If the plan visibility cannot be determined by the user,
+            //we can still check if the plan is visible to the public:
+            $plan_is_visible = $this->resource->bookingPlanVisibleForUser($current_user);
         }
         if (!$plan_is_visible) {
             throw new AccessDeniedException(
@@ -351,18 +350,16 @@ class Resources_RoomPlanningController extends AuthenticatedController
                 }
             }
         }
-        if ($this->resource instanceof Room && $this->resource->booking_plan_is_public) {
-            $actions->addLink(
-                _('QR-Code anzeigen'),
-                $this->resource->getActionURL('booking_plan'),
-                Icon::create('code-qr'),
-                [
-                    'data-qr-code'       => '',
-                    'data-qr-code-print' => '1',
-                    'data-qr-title'      => _('Aktueller Belegungsplan')
-                ]
-            );
-        }
+        $actions->addLink(
+            _('QR-Code anzeigen'),
+            $this->resource->getActionURL('booking_plan'),
+            Icon::create('code-qr'),
+            [
+                'data-qr-code'       => '',
+                'data-qr-code-print' => '1',
+                'data-qr-title'      => _('Aktueller Belegungsplan')
+            ]
+        );
 
         if ($current_user instanceof User) {
             //No check necessary here: This part of the controller is only called
diff --git a/app/controllers/room_management/planning.php b/app/controllers/room_management/planning.php
index 06a460b00b4..7aafe68fcf9 100644
--- a/app/controllers/room_management/planning.php
+++ b/app/controllers/room_management/planning.php
@@ -457,10 +457,7 @@ class RoomManagement_PlanningController extends AuthenticatedController
             }
 
             //Check the permissions for the room:
-            $sufficient_permissions =
-                $room->userHasPermission($current_user, 'user')
-                || $room->booking_plan_is_public;
-            if (!$sufficient_permissions) {
+            if (!$room->bookingPlanVisibleForUser($current_user)) {
                 throw new AccessDeniedException();
             }
         }
diff --git a/app/routes/Resources.php b/app/routes/Resources.php
index bf826ba818a..c3d0da06835 100644
--- a/app/routes/Resources.php
+++ b/app/routes/Resources.php
@@ -308,7 +308,7 @@ class Resources extends \RESTAPI\RouteMap
                 throw new \AccessDeniedException();
             }
         } elseif ($resource instanceof \Room) {
-            if (!$resource->booking_plan_is_public) {
+            if (!$resource->bookingPlanVisibleForUser($current_user)) {
                 throw new \AccessDeniedException();
             }
         }
diff --git a/app/views/resources/search/rooms.php b/app/views/resources/search/rooms.php
index a1bb2e7db4d..e9505013bd4 100644
--- a/app/views/resources/search/rooms.php
+++ b/app/views/resources/search/rooms.php
@@ -28,7 +28,7 @@
                         ) ?>
                     </td>
                     <td>
-                        <? if ($room->userHasPermission($current_user, 'autor') || $room->booking_plan_is_public && Config::get()->RESOURCES_SHOW_PUBLIC_ROOM_PLANS): ?>
+                        <? if ($room->bookingPlanVisibleForUser($current_user)): ?>
                             <a href="<?= $room->getActionLink('booking_plan', $booking_plan_action_params) ?>" data-dialog="size=big">
                                 <?= htmlReady($room->name) ?>
                             </a>
diff --git a/lib/models/resources/Resource.class.php b/lib/models/resources/Resource.class.php
index 0cdd517a060..532b571f00a 100644
--- a/lib/models/resources/Resource.class.php
+++ b/lib/models/resources/Resource.class.php
@@ -2403,8 +2403,8 @@ class Resource extends SimpleORMap implements StudipItem
     /**
      * Determines if a user has the specified permission.
      *
-     * @param User $user The user whose permissions shall be checked on this
-     *     resource object.
+     * @param ?User $user The user whose permissions shall be checked on this
+     *     resource object. May be null.
      * @param string $permission The permission level.
      * @param $time_range @TODO
      *
@@ -2412,12 +2412,12 @@ class Resource extends SimpleORMap implements StudipItem
      *     false otherwise.
      */
     public function userHasPermission(
-        User $user,
+        ?User $user,
         string $permission = 'user',
         array $time_range = []
     )
     {
-        if (!in_array($permission, ['user', 'autor', 'tutor', 'admin'])) {
+        if (!in_array($permission, ['user', 'autor', 'tutor', 'admin']) || $user === null) {
             return false;
         }
 
@@ -2559,8 +2559,8 @@ class Resource extends SimpleORMap implements StudipItem
      * Determines if the booking plan of the resource is visible for a
      * specified user.
      *
-     * @param User $user The user whose permission to view the booking plan
-     *     shall be determined.
+     * @param ?User $user The user whose permission to view the booking plan
+     *     shall be determined. May be null.
      *
      * @param DateTime[] $time_range An optional time range for the
      *     permission check.
@@ -2569,10 +2569,9 @@ class Resource extends SimpleORMap implements StudipItem
      * @see Resource::getUserPermission
      *
      */
-    public function bookingPlanVisibleForUser(User $user, $time_range = [])
+    public function bookingPlanVisibleForUser(?User $user, $time_range = [])
     {
-        return $this->userHasPermission($user, 'user', $time_range)
-            || $GLOBALS['perm']->have_perm('root', $user->id);
+        return $this->userHasPermission($user, 'user', $time_range);
     }
 
     /**
diff --git a/lib/models/resources/Room.class.php b/lib/models/resources/Room.class.php
index d8f2ad4685f..1c9a0e401d7 100644
--- a/lib/models/resources/Room.class.php
+++ b/lib/models/resources/Room.class.php
@@ -645,10 +645,10 @@ class Room extends Resource
      * @see Resource::bookingPlanVisibleForUser
      * @inheritDoc
      */
-    public function bookingPlanVisibleForUser(User $user, $time_range = [])
+    public function bookingPlanVisibleForUser(?User $user, $time_range = [])
     {
         return parent::bookingPlanVisibleForUser($user, $time_range)
-            || $this->booking_plan_is_public;
+            || $this->booking_plan_is_public && Config::get()->RESOURCES_SHOW_PUBLIC_ROOM_PLANS;
     }
 
 
-- 
GitLab