From 9446f128c908167fa866c115466e5616a6b58017 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Tollk=C3=BChn?= Date: Mon, 26 Oct 2026 13:43:58 +0100 Subject: [PATCH] Rate limit contact search, skip short queries The contact-search endpoint had no request-rate limit and ran an address-book search even for an empty or single-character query, the most expensive case. Authenticated users could trigger repeated scans with simple scripted requests. Apply the framework's per-user UserRateLimit attribute (120 requests per minute) and return an empty result without touching the address book for fragments under two characters. The unfiltered datalist on the confirm page is unchanged. --- lib/Controller/CalendarController.php | 27 +++++++++++++++++++++++++ tests/Unit/CalendarControllerTest.php | 29 +++++++++++++++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/lib/Controller/CalendarController.php b/lib/Controller/CalendarController.php index f8e6487..59620fd 100644 --- a/lib/Controller/CalendarController.php +++ b/lib/Controller/CalendarController.php @@ -10,6 +10,7 @@ use OCP\AppFramework\Controller; use OCP\AppFramework\Http; use OCP\AppFramework\Http\Attribute\NoAdminRequired; use OCP\AppFramework\Http\Attribute\NoCSRFRequired; +use OCP\AppFramework\Http\Attribute\UserRateLimit; use OCP\AppFramework\Http\RedirectResponse; use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\Http\TemplateResponse; @@ -76,6 +77,19 @@ class CalendarController extends Controller */ private const MAX_CONTACT_SUGGESTIONS = 50; + /* + * Short fragments are not distinctive enough for an + * address-book search, so they skip it entirely. + */ + private const MIN_SEARCH_QUERY = 2; + + /* + * Live-search requests per minute per user. The JS debounce + * produces at most four per second while typing. + */ + private const SEARCH_RATE_LIMIT = 120; + private const SEARCH_RATE_PERIOD = 60; + private IManager $calendarManager; private IContactsManager $contactsManager; private IAppManager $appManager; @@ -310,8 +324,21 @@ class CalendarController extends Controller */ #[NoAdminRequired] #[NoCSRFRequired] + #[UserRateLimit( + limit: self::SEARCH_RATE_LIMIT, + period: self::SEARCH_RATE_PERIOD + )] public function searchContacts(string $query = ''): DataResponse { + /* + * An empty or single-character query means a scan of the + * whole address book: the most expensive case, skipped + * because such fragments match too much anyway. + */ + if (mb_strlen(trim($query)) < self::MIN_SEARCH_QUERY) { + return new DataResponse([]); + } + return new DataResponse($this->contactSuggestions($query)); } diff --git a/tests/Unit/CalendarControllerTest.php b/tests/Unit/CalendarControllerTest.php index 52a1376..5400f86 100644 --- a/tests/Unit/CalendarControllerTest.php +++ b/tests/Unit/CalendarControllerTest.php @@ -9,6 +9,7 @@ use OCP\App\IAppManager; use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\Http\RedirectResponse; use OCP\AppFramework\Http\TemplateResponse; +use OCP\AppFramework\Http\Attribute\UserRateLimit; use OCP\Calendar\ICalendar; use OCP\Calendar\ICalendarEventBuilder; use OCP\Calendar\ICreateFromString; @@ -1085,6 +1086,34 @@ class CalendarControllerTest extends TestCase $this->assertCount(50, $response->getData()); } + /* + * searchContacts(): trivial queries must not reach the + * address book + */ + + public function testSearchContactsSkipsShortQueries(): void + { + $this->contactsManager->expects($this->never())->method('search'); + + $c = $this->newController(); + + $this->assertSame([], $c->searchContacts('')->getData()); + $this->assertSame([], $c->searchContacts('a')->getData()); + $this->assertSame([], $c->searchContacts(' b ')->getData()); + } + + public function testSearchContactsIsRateLimited(): void + { + $method = new \ReflectionMethod(CalendarController::class, 'searchContacts'); + $attributes = $method->getAttributes(UserRateLimit::class); + + $this->assertCount(1, $attributes); + + $limit = $attributes[0]->newInstance(); + $this->assertSame(120, $limit->getLimit()); + $this->assertSame(60, $limit->getPeriod()); + } + public function testSearchContactsTruncatesQuery(): void { $this->contactsManager->expects($this->once())->method('search')