From 05a2088bbaa63b2d00d966b237543c91ef083403 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Thu, 3 Aug 2023 10:20:30 +0200 Subject: [PATCH 1/2] fix(page): Decouple the index controller from the executing method Signed-off-by: Joas Schilling --- lib/Controller/PageController.php | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/lib/Controller/PageController.php b/lib/Controller/PageController.php index 5c2fb75398..37602cd3f4 100644 --- a/lib/Controller/PageController.php +++ b/lib/Controller/PageController.php @@ -156,25 +156,24 @@ public function showCall(string $token): Response { #[BruteForceProtection(action: 'talkRoomPassword')] public function authenticatePassword(string $token, string $password = ''): Response { // This is the entry point from the `/call/{token}` URL which is hardcoded in the server. - return $this->index($token, '', $password); + return $this->pageHandler($token, '', $password); } #[NoCSRFRequired] #[PublicPage] public function notFound(): Response { - return $this->index(); + return $this->pageHandler(); } #[NoCSRFRequired] #[PublicPage] public function duplicateSession(): Response { - return $this->index(); + return $this->pageHandler(); } /** * @param string $token * @param string $callUser - * @param string $password * @return TemplateResponse|RedirectResponse * @throws HintException */ @@ -182,7 +181,21 @@ public function duplicateSession(): Response { #[PublicPage] #[BruteForceProtection(action: 'talkRoomToken')] #[UseSession] - public function index(string $token = '', string $callUser = '', string $password = ''): Response { + public function index(string $token = '', string $callUser = ''): Response { + if ($callUser !== '') { + $token = ''; + } + return $this->pageHandler($token, $callUser); + } + + /** + * @param string $token + * @param string $callUser + * @param string $password + * @return TemplateResponse|RedirectResponse + * @throws HintException + */ + protected function pageHandler(string $token = '', string $callUser = '', string $password = ''): Response { $bruteForceToken = $token; $user = $this->userSession->getUser(); if (!$user instanceof IUser) { From 82e4ba1236f97fd3a0441e707cc9b373a6f6071e Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 30 Aug 2023 10:33:38 +0200 Subject: [PATCH 2/2] fix(page): Add integration test Signed-off-by: Joas Schilling --- .../features/bootstrap/FeatureContext.php | 24 +++++++++++++++++++ .../bruteforce-protection.feature | 18 +++++++++++++- 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/tests/integration/features/bootstrap/FeatureContext.php b/tests/integration/features/bootstrap/FeatureContext.php index a594f77338..751390999b 100644 --- a/tests/integration/features/bootstrap/FeatureContext.php +++ b/tests/integration/features/bootstrap/FeatureContext.php @@ -1088,6 +1088,30 @@ public function userViewsCallURL(string $user, string $identifier, int $statusCo } } + /** + * @Then /^user "([^"]*)" views URL "([^"]*)" with query parameters and status code (\d+)$/ + * + * @param string $user + * @param string $page + * @param int $statusCode + * @param null|TableNode $formData + */ + public function userViewsURLWithQuery(string $user, string $page, int $statusCode, TableNode $formData = null): void { + $parameters = []; + if ($formData instanceof TableNode) { + foreach ($formData->getRowsHash() as $key => $value) { + $parameters[$key] = $key === 'token' ? (self::$identifierToToken[$value] ?? $value) : $value; + } + } + + $this->setCurrentUser($user); + $this->sendFrontpageRequest( + 'GET', '/' . $page . '?' . http_build_query($parameters) + ); + + $this->assertStatusCode($this->response, $statusCode); + } + /** * @Then /^user "([^"]*)" sets notifications to (default|disabled|mention|all) for room "([^"]*)" \((v4)\)$/ * diff --git a/tests/integration/features/conversation/bruteforce-protection.feature b/tests/integration/features/conversation/bruteforce-protection.feature index ce08809775..7f15ee33c9 100644 --- a/tests/integration/features/conversation/bruteforce-protection.feature +++ b/tests/integration/features/conversation/bruteforce-protection.feature @@ -63,7 +63,6 @@ Feature: conversation/bruteforce-protection Then the following brute force attempts are registered And disable brute force protection - # Note: This test takes quite long … Scenario: User gets blocked after some attempts Given enable brute force protection Then the following brute force attempts are registered @@ -81,3 +80,20 @@ Feature: conversation/bruteforce-protection Then the following brute force attempts are registered | talkRoomToken | 11 | And disable brute force protection + + Scenario: Prevent brute forcing on an endpoint that is not meant to handle the password + Given enable brute force protection + And user "participant1" creates room "room" (v4) + | roomType | 3 | + | roomName | room | + And user "participant1" sets password "foobar" for room "room" with 200 (v4) + Then the following brute force attempts are registered + And user "participant2" joins room "room" with 403 (v4) + Then the following brute force attempts are registered + | talkRoomPassword | 1 | + When user "participant2" views URL "apps/spreed" with query parameters and status code 200 + | token | room | + | password | foobar | + Then the following brute force attempts are registered + | talkRoomPassword | 1 | + And disable brute force protection