Skip to content
Prev Previous commit
Next Next commit
Fix owner being able to add more users to a password request room
Only the owner and another participant will be allowed to join a
password request room, so there is no point in being able to add more
participants to those rooms.

Although throwing the exception in the listener is enough to prevent
adding the participants unhandled exceptions in the endpoint are
returned as error 404, but the expected error would be 400. To minimize
conflicts with other pull requests and backports it is explicitly
checked if the room is a password request room instead of refactoring
the code to handle the exception.

Signed-off-by: Daniel Calviño Sánchez <[email protected]>
  • Loading branch information
danxuliu committed Dec 18, 2020
commit 3d9bcbb30123e2621b54a91319e0ff49d540a255
2 changes: 1 addition & 1 deletion docs/participant.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,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
Expand Down
2 changes: 1 addition & 1 deletion lib/Controller/RoomController.php
Original file line number Diff line number Diff line change
Expand Up @@ -944,7 +944,7 @@ public function getParticipants(bool $includeStatus = false): DataResponse {
* @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);
}

Expand Down
34 changes: 34 additions & 0 deletions lib/PublicShareAuth/Listener.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

namespace OCA\Talk\PublicShareAuth;

use OCA\Talk\Events\AddParticipantsEvent;
use OCA\Talk\Events\JoinRoomGuestEvent;
use OCA\Talk\Events\JoinRoomUserEvent;
use OCA\Talk\Events\RoomEvent;
Expand Down Expand Up @@ -57,6 +58,11 @@ public static function register(IEventDispatcher $dispatcher): void {
};
$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());
};
Expand Down Expand Up @@ -113,6 +119,34 @@ public static function preventExtraGuestsFromJoining(Room $room): void {
}
}

/**
* 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;
}

// 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');
}
}

/**
* Destroys the PublicShareAuth room as soon as one of the participant
* leaves the room.
Expand Down
11 changes: 11 additions & 0 deletions tests/integration/features/conversation/password-request.feature
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,17 @@ Feature: conversation/password-request



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 |
Expand Down