From ac0bfcc24b4af4c8c884cfd2603b77ed9e5b06c1 Mon Sep 17 00:00:00 2001
From: Anna Larch <anna@nextcloud.com>
Date: Thu, 5 Dec 2024 13:46:49 +0100
Subject: [PATCH] fix(conversations): fix password enforcement logic error

Signed-off-by: Anna Larch <anna@nextcloud.com>
---
 lib/Manager.php                                |  1 +
 lib/Service/RoomService.php                    |  8 +++++---
 .../features/chat-1/password.feature           | 12 ++++++++++++
 .../integration/features/chat-3/public.feature |  6 ++++++
 tests/php/Service/RoomServiceTest.php          | 18 ++++++++++++------
 5 files changed, 36 insertions(+), 9 deletions(-)

diff --git a/lib/Manager.php b/lib/Manager.php
index a4e87a89efb..c25ac9080f2 100644
--- 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
index 9d0632c6902..59b5228e421 100644
--- 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
index 4c45438ca9d..059c83373e0 100644
--- 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
index a472916a3cf..8fab46cc9eb 100644
--- 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
index 4a315f87035..46764217719 100644
--- 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 {