diff options
author | Joas Schilling <213943+nickvergessen@users.noreply.github.com> | 2020-12-18 12:46:10 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-12-18 12:46:10 +0300 |
commit | 3ba613b97507f0c068015111794c07444339edd6 (patch) | |
tree | 80b44c21cf529863496ab09baa67dcfae0322947 | |
parent | e841af7a9956dd8c8e755c97be8ea27b38dce6a5 (diff) | |
parent | 5d033454d684f766566a87fa886ccfcd25d78ec2 (diff) |
Merge pull request #4797 from nextcloud/backport/4656/stable19-fix-user-management-in-password-request-rooms
[stable19] Fix user management in password request rooms
-rw-r--r-- | docs/participant.md | 2 | ||||
-rw-r--r-- | lib/Controller/RoomController.php | 2 | ||||
-rw-r--r-- | lib/PublicShareAuth/Listener.php | 42 | ||||
-rw-r--r-- | src/components/RightSidebar/Participants/ParticipantsTab.vue | 1 | ||||
-rw-r--r-- | src/components/RightSidebar/RightSidebar.vue | 2 | ||||
-rw-r--r-- | tests/integration/features/bootstrap/FeatureContext.php | 89 | ||||
-rw-r--r-- | tests/integration/features/conversation/password-request.feature | 202 |
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 | [] | |