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.
This commit is contained in:
1 parent
2b3e96e50a
commit
9446f128c9
2 files changed
+56
No files matched your search
@@ -10,6 +10,7 @@ use OCP\AppFramework\Controller;
|
|||||||
use OCP\AppFramework\Http;
|
use OCP\AppFramework\Http;
|
||||||
use OCP\AppFramework\Http\Attribute\NoAdminRequired;
|
use OCP\AppFramework\Http\Attribute\NoAdminRequired;
|
||||||
use OCP\AppFramework\Http\Attribute\NoCSRFRequired;
|
use OCP\AppFramework\Http\Attribute\NoCSRFRequired;
|
||||||
|
use OCP\AppFramework\Http\Attribute\UserRateLimit;
|
||||||
use OCP\AppFramework\Http\RedirectResponse;
|
use OCP\AppFramework\Http\RedirectResponse;
|
||||||
use OCP\AppFramework\Http\DataResponse;
|
use OCP\AppFramework\Http\DataResponse;
|
||||||
use OCP\AppFramework\Http\TemplateResponse;
|
use OCP\AppFramework\Http\TemplateResponse;
|
||||||
@@ -76,6 +77,19 @@ class CalendarController extends Controller
|
|||||||
*/
|
*/
|
||||||
private const MAX_CONTACT_SUGGESTIONS = 50;
|
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 IManager $calendarManager;
|
||||||
private IContactsManager $contactsManager;
|
private IContactsManager $contactsManager;
|
||||||
private IAppManager $appManager;
|
private IAppManager $appManager;
|
||||||
@@ -310,8 +324,21 @@ class CalendarController extends Controller
|
|||||||
*/
|
*/
|
||||||
#[NoAdminRequired]
|
#[NoAdminRequired]
|
||||||
#[NoCSRFRequired]
|
#[NoCSRFRequired]
|
||||||
|
#[UserRateLimit(
|
||||||
|
limit: self::SEARCH_RATE_LIMIT,
|
||||||
|
period: self::SEARCH_RATE_PERIOD
|
||||||
|
)]
|
||||||
public function searchContacts(string $query = ''): DataResponse
|
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));
|
return new DataResponse($this->contactSuggestions($query));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -9,6 +9,7 @@ use OCP\App\IAppManager;
|
|||||||
use OCP\AppFramework\Http\DataResponse;
|
use OCP\AppFramework\Http\DataResponse;
|
||||||
use OCP\AppFramework\Http\RedirectResponse;
|
use OCP\AppFramework\Http\RedirectResponse;
|
||||||
use OCP\AppFramework\Http\TemplateResponse;
|
use OCP\AppFramework\Http\TemplateResponse;
|
||||||
|
use OCP\AppFramework\Http\Attribute\UserRateLimit;
|
||||||
use OCP\Calendar\ICalendar;
|
use OCP\Calendar\ICalendar;
|
||||||
use OCP\Calendar\ICalendarEventBuilder;
|
use OCP\Calendar\ICalendarEventBuilder;
|
||||||
use OCP\Calendar\ICreateFromString;
|
use OCP\Calendar\ICreateFromString;
|
||||||
@@ -1085,6 +1086,34 @@ class CalendarControllerTest extends TestCase
|
|||||||
$this->assertCount(50, $response->getData());
|
$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
|
public function testSearchContactsTruncatesQuery(): void
|
||||||
{
|
{
|
||||||
$this->contactsManager->expects($this->once())->method('search')
|
$this->contactsManager->expects($this->once())->method('search')
|
||||||
|
|||||||
Reference in new issue
Block a user