Merge pull request #4797 from nextcloud/backport/4656/stable19-fix-user-management-in-password-request-rooms

[stable19] Fix user management in password request rooms

7 files changed, 326 insertions, 14 deletions

diff --git a/docs/participant.md b/docs/participant.md
index bb72dd0e4..ea014f9cf 100644
--- a/docs/participant.md
+++ b/docs/participant.md
@@ -40,7 +40,7 @@ Base endpoint is: `/ocs/v2.php/apps/spreed/api/v1`
     - Status code:
         + `200 OK`
         + `400 Bad Request` When the source type is unknown, currently `users`, `groups`, `emails` are supported. `circles` are supported with `circles-support` capability
-        + `400 Bad Request` When the conversation is a one-to-one conversation
+        + `400 Bad Request` When the conversation is a one-to-one conversation or a conversation to request a password for a share
         + `403 Forbidden` When the current user is not a moderator or owner
         + `404 Not Found` When the conversation could not be found for the participant
         + `404 Not Found` When the user or group to add could not be found
diff --git a/lib/Controller/RoomController.php b/lib/Controller/RoomController.php
index 04a0b06b5..65d6f56dd 100644
--- a/lib/Controller/RoomController.php
+++ b/lib/Controller/RoomController.php
@@ -903,7 +903,7 @@ class RoomController extends AEnvironmentAwareController {
 	 * @return DataResponse
 	 */
 	public function addParticipantToRoom(string $newParticipant, string $source = 'users'): DataResponse {
-		if ($this->room->getType() === Room::ONE_TO_ONE_CALL) {
+		if ($this->room->getType() === Room::ONE_TO_ONE_CALL || $this->room->getObjectType() === 'share:password') {
 			return new DataResponse([], Http::STATUS_BAD_REQUEST);
 		}
 
diff --git a/lib/PublicShareAuth/Listener.php b/lib/PublicShareAuth/Listener.php
index a0518504f..192c9f262 100644
--- a/lib/PublicShareAuth/Listener.php
+++ b/lib/PublicShareAuth/Listener.php
@@ -24,6 +24,7 @@ declare(strict_types=1);
 
 namespace OCA\Talk\PublicShareAuth;
 
+use OCA\Talk\Events\AddParticipantsEvent;
 use OCA\Talk\Events\JoinRoomGuestEvent;
 use OCA\Talk\Events\JoinRoomUserEvent;
 use OCA\Talk\Events\RoomEvent;
@@ -57,6 +58,11 @@ class Listener {
 		};
 		$dispatcher->addListener(Room::EVENT_BEFORE_GUEST_CONNECT, $listener);
 
+		$listener = static function (AddParticipantsEvent $event) {
+			self::preventExtraUsersFromBeingAdded($event->getRoom(), $event->getParticipants());
+		};
+		$dispatcher->addListener(Room::EVENT_BEFORE_USERS_ADD, $listener);
+
 		$listener = static function (RoomEvent $event) {
 			self::destroyRoomOnParticipantLeave($event->getRoom());
 		};
@@ -89,7 +95,7 @@ class Listener {
 		} catch (ParticipantNotFoundException $e) {
 		}
 
-		if ($room->getActiveGuests() > 0 || \count($room->getParticipantUserIds()) > 1) {
+		if ($room->getNumberOfParticipants(false) > 1) {
 			throw new \OverflowException('Only the owner and another participant are allowed in rooms to request the password for a share');
 		}
 	}
@@ -108,7 +114,39 @@ class Listener {
 			return;
 		}
 
-		if ($room->getActiveGuests() > 0 || \count($room->getParticipantUserIds()) > 1) {
+		if ($room->getNumberOfParticipants(false) > 1) {
+			throw new \OverflowException('Only the owner and another participant are allowed in rooms to request the password for a share');
+		}
+	}
+
+	/**
+	 * Prevents other users from being added to the room (as they will not be
+	 * able to join).
+	 *
+	 * This method should be called before a user is added to a room.
+	 *
+	 * @param Room $room
+	 * @param array[] $participants
+	 * @throws \OverflowException
+	 */
+	public static function preventExtraUsersFromBeingAdded(Room $room, array $participants): void {
+		if ($room->getObjectType() !== 'share:password') {
+			return;
+		}
+
+		if (empty($participants)) {
+			return;
+		}
+
+		// Events with more than one participant can be directly aborted, as
+		// when the owner is added during room creation or a user self-joins the
+		// event will always have just one participant.
+		if (count($participants) > 1) {
+			throw new \OverflowException('Only the owner and another participant are allowed in rooms to request the password for a share');
+		}
+
+		$participant = $participants[0];
+		if ($participant['participantType'] !== Participant::OWNER && $participant['participantType'] !== Participant::USER_SELF_JOINED) {
 			throw new \OverflowException('Only the owner and another participant are allowed in rooms to request the password for a share');
 		}
 	}
diff --git a/src/components/RightSidebar/Participants/ParticipantsTab.vue b/src/components/RightSidebar/Participants/ParticipantsTab.vue
index 6397e7da9..3ec9d12cf 100644
--- a/src/components/RightSidebar/Participants/ParticipantsTab.vue
+++ b/src/components/RightSidebar/Participants/ParticipantsTab.vue
@@ -208,6 +208,7 @@ export default {
 				this.cancelableGetParticipants()
 			} catch (exception) {
 				console.debug(exception)
+				showError(t('spreed', 'An error occurred while adding the participants'))
 			}
 		},
 
diff --git a/src/components/RightSidebar/RightSidebar.vue b/src/components/RightSidebar/RightSidebar.vue
index c48fc0330..a98d48395 100644
--- a/src/components/RightSidebar/RightSidebar.vue
+++ b/src/components/RightSidebar/RightSidebar.vue
@@ -156,7 +156,7 @@ export default {
 		displaySearchBox() {
 			return this.canFullModerate
 				&& (this.conversation.type === CONVERSATION.TYPE.GROUP
-					|| this.conversation.type === CONVERSATION.TYPE.PUBLIC)
+					|| (this.conversation.type === CONVERSATION.TYPE.PUBLIC && this.conversation.objectType !== 'share:password'))
 		},
 		isSearching() {
 			return this.searchText !== ''
diff --git a/tests/integration/features/bootstrap/FeatureContext.php b/tests/integration/features/bootstrap/FeatureContext.php
index 891f30d8e..b3ffaf4c3 100644
--- a/tests/integration/features/bootstrap/FeatureContext.php
+++ b/tests/integration/features/bootstrap/FeatureContext.php
@@ -143,6 +143,14 @@ class FeatureContext implements Context, SnippetAcceptingContext {
 			return;
 		}
 
+		$this->assertRooms($rooms, $formData);
+	}
+
+	/**
+	 * @param array $rooms
+	 * @param TableNode $formData
+	 */
+	private function assertRooms($rooms, TableNode $formData) {
 		Assert::assertCount(count($formData->getHash()), $rooms, 'Room count does not match');
 		Assert::assertEquals($formData->getHash(), array_map(function ($room, $expectedRoom) {
 			$participantNames = array_map(function ($participant) {
@@ -169,12 +177,30 @@ class FeatureContext implements Context, SnippetAcceptingContext {
 				$participantNames[$lastParticipantKey] .= ' [exact order]';
 			}
 
-			return [
-				'id' => self::$tokenToIdentifier[$room['token']],
-				'type' => (string) $room['type'],
-				'participantType' => (string) $room['participantType'],
-				'participants' => implode(', ', $participantNames),
-			];
+			$data = [];
+			if (isset($expectedRoom['id'])) {
+				$data['id'] = self::$tokenToIdentifier[$room['token']];
+			}
+			if (isset($expectedRoom['name'])) {
+				$data['name'] = $room['name'];
+			}
+			if (isset($expectedRoom['type'])) {
+				$data['type'] = (string) $room['type'];
+			}
+			if (isset($expectedRoom['hasPassword'])) {
+				$data['hasPassword'] = (string) $room['hasPassword'];
+			}
+			if (isset($expectedRoom['readOnly'])) {
+				$data['readOnly'] = (string) $room['readOnly'];
+			}
+			if (isset($expectedRoom['participantType'])) {
+				$data['participantType'] = (string) $room['participantType'];
+			}
+			if (isset($expectedRoom['participants'])) {
+				$data['participants'] = implode(', ', $participantNames);
+			}
+
+			return $data;
 		}, $rooms, $formData->getHash()));
 	}
 
@@ -184,10 +210,11 @@ class FeatureContext implements Context, SnippetAcceptingContext {
 	 * @param string $user
 	 * @param string $isOrNotParticipant
 	 * @param string $identifier
+	 * @param TableNode|null $formData
 	 */
-	public function userIsParticipantOfRoom($user, $isOrNotParticipant, $identifier) {
+	public function userIsParticipantOfRoom($user, $isOrNotParticipant, $identifier, TableNode $formData = null) {
 		if (strpos($user, 'guest') === 0) {
-			$this->guestIsParticipantOfRoom($user, $isOrNotParticipant, $identifier);
+			$this->guestIsParticipantOfRoom($user, $isOrNotParticipant, $identifier, $formData);
 
 			return;
 		}
@@ -211,6 +238,15 @@ class FeatureContext implements Context, SnippetAcceptingContext {
 		foreach ($rooms as $room) {
 			if (self::$tokenToIdentifier[$room['token']] === $identifier) {
 				Assert::assertEquals($isParticipant, true, 'Room ' . $identifier . ' found in user´s room list');
+
+				if ($formData) {
+					$this->sendRequest('GET', '/apps/spreed/api/v1/room/' . self::$identifierToToken[$identifier]);
+
+					$rooms = [$this->getDataFromResponse($this->response)];
+
+					$this->assertRooms($rooms, $formData);
+				}
+
 				return;
 			}
 		}
@@ -222,8 +258,9 @@ class FeatureContext implements Context, SnippetAcceptingContext {
 	 * @param string $guest
 	 * @param string $isOrNotParticipant
 	 * @param string $identifier
+	 * @param TableNode|null $formData
 	 */
-	private function guestIsParticipantOfRoom($guest, $isOrNotParticipant, $identifier) {
+	private function guestIsParticipantOfRoom($guest, $isOrNotParticipant, $identifier, TableNode $formData = null) {
 		$this->setCurrentUser($guest);
 		$this->sendRequest('GET', '/apps/spreed/api/v1/room/' . self::$identifierToToken[$identifier]);
 
@@ -231,6 +268,12 @@ class FeatureContext implements Context, SnippetAcceptingContext {
 
 		$isParticipant = $isOrNotParticipant === 'is';
 
+		if ($formData) {
+			$rooms = [$response];
+
+			$this->assertRooms($rooms, $formData);
+		}
+
 		if ($isParticipant) {
 			$this->assertStatusCode($this->response, 200);
 			Assert::assertEquals(self::$userToSessionId[$guest], $response['sessionId']);
@@ -405,6 +448,30 @@ class FeatureContext implements Context, SnippetAcceptingContext {
 	}
 
 	/**
+	 * @Then /^user "([^"]*)" creates the password request room for last share with (\d+)$/
+	 *
+	 * @param string $user
+	 * @param int $statusCode
+	 */
+	public function userCreatesThePasswordRequestRoomForLastShare($user, $statusCode) {
+		$shareToken = $this->sharingContext->getLastShareToken();
+
+		$this->setCurrentUser($user);
+		$this->sendRequest('POST', '/apps/spreed/api/v1/publicshareauth', ['shareToken' => $shareToken]);
+		$this->assertStatusCode($this->response, $statusCode);
+
+		if ($statusCode !== '201') {
+			return;
+		}
+
+		$response = $this->getDataFromResponse($this->response);
+
+		$identifier = 'password request for last share room';
+		self::$identifierToToken[$identifier] = $response['token'];
+		self::$tokenToIdentifier[$response['token']] = $identifier;
+	}
+
+	/**
 	 * @Then /^user "([^"]*)" joins room "([^"]*)" with (\d+)$/
 	 *
 	 * @param string $user
@@ -420,6 +487,10 @@ class FeatureContext implements Context, SnippetAcceptingContext {
 		);
 		$this->assertStatusCode($this->response, $statusCode);
 
+		if ($statusCode !== '200') {
+			return;
+		}
+
 		$response = $this->getDataFromResponse($this->response);
 		if (array_key_exists('sessionId', $response)) {
 			// In the chat guest users are identified by their sessionId. The
diff --git a/tests/integration/features/conversation/password-request.feature b/tests/integration/features/conversation/password-request.feature
new file mode 100644
index 000000000..d8a1b83fe
--- /dev/null
+++ b/tests/integration/features/conversation/password-request.feature
@@ -0,0 +1,202 @@
+Feature: conversation/password-request
+
+  Background:
+    Given user "participant1" exists
+    Given user "participant2" exists
+    Given user "participant3" exists
+
+  Scenario: create password-request room for file shared by link
+    Given user "participant1" shares "welcome.txt" by link with OCS 100
+      | password | 123456 |
+      | sendPasswordByTalk | true |
+    When user "guest" creates the password request room for last share with 201
+    Then user "participant1" is participant of room "password request for last share room"
+      | name        | type | participantType | participants |
+      | welcome.txt | 3    | 1               | participant1-displayname |
+    And user "guest" is not participant of room "password request for last share room"
+
+  Scenario: create password-request room for folder shared by link
+    Given user "participant1" creates folder "/test"
+    And user "participant1" shares "test" by link with OCS 100
+      | password | 123456 |
+      | sendPasswordByTalk | true |
+    When user "guest" creates the password request room for last share with 201
+    Then user "participant1" is participant of room "password request for last share room"
+      | name | type | participantType | participants |
+      | test | 3    | 1               | participant1-displayname |
+    And user "guest" is not participant of room "password request for last share room"
+
+  Scenario: create password-request room for folder reshared by link
+    Given user "participant1" creates folder "/test"
+    And user "participant1" shares "test" with user "participant2" with OCS 100
+    And user "participant2" shares "test" by link with OCS 100
+      | password | 123456 |
+      | sendPasswordByTalk | true |
+    When user "guest" creates the password request room for last share with 201
+    Then user "participant2" is participant of room "password request for last share room"
+      | name | type | participantType | participants |
+      | test | 3    | 1               | participant2-displayname |
+    And user "participant1" is not participant of room "password request for last share room"
+    And user "guest" is not participant of room "password request for last share room"
+
+  Scenario: create password-request room for file shared by link but not protected by Talk
+    Given user "participant1" shares "welcome.txt" by link with OCS 100
+      | password | 123456 |
+    When user "guest" creates the password request room for last share with 404
+
+
+
+  # Creating and joining the password request room is a two steps process.
+  # Technically one guest or user could create the room and a different one
+  # join it, but it does not really matter who created the room, only who joins
+  # it and talks with the owner (and, besides that, the WebUI joins the room
+  # immediately after creating it).
+
+  Scenario: guest can join the password request room
+    Given user "participant1" shares "welcome.txt" by link with OCS 100
+      | password | 123456 |
+      | sendPasswordByTalk | true |
+    And user "guest" creates the password request room for last share with 201
+    When user "guest" joins room "password request for last share room" with 200
+    Then user "guest" is participant of room "password request for last share room"
+
+  Scenario: user can join the password request room
+    Given user "participant1" shares "welcome.txt" by link with OCS 100
+      | password | 123456 |
+      | sendPasswordByTalk | true |
+    And user "participant2" creates the password request room for last share with 201
+    When user "participant2" joins room "password request for last share room" with 200
+    Then user "participant2" is participant of room "password request for last share room"
+
+  Scenario: owner can join the password request room
+    Given user "participant1" shares "welcome.txt" by link with OCS 100
+      | password | 123456 |
+      | sendPasswordByTalk | true |
+    And user "guest" creates the password request room for last share with 201
+    When user "participant1" joins room "password request for last share room" with 200
+
+  Scenario: other guests can not join the password request room when a guest already joined
+    Given user "participant1" shares "welcome.txt" by link with OCS 100
+      | password | 123456 |
+      | sendPasswordByTalk | true |
+    And user "guest" creates the password request room for last share with 201
+    And user "guest" joins room "password request for last share room" with 200
+    When user "guest2" joins room "password request for last share room" with 404
+    Then user "guest2" is not participant of room "password request for last share room"
+
+  Scenario: other guests can not join the password request room when a user already joined
+    Given user "participant1" shares "welcome.txt" by link with OCS 100
+      | password | 123456 |
+      | sendPasswordByTalk | true |
+    And user "participant2" creates the password request room for last share with 201
+    And user "participant2" joins room "password request for last share room" with 200
+    When user "guest" joins room "password request for last share room" with 404
+    Then user "guest" is not participant of room "password request for last share room"
+
+  Scenario: other users can not join the password request room when a guest already joined
+    Given user "participant1" shares "welcome.txt" by link with OCS 100
+      | password | 123456 |
+      | sendPasswordByTalk | true |
+    And user "guest" creates the password request room for last share with 201
+    And user "guest" joins room "password request for last share room" with 200
+    When user "participant2" joins room "password request for last share room" with 404
+    Then user "participant2" is not participant of room "password request for last share room"
+
+  Scenario: other users can not join the password request room when a user already joined
+    Given user "participant1" shares "welcome.txt" by link with OCS 100
+      | password | 123456 |
+      | sendPasswordByTalk | true |
+    And user "participant2" creates the password request room for last share with 201
+    And user "participant2" joins room "password request for last share room" with 200
+    When user "participant3" joins room "password request for last share room" with 404
+    Then user "participant3" is not participant of room "password request for last share room"
+
+
+
+  Scenario: owner can not add other users to a password request room
+    Given user "participant1" shares "welcome.txt" by link with OCS 100
+      | password | 123456 |
+      | sendPasswordByTalk | true |
+    And user "guest" creates the password request room for last share with 201
+    And user "participant1" joins room "password request for last share room" with 200
+    When user "participant1" adds "participant2" to room "password request for last share room" with 400
+    Then user "participant2" is not participant of room "password request for last share room"
+
+
+
+  Scenario: guest leaves the password request room
+    Given user "participant1" shares "welcome.txt" by link with OCS 100
+      | password | 123456 |
+      | sendPasswordByTalk | true |
+    And user "guest" creates the password request room for last share with 201
+    And user "guest" joins room "password request for last share room" with 200
+    And user "participant1" joins room "password request for last share room" with 200
+    When user "guest" leaves room "password request for last share room" with 200
+    Then user "participant1" is not participant of room "password request for last share room"
+    And user "guest" is not participant of room "password request for last share room"
+
+  Scenario: user leaves the password request room
+    Given user "participant1" shares "welcome.txt" by link with OCS 100
+      | password | 123456 |
+      | sendPasswordByTalk | true |
+    And user "participant2" creates the password request room for last share with 201
+    And user "participant2" joins room "password request for last share room" with 200
+    And user "participant1" joins room "password request for last share room" with 200
+    When user "participant2" leaves room "password request for last share room" with 200
+    Then user "participant1" is not participant of room "password request for last share room"
+    And user "participant2" is not participant of room "password request for last share room"
+
+  Scenario: owner leaves the password request room
+    Given user "participant1" shares "welcome.txt" by link with OCS 100
+      | password | 123456 |
+      | sendPasswordByTalk | true |
+    And user "guest" creates the password request room for last share with 201
+    And user "guest" joins room "password request for last share room" with 200
+    And user "participant1" joins room "password request for last share room" with 200
+    When user "participant1" leaves room "password request for last share room" with 200
+    Then user "participant1" is not participant of room "password request for last share room"
+    And user "guest" is not participant of room "password request for last share room"
+
+
+
+  Scenario: guest can start a call
+    Given user "participant1" shares "welcome.txt" by link with OCS 100
+      | password | 123456 |
+      | sendPasswordByTalk | true |
+    And user "guest" creates the password request room for last share with 201
+    And user "guest" joins room "password request for last share room" with 200
+    When user "guest" joins call "password request for last share room" with 200
+    Then user "guest" sees 1 peers in call "password request for last share room" with 200
+    And user "participant1" sees 1 peers in call "password request for last share room" with 200
+
+  Scenario: owner can join a call
+    Given user "participant1" shares "welcome.txt" by link with OCS 100
+      | password | 123456 |
+      | sendPasswordByTalk | true |
+    And user "guest" creates the password request room for last share with 201
+    And user "guest" joins room "password request for last share room" with 200
+    And user "participant1" joins room "password request for last share room" with 200
+    And user "guest" joins call "password request for last share room" with 200
+    When user "participant1" joins call "password request for last share room" with 200
+    Then user "guest" sees 2 peers in call "password request for last share room" with 200
+    And user "participant1" sees 2 peers in call "password request for last share room" with 200
+
+
+
+  Scenario: participants can send and receive chat messages
+    Given user "participant1" shares "welcome.txt" by link with OCS 100
+      | password | 123456 |
+      | sendPasswordByTalk | true |
+    And user "guest" creates the password request room for last share with 201
+    And user "participant1" joins room "password request for last share room" with 200
+    And user "guest" joins room "password request for last share room" with 200
+    When user "participant1" sends message "Message 1" to room "password request for last share room" with 201
+    And user "guest" sends message "Message 2" to room "password request for last share room" with 201
+    Then user "participant1" sees the following messages in room "password request for last share room" with 200
+      | room                                  | actorType | actorId      | actorDisplayName         | message   | messageParameters |
+      | password request for last share room | guests    | guest        |                          | Message 2 | []                |
+      | password request for last share room | users     | participant1 | participant1-displayname | Message 1 | []                |
+    And user "guest" sees the following messages in room "password request for last share room" with 200
+      | room                                  | actorType | actorId      | actorDisplayName         | message   | messageParameters |
+      | password request for last share room | guests    | guest        |                          | Message 2 | []                |
+      | password request for last share room | users     | participant1 | participant1-displayname | Message 1 | []                |