fix(conversations): fix password enforcement logic error

Signed-off-by: Anna Larch <[email protected]>
nextcloud · Dec 11, 2024 · ac0bfcc · ac0bfcc
1 parent a477e6c
commit ac0bfcc
Show file tree

Hide file tree

Showing 5 changed files with 36 additions and 9 deletions.
diff --git a/lib/Manager.php b/lib/Manager.php
@@ -1107,6 +1107,7 @@ public function getChangelogRoom(string $userId): Room {
 	 * @param string $name
 	 * @param string $objectType
 	 * @param string $objectId
+	 * @param string $password
 	 * @return Room
 	 */
 	public function createRoom(int $type, string $name = '', string $objectType = '', string $objectId = '', string $password = ''): Room {

diff --git a/lib/Service/RoomService.php b/lib/Service/RoomService.php
@@ -165,10 +165,12 @@ public function createConversation(int $type, string $name, ?IUser $owner = null
 			throw new InvalidArgumentException('object');
 		}
 
-		if ($type !== Room::TYPE_PUBLIC || !$this->config->isPasswordEnforced()) {
-			$room = $this->manager->createRoom($type, $name, $objectType, $objectId);
-		} elseif ($password === '') {
+		if ($type === Room::TYPE_PUBLIC && $password === '' && $this->config->isPasswordEnforced()) {
 			throw new PasswordException(PasswordException::REASON_VALUE, $this->l10n->t('Password needs to be set'));
+		}
+
+		if ($type !== Room::TYPE_PUBLIC) {
+			$room = $this->manager->createRoom($type, $name, $objectType, $objectId);
 		} else {
 			$event = new ValidatePasswordPolicyEvent($password);
 			try {

diff --git a/tests/integration/features/chat-1/password.feature b/tests/integration/features/chat-1/password.feature
@@ -25,6 +25,18 @@ Feature: chat/password
       | room                           | actorType | actorId      | actorDisplayName         | message   | messageParameters |
       | public password protected room | users     | participant2 | participant2-displayname | Message 1 | []                |
 
+  Scenario: invited user can send and receive chat messages to and from public password protected room with initial password
+    Given user "participant1" creates room "public password protected room" (v4)
+      | roomType | 3 |
+      | roomName | room |
+      | password | ARoomPassword123. |
+    And user "participant1" sets password "foobar" for room "public password protected room" with 200 (v4)
+    And user "participant1" adds user "participant2" to room "public password protected room" with 200 (v4)
+    When user "participant2" sends message "Message 1" to room "public password protected room" with 201
+    Then user "participant2" sees the following messages in room "public password protected room" with 200
+      | room                           | actorType | actorId      | actorDisplayName         | message   | messageParameters |
+      | public password protected room | users     | participant2 | participant2-displayname | Message 1 | []                |
+
   Scenario: not invited but joined with password user can send and receive chat messages to and from public password protected room
     Given user "participant1" creates room "public password protected room" (v4)
       | roomType | 3 |

diff --git a/tests/integration/features/chat-3/public.feature b/tests/integration/features/chat-3/public.feature
@@ -111,3 +111,9 @@ Feature: chat-2/public
     And user "participant1" renames room "public room" to "A name with 260 chars 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 12345678" with 400 (v4)
     # 255 chars
     And user "participant1" renames room "public room" to "Another name with 255 chars 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 1234567" with 200 (v4)
+
+  Scenario: Create public room with a password
+    When user "participant1" creates room "public room" with 201 (v4)
+      | roomType | 3 |
+      | roomName | A room name |
+      | password | ARoomPassword123. |
diff --git a/tests/php/Service/RoomServiceTest.php b/tests/php/Service/RoomServiceTest.php
@@ -246,16 +246,17 @@ public function testCreateConversationInvalidObjects(string $type, string $id, s
 
 	public static function dataCreateConversation(): array {
 		return [
-			[Room::TYPE_GROUP, 'Group conversation', 'admin', '', ''],
-			[Room::TYPE_PUBLIC, 'Public conversation', '', 'files', '123456'],
-			[Room::TYPE_CHANGELOG, 'Talk updates ✅', 'test1', 'changelog', 'conversation'],
+			[Room::TYPE_GROUP, 'Group conversation', 'admin', '', '', ''],
+			[Room::TYPE_PUBLIC, 'Public conversation', '', 'files', '123456', ''],
+			[Room::TYPE_PUBLIC, 'Public conversation', '', 'files', '123456', 'AGoodPassword123?'],
+			[Room::TYPE_CHANGELOG, 'Talk updates ✅', 'test1', 'changelog', 'conversation', ''],
 		];
 	}
 
 	/**
 	 * @dataProvider dataCreateConversation
 	 */
-	public function testCreateConversation(int $type, string $name, string $ownerId, string $objectType, string $objectId): void {
+	public function testCreateConversation(int $type, string $name, string $ownerId, string $objectType, string $objectId, string $password): void {
 		$room = $this->createMock(Room::class);
 
 		if ($ownerId !== '') {
@@ -279,12 +280,17 @@ public function testCreateConversation(int $type, string $name, string $ownerId,
 				->method('addUsers');
 		}
 
+		if ($password !== '') {
+			$this->hasher->expects(self::once())
+				->method('hash')
+				->willReturn($password);
+		}
 		$this->manager->expects($this->once())
 			->method('createRoom')
-			->with($type, $name, $objectType, $objectId)
+			->with($type, $name, $objectType, $objectId, $password)
 			->willReturn($room);
 
-		$this->assertSame($room, $this->service->createConversation($type, $name, $owner, $objectType, $objectId));
+		$this->assertSame($room, $this->service->createConversation($type, $name, $owner, $objectType, $objectId, $password));
 	}
 
 	public static function dataPrepareConversationName(): array {