Merge pull request #10545 from nextcloud/bugfix/noid/split-index-cont…

…roller-method fix(page): Decouple the index controller from the executing method
nextcloud · Sep 21, 2023 · 939f0d3 · 939f0d3
2 parents 3916ee2 + 2b36f0f
commit 939f0d3
Show file tree

Hide file tree

Showing 3 changed files with 59 additions and 6 deletions.
diff --git a/lib/Controller/PageController.php b/lib/Controller/PageController.php
@@ -127,33 +127,46 @@ 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
 	 */
 	#[NoCSRFRequired]
 	#[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) {

diff --git 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
@@ -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