From 4ab2aba9254a77ee0ae203bb1516808ef7e0c5f6 Mon Sep 17 00:00:00 2001
From: Elmar Ludwig <elmar.ludwig@uni-osnabrueck.de>
Date: Tue, 9 Aug 2022 13:04:35 +0000
Subject: [PATCH] switch resource locking to check the booking interval, fixes
#1350 and #185
Closes #1350 and #185
Merge request studip/studip!834
---
app/controllers/resources/admin.php | 20 +-----
app/routes/Resources.php | 2 +-
app/views/resources/admin/global_locks.php | 13 +---
.../resources/admin/user_permissions.php | 25 -------
.../resources/GlobalResourceLock.class.php | 12 +---
lib/models/resources/Resource.class.php | 71 +++++++------------
.../resources/ResourceBooking.class.php | 2 +-
lib/raumzeit/SingleDate.class.php | 2 +-
lib/resources/ResourceManager.class.php | 10 ---
9 files changed, 34 insertions(+), 123 deletions(-)
diff --git a/app/controllers/resources/admin.php b/app/controllers/resources/admin.php
index c4968c3c82e..084839f5dc1 100644
--- a/app/controllers/resources/admin.php
+++ b/app/controllers/resources/admin.php
@@ -93,22 +93,7 @@ class Resources_AdminController extends AuthenticatedController
);
$this->sidebar->addWidget($actions);
- $now = time();
-
- $this->current_lock = GlobalResourceLock::findOneBySql(
- 'begin <= :now AND end >= :now',
- [
- 'now' => $now
- ]
- );
-
- $this->future_locks = GlobalResourceLock::findBySql(
- '(begin > :now) OR (end > :now)
- ORDER BY begin ASC, end ASC',
- [
- 'now' => $now
- ]
- );
+ $this->locks = GlobalResourceLock::findBySql('1 ORDER BY begin, end');
}
@@ -148,9 +133,6 @@ class Resources_AdminController extends AuthenticatedController
$this->user->perms
)
);
- if (GlobalResourceLock::currentlyLocked()) {
- $this->current_global_lock = true;
- }
//get the permissions of that user:
diff --git a/app/routes/Resources.php b/app/routes/Resources.php
index c3d0da06835..b2113b1b65b 100644
--- a/app/routes/Resources.php
+++ b/app/routes/Resources.php
@@ -926,7 +926,7 @@ class Resources extends \RESTAPI\RouteMap
$resource = $resource->getDerivedClassInstance();
- if (!$resource->userHasPermission(\User::findCurrent(), 'autor')) {
+ if (!$resource->userHasPermission(\User::findCurrent(), 'autor', [$interval->begin, $interval->end])) {
$this->halt(403, 'You do not have sufficient permissions to modify the interval!');
}
diff --git a/app/views/resources/admin/global_locks.php b/app/views/resources/admin/global_locks.php
index 509309f685b..ef6bccd74ff 100644
--- a/app/views/resources/admin/global_locks.php
+++ b/app/views/resources/admin/global_locks.php
@@ -1,13 +1,4 @@
-<? if ($current_lock): ?>
- <?= MessageBox::info(
- sprintf(
- _('Die Raumverwaltung ist vom %1$s bis zum %2$s gesperrt.'),
- date('d.m.Y H:i', $current_lock->begin),
- date('d.m.Y H:i', $current_lock->end)
- )
- ) ?>
-<? endif ?>
-<? if ($future_locks): ?>
+<? if ($locks): ?>
<table class="default">
<thead>
<tr>
@@ -18,7 +9,7 @@
</tr>
</thead>
<tbody>
- <? foreach ($future_locks as $lock): ?>
+ <? foreach ($locks as $lock): ?>
<tr>
<td><?= date('d.m.Y H:i', $lock->begin) ?></td>
<td><?= date('d.m.Y H:i', $lock->end) ?></td>
diff --git a/app/views/resources/admin/user_permissions.php b/app/views/resources/admin/user_permissions.php
index 33a9980af13..143f2ad5371 100644
--- a/app/views/resources/admin/user_permissions.php
+++ b/app/views/resources/admin/user_permissions.php
@@ -5,14 +5,6 @@
<dd>
<? if ($global_permission): ?>
<?= htmlReady($global_permission->perms) ?>
- <? if ($current_global_lock and ($global_permission->perms != 'admin')): ?>
- <?= Icon::create('exclaim', 'attention')->asImg(
- [
- 'class' => 'text-bottom',
- 'title' => _('Die Berechtigung kann zurzeit aufgrund einer globalen Sperrung der Raumverwaltung nicht genutzt werden!')
- ]
- )?>
- <? endif ?>
<? else: ?>
<?= _('keine') ?>
<? endif ?>
@@ -94,15 +86,6 @@
</td>
<td>
<?= htmlReady($permission->perms) ?>
- <? if ($current_global_lock) : ?>
- <?= Icon::create('exclaim', 'attention')->asImg(
- '20px',
- [
- 'class' => 'text-bottom',
- 'title' => _('Die Berechtigung kann aufgrund einer globalen Sperrung der Raumverwaltung zurzeit nicht genutzt werden!')
- ]
- )?>
- <? endif ?>
</td>
<td>
<?= date('d.m.Y H:i', $permission->begin) ?>
@@ -164,14 +147,6 @@
</td>
<td>
<?= htmlReady($permission->perms) ?>
- <? if ($current_global_lock and ($permission->perms != 'admin')): ?>
- <?= Icon::create('exclaim', 'attention')->asImg(
- [
- 'class' => 'text-bottom',
- 'title' => _('Die Berechtigung kann zurzeit aufgrund einer globalen Sperrung der Raumverwaltung nicht genutzt werden!')
- ]
- )?>
- <? endif ?>
</td>
<td class="actions">
<a href="<?= $permission->resource->getActionLink(
diff --git a/lib/models/resources/GlobalResourceLock.class.php b/lib/models/resources/GlobalResourceLock.class.php
index 4b0e009c30f..541cc0ffe28 100644
--- a/lib/models/resources/GlobalResourceLock.class.php
+++ b/lib/models/resources/GlobalResourceLock.class.php
@@ -53,17 +53,11 @@ class GlobalResourceLock extends SimpleORMap
parent::configure($config);
}
- public static function currentlyLocked()
+ public static function isLocked($begin, $end)
{
- $now = time();
- return self::countBySql(
- 'begin <= :now AND end >= :now',
- [
- 'now' => $now
- ]
- ) > 0;
+ return self::countBySql('begin < :end AND end > :begin', compact('begin', 'end')) > 0;
}
-
+
/**
* Returns a list of defined lock types.
*
diff --git a/lib/models/resources/Resource.class.php b/lib/models/resources/Resource.class.php
index 532b571f00a..89fc3875e51 100644
--- a/lib/models/resources/Resource.class.php
+++ b/lib/models/resources/Resource.class.php
@@ -1256,7 +1256,7 @@ class Resource extends SimpleORMap implements StudipItem
$internal_comment = ''
)
{
- if (!$this->userHasPermission($user, 'admin')) {
+ if (!$this->userHasPermission($user, 'admin', [$begin, $end])) {
throw new AccessDeniedException(
sprintf(
_('%s: Unzureichende Berechtigungen zum Erstellen einer Sperrbuchung!'),
@@ -2304,25 +2304,25 @@ class Resource extends SimpleORMap implements StudipItem
$perm_string = '';
$temp_perm = null;
- if (!$permanent_only) {
- $begin = time();
- $end = $begin;
-
- //If $time range is set and contains two DateTime objects
- //we can include that in the search for temporary permissions.
- if ($time_range) {
- if ($time_range[0] instanceof DateTime) {
- $begin = $time_range[0]->getTimestamp();
- } else {
- $begin = $time_range[0];
- }
- if ($time_range[1] instanceof DateTime) {
- $end = $time_range[1]->getTimestamp();
- } else {
- $end = $time_range[1];
- }
+ $begin = time();
+ $end = $begin;
+
+ //If $time range is set and contains two DateTime objects
+ //we can include that in the search for temporary permissions.
+ if ($time_range) {
+ if ($time_range[0] instanceof DateTime) {
+ $begin = $time_range[0]->getTimestamp();
+ } else {
+ $begin = $time_range[0];
+ }
+ if ($time_range[1] instanceof DateTime) {
+ $end = $time_range[1]->getTimestamp();
+ } else {
+ $end = $time_range[1];
}
+ }
+ if (!$permanent_only) {
$temp_perm = ResourceTemporaryPermission::findOneBySql(
'(resource_id = :resource_id) AND (user_id = :user_id)
AND (begin <= :begin) AND (end >= :end)',
@@ -2379,11 +2379,8 @@ class Resource extends SimpleORMap implements StudipItem
}
//Now we must check for global resource locks:
- if ($this->lockable && GlobalResourceLock::currentlyLocked()) {
- //The resource management system is currently locked.
- //permission level 'user' for all other permission
- //levels.
- if ($perm_string) {
+ if ($perm_string && $time_range && $this->lockable) {
+ if (GlobalResourceLock::isLocked($begin, $end)) {
//A permission level exists for the user.
//The user gets "user" permissions in case
//a global lock is active.
@@ -2439,28 +2436,18 @@ class Resource extends SimpleORMap implements StudipItem
return false;
}
} elseif ($permission === 'autor') {
- if ($this->lockable && GlobalResourceLock::currentlyLocked()) {
- //A global resource lock means no writing actions are permitted.
- return false;
- }
if (in_array($perm_level, ['autor', 'tutor', 'admin'])) {
return true;
} else {
return false;
}
} elseif ($permission === 'tutor') {
- if ($this->lockable && GlobalResourceLock::currentlyLocked()) {
- //A global resource lock means no writing actions are permitted.
- return false;
- }
if (in_array($perm_level, ['tutor', 'admin'])) {
return true;
} else {
return false;
}
} elseif ($permission === 'admin') {
- //No check for global resource locks here:
- //Admins may always do write actions in the resource management.
if ($perm_level == 'admin') {
return true;
} else {
@@ -2537,22 +2524,14 @@ class Resource extends SimpleORMap implements StudipItem
$end = null
)
{
- if (!$begin) {
- $begin = time();
- }
- if (!$end) {
- $end = $begin;
+ if ($begin && $end) {
+ $time_range = [$begin, $end];
+ } else {
+ $time_range = [];
}
//Check the permissions on this resource and the global permissions:
- return $this->userHasPermission(
- $user,
- 'autor',
- [
- $begin,
- $end
- ]
- );
+ return $this->userHasPermission($user, 'autor', $time_range);
}
/**
diff --git a/lib/models/resources/ResourceBooking.class.php b/lib/models/resources/ResourceBooking.class.php
index a16d5e7f04f..a8e65dd07e4 100644
--- a/lib/models/resources/ResourceBooking.class.php
+++ b/lib/models/resources/ResourceBooking.class.php
@@ -492,7 +492,7 @@ class ResourceBooking extends SimpleORMap implements PrivacyObject, Studip\Calen
//(the moment this booking is saved).
$derived_resource = $this->resource->getDerivedClassInstance();
$user_has_booking_rights = $derived_resource->userHasBookingRights(
- $this->booking_user
+ $this->booking_user, $this->begin, $this->end
);
if (!$user_has_booking_rights) {
throw new ResourcePermissionException(
diff --git a/lib/raumzeit/SingleDate.class.php b/lib/raumzeit/SingleDate.class.php
index b6b8f0273e3..3cda04d027a 100644
--- a/lib/raumzeit/SingleDate.class.php
+++ b/lib/raumzeit/SingleDate.class.php
@@ -437,7 +437,7 @@ class SingleDate
}
// check permissions (is current user allowed to book the passed room?)
- if (!$room->userHasBookingRights(User::findCurrent())) {
+ if (!$room->userHasBookingRights(User::findCurrent(), $this->date, $this->end_time)) {
return false;
}
diff --git a/lib/resources/ResourceManager.class.php b/lib/resources/ResourceManager.class.php
index 13588a47814..2c4e6ce5918 100644
--- a/lib/resources/ResourceManager.class.php
+++ b/lib/resources/ResourceManager.class.php
@@ -870,16 +870,6 @@ class ResourceManager
return '';
}
- if (GlobalResourceLock::currentlyLocked()) {
- //A global permission object exist. But since the
- //resource management is locked only 'user' permissions
- //are allowed, when the user does not have 'admin' permissions:
- return (
- $permission->perms == 'admin'
- ? 'admin'
- : 'user'
- );
- }
return $permission->perms;
}
--
GitLab