From 0f77b41942d67c58daccdac1ff3a7f80d3b569a7 Mon Sep 17 00:00:00 2001 From: Christoph Wurst Date: Thu, 4 Jan 2024 10:04:35 +0100 Subject: [PATCH] fix(appointments): Rate limit config creation and booking Abusing the appointment config endpoint can lead to additional server load. Sending bulks of booking requests can lead to mass notifications and emails and server load, too. Signed-off-by: Christoph Wurst --- lib/Controller/AppointmentConfigController.php | 3 +++ lib/Controller/BookingController.php | 7 +++++++ src/components/AppointmentConfigModal.vue | 13 ++++++++++++- .../Appointments/AppointmentDetails.vue | 16 +++++++++++++--- src/views/Appointments/Booking.vue | 9 ++++++++- tests/psalm-baseline.xml | 11 ++++++++++- 6 files changed, 53 insertions(+), 6 deletions(-) diff --git a/lib/Controller/AppointmentConfigController.php b/lib/Controller/AppointmentConfigController.php index 84260799a..a2603958b 100644 --- a/lib/Controller/AppointmentConfigController.php +++ b/lib/Controller/AppointmentConfigController.php @@ -35,6 +35,7 @@ use OCA\Calendar\Http\JsonResponse; use OCA\Calendar\Service\Appointments\AppointmentConfigService; use OCP\AppFramework\Controller; use OCP\AppFramework\Http; +use OCP\AppFramework\Http\Attribute\UserRateLimit; use OCP\IRequest; use Psr\Log\LoggerInterface; use function array_keys; @@ -147,7 +148,9 @@ class AppointmentConfigController extends Controller { * @param int|null $end * @param int|null $futureLimit * @return JsonResponse + * @UserRateThrottle(limit=10, period=1200) */ + #[UserRateLimit(limit: 10, period: 1200)] public function create( string $name, string $description, diff --git a/lib/Controller/BookingController.php b/lib/Controller/BookingController.php index 7697fd26e..c51895b44 100644 --- a/lib/Controller/BookingController.php +++ b/lib/Controller/BookingController.php @@ -37,6 +37,8 @@ use OCA\Calendar\Service\Appointments\AppointmentConfigService; use OCA\Calendar\Service\Appointments\BookingService; use OCP\AppFramework\Controller; use OCP\AppFramework\Http; +use OCP\AppFramework\Http\Attribute\AnonRateLimit; +use OCP\AppFramework\Http\Attribute\UserRateLimit; use OCP\AppFramework\Http\TemplateResponse; use OCP\AppFramework\Services\IInitialState; use OCP\AppFramework\Utility\ITimeFactory; @@ -162,7 +164,12 @@ class BookingController extends Controller { * @param string $description * @param string $timeZone * @return JsonResponse + * + * @AnonRateThrottle(limit=10, period=1200) + * @UserRateThrottle(limit=10, period=300) */ + #[AnonRateLimit(limit: 10, period: 1200)] + #[UserRateLimit(limit: 10, period: 300)] public function bookSlot(int $appointmentConfigId, int $start, int $end, diff --git a/src/components/AppointmentConfigModal.vue b/src/components/AppointmentConfigModal.vue index 2d35cd0f8..27a655034 100644 --- a/src/components/AppointmentConfigModal.vue +++ b/src/components/AppointmentConfigModal.vue @@ -134,6 +134,10 @@ + + {{ t('calendar', 'It seems a rate limit has been reached. Please try again later.') }} + import { CalendarAvailability } from '@nextcloud/calendar-availability-vue' -import { NcModal as Modal, NcButton, NcCheckboxRadioSwitch } from '@nextcloud/vue' +import { NcModal as Modal, NcButton, NcCheckboxRadioSwitch, NcNoteCard } from '@nextcloud/vue' import TextInput from './AppointmentConfigModal/TextInput.vue' import TextArea from './AppointmentConfigModal/TextArea.vue' import AppointmentConfig from '../models/appointmentConfig.js' @@ -177,6 +181,7 @@ export default { Confirmation, NcButton, NcCheckboxRadioSwitch, + NcNoteCard, }, props: { config: { @@ -194,6 +199,7 @@ export default { enablePreparationDuration: false, enableFollowupDuration: false, enableFutureLimit: false, + rateLimitingReached: false, showConfirmation: false, } }, @@ -282,6 +288,8 @@ export default { this.editing.calendarFreeBusyUris = this.editing.calendarFreeBusyUris.filter(uri => uri !== this.calendarUrlToUri(calendar.url)) }, async save() { + this.rateLimitingReached = false + if (!this.enablePreparationDuration) { this.editing.preparationDuration = this.defaultConfig.preparationDuration } @@ -307,6 +315,9 @@ export default { } this.showConfirmation = true } catch (error) { + if (error?.response?.status === 429) { + this.rateLimitingReached = true + } logger.error('Failed to save config', { error, config, isNew: this.isNew }) } }, diff --git a/src/components/Appointments/AppointmentDetails.vue b/src/components/Appointments/AppointmentDetails.vue index 89cccb40a..ede9c2245 100644 --- a/src/components/Appointments/AppointmentDetails.vue +++ b/src/components/Appointments/AppointmentDetails.vue @@ -53,10 +53,14 @@ :disabled="isLoading" /> -
+ + {{ $t('calendar', 'It seems a rate limit has been reached. Please try again later.') }} + + {{ $t('calendar', 'Could not book the appointment. Please try again later or contact the organizer.') }} -
+
@@ -73,6 +77,7 @@ import { NcButton, NcLoadingIcon, NcModal as Modal, + NcNoteCard, } from '@nextcloud/vue' import autosize from '../../directives/autosize.js' @@ -85,6 +90,7 @@ export default { NcButton, NcLoadingIcon, Modal, + NcNoteCard, }, directives: { autosize, @@ -110,6 +116,10 @@ export default { required: true, type: String, }, + showRateLimitingWarning: { + required: true, + type: Boolean, + }, showError: { required: true, type: Boolean, diff --git a/src/views/Appointments/Booking.vue b/src/views/Appointments/Booking.vue index 0c7c68107..f2c77ad6c 100644 --- a/src/views/Appointments/Booking.vue +++ b/src/views/Appointments/Booking.vue @@ -76,6 +76,7 @@ :visitor-info="visitorInfo" :time-zone-id="timeZone" :show-error="bookingError" + :show-rate-limiting-warning="bookingRateLimit" :is-loading="bookingLoading" @save="onSave" @close="selectedSlot = undefined" /> @@ -172,6 +173,7 @@ export default { bookingConfirmed: false, bookingError: false, bookingLoading: false, + bookingRateLimit: false, } }, watch: { @@ -229,6 +231,7 @@ export default { }) this.bookingError = false + this.bookingRateLimit = false try { await bookSlot(this.config, slot, displayName, email, description, timeZone) @@ -238,7 +241,11 @@ export default { this.bookingConfirmed = true } catch (e) { console.error('could not book appointment', e) - this.bookingError = true + if (e?.response?.status === 429) { + this.bookingRateLimit = true + } else { + this.bookingError = true + } } finally { this.bookingLoading = false } diff --git a/tests/psalm-baseline.xml b/tests/psalm-baseline.xml index 7655a6ae6..337c6cd8a 100644 --- a/tests/psalm-baseline.xml +++ b/tests/psalm-baseline.xml @@ -1,5 +1,5 @@ - + CalendarWidgetV2 @@ -13,6 +13,15 @@ $expectedDayKeys !== $actualDayKeys + + UserRateLimit + + + + + AnonRateLimit + UserRateLimit +