Skip to content

Commit

Permalink
fix(appointments): Rate limit config creation and booking
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
ChristophWurst committed Jan 10, 2024
1 parent af21e82 commit dd0f551
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 2 deletions.
3 changes: 3 additions & 0 deletions lib/Controller/AppointmentConfigController.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -148,7 +149,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,
Expand Down
7 changes: 7 additions & 0 deletions lib/Controller/BookingController.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,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;
Expand Down Expand Up @@ -163,7 +165,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,
Expand Down
11 changes: 10 additions & 1 deletion src/components/AppointmentConfigModal.vue
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,10 @@
</div>
</fieldset>
</div>
<button class="primary appointment-config-modal__submit-button"
<div v-if="rateLimitingReached">
{{ t('calendar', 'It seems a rate limit has been reached. Please try again later.') }}
</div>
<button class="appointment-config-modal__submit-button"
:disabled="!editing.name || editing.length === 0"
@click="save">
{{ saveButtonText }}
Expand Down Expand Up @@ -184,6 +187,7 @@ export default {
enablePreparationDuration: false,
enableFollowupDuration: false,
enableFutureLimit: false,
rateLimitingReached: false,
showConfirmation: false,
}
},
Expand Down Expand Up @@ -267,6 +271,8 @@ export default {
this.editing.calendarFreeBusyUris = this.editing.calendarFreeBusyUris.filter(uri => uri !== this.calendarUrlToUri(calendar.url))
},
async save() {
this.rateLimitingReached = false

Check warning on line 274 in src/components/AppointmentConfigModal.vue

View check run for this annotation

Codecov / codecov/patch

src/components/AppointmentConfigModal.vue#L274

Added line #L274 was not covered by tests

if (!this.enablePreparationDuration) {
this.editing.preparationDuration = this.defaultConfig.preparationDuration
}
Expand All @@ -292,6 +298,9 @@ export default {
}
this.showConfirmation = true
} catch (error) {
if (error?.response?.status === 429) {
this.rateLimitingReached = true

Check warning on line 302 in src/components/AppointmentConfigModal.vue

View check run for this annotation

Codecov / codecov/patch

src/components/AppointmentConfigModal.vue#L301-L302

Added lines #L301 - L302 were not covered by tests
}
logger.error('Failed to save config', { error, config, isNew: this.isNew })
}
},
Expand Down
8 changes: 8 additions & 0 deletions src/components/Appointments/AppointmentDetails.vue
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@
autocomplete="off" />
</div>
</div>
<div v-if="showRateLimitingWarning"
class="booking-error">
{{ $t('calendar', 'It seems a rate limit has been reached. Please try again later.') }}
</div>
<div v-if="showError"
class="booking-error">
{{ $t('calendar', 'Could not book the appointment. Please try again later or contact the organizer.') }}
Expand Down Expand Up @@ -101,6 +105,10 @@ export default {
required: true,
type: String,
},
showRateLimitingWarning: {
required: true,
type: Boolean,
},
showError: {
required: true,
type: Boolean,
Expand Down
9 changes: 8 additions & 1 deletion src/views/Appointments/Booking.vue
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@
:visitor-info="visitorInfo"
:time-zone-id="timeZone"
:show-error="bookingError"
:show-rate-limiting-warning="bookingRateLimit"

Check failure on line 76 in src/views/Appointments/Booking.vue

View workflow job for this annotation

GitHub Actions / ESLint

Expected indentation of 4 tabs but found 5 tabs
@save="onSave"
@close="selectedSlot = undefined" />
</div>
Expand Down Expand Up @@ -146,6 +147,7 @@ export default {
selectedSlot: undefined,
bookingConfirmed: false,
bookingError: false,
bookingRateLimit: false,
}
},
watch: {
Expand Down Expand Up @@ -206,6 +208,7 @@ export default {
})

this.bookingError = false
this.bookingRateLimit = false

Check warning on line 211 in src/views/Appointments/Booking.vue

View check run for this annotation

Codecov / codecov/patch

src/views/Appointments/Booking.vue#L211

Added line #L211 was not covered by tests
try {
await bookSlot(this.config, slot, displayName, email, description, timeZone)

Expand All @@ -215,7 +218,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

Check warning on line 224 in src/views/Appointments/Booking.vue

View check run for this annotation

Codecov / codecov/patch

src/views/Appointments/Booking.vue#L221-L224

Added lines #L221 - L224 were not covered by tests
}
}
},
onSlotClicked(slot) {
Expand Down

0 comments on commit dd0f551

Please sign in to comment.