From 4774b0301b6e22ebcdc2458ededdc25527b20aa7 Mon Sep 17 00:00:00 2001 From: Johan Cwiklinski Date: Sat, 26 Sep 2026 09:45:41 +0200 Subject: [PATCH 1/8] Require authentication to list bookings and to add activities --- _routes.php | 4 +- lib/GaletteEvents/PluginGaletteEvents.php | 26 +-- tests/EventsFixtures.php | 203 ++++++++++++++++++ .../Crud/tests/units/ActivitiesController.php | 89 ++++++++ .../Crud/tests/units/BookingsController.php | 51 +++++ .../tests/units/PluginGaletteEvents.php | 82 +++++++ tests/TestsBootstrap.php | 1 + 7 files changed, 438 insertions(+), 18 deletions(-) create mode 100644 tests/EventsFixtures.php create mode 100644 tests/GaletteEvents/Controllers/Crud/tests/units/ActivitiesController.php create mode 100644 tests/GaletteEvents/Controllers/Crud/tests/units/BookingsController.php create mode 100644 tests/GaletteEvents/tests/units/PluginGaletteEvents.php diff --git a/_routes.php b/_routes.php index 6278dc94..b171427d 100644 --- a/_routes.php +++ b/_routes.php @@ -75,7 +75,7 @@ $app->get( '/bookings/{event:guess|all|\d+}[/{option:page|order|clear_filter}/{value:\d+}]', [BookingsController::class, 'listBookings'] -)->setName('events_bookings'); +)->setName('events_bookings')->add(Authenticate::class); //bookings list filtering $app->post( @@ -158,7 +158,7 @@ $app->post( '/activity/add', [ActivitiesController::class, 'doAdd'] -)->setName('events_storeactivity_add'); +)->setName('events_storeactivity_add')->add(Authenticate::class); $app->post( '/activity/store', diff --git a/lib/GaletteEvents/PluginGaletteEvents.php b/lib/GaletteEvents/PluginGaletteEvents.php index be7c1205..99fd8700 100644 --- a/lib/GaletteEvents/PluginGaletteEvents.php +++ b/lib/GaletteEvents/PluginGaletteEvents.php @@ -68,26 +68,20 @@ public function getMenus(): array 'name' => 'events_calendar', ] ], + [ + 'label' => _T('Bookings', 'events'), + 'route' => [ + 'name' => 'events_bookings', + 'args' => [ + 'event' => 'all' + ], + 'aliases' => ['events_booking_add', 'events_booking_edit'] + ] + ], ] ]; } - $menus['plugin_events']['items'] = array_merge( - $menus['plugin_events']['items'], - [ - [ - 'label' => _T('Bookings', 'events'), - 'route' => [ - 'name' => 'events_bookings', - 'args' => [ - 'event' => 'all' - ], - 'aliases' => ['events_booking_add', 'events_booking_edit'] - ] - ] - ] - ); - if ($login->isAdmin() || $login->isStaff()) { $menus['plugin_events']['items'] = array_merge( $menus['plugin_events']['items'], diff --git a/tests/EventsFixtures.php b/tests/EventsFixtures.php new file mode 100644 index 00000000..b34c286d --- /dev/null +++ b/tests/EventsFixtures.php @@ -0,0 +1,203 @@ + + */ +trait EventsFixtures +{ + /** + * Remove plugin data + */ + protected function cleanEvents(): void + { + foreach (['activitiesbookings', 'activitiesevents', Booking::TABLE, Event::TABLE, Activity::TABLE] as $table) { + $this->zdb->execute($this->zdb->delete(EVENTS_PREFIX . $table)); + } + } + + /** + * Log in given member + * + * @param array $mdata Member data + */ + protected function logMember(array $mdata): void + { + $this->assertTrue($this->login->login($mdata['login_adh'], $mdata['mdp_adh'])); + } + + /** + * Create a group + * + * @param string $name Group name + * @param Adherent[] $managers Group managers + * @param Adherent[] $members Group members + */ + protected function createGroup(string $name, array $managers = [], array $members = []): Group + { + $group = new Group(); + $group->setName($name); + $this->assertTrue($group->store()); + if (count($managers)) { + $this->assertTrue($group->setManagers($managers)); + } + if (count($members)) { + $this->assertTrue($group->setMembers($members)); + } + return $group; + } + + /** + * Insert an event, open and in the future by default + * + * @param string $name Event name + * @param array $data Values to override + * + * @return int Event ID + */ + protected function insertEvent(string $name, array $data = []): int + { + $values = $data + [ + 'name' => $name, + 'town' => 'Lille', + 'begin_date' => date('Y-m-d', strtotime('+10 days')), + 'end_date' => date('Y-m-d', strtotime('+11 days')), + 'creation_date' => date('Y-m-d'), + 'is_open' => true, + 'id_group' => null, + 'comment' => '', + ]; + if (!$this->zdb->isPostgres()) { + $values['is_open'] = (int)$values['is_open']; + } else { + $values['is_open'] = $values['is_open'] ? 'true' : 'false'; + } + $insert = $this->zdb->insert(EVENTS_PREFIX . Event::TABLE); + $insert->values($values); + $this->zdb->execute($insert); + + return $this->getIdByName(Event::TABLE, Event::PK, $name); + } + + /** + * Insert an activity + * + * @param string $name Activity name + * + * @return int Activity ID + */ + protected function insertActivity(string $name): int + { + $insert = $this->zdb->insert(EVENTS_PREFIX . Activity::TABLE); + $insert->values([ + 'name' => $name, + 'creation_date' => date('Y-m-d'), + 'comment' => '', + ]); + $this->zdb->execute($insert); + + return $this->getIdByName(Activity::TABLE, Activity::PK, $name); + } + + /** + * Link an activity to an event + * + * @param int $event Event ID + * @param int $activity Activity ID + * @param int $status One of Activity::YES or Activity::REQUIRED + */ + protected function linkActivity(int $event, int $activity, int $status = Activity::YES): void + { + $insert = $this->zdb->insert(EVENTS_PREFIX . 'activitiesevents'); + $insert->values([ + Event::PK => $event, + Activity::PK => $activity, + 'status' => $status, + ]); + $this->zdb->execute($insert); + } + + /** + * Insert a booking + * + * @param int $event Event ID + * @param int $member Member ID + * @param array $data Values to override + * + * @return int Booking ID + */ + protected function insertBooking(int $event, int $member, array $data = []): int + { + $insert = $this->zdb->insert(EVENTS_PREFIX . Booking::TABLE); + $insert->values($data + [ + Event::PK => $event, + Adherent::PK => $member, + 'booking_date' => date('Y-m-d'), + 'number_people' => 1, + 'creation_date' => date('Y-m-d'), + 'comment' => '', + ]); + $this->zdb->execute($insert); + + $select = $this->zdb->select(EVENTS_PREFIX . Booking::TABLE); + $select->where([Event::PK => $event, Adherent::PK => $member]); + return (int)$this->zdb->execute($select)->current()[Booking::PK]; + } + + /** + * Get a booking stored values + * + * @param int $id Booking ID + * + * @return array + */ + protected function getBookingRow(int $id): array + { + $select = $this->zdb->select(EVENTS_PREFIX . Booking::TABLE); + $select->where([Booking::PK => $id]); + return (array)$this->zdb->execute($select)->current(); + } + + /** + * Count bookings of an event + * + * @param int $event Event ID + */ + protected function countBookings(int $event): int + { + $select = $this->zdb->select(EVENTS_PREFIX . Booking::TABLE); + $select->where([Event::PK => $event]); + return $this->zdb->execute($select)->count(); + } + + /** + * Get an ID from a name + * + * @param string $table Table name, without prefixes + * @param string $pk Primary key + * @param string $name Name + */ + private function getIdByName(string $table, string $pk, string $name): int + { + $select = $this->zdb->select(EVENTS_PREFIX . $table); + $select->where(['name' => $name]); + return (int)$this->zdb->execute($select)->current()[$pk]; + } +} diff --git a/tests/GaletteEvents/Controllers/Crud/tests/units/ActivitiesController.php b/tests/GaletteEvents/Controllers/Crud/tests/units/ActivitiesController.php new file mode 100644 index 00000000..2aa8c058 --- /dev/null +++ b/tests/GaletteEvents/Controllers/Crud/tests/units/ActivitiesController.php @@ -0,0 +1,89 @@ + + */ +class ActivitiesController extends GaletteRoutingTestCase +{ + use EventsFixtures; + + protected int $seed = 20260926101512; + protected bool $load_plugins = true; + + /** + * Cleanup after each test method + */ + public function tearDown(): void + { + $this->login->logout(); + $this->cleanEvents(); + parent::tearDown(); + } + + /** + * Count activities with given name + * + * @param string $name Activity name + */ + private function countActivities(string $name): int + { + $select = $this->zdb->select(EVENTS_PREFIX . Activity::TABLE); + $select->where(['name' => $name]); + return $this->zdb->execute($select)->count(); + } + + /** + * Post an activity + * + * @param array $data Posted data + */ + private function postActivity(array $data): \Psr\Http\Message\ResponseInterface + { + $request = $this->createRequest('events_storeactivity_add', [], 'POST') + ->withParsedBody($data); + return $this->app->handle($request); + } + + /** + * Visitors can neither create nor change activities + */ + public function testVisitorCannotStoreActivity(): void + { + $id = $this->insertActivity('Dinner'); + + $this->expectLogin($this->postActivity(['name' => 'Created by a visitor', 'active' => '1', 'comment' => ''])); + $this->expectLogin($this->postActivity(['id' => (string)$id, 'name' => 'Renamed by a visitor', 'comment' => ''])); + + $this->assertSame(0, $this->countActivities('Created by a visitor')); + $this->assertSame(0, $this->countActivities('Renamed by a visitor')); + $this->assertSame(1, $this->countActivities('Dinner')); + } + + /** + * Members cannot create activities + */ + public function testMemberCannotStoreActivity(): void + { + $this->getMemberOne(); + $this->logMember($this->dataAdherentOne()); + + $this->expectAuthMiddlewareRefused($this->postActivity(['name' => 'Created by a member', 'active' => '1', 'comment' => ''])); + $this->assertSame(0, $this->countActivities('Created by a member')); + } +} diff --git a/tests/GaletteEvents/Controllers/Crud/tests/units/BookingsController.php b/tests/GaletteEvents/Controllers/Crud/tests/units/BookingsController.php new file mode 100644 index 00000000..623ae53c --- /dev/null +++ b/tests/GaletteEvents/Controllers/Crud/tests/units/BookingsController.php @@ -0,0 +1,51 @@ + + */ +class BookingsController extends GaletteRoutingTestCase +{ + use EventsFixtures; + + protected int $seed = 20260926101512; + protected bool $load_plugins = true; + + /** + * Cleanup after each test method + */ + public function tearDown(): void + { + $this->login->logout(); + $this->cleanEvents(); + parent::tearDown(); + } + + /** + * Visitors cannot list bookings + */ + public function testVisitorCannotListBookings(): void + { + $member_one = $this->getMemberOne(); + $this->insertBooking($this->insertEvent('Public event'), $member_one->id); + + foreach (['all', 'guess'] as $event) { + $request = $this->createRequest('events_bookings', ['event' => $event]); + $this->expectLogin($this->app->handle($request)); + } + } +} diff --git a/tests/GaletteEvents/tests/units/PluginGaletteEvents.php b/tests/GaletteEvents/tests/units/PluginGaletteEvents.php new file mode 100644 index 00000000..346419ed --- /dev/null +++ b/tests/GaletteEvents/tests/units/PluginGaletteEvents.php @@ -0,0 +1,82 @@ + + */ +class PluginGaletteEvents extends GaletteTestCase +{ + protected int $seed = 20260926101512; + + /** + * Cleanup after each test method + */ + public function tearDown(): void + { + $this->login->logout(); + parent::tearDown(); + } + + /** + * Get plugin instance + */ + private function getPlugin(): \GaletteEvents\PluginGaletteEvents + { + return $this->container->get(\GaletteEvents\PluginGaletteEvents::class); + } + + /** + * Get routes names of menus entries + * + * @param array $menus Menus + * + * @return array> + */ + private function getMenusRoutes(array $menus): array + { + $routes = []; + foreach ($menus as $section => $menu) { + $routes[$section] = array_map( + fn(array $item): string => $item['route']['name'], + $menu['items'] + ); + } + return $routes; + } + + /** + * Test menus + */ + public function testMenus(): void + { + $plugin = $this->getPlugin(); + $this->assertSame([], $plugin->getMenus()); + + $this->getMemberOne(); + $this->assertTrue($this->login->login($this->dataAdherentOne()['login_adh'], $this->dataAdherentOne()['mdp_adh'])); + $this->assertSame( + ['plugin_events' => ['events_events', 'events_calendar', 'events_bookings']], + $this->getMenusRoutes($plugin->getMenus()) + ); + $this->login->logout(); + + $this->logSuperAdmin(); + $this->assertSame( + ['plugin_events' => ['events_events', 'events_calendar', 'events_bookings', 'events_activities']], + $this->getMenusRoutes($plugin->getMenus()) + ); + } +} diff --git a/tests/TestsBootstrap.php b/tests/TestsBootstrap.php index d4461b59..692c490b 100644 --- a/tests/TestsBootstrap.php +++ b/tests/TestsBootstrap.php @@ -17,3 +17,4 @@ include_once '../../../tests/TestsBootstrap.php'; require_once __DIR__ . '/../_config.inc.php'; +require_once __DIR__ . '/EventsFixtures.php'; From 9cb9d55c7498ba69360aa727846b1bd41713f6bb Mon Sep 17 00:00:00 2001 From: Johan Cwiklinski Date: Sat, 26 Sep 2026 09:46:36 +0200 Subject: [PATCH 2/8] Escape user values displayed in the calendar --- calendar.js | 5 +- lib/GaletteEvents/Repository/Events.php | 16 +++- .../Crud/tests/units/EventsController.php | 79 +++++++++++++++++++ 3 files changed, 96 insertions(+), 4 deletions(-) create mode 100644 tests/GaletteEvents/Controllers/Crud/tests/units/EventsController.php diff --git a/calendar.js b/calendar.js index e510e815..744a59e8 100644 --- a/calendar.js +++ b/calendar.js @@ -43,7 +43,10 @@ $(function() { } else { _modal_actions[1].click = _booking_action; } - var _elt = $('
' + _infos.name + ' (' + _infos.begin_date_fmt + ' - ' + _infos.end_date_fmt + ')
' + _infos.description + '
'); + //description is built and escaped server side, other values must be displayed as text + var _elt = $('
'); + _elt.find('.header').text(_infos.name + ' (' + _infos.begin_date_fmt + ' - ' + _infos.end_date_fmt + ')'); + _elt.find('.content').html(_infos.description); _elt.appendTo('body'); _elt.modal({ onApprove: function() { diff --git a/lib/GaletteEvents/Repository/Events.php b/lib/GaletteEvents/Repository/Events.php index 2fbfa028..132678a9 100644 --- a/lib/GaletteEvents/Repository/Events.php +++ b/lib/GaletteEvents/Repository/Events.php @@ -199,9 +199,9 @@ public function getList(bool $onlyevents = false, bool $fullcalendar = false): a $pattern = '
  • %1$s %2$s
    • '; $description .= sprintf($pattern, _T("Start date:", "events"), $event->getBeginDate()); $description .= sprintf($pattern, _T("End date:", "events"), $event->getEndDate()); - $description .= sprintf($pattern, _T("Location:", "events"), $event->getTown()); + $description .= sprintf($pattern, _T("Location:", "events"), $this->escape($event->getTown())); if ($comment = $event->getComment()) { - $description .= sprintf($pattern, _T("Comment:", "events"), $comment); + $description .= sprintf($pattern, _T("Comment:", "events"), $this->escape($comment)); } /** @var ResultSet $attendees */ @@ -234,7 +234,7 @@ public function getList(bool $onlyevents = false, bool $fullcalendar = false): a $description .= '

    ' . _T('Activities', 'events') . '

    '; $description .= '
      '; foreach ($activities as $activity) { - $description .= '
    • ' . $activity['activity']->getName() . '
      • '; + $description .= '
    • ' . $this->escape($activity['activity']->getName()) . '
      • '; } $description .= '
    '; } @@ -255,6 +255,16 @@ public function getList(bool $onlyevents = false, bool $fullcalendar = false): a } } + /** + * Escape a value typed by users for the calendar HTML description + * + * @param string $value Value to escape + */ + private function escape(string $value): string + { + return htmlspecialchars($value, ENT_QUOTES | ENT_SUBSTITUTE, 'UTF-8'); + } + /** * Is field allowed to order? it should be present in * provided fields list (those that are SELECT'ed). diff --git a/tests/GaletteEvents/Controllers/Crud/tests/units/EventsController.php b/tests/GaletteEvents/Controllers/Crud/tests/units/EventsController.php new file mode 100644 index 00000000..9266ae23 --- /dev/null +++ b/tests/GaletteEvents/Controllers/Crud/tests/units/EventsController.php @@ -0,0 +1,79 @@ + + */ +class EventsController extends GaletteRoutingTestCase +{ + use EventsFixtures; + + protected int $seed = 20260926101512; + protected bool $load_plugins = true; + + /** + * Cleanup after each test method + */ + public function tearDown(): void + { + $this->login->logout(); + $this->cleanEvents(); + parent::tearDown(); + } + + /** + * Calendar event description is HTML: values typed by users must be escaped + */ + public function testCalendarDescriptionIsEscaped(): void + { + $this->getMemberOne(); + $event = $this->insertEvent( + 'Party name', + [ + 'town' => 'Lille', + 'comment' => '', + ] + ); + $this->linkActivity($event, $this->insertActivity('')); + $this->logMember($this->dataAdherentOne()); + + $request = $this->createRequest( + 'ajax-events_calendar', + query_params: [ + 'start' => date('Y-m-d'), + 'end' => date('Y-m-d', strtotime('+1 month')), + ] + ); + $test_response = $this->app->handle($request); + $this->assertSame(200, $test_response->getStatusCode()); + + $events = json_decode((string)$test_response->getBody(), true); + $this->assertIsArray($events); + $this->assertCount(1, $events); + $description = $events[0]['description']; + + $this->assertStringNotContainsString('assertStringNotContainsString('assertStringNotContainsString('', $description); + $this->assertStringContainsString('<img src=x onerror=alert(1)>', $description); + $this->assertStringContainsString('<script>alert(2)</script>', $description); + $this->assertStringContainsString('<b>Lille</b>', $description); + //raw values stay raw in JSON, the script displays them as text + $this->assertSame('Party name', $events[0]['name']); + $this->assertSame('Party name', $events[0]['title']); + } +} From 8d0436e7bb7ae610d179161b770f49b903b63798 Mon Sep 17 00:00:00 2001 From: Johan Cwiklinski Date: Sat, 26 Sep 2026 09:48:32 +0200 Subject: [PATCH 3/8] Check who can edit a booking --- lib/GaletteEvents/Booking.php | 22 +++ .../Controllers/Crud/BookingsController.php | 29 ++++ .../Crud/tests/units/BookingsController.php | 162 ++++++++++++++++++ 3 files changed, 213 insertions(+) diff --git a/lib/GaletteEvents/Booking.php b/lib/GaletteEvents/Booking.php index 482ac866..c18cab00 100644 --- a/lib/GaletteEvents/Booking.php +++ b/lib/GaletteEvents/Booking.php @@ -735,6 +735,28 @@ public function getActivities(): array return $this->activities; } + /** + * Can current logged-in user edit booking + * + * Admins and staff members can edit any booking, members their own ones, + * and group managers the ones on events of the groups they manage. + * + * @param Login $login Login instance + */ + public function canEdit(Login $login): bool + { + if ($login->isAdmin() || $login->isStaff()) { + return true; + } + + if ($this->getMemberId() !== null && $this->getMemberId() === $login->id) { + return true; + } + + $group = $this->getEvent()?->getGroup(); + return $group !== null && $login->isGroupManager($group); + } + /** * Get row class related to current fee status * diff --git a/lib/GaletteEvents/Controllers/Crud/BookingsController.php b/lib/GaletteEvents/Controllers/Crud/BookingsController.php index cf1de0f9..29008cf2 100644 --- a/lib/GaletteEvents/Controllers/Crud/BookingsController.php +++ b/lib/GaletteEvents/Controllers/Crud/BookingsController.php @@ -10,6 +10,7 @@ namespace GaletteEvents\Controllers\Crud; +use Analog\Analog; use Galette\Entity\Adherent; use Galette\Repository\Groups; use Galette\Repository\Members; @@ -328,6 +329,10 @@ public function edit(Request $request, Response $response, ?int $id = null, stri $booking->load($id); } + if ($booking->getId() !== null && !$booking->canEdit($this->login)) { + return $this->redirectForbidden($response, $booking); + } + // template variable declaration $title = _T("Booking", "events"); if ($booking->getId() != '') { @@ -422,6 +427,10 @@ public function doEdit(Request $request, Response $response, ?int $id = null, st $booking->load((int)$post['id']); } + if ($booking->getId() !== null && !$booking->canEdit($this->login)) { + return $this->redirectForbidden($response, $booking); + } + if (isset($post['cancel'])) { $redirect_url = $this->routeparser->urlFor( 'events_bookings', @@ -527,6 +536,26 @@ public function doEdit(Request $request, Response $response, ?int $id = null, st ->withHeader('Location', $redirect_url); } + /** + * Redirect when current logged-in user cannot edit a booking + * + * @param Booking $booking Booking + */ + private function redirectForbidden(Response $response, Booking $booking): Response + { + Analog::log( + 'Logged in member ' . $this->login->login + . ' has tried to edit booking #' . $booking->getId() + . ' without the right to do so.', + Analog::WARNING + ); + return $this->redirectWithErrors( + response: $response, + errors: [_T("You do not have permission for requested URL.")], + redirect_url: $this->routeparser->urlFor('events_bookings', ['event' => 'all']) + ); + } + // /CRUD - Update // CRUD - Delete diff --git a/tests/GaletteEvents/Controllers/Crud/tests/units/BookingsController.php b/tests/GaletteEvents/Controllers/Crud/tests/units/BookingsController.php index 623ae53c..25a1a924 100644 --- a/tests/GaletteEvents/Controllers/Crud/tests/units/BookingsController.php +++ b/tests/GaletteEvents/Controllers/Crud/tests/units/BookingsController.php @@ -10,6 +10,7 @@ namespace GaletteEvents\Controllers\Crud\tests\units; +use Analog\Analog; use Galette\Tests\GaletteRoutingTestCase; use GaletteEvents\tests\EventsFixtures; @@ -35,6 +36,69 @@ public function tearDown(): void parent::tearDown(); } + /** + * Get booking form + * + * @param int $id Booking ID + */ + private function getBookingForm(int $id): \Psr\Http\Message\ResponseInterface + { + return $this->app->handle($this->createRequest('events_booking_edit', ['id' => (string)$id])); + } + + /** + * Post a booking + * + * @param ?int $id Booking ID, null to add a new one + * @param array $data Posted data + */ + private function postBooking(?int $id, array $data): \Psr\Http\Message\ResponseInterface + { + if ($id === null) { + $request = $this->createRequest('events_storebooking_add', [], 'POST'); + } else { + $request = $this->createRequest('events_storebooking_edit', ['id' => (string)$id], 'POST'); + $data += ['id' => (string)$id]; + } + return $this->app->handle($request->withParsedBody($data + [ + 'booking_date' => date('Y-m-d'), + 'number_people' => '1', + 'comment' => '', + 'save' => '1', + ])); + } + + /** + * Assert access to a booking has been refused + * + * @param \Psr\Http\Message\ResponseInterface $test_response Response + * @param int $id Booking ID + */ + private function expectBookingRefused(\Psr\Http\Message\ResponseInterface $test_response, int $id): void + { + $this->assertSame( + ['Location' => [$this->routeparser->urlFor('events_bookings', ['event' => 'all'])]], + $test_response->getHeaders() + ); + $this->assertSame(301, $test_response->getStatusCode()); + //message comes in the language of the logged-in member + $this->expectFlashData(['error_detected' => [_T('You do not have permission for requested URL.')]]); + $this->expectLogEntry(Analog::WARNING, 'has tried to edit booking #' . $id); + $this->expectNoLogEntry(); + } + + /** + * Assert booking form is displayed + * + * @param \Psr\Http\Message\ResponseInterface $test_response Response + */ + private function expectBookingForm(\Psr\Http\Message\ResponseInterface $test_response): void + { + $this->assertSame(200, $test_response->getStatusCode()); + $this->assertStringContainsString('name="save"', (string)$test_response->getBody()); + $this->expectNoLogEntry(); + } + /** * Visitors cannot list bookings */ @@ -48,4 +112,102 @@ public function testVisitorCannotListBookings(): void $this->expectLogin($this->app->handle($request)); } } + + /** + * Members can neither display nor change bookings of other members + */ + public function testMemberCannotEditOtherMemberBooking(): void + { + //member two speaks Catalan, member one gets messages in English + $this->getMemberOne(); + $member_two = $this->getMemberTwo(); + $event = $this->insertEvent('Public event'); + $booking = $this->insertBooking($event, $member_two->id, ['comment' => 'Vegetarian']); + + $this->logMember($this->dataAdherentOne()); + $this->expectBookingRefused($this->getBookingForm($booking), $booking); + $this->expectBookingRefused( + $this->postBooking( + $booking, + [ + 'event' => (string)$event, + 'member' => (string)$member_two->id, + 'number_people' => '5', + 'comment' => 'Changed', + ] + ), + $booking + ); + + $row = $this->getBookingRow($booking); + $this->assertSame('Vegetarian', $row['comment']); + $this->assertSame(1, (int)$row['number_people']); + } + + /** + * Members display and change their own bookings + */ + public function testMemberEditsOwnBooking(): void + { + $member_one = $this->getMemberOne(); + $event = $this->insertEvent('Public event'); + $booking = $this->insertBooking($event, $member_one->id); + + $this->logMember($this->dataAdherentOne()); + $this->expectBookingForm($this->getBookingForm($booking)); + + $test_response = $this->postBooking($booking, ['event' => (string)$event, 'comment' => 'Changed']); + $this->assertSame( + ['Location' => [$this->routeparser->urlFor('events_bookings', ['event' => (string)$event])]], + $test_response->getHeaders() + ); + $this->expectFlashData(['success_detected' => ['Booking has been modified.']]); + $this->assertSame('Changed', $this->getBookingRow($booking)['comment']); + } + + /** + * Group managers display bookings on events of the groups they manage only + */ + public function testManagerEditsBookingsOfManagedGroupsOnly(): void + { + $member_one = $this->getMemberOne(); + $member_two = $this->getMemberTwo(); + $managed = $this->createGroup('Managed group', [$member_two], [$member_one]); + //member two belongs to this one, but does not manage it + $other = $this->createGroup('Other group', [], [$member_one, $member_two]); + + $managed_booking = $this->insertBooking( + $this->insertEvent('Managed event', ['id_group' => $managed->getId()]), + $member_one->id + ); + $other_booking = $this->insertBooking( + $this->insertEvent('Other event', ['id_group' => $other->getId()]), + $member_one->id + ); + $public_booking = $this->insertBooking($this->insertEvent('Public event'), $member_one->id); + + $this->logMember($this->dataAdherentTwo()); + $this->expectBookingForm($this->getBookingForm($managed_booking)); + $this->expectBookingRefused($this->getBookingForm($other_booking), $other_booking); + $this->expectBookingRefused($this->getBookingForm($public_booking), $public_booking); + } + + /** + * Staff members display any booking + */ + public function testStaffEditsAnyBooking(): void + { + $staff = $this->getStaffMember($this->getMemberOne()); + $member_two = $this->getMemberTwo(); + $group = $this->createGroup('Group', [], [$member_two]); + $booking = $this->insertBooking( + $this->insertEvent('Group event', ['id_group' => $group->getId()]), + $member_two->id + ); + + $this->logMember($this->dataAdherentOne()); + $this->assertTrue($this->login->isStaff()); + $this->expectBookingForm($this->getBookingForm($booking)); + $this->resetStaffStatus($staff, $this->getMemberTwo()); + } } From 00b6e88a97b70fd46d16c97ba2ada9801ed88fdd Mon Sep 17 00:00:00 2001 From: Johan Cwiklinski Date: Sat, 26 Sep 2026 09:50:11 +0200 Subject: [PATCH 4/8] Do not trust the posted member of a booking --- lib/GaletteEvents/Booking.php | 28 +++-- .../Crud/tests/units/BookingsController.php | 103 ++++++++++++++++++ 2 files changed, 122 insertions(+), 9 deletions(-) diff --git a/lib/GaletteEvents/Booking.php b/lib/GaletteEvents/Booking.php index c18cab00..f8cba098 100644 --- a/lib/GaletteEvents/Booking.php +++ b/lib/GaletteEvents/Booking.php @@ -236,18 +236,28 @@ public function check(array $values): array|bool } //booking information - if (!isset($values['member']) || empty($values['member'])) { + if (!$this->login->isAdmin() && !$this->login->isStaff() && !$this->login->isGroupManager()) { + //members book for themselves only + $this->member = $this->login->id; + } elseif (!isset($values['member']) || empty($values['member'])) { + $this->errors[] = _T('Member is mandatory', 'events'); + } else { + $member = (int)$values['member']; if ( - $this->login->isAdmin() - || $this->login->isStaff() - || $this->login->isGroupManager() + !$this->login->isAdmin() + && !$this->login->isStaff() + && $member !== $this->login->id + && $member !== $this->getMemberId() ) { - $this->errors[] = _T('Member is mandatory', 'events'); - } else { - $this->member = $this->login->id; + //group managers book for members of the groups they manage, on events of those groups + $group = $this->getEvent()?->getGroup(); + if (!(new Adherent($this->zdb, $member))->canShow($this->login)) { + $this->errors[] = _T("- Please select a member from a group you manage."); + } elseif ($group === null || !$this->login->isGroupManager($group)) { + $this->errors[] = _T('You can only book other members on events of groups you manage.', 'events'); + } } - } else { - $this->member = (int)$values['member']; + $this->member = $member; } if (isset($values['number_people'])) { diff --git a/tests/GaletteEvents/Controllers/Crud/tests/units/BookingsController.php b/tests/GaletteEvents/Controllers/Crud/tests/units/BookingsController.php index 25a1a924..ce85c7ce 100644 --- a/tests/GaletteEvents/Controllers/Crud/tests/units/BookingsController.php +++ b/tests/GaletteEvents/Controllers/Crud/tests/units/BookingsController.php @@ -99,6 +99,36 @@ private function expectBookingForm(\Psr\Http\Message\ResponseInterface $test_res $this->expectNoLogEntry(); } + /** + * Get member of the only booking of an event + * + * @param int $event Event ID + */ + private function getBookedMember(int $event): int + { + $this->assertSame(1, $this->countBookings($event)); + $select = $this->zdb->select(EVENTS_PREFIX . \GaletteEvents\Booking::TABLE); + $select->where([\GaletteEvents\Event::PK => $event]); + return (int)$this->zdb->execute($select)->current()['id_adh']; + } + + /** + * Assert new booking has been refused by validation + * + * @param \Psr\Http\Message\ResponseInterface $test_response Response + * @param string $error Expected error message + */ + private function expectBookingInvalid(\Psr\Http\Message\ResponseInterface $test_response, string $error): void + { + $this->assertSame( + ['Location' => [$this->routeparser->urlFor('events_booking_add')]], + $test_response->getHeaders() + ); + $this->expectFlashData(['error_detected' => [$error]]); + $this->expectLogEntry(Analog::ERROR, 'Some errors has been threw attempting to edit/store a booking'); + $this->expectNoLogEntry(); + } + /** * Visitors cannot list bookings */ @@ -210,4 +240,77 @@ public function testStaffEditsAnyBooking(): void $this->expectBookingForm($this->getBookingForm($booking)); $this->resetStaffStatus($staff, $this->getMemberTwo()); } + + /** + * Members book for themselves, whatever member is posted + */ + public function testMemberBooksForThemselvesOnly(): void + { + $member_one = $this->getMemberOne(); + $member_two = $this->getMemberTwo(); + $event = $this->insertEvent('Public event'); + + $this->logMember($this->dataAdherentOne()); + $test_response = $this->postBooking(null, ['event' => (string)$event, 'member' => (string)$member_two->id]); + $this->assertSame( + ['Location' => [$this->routeparser->urlFor('events_bookings', ['event' => (string)$event])]], + $test_response->getHeaders() + ); + $this->expectFlashData(['success_detected' => ['New booking has been successfully added.']]); + $this->assertSame($member_one->id, $this->getBookedMember($event)); + } + + /** + * Group managers book members of the groups they manage, on events of those groups + */ + public function testManagerBooksMembersOfManagedGroups(): void + { + $member_one = $this->getMemberOne(); + $member_two = $this->getMemberTwo(); + $managed = $this->createGroup('Managed group', [$member_two], [$member_one]); + $managed_event = $this->insertEvent('Managed event', ['id_group' => $managed->getId()]); + $public_event = $this->insertEvent('Public event'); + + $this->logMember($this->dataAdherentTwo()); + + //a public event is not an event of a managed group + $test_response = $this->postBooking( + null, + ['event' => (string)$public_event, 'member' => (string)$member_one->id] + ); + $this->expectBookingInvalid( + $test_response, + _T('You can only book other members on events of groups you manage.', 'events') + ); + $this->assertSame(0, $this->countBookings($public_event)); + + //but group managers can still book for themselves + $this->postBooking(null, ['event' => (string)$public_event, 'member' => (string)$member_two->id]); + $this->flash_data = []; + $this->assertSame($member_two->id, $this->getBookedMember($public_event)); + + $this->postBooking(null, ['event' => (string)$managed_event, 'member' => (string)$member_one->id]); + $this->expectFlashData(['success_detected' => [_T('New booking has been successfully added.', 'events')]]); + $this->assertSame($member_one->id, $this->getBookedMember($managed_event)); + } + + /** + * Group managers cannot book members out of the groups they manage + */ + public function testManagerCannotBookOtherMembers(): void + { + $member_one = $this->getMemberOne(); + $member_two = $this->getMemberTwo(); + $managed = $this->createGroup('Managed group', [$member_two]); + $this->createGroup('Other group', [], [$member_one, $member_two]); + $managed_event = $this->insertEvent('Managed event', ['id_group' => $managed->getId()]); + + $this->logMember($this->dataAdherentTwo()); + $test_response = $this->postBooking( + null, + ['event' => (string)$managed_event, 'member' => (string)$member_one->id] + ); + $this->expectBookingInvalid($test_response, _T('- Please select a member from a group you manage.')); + $this->assertSame(0, $this->countBookings($managed_event)); + } } From 6743555a2b7bd343048edc3db799842d07f278e1 Mon Sep 17 00:00:00 2001 From: Johan Cwiklinski Date: Sat, 26 Sep 2026 09:51:41 +0200 Subject: [PATCH 5/8] Check the booked event is open and visible --- lib/GaletteEvents/Booking.php | 35 +++++++++- .../Crud/tests/units/BookingsController.php | 67 +++++++++++++++++++ 2 files changed, 100 insertions(+), 2 deletions(-) diff --git a/lib/GaletteEvents/Booking.php b/lib/GaletteEvents/Booking.php index f8cba098..d557e540 100644 --- a/lib/GaletteEvents/Booking.php +++ b/lib/GaletteEvents/Booking.php @@ -15,6 +15,7 @@ use Galette\Core\Login; use Galette\Entity\Adherent; use Galette\Entity\PaymentType; +use Galette\Repository\Groups; use Analog\Analog; /** @@ -174,8 +175,12 @@ public function check(array $values): array|bool if (!isset($values['event']) || empty($values['event']) || $values['event'] == -1) { $this->errors[] = _T('Event is mandatory', 'events'); } else { + $event_changed = $this->getId() === null || $this->getEventId() !== (int)$values['event']; $this->event = (int)$values['event']; $event = $this->getEvent(); + if ($event_changed && !$this->canBook($event)) { + $this->errors[] = _T('This event cannot be booked.', 'events'); + } $activities = $event->getActivities(); foreach ($activities as $aid => $entry) { if ( @@ -250,7 +255,7 @@ public function check(array $values): array|bool && $member !== $this->getMemberId() ) { //group managers book for members of the groups they manage, on events of those groups - $group = $this->getEvent()?->getGroup(); + $group = $this->getEvent()?->getGroup() ?: null; if (!(new Adherent($this->zdb, $member))->canShow($this->login)) { $this->errors[] = _T("- Please select a member from a group you manage."); } elseif ($group === null || !$this->login->isGroupManager($group)) { @@ -745,6 +750,31 @@ public function getActivities(): array return $this->activities; } + /** + * Can current logged-in user book an event + * + * Admins and staff members can book any event, others open events + * that are public or restricted to one of their groups. + * + * @param Event $event Event + */ + private function canBook(Event $event): bool + { + if ($this->login->isAdmin() || $this->login->isStaff()) { + return $event->getId() !== null; + } + + if ($event->getId() === null || !$event->isOpen()) { + return false; + } + + //public events have no group, loaded as 0 + $group = $event->getGroup() ?: null; + return $group === null + || $this->login->isGroupManager($group) + || in_array($group, array_map('intval', Groups::loadGroups($this->login->id, false, false)), true); + } + /** * Can current logged-in user edit booking * @@ -763,7 +793,8 @@ public function canEdit(Login $login): bool return true; } - $group = $this->getEvent()?->getGroup(); + //public events have no group, loaded as 0 + $group = $this->getEvent()?->getGroup() ?: null; return $group !== null && $login->isGroupManager($group); } diff --git a/tests/GaletteEvents/Controllers/Crud/tests/units/BookingsController.php b/tests/GaletteEvents/Controllers/Crud/tests/units/BookingsController.php index ce85c7ce..7bf42f9c 100644 --- a/tests/GaletteEvents/Controllers/Crud/tests/units/BookingsController.php +++ b/tests/GaletteEvents/Controllers/Crud/tests/units/BookingsController.php @@ -313,4 +313,71 @@ public function testManagerCannotBookOtherMembers(): void $this->expectBookingInvalid($test_response, _T('- Please select a member from a group you manage.')); $this->assertSame(0, $this->countBookings($managed_event)); } + + /** + * Members book open events that are public or restricted to their groups + */ + public function testMemberBooksVisibleOpenEventsOnly(): void + { + $member_one = $this->getMemberOne(); + $member_two = $this->getMemberTwo(); + $own_group = $this->createGroup('Own group', [], [$member_one]); + $other_group = $this->createGroup('Other group', [], [$member_two]); + + $refused = [ + 'closed' => $this->insertEvent('Closed event', ['is_open' => false]), + 'past' => $this->insertEvent( + 'Past event', + ['begin_date' => date('Y-m-d', strtotime('-2 days')), 'end_date' => date('Y-m-d', strtotime('-1 day'))] + ), + 'other' => $this->insertEvent('Other group event', ['id_group' => $other_group->getId()]), + ]; + $own_event = $this->insertEvent('Own group event', ['id_group' => $own_group->getId()]); + + $this->logMember($this->dataAdherentOne()); + foreach ($refused + ['unknown' => $own_event + 1000] as $event) { + $this->expectBookingInvalid( + $this->postBooking(null, ['event' => (string)$event]), + 'This event cannot be booked.' + ); + } + foreach ($refused as $event) { + $this->assertSame(0, $this->countBookings($event)); + } + + $this->postBooking(null, ['event' => (string)$own_event]); + $this->expectFlashData(['success_detected' => ['New booking has been successfully added.']]); + $this->assertSame($member_one->id, $this->getBookedMember($own_event)); + } + + /** + * Members still change their bookings once the event has been closed + */ + public function testMemberEditsBookingOfClosedEvent(): void + { + $member_one = $this->getMemberOne(); + $event = $this->insertEvent('Closed event', ['is_open' => false]); + $booking = $this->insertBooking($event, $member_one->id); + + $this->logMember($this->dataAdherentOne()); + $this->postBooking($booking, ['event' => (string)$event, 'comment' => 'Changed']); + $this->expectFlashData(['success_detected' => ['Booking has been modified.']]); + $this->assertSame('Changed', $this->getBookingRow($booking)['comment']); + } + + /** + * Staff members book closed events + */ + public function testStaffBooksClosedEvent(): void + { + $staff = $this->getStaffMember($this->getMemberOne()); + $member_two = $this->getMemberTwo(); + $event = $this->insertEvent('Closed event', ['is_open' => false]); + + $this->logMember($this->dataAdherentOne()); + $this->postBooking(null, ['event' => (string)$event, 'member' => (string)$member_two->id]); + $this->expectFlashData(['success_detected' => ['New booking has been successfully added.']]); + $this->assertSame($member_two->id, $this->getBookedMember($event)); + $this->resetStaffStatus($staff, $member_two); + } } From a099d5a14a2d9b4a28ab03851c1ce9bc419958f5 Mon Sep 17 00:00:00 2001 From: Johan Cwiklinski Date: Sat, 26 Sep 2026 09:53:44 +0200 Subject: [PATCH 6/8] Restrict group managers to bookings on events of the groups they manage --- .../Controllers/Crud/BookingsController.php | 10 ++ lib/GaletteEvents/Repository/Bookings.php | 50 ++-------- tests/EventsFixtures.php | 2 + .../Crud/tests/units/BookingsController.php | 40 ++++++++ .../Controllers/tests/units/CsvController.php | 76 +++++++++++++++ .../Repository/tests/units/Bookings.php | 92 +++++++++++++++++++ 6 files changed, 230 insertions(+), 40 deletions(-) create mode 100644 tests/GaletteEvents/Controllers/tests/units/CsvController.php create mode 100644 tests/GaletteEvents/Repository/tests/units/Bookings.php diff --git a/lib/GaletteEvents/Controllers/Crud/BookingsController.php b/lib/GaletteEvents/Controllers/Crud/BookingsController.php index 29008cf2..93811994 100644 --- a/lib/GaletteEvents/Controllers/Crud/BookingsController.php +++ b/lib/GaletteEvents/Controllers/Crud/BookingsController.php @@ -231,11 +231,21 @@ public function handleBatch(Request $request, Response $response): Response //$this->session->filter_bookings = $filters; $filters->selected = $post['entries_sel']; + //selection is restricted to bookings current logged-in user can list $bookings = new Bookings($this->zdb, $this->login, $filters); $members = []; foreach ($bookings->getList() as $booking) { $members[] = $booking->getMemberId(); } + if (count($members) === 0) { + $this->flash->addMessage( + 'error_detected', + _T("No booking was selected, please check at least one.", "events") + ); + return $response + ->withStatus(301) + ->withHeader('Location', $this->routeparser->urlFor('events_events')); + } $mfilter = new MembersList(); $mfilter->selected = $members; diff --git a/lib/GaletteEvents/Repository/Bookings.php b/lib/GaletteEvents/Repository/Bookings.php index e993e73b..753fc435 100644 --- a/lib/GaletteEvents/Repository/Bookings.php +++ b/lib/GaletteEvents/Repository/Bookings.php @@ -18,7 +18,6 @@ use Galette\Core\Db; use Galette\Entity\Adherent; use Galette\Entity\Group; -use Galette\Repository\Groups; use GaletteEvents\Event; use GaletteEvents\Booking; use GaletteEvents\Filters\BookingsList; @@ -235,49 +234,20 @@ private function buildWhereClause(Select $select): void } if (!$this->login->isAdmin() && !$this->login->isStaff()) { - $groups = Groups::loadGroups( - $this->login->id, - false, - false - ); - - if ($this->login->isGroupManager() && count($this->login->managed_groups)) { - $groups = array_merge($groups, $this->login->managed_groups); - } - - $set = [new PredicateSet( - [ - new Predicate\IsNull(Group::PK), - new Predicate\Operator( - 'is_open', - '=', - true - ), - new Predicate\Operator( - 'begin_date', - '>=', - date('Y-m-d') - ) - ] - )]; - - if (count($groups)) { - $set[] = new Predicate\In( - Group::PK, - $groups - ); - } - - if (!$this->login->isSuperAdmin()) { - $set[] = new Predicate\Operator( + //members see their own bookings, group managers also the ones on events of groups they manage + $set = [ + new Predicate\Operator( 'a.' . Adherent::PK, '=', $this->login->id - ); + ) + ]; - if (!$this->login->isGroupManager()) { - $select->where(['a.' . Adherent::PK => $this->login->id]); - } + if ($this->login->isGroupManager() && count($this->login->managed_groups)) { + $set[] = new Predicate\In( + 'e.' . Group::PK, + $this->login->managed_groups + ); } $select->where( diff --git a/tests/EventsFixtures.php b/tests/EventsFixtures.php index b34c286d..0afe2fd9 100644 --- a/tests/EventsFixtures.php +++ b/tests/EventsFixtures.php @@ -12,6 +12,7 @@ use Galette\Entity\Adherent; use Galette\Entity\Group; +use Galette\Entity\PaymentType; use GaletteEvents\Activity; use GaletteEvents\Booking; use GaletteEvents\Event; @@ -150,6 +151,7 @@ protected function insertBooking(int $event, int $member, array $data = []): int Event::PK => $event, Adherent::PK => $member, 'booking_date' => date('Y-m-d'), + 'payment_method' => PaymentType::OTHER, 'number_people' => 1, 'creation_date' => date('Y-m-d'), 'comment' => '', diff --git a/tests/GaletteEvents/Controllers/Crud/tests/units/BookingsController.php b/tests/GaletteEvents/Controllers/Crud/tests/units/BookingsController.php index 7bf42f9c..dc286cb5 100644 --- a/tests/GaletteEvents/Controllers/Crud/tests/units/BookingsController.php +++ b/tests/GaletteEvents/Controllers/Crud/tests/units/BookingsController.php @@ -380,4 +380,44 @@ public function testStaffBooksClosedEvent(): void $this->assertSame($member_two->id, $this->getBookedMember($event)); $this->resetStaffStatus($staff, $member_two); } + + /** + * Group managers run batch actions on bookings of the groups they manage only + */ + public function testManagerBatchOnManagedGroupsBookingsOnly(): void + { + $member_one = $this->getMemberOne(); + $member_two = $this->getMemberTwo(); + $managed = $this->createGroup('Managed group', [$member_two], [$member_one]); + $other = $this->createGroup('Other group', [], [$member_one, $member_two]); + $managed_booking = $this->insertBooking( + $this->insertEvent('Managed event', ['id_group' => $managed->getId()]), + $member_one->id + ); + $other_booking = $this->insertBooking( + $this->insertEvent('Other event', ['id_group' => $other->getId()]), + $member_one->id + ); + + $this->logMember($this->dataAdherentTwo()); + $batch = function (array $selected): \Psr\Http\Message\ResponseInterface { + $request = $this->createRequest('batch-eventslist', [], 'POST')->withParsedBody([ + 'entries_sel' => array_map('strval', $selected), + 'csv' => '1', + ]); + return $this->app->handle($request); + }; + + $test_response = $batch([$other_booking]); + $this->assertSame( + ['Location' => [$this->routeparser->urlFor('events_events')]], + $test_response->getHeaders() + ); + $this->expectFlashData(['error_detected' => [_T('No booking was selected, please check at least one.', 'events')]]); + $this->assertFalse(isset($this->session->{'plugin-events-members'})); + + $test_response = $batch([$other_booking, $managed_booking]); + $this->assertSame(307, $test_response->getStatusCode()); + $this->assertSame([$member_one->id], $this->session->{'plugin-events-members'}->selected); + } } diff --git a/tests/GaletteEvents/Controllers/tests/units/CsvController.php b/tests/GaletteEvents/Controllers/tests/units/CsvController.php new file mode 100644 index 00000000..f517d564 --- /dev/null +++ b/tests/GaletteEvents/Controllers/tests/units/CsvController.php @@ -0,0 +1,76 @@ + + */ +class CsvController extends GaletteRoutingTestCase +{ + use EventsFixtures; + + protected int $seed = 20260926101512; + protected bool $load_plugins = true; + + /** + * Cleanup after each test method + */ + public function tearDown(): void + { + $this->login->logout(); + $this->cleanEvents(); + parent::tearDown(); + } + + /** + * Export bookings of an event + * + * @param int $event Event ID + */ + private function exportEvent(int $event): string + { + $test_response = $this->app->handle( + $this->createRequest('event_bookings_export', ['id' => (string)$event]) + ); + $this->assertSame(200, $test_response->getStatusCode()); + $this->assertSame(['text/csv'], $test_response->getHeader('Content-Type')); + return (string)$test_response->getBody(); + } + + /** + * Group managers export bookings on events of the groups they manage only + */ + public function testManagerExportsManagedGroupsBookingsOnly(): void + { + $member_one = $this->getMemberOne(); + $member_two = $this->getMemberTwo(); + $managed = $this->createGroup('Managed group', [$member_two], [$member_one]); + //member two belongs to this one, but does not manage it + $other = $this->createGroup('Other group', [], [$member_one, $member_two]); + + $managed_event = $this->insertEvent('Managed event', ['id_group' => $managed->getId()]); + $other_event = $this->insertEvent('Other event', ['id_group' => $other->getId()]); + $public_event = $this->insertEvent('Public event'); + foreach ([$managed_event, $other_event, $public_event] as $event) { + $this->insertBooking($event, $member_one->id); + } + + $this->logMember($this->dataAdherentTwo()); + $this->assertStringContainsString($member_one->email, $this->exportEvent($managed_event)); + $this->assertStringNotContainsString($member_one->email, $this->exportEvent($other_event)); + $this->assertStringNotContainsString($member_one->email, $this->exportEvent($public_event)); + } +} diff --git a/tests/GaletteEvents/Repository/tests/units/Bookings.php b/tests/GaletteEvents/Repository/tests/units/Bookings.php new file mode 100644 index 00000000..f1b6e56d --- /dev/null +++ b/tests/GaletteEvents/Repository/tests/units/Bookings.php @@ -0,0 +1,92 @@ + + */ +class Bookings extends GaletteTestCase +{ + use EventsFixtures; + + protected int $seed = 20260926101512; + + /** + * Cleanup after each test method + */ + public function tearDown(): void + { + $this->login->logout(); + $this->cleanEvents(); + parent::tearDown(); + } + + /** + * Get IDs of bookings current logged-in user can list, and their sum + * + * @return array{ids: array, sum: float} + */ + private function getVisibleBookings(): array + { + $bookings = new \GaletteEvents\Repository\Bookings($this->zdb, $this->login); + $ids = array_map(fn(Booking $booking): ?int => $booking->getId(), $bookings->getList(true)); + sort($ids); + return ['ids' => $ids, 'sum' => $bookings->getSum()]; + } + + /** + * Members list their own bookings, group managers the ones on events of groups they manage as well + */ + public function testListScope(): void + { + $member_one = $this->getMemberOne(); + $member_two = $this->getMemberTwo(); + $managed = $this->createGroup('Managed group', [$member_two], [$member_one]); + //member two belongs to this one, but does not manage it + $other = $this->createGroup('Other group', [], [$member_one, $member_two]); + + $managed_event = $this->insertEvent('Managed event', ['id_group' => $managed->getId()]); + $other_event = $this->insertEvent('Other event', ['id_group' => $other->getId()]); + $public_event = $this->insertEvent('Public event'); + + $one_managed = $this->insertBooking($managed_event, $member_one->id, ['payment_amount' => 1]); + $one_other = $this->insertBooking($other_event, $member_one->id, ['payment_amount' => 10]); + $one_public = $this->insertBooking($public_event, $member_one->id, ['payment_amount' => 100]); + $two_public = $this->insertBooking($public_event, $member_two->id, ['payment_amount' => 1000]); + + $this->logMember($this->dataAdherentOne()); + $this->assertSame( + ['ids' => [$one_managed, $one_other, $one_public], 'sum' => 111.0], + $this->getVisibleBookings() + ); + $this->login->logout(); + + $this->logMember($this->dataAdherentTwo()); + $this->assertTrue($this->login->isGroupManager()); + $this->assertSame( + ['ids' => [$one_managed, $two_public], 'sum' => 1001.0], + $this->getVisibleBookings() + ); + $this->login->logout(); + + $this->logSuperAdmin(); + $this->assertSame( + ['ids' => [$one_managed, $one_other, $one_public, $two_public], 'sum' => 1111.0], + $this->getVisibleBookings() + ); + } +} From 3726e592284d8e4a2efae09b4102e950e917d8ee Mon Sep 17 00:00:00 2001 From: Johan Cwiklinski Date: Sat, 26 Sep 2026 09:55:26 +0200 Subject: [PATCH 7/8] Apply group managers exports and mailings preferences to bookings --- .../Controllers/Crud/BookingsController.php | 35 ++++++++++++++ .../Controllers/CsvController.php | 18 ++++++++ .../Crud/tests/units/BookingsController.php | 46 +++++++++++++++++++ .../Controllers/tests/units/CsvController.php | 34 ++++++++++++++ 4 files changed, 133 insertions(+) diff --git a/lib/GaletteEvents/Controllers/Crud/BookingsController.php b/lib/GaletteEvents/Controllers/Crud/BookingsController.php index 93811994..41711f40 100644 --- a/lib/GaletteEvents/Controllers/Crud/BookingsController.php +++ b/lib/GaletteEvents/Controllers/Crud/BookingsController.php @@ -221,6 +221,22 @@ public function handleBatch(Request $request, Response $response): Response { $post = $request->getParsedBody(); + foreach (['mailing', 'csv', 'csvbooking', 'labels'] as $action) { + if (isset($post[$action]) && !$this->canBatch($action)) { + Analog::log( + 'Logged in member ' . $this->login->login + . ' has tried to run "' . $action . '" batch action on bookings' + . ' without the right to do so.', + Analog::WARNING + ); + return $this->redirectWithErrors( + response: $response, + errors: [_T("You do not have permission for requested URL.")], + redirect_url: $this->routeparser->urlFor('events_bookings', ['event' => 'all']) + ); + } + } + if (isset($post['entries_sel'])) { if (isset($this->session->filter_bookings)) { $filters = clone $this->session->filter_bookings; @@ -313,6 +329,25 @@ public function handleBatch(Request $request, Response $response): Response ->withHeader('Location', $this->routeparser->urlFor('events_events')); } + /** + * Can current logged-in user run a batch action on bookings + * + * Group managers run exports and mailings as core preferences allow them to. + * + * @param string $action Batch action + */ + private function canBatch(string $action): bool + { + if ($this->login->isAdmin() || $this->login->isStaff()) { + return true; + } + + if ($action === 'mailing') { + return (bool)$this->preferences->pref_bool_groupsmanagers_mailings; + } + return (bool)$this->preferences->pref_bool_groupsmanagers_exports; + } + // /CRUD - Read // CRUD - Update diff --git a/lib/GaletteEvents/Controllers/CsvController.php b/lib/GaletteEvents/Controllers/CsvController.php index d005810e..d317f683 100644 --- a/lib/GaletteEvents/Controllers/CsvController.php +++ b/lib/GaletteEvents/Controllers/CsvController.php @@ -10,6 +10,7 @@ namespace GaletteEvents\Controllers; +use Analog\Analog; use Slim\Psr7\Request; use Slim\Psr7\Response; use Galette\IO\Csv; @@ -32,6 +33,23 @@ class CsvController extends \Galette\Controllers\CsvController */ public function bookingsExport(Request $request, Response $response, ?int $id = null): Response { + if ( + !$this->login->isAdmin() + && !$this->login->isStaff() + && !$this->preferences->pref_bool_groupsmanagers_exports + ) { + Analog::log( + 'Logged in member ' . $this->login->login + . ' has tried to export bookings without the right to do so.', + Analog::WARNING + ); + return $this->redirectWithErrors( + response: $response, + errors: [_T("You do not have permission for requested URL.")], + redirect_url: $this->routeparser->urlFor('events_bookings', ['event' => 'all']) + ); + } + $post = $request->getParsedBody(); $get = $request->getQueryParams(); $csv = new CsvOut(); diff --git a/tests/GaletteEvents/Controllers/Crud/tests/units/BookingsController.php b/tests/GaletteEvents/Controllers/Crud/tests/units/BookingsController.php index dc286cb5..84faa654 100644 --- a/tests/GaletteEvents/Controllers/Crud/tests/units/BookingsController.php +++ b/tests/GaletteEvents/Controllers/Crud/tests/units/BookingsController.php @@ -32,6 +32,8 @@ class BookingsController extends GaletteRoutingTestCase public function tearDown(): void { $this->login->logout(); + $this->preferences->pref_bool_groupsmanagers_exports = true; + $this->preferences->pref_bool_groupsmanagers_mailings = false; $this->cleanEvents(); parent::tearDown(); } @@ -420,4 +422,48 @@ public function testManagerBatchOnManagedGroupsBookingsOnly(): void $this->assertSame(307, $test_response->getStatusCode()); $this->assertSame([$member_one->id], $this->session->{'plugin-events-members'}->selected); } + + /** + * Group managers run exports and mailings as core preferences allow them to + */ + public function testManagerBatchAsCoreAllows(): void + { + $member_one = $this->getMemberOne(); + $member_two = $this->getMemberTwo(); + $managed = $this->createGroup('Managed group', [$member_two], [$member_one]); + $booking = $this->insertBooking( + $this->insertEvent('Managed event', ['id_group' => $managed->getId()]), + $member_one->id + ); + $this->preferences->pref_bool_groupsmanagers_exports = false; + + $this->logMember($this->dataAdherentTwo()); + $batch = function (string $action) use ($booking): \Psr\Http\Message\ResponseInterface { + $request = $this->createRequest('batch-eventslist', [], 'POST')->withParsedBody([ + 'entries_sel' => [(string)$booking], + $action => '1', + ]); + return $this->app->handle($request); + }; + + foreach (['mailing', 'csv', 'csvbooking', 'labels'] as $action) { + $test_response = $batch($action); + $this->assertSame( + ['Location' => [$this->routeparser->urlFor('events_bookings', ['event' => 'all'])]], + $test_response->getHeaders(), + $action + ); + $this->expectFlashData(['error_detected' => [_T('You do not have permission for requested URL.')]]); + $this->expectLogEntry(Analog::WARNING, 'has tried to run "' . $action . '" batch action on bookings'); + $this->expectNoLogEntry(); + } + + $this->preferences->pref_bool_groupsmanagers_exports = true; + $this->preferences->pref_bool_groupsmanagers_mailings = true; + $this->assertSame( + [$this->routeparser->urlFor('mailing') . '?mailing_new=true'], + $batch('mailing')->getHeader('Location') + ); + $this->assertSame(307, $batch('csv')->getStatusCode()); + } } diff --git a/tests/GaletteEvents/Controllers/tests/units/CsvController.php b/tests/GaletteEvents/Controllers/tests/units/CsvController.php index f517d564..32663610 100644 --- a/tests/GaletteEvents/Controllers/tests/units/CsvController.php +++ b/tests/GaletteEvents/Controllers/tests/units/CsvController.php @@ -10,6 +10,7 @@ namespace GaletteEvents\Controllers\tests\units; +use Analog\Analog; use Galette\Tests\GaletteRoutingTestCase; use GaletteEvents\tests\EventsFixtures; @@ -31,6 +32,7 @@ class CsvController extends GaletteRoutingTestCase public function tearDown(): void { $this->login->logout(); + $this->preferences->pref_bool_groupsmanagers_exports = true; $this->cleanEvents(); parent::tearDown(); } @@ -73,4 +75,36 @@ public function testManagerExportsManagedGroupsBookingsOnly(): void $this->assertStringNotContainsString($member_one->email, $this->exportEvent($other_event)); $this->assertStringNotContainsString($member_one->email, $this->exportEvent($public_event)); } + + /** + * Group managers export bookings as core preferences allow them to + */ + public function testManagerExportsAsCoreAllows(): void + { + $member_one = $this->getMemberOne(); + $member_two = $this->getMemberTwo(); + $managed = $this->createGroup('Managed group', [$member_two], [$member_one]); + $event = $this->insertEvent('Managed event', ['id_group' => $managed->getId()]); + $this->insertBooking($event, $member_one->id); + $this->preferences->pref_bool_groupsmanagers_exports = false; + + $this->logMember($this->dataAdherentTwo()); + foreach (['event_bookings_export' => ['id' => (string)$event], 'events_bookings_export' => []] as $route => $args) { + $test_response = $this->app->handle($this->createRequest($route, $args, $args === [] ? 'POST' : 'GET')); + $this->assertSame( + ['Location' => [$this->routeparser->urlFor('events_bookings', ['event' => 'all'])]], + $test_response->getHeaders() + ); + $this->expectFlashData(['error_detected' => [_T('You do not have permission for requested URL.')]]); + $this->expectLogEntry(Analog::WARNING, 'has tried to export bookings without the right to do so'); + $this->expectNoLogEntry(); + } + $this->login->logout(); + + //preference is for group managers only + $staff = $this->getStaffMember($member_one); + $this->logMember($this->dataAdherentOne()); + $this->assertStringContainsString($member_one->email, $this->exportEvent($event)); + $this->resetStaffStatus($staff, $member_two); + } } From c3f8bf0c0f5615c68f814db251b037cbf8a5666b Mon Sep 17 00:00:00 2001 From: Johan Cwiklinski Date: Sat, 26 Sep 2026 09:55:53 +0200 Subject: [PATCH 8/8] Read bookings export filters from plugin session keys only --- lib/GaletteEvents/Controllers/CsvController.php | 6 +++++- .../Controllers/tests/units/CsvController.php | 16 ++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/lib/GaletteEvents/Controllers/CsvController.php b/lib/GaletteEvents/Controllers/CsvController.php index d317f683..389c35ab 100644 --- a/lib/GaletteEvents/Controllers/CsvController.php +++ b/lib/GaletteEvents/Controllers/CsvController.php @@ -54,8 +54,12 @@ public function bookingsExport(Request $request, Response $response, ?int $id = $get = $request->getQueryParams(); $csv = new CsvOut(); + //filters come from bookings list, or from its batch actions $session_var = $post['session_var'] ?? $get['session_var'] ?? 'filter_bookings'; - if (isset($this->session->$session_var) && $id === null) { + if (!in_array($session_var, ['filter_bookings', 'plugin-events-bookings'], true)) { + $session_var = 'filter_bookings'; + } + if ($id === null && ($this->session->$session_var ?? null) instanceof BookingsList) { $filters = $this->session->$session_var; } else { $filters = new BookingsList(); diff --git a/tests/GaletteEvents/Controllers/tests/units/CsvController.php b/tests/GaletteEvents/Controllers/tests/units/CsvController.php index 32663610..5a84ac68 100644 --- a/tests/GaletteEvents/Controllers/tests/units/CsvController.php +++ b/tests/GaletteEvents/Controllers/tests/units/CsvController.php @@ -107,4 +107,20 @@ public function testManagerExportsAsCoreAllows(): void $this->assertStringContainsString($member_one->email, $this->exportEvent($event)); $this->resetStaffStatus($staff, $member_two); } + + /** + * Export reads bookings filters only from the session + */ + public function testExportReadsBookingsFiltersOnly(): void + { + $this->logSuperAdmin(); + $this->session->filter_members = new \Galette\Filters\MembersList(); + + $test_response = $this->app->handle( + $this->createRequest('events_bookings_export', [], 'POST') + ->withParsedBody(['session_var' => 'filter_members']) + ); + $this->assertSame(200, $test_response->getStatusCode()); + $this->assertSame(['text/csv'], $test_response->getHeader('Content-Type')); + } }