Merge pull request #5680 from nextcloud/fix/appointments/rate-limit-c…

…onfig-creation fix(appointments): Rate limit config creation and booking
nextcloud · Jan 10, 2024 · b19443d · b19443d
2 parents ceb2673 + 0f77b41
commit b19443d
Show file tree

Hide file tree

Showing 6 changed files with 53 additions and 6 deletions.
diff --git a/lib/Controller/AppointmentConfigController.php b/lib/Controller/AppointmentConfigController.php
@@ -35,6 +35,7 @@
 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 @@ private function validateAvailability(array $availability): void {
 	 * @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
@@ -37,6 +37,8 @@
 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 @@ public function getBookableSlots(int $appointmentConfigId,
 	 * @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
@@ -134,6 +134,10 @@
 						</div>
 					</fieldset>
 				</div>
+				<NcNoteCard v-if="rateLimitingReached"
+					type="warning">
+					{{ t('calendar', 'It seems a rate limit has been reached. Please try again later.') }}
+				</NcNoteCard>
 				<NcButton class="appointment-config-modal__submit-button"
 					type="primary"
 					:disabled="!editing.name || editing.length === 0"
@@ -147,7 +151,7 @@
 
 <script>
 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
@@ -53,10 +53,14 @@
 							:disabled="isLoading" />
 					</div>
 				</div>
-				<div v-if="showError"
-					class="booking-error">
+				<NcNoteCard v-if="showRateLimitingWarning"
+					type="warning">
+					{{ $t('calendar', 'It seems a rate limit has been reached. Please try again later.') }}
+				</NcNoteCard>
+				<NcNoteCard v-if="showError"
+					type="error">
 					{{ $t('calendar', 'Could not book the appointment. Please try again later or contact the organizer.') }}
-				</div>
+				</NcNoteCard>
 				<div class="buttons">
 					<NcLoadingIcon v-if="isLoading" :size="32" class="loading-icon" />
 					<NcButton type="primary" :disabled="isLoading" @click="save">
@@ -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
@@ -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
@@ -1,5 +1,5 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<files psalm-version="5.15.0@5c774aca4746caf3d239d9c8cadb9f882ca29352">
+<files psalm-version="5.19.0@06b71be009a6bd6d81b9811855d6629b9fe90e1b">
   <file src="lib/AppInfo/Application.php">
     <MissingDependency>
       <code>CalendarWidgetV2</code>
@@ -13,6 +13,15 @@
       <code>$expectedDayKeys !== $actualDayKeys</code>
       <code><![CDATA[$slotKeys !== ['end', 'start']]]></code>
     </RedundantCondition>
+    <UndefinedAttributeClass>
+      <code>UserRateLimit</code>
+    </UndefinedAttributeClass>
+  </file>
+  <file src="lib/Controller/BookingController.php">
+    <UndefinedAttributeClass>
+      <code>AnonRateLimit</code>
+      <code>UserRateLimit</code>
+    </UndefinedAttributeClass>
   </file>
   <file src="lib/Dashboard/CalendarWidgetV2.php">
     <UndefinedClass>