fix(ban): Correctly allow IP ranges

Signed-off-by: Joas Schilling <coding@schilljs.com>
nextcloud · Aug 12, 2024 · e662bc4 · e662bc4
1 parent d6f329e
commit e662bc4
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 7 deletions.
diff --git a/lib/Service/BanService.php b/lib/Service/BanService.php
@@ -62,9 +62,9 @@ public function createBan(Room $room, string $moderatorActorType, string $modera
 			} catch (\InvalidArgumentException) {
 				// Not an IP, check if it's a range
 				try {
-					$this->ipFactory->addressFromString($bannedActorId);
+					$this->ipFactory->rangeFromString($bannedActorId);
 				} catch (\InvalidArgumentException) {
-					// Not an IP, see if it's a range
+					// Not an IP range either
 					throw new \InvalidArgumentException('bannedActor');
 				}
 			}

diff --git a/tests/integration/features/bootstrap/FeatureContext.php b/tests/integration/features/bootstrap/FeatureContext.php
@@ -1547,7 +1547,7 @@ public function userRemovesAttendeeFromRoom(string $user, string $actorType, str
 	}
 
 	/**
-	 * @When /^user "([^"]*)" bans (user|group|email|remote|guest) "([^"]*)" from room "([^"]*)" with (\d+) \((v1)\)$/
+	 * @When /^user "([^"]*)" bans ([^ ]*) "([^"]*)" from room "([^"]*)" with (\d+) \((v1)\)$/
 	 */
 	public function userBansUserFromRoom(string $user, string $actorType, string $actorId, string $identifier, int $statusCode, string $apiVersion = 'v1', ?TableNode $internalNote = null): void {
 		if ($actorType === 'guest') {
@@ -1561,7 +1561,9 @@ public function userBansUserFromRoom(string $user, string $actorType, string $ac
 			}
 		}
 
-		$actorType .= 's';
+		if ($actorType !== 'ip') {
+			$actorType .= 's';
+		}
 
 		$this->setCurrentUser($user);
 		$body = [
@@ -1581,17 +1583,16 @@ public function userBansUserFromRoom(string $user, string $actorType, string $ac
 			'POST', '/apps/spreed/api/' . $apiVersion . '/ban/' . self::$identifierToToken[$identifier], $body
 		);
 
-		$this->assertStatusCode($this->response, $statusCode);
+		$data = $this->getDataFromResponse($this->response);
+		$this->assertStatusCode($this->response, $statusCode, print_r($data, true));
 
 		if ($statusCode === 200) {
-			$data = $this->getDataFromResponse($this->response);
 			self::$userToBanId[self::$identifierToToken[$identifier]] ??= [];
 			self::$userToBanId[self::$identifierToToken[$identifier]][$actorType] ??= [];
 			self::$userToBanId[self::$identifierToToken[$identifier]][$actorType][$actorId] = $data['id'];
 		} else if ($internalNote !== null) {
 			$internalNoteData = $internalNote->getRowsHash();
 			if (isset($internalNoteData['error'])) {
-				$data = $this->getDataFromResponse($this->response);
 				Assert::assertSame($internalNoteData['error'], $data['error']);
 			}
 		}

diff --git a/tests/integration/features/conversation-1/ban.feature b/tests/integration/features/conversation-1/ban.feature
@@ -188,3 +188,35 @@ Feature: conversation/ban
       | invite   | participant2 |
     And user "participant1" bans user "participant2" from room "one-to-one room" with 400 (v1)
       | error | room |
+
+  Scenario: Invalid banned actor type
+    Given user "participant1" creates room "room" (v4)
+      | roomType | 3 |
+      | roomName | room |
+    And user "participant1" bans range "participant2" from room "room" with 400 (v1)
+      | error | bannedActor |
+
+  Scenario: Invalid IP address
+    Given user "participant1" creates room "room" (v4)
+      | roomType | 3 |
+      | roomName | room |
+    And user "participant1" bans ip "participant2" from room "room" with 400 (v1)
+      | error | bannedActor |
+
+  Scenario: Invalid IP address range
+    Given user "participant1" creates room "room" (v4)
+      | roomType | 3 |
+      | roomName | room |
+    And user "participant1" bans ip "127.0.0.1/64" from room "room" with 400 (v1)
+      | error | bannedActor |
+
+  Scenario: Test valid IP bans
+    Given user "participant1" creates room "room" (v4)
+      | roomType | 3 |
+      | roomName | room |
+    And user "participant1" bans ip "127.0.0.1" from room "room" with 200 (v1)
+    And user "participant1" bans ip "127.0.0.1/24" from room "room" with 200 (v1)
+    And user "participant1" bans ip "127.0.0.1/32" from room "room" with 200 (v1)
+    And user "participant1" bans ip "::1" from room "room" with 200 (v1)
+    And user "participant1" bans ip "::1/32" from room "room" with 200 (v1)
+    And user "participant1" bans ip "::1/64" from room "room" with 200 (v1)