From 7c1268acbce3884cb07a7447dba067d0ddbccfec Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 27 Apr 2022 17:05:07 +0200 Subject: [PATCH 1/7] Introduce a chat permission on the API Signed-off-by: Joas Schilling --- docs/constants.md | 1 + lib/Controller/ChatController.php | 4 + lib/Controller/ReactionController.php | 2 + lib/Exceptions/PermissionsException.php | 28 +++ lib/Middleware/InjectionMiddleware.php | 27 ++- lib/Model/Attendee.php | 2 + .../features/bootstrap/FeatureContext.php | 1 + .../integration/features/chat/delete.feature | 16 ++ tests/integration/features/chat/group.feature | 12 + .../integration/features/chat/public.feature | 13 ++ .../features/chat/rich-object-share.feature | 25 ++ .../set-participant-permissions.feature | 214 +++++++++--------- .../conversation-2/set-permissions.feature | 48 ++-- .../features/reaction/react.feature | 24 ++ 14 files changed, 285 insertions(+), 132 deletions(-) create mode 100644 lib/Exceptions/PermissionsException.php diff --git a/docs/constants.md b/docs/constants.md index 108bfb6689b..f170118c8c6 100644 --- a/docs/constants.md +++ b/docs/constants.md @@ -70,6 +70,7 @@ title: Constants * `16` Can publish audio stream * `32` Can publish video stream * `64` Can publish screen sharing stream +* `128` Can post chat message, share items and do reactions ### Attendee permission modifications * `set` - Setting this permission set. diff --git a/lib/Controller/ChatController.php b/lib/Controller/ChatController.php index 39db47d831f..8e508d0b083 100644 --- a/lib/Controller/ChatController.php +++ b/lib/Controller/ChatController.php @@ -180,6 +180,7 @@ public function parseCommentToResponse(IComment $comment, Message $parentMessage * @PublicPage * @RequireParticipant * @RequireReadWriteConversation + * @RequirePermissions(permissions=chat) * @RequireModeratorOrNoLobby * * Sends a new chat message to the given room. @@ -235,6 +236,7 @@ public function sendMessage(string $message, string $actorDisplayName = '', stri * @PublicPage * @RequireParticipant * @RequireReadWriteConversation + * @RequirePermissions(permissions=chat) * @RequireModeratorOrNoLobby * * Sends a rich-object to the given room. @@ -575,6 +577,7 @@ protected function loadSelfReactions(array $messages, array $commentIdToIndex): * @NoAdminRequired * @RequireParticipant * @RequireReadWriteConversation + * @RequirePermissions(permissions=chat) * @RequireModeratorOrNoLobby * * @param int $messageId @@ -825,6 +828,7 @@ protected function getMessagesForRoom(Room $room, array $messageIds): array { * @PublicPage * @RequireParticipant * @RequireReadWriteConversation + * @RequirePermissions(permissions=chat) * @RequireModeratorOrNoLobby * * @param string $search diff --git a/lib/Controller/ReactionController.php b/lib/Controller/ReactionController.php index a5b6fa89ee5..d99553914f5 100644 --- a/lib/Controller/ReactionController.php +++ b/lib/Controller/ReactionController.php @@ -48,6 +48,7 @@ public function __construct(string $appName, * @PublicPage * @RequireParticipant * @RequireReadWriteConversation + * @RequirePermissions(permissions=chat) * @RequireModeratorOrNoLobby * * @param int $messageId for reaction @@ -78,6 +79,7 @@ public function react(int $messageId, string $reaction): DataResponse { * @PublicPage * @RequireParticipant * @RequireReadWriteConversation + * @RequirePermissions(permissions=chat) * @RequireModeratorOrNoLobby * * @param int $messageId for reaction diff --git a/lib/Exceptions/PermissionsException.php b/lib/Exceptions/PermissionsException.php new file mode 100644 index 00000000000..3677eb0d031 --- /dev/null +++ b/lib/Exceptions/PermissionsException.php @@ -0,0 +1,28 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + + +namespace OCA\Talk\Exceptions; + +class PermissionsException extends \Exception { +} diff --git a/lib/Middleware/InjectionMiddleware.php b/lib/Middleware/InjectionMiddleware.php index 18d98b1acf6..0671aac8c55 100644 --- a/lib/Middleware/InjectionMiddleware.php +++ b/lib/Middleware/InjectionMiddleware.php @@ -25,6 +25,7 @@ use OCA\Talk\Controller\AEnvironmentAwareController; use OCA\Talk\Exceptions\ParticipantNotFoundException; +use OCA\Talk\Exceptions\PermissionsException; use OCA\Talk\Exceptions\RoomNotFoundException; use OCA\Talk\Manager; use OCA\Talk\Middleware\Exceptions\LobbyException; @@ -108,6 +109,11 @@ public function beforeController($controller, $methodName): void { if ($this->reflector->hasAnnotation('RequireModeratorOrNoLobby')) { $this->checkLobbyState($controller); } + + $requiredPermissions = $this->reflector->getAnnotationParameter('RequirePermissions', 'permissions'); + if ($requiredPermissions) { + $this->checkPermissions($controller, $requiredPermissions); + } } /** @@ -188,6 +194,24 @@ protected function checkReadOnlyState(AEnvironmentAwareController $controller): } } + /** + * @param AEnvironmentAwareController $controller + * @throws PermissionsException + */ + protected function checkPermissions(AEnvironmentAwareController $controller, string $permissions): void { + $textPermissions = explode(',', $permissions); + $participant = $controller->getParticipant(); + if (!$participant instanceof Participant) { + throw new PermissionsException(); + } + + foreach ($textPermissions as $textPermission) { + if ($textPermission === 'chat' && !($participant->getPermissions() & Attendee::PERMISSIONS_CHAT)) { + throw new PermissionsException(); + } + } + } + /** * @param AEnvironmentAwareController $controller * @throws LobbyException @@ -238,7 +262,8 @@ public function afterException($controller, $methodName, \Exception $exception): } if ($exception instanceof NotAModeratorException || - $exception instanceof ReadOnlyException) { + $exception instanceof ReadOnlyException || + $exception instanceof PermissionsException) { if ($controller instanceof OCSController) { throw new OCSException('', Http::STATUS_FORBIDDEN); } diff --git a/lib/Model/Attendee.php b/lib/Model/Attendee.php index 32c03f66637..cabda580faa 100644 --- a/lib/Model/Attendee.php +++ b/lib/Model/Attendee.php @@ -78,6 +78,7 @@ class Attendee extends Entity { public const PERMISSIONS_PUBLISH_AUDIO = 16; public const PERMISSIONS_PUBLISH_VIDEO = 32; public const PERMISSIONS_PUBLISH_SCREEN = 64; + public const PERMISSIONS_CHAT = 128; public const PERMISSIONS_MAX_DEFAULT = // Max int (when all permissions are granted as default) self::PERMISSIONS_CALL_START | self::PERMISSIONS_CALL_JOIN @@ -85,6 +86,7 @@ class Attendee extends Entity { | self::PERMISSIONS_PUBLISH_AUDIO | self::PERMISSIONS_PUBLISH_VIDEO | self::PERMISSIONS_PUBLISH_SCREEN + | self::PERMISSIONS_CHAT ; public const PERMISSIONS_MAX_CUSTOM = self::PERMISSIONS_MAX_DEFAULT | self::PERMISSIONS_CUSTOM; // Max int (when all permissions are granted as custom) diff --git a/tests/integration/features/bootstrap/FeatureContext.php b/tests/integration/features/bootstrap/FeatureContext.php index 23a22880bb1..992fa9ddd4b 100644 --- a/tests/integration/features/bootstrap/FeatureContext.php +++ b/tests/integration/features/bootstrap/FeatureContext.php @@ -63,6 +63,7 @@ class FeatureContext implements Context, SnippetAcceptingContext { 'A' => 16, // PERMISSIONS_PUBLISH_AUDIO 'V' => 32, // PERMISSIONS_PUBLISH_VIDEO 'P' => 64, // PERMISSIONS_PUBLISH_SCREEN + 'M' => 128, // PERMISSIONS_CHAT ]; /** @var string */ diff --git a/tests/integration/features/chat/delete.feature b/tests/integration/features/chat/delete.feature index 518c0f1732d..28842977122 100644 --- a/tests/integration/features/chat/delete.feature +++ b/tests/integration/features/chat/delete.feature @@ -47,6 +47,22 @@ Feature: chat/reply Then user "participant1" received a system messages in room "group room" to delete "Message 1" Then user "participant2" received a system messages in room "group room" to delete "Message 1" + Scenario: user cannot delete without chat permission + Given user "participant1" creates room "group room" (v4) + | roomType | 2 | + | roomName | room | + And user "participant1" adds user "participant2" to room "group room" with 200 (v4) + And user "participant2" sends message "Message 1" to room "group room" with 201 + Then user "participant1" sees the following messages in room "group room" with 200 + | room | actorType | actorId | actorDisplayName | message | messageParameters | parentMessage | + | group room | users | participant2 | participant2-displayname | Message 1 | [] | | + # Removing chat permission only + Then user "participant1" sets permissions for "participant2" in room "group room" to "CSJLAVP" with 200 (v4) + And user "participant2" deletes message "Message 1" from room "group room" with 403 + Then user "participant1" sees the following messages in room "group room" with 200 + | room | actorType | actorId | actorDisplayName | message | messageParameters | parentMessage | + | group room | users | participant2 | participant2-displayname | Message 1 | [] | | + Scenario: moderator deletes other user message Given user "participant1" creates room "group room" (v4) | roomType | 2 | diff --git a/tests/integration/features/chat/group.feature b/tests/integration/features/chat/group.feature index 25525845006..9021ce8c90f 100644 --- a/tests/integration/features/chat/group.feature +++ b/tests/integration/features/chat/group.feature @@ -24,6 +24,18 @@ Feature: chat/group | room | actorType | actorId | actorDisplayName | message | messageParameters | | group room | users | participant2 | participant2-displayname | Message 1 | [] | + Scenario: invited user can not send without chat permissions + Given user "participant1" creates room "group room" (v4) + | roomType | 2 | + | invite | attendees1 | + # Removing chat permission only + Then user "participant1" sets permissions for "participant2" in room "group room" to "CSJLAVP" with 200 (v4) + When user "participant2" sends message "Message 1" to room "group room" with 403 + When user "participant1" sends message "Message 2" to room "group room" with 201 + Then user "participant2" sees the following messages in room "group room" with 200 + | room | actorType | actorId | actorDisplayName | message | messageParameters | + | group room | users | participant1 | participant1-displayname | Message 2 | [] | + Scenario: not invited user can not send nor receive chat messages to nor from group room Given user "participant1" creates room "group room" (v4) | roomType | 2 | diff --git a/tests/integration/features/chat/public.feature b/tests/integration/features/chat/public.feature index d0617b4566b..7f3391ec96a 100644 --- a/tests/integration/features/chat/public.feature +++ b/tests/integration/features/chat/public.feature @@ -23,6 +23,19 @@ Feature: chat/public | room | actorType | actorId | actorDisplayName | message | messageParameters | | public room | users | participant2 | participant2-displayname | Message 1 | [] | + Scenario: invited user can not send without chat permissions + Given user "participant1" creates room "public room" (v4) + | roomType | 3 | + | roomName | room | + And user "participant1" adds user "participant2" to room "public room" with 200 (v4) + # Removing chat permission only + Then user "participant1" sets permissions for "participant2" in room "public room" to "CSJLAVP" with 200 (v4) + When user "participant2" sends message "Message 1" to room "public room" with 403 + When user "participant1" sends message "Message 2" to room "public room" with 201 + Then user "participant2" sees the following messages in room "public room" with 200 + | room | actorType | actorId | actorDisplayName | message | messageParameters | + | public room | users | participant1 | participant1-displayname | Message 2 | [] | + Scenario: not invited but joined user can send and receive chat messages to and from public room Given user "participant1" creates room "public room" (v4) | roomType | 3 | diff --git a/tests/integration/features/chat/rich-object-share.feature b/tests/integration/features/chat/rich-object-share.feature index 467a7250142..7cc083b3ae0 100644 --- a/tests/integration/features/chat/rich-object-share.feature +++ b/tests/integration/features/chat/rich-object-share.feature @@ -1,6 +1,7 @@ Feature: chat/public Background: Given user "participant1" exists + Given user "participant2" exists Scenario: Share a rich object to a chat Given user "participant1" creates room "public room" (v4) @@ -11,6 +12,17 @@ Feature: chat/public | room | actorType | actorId | actorDisplayName | message | messageParameters | | public room | users | participant1 | participant1-displayname | {object} | {"actor":{"type":"user","id":"participant1","name":"participant1-displayname"},"object":{"name":"Another room","call-type":"group","type":"call","id":"R4nd0mT0k3n"}} | + Scenario: Can not share without chat permission + Given user "participant1" creates room "public room" (v4) + | roomType | 3 | + | roomName | room | + And user "participant1" adds user "participant2" to room "public room" with 200 (v4) + # Removing chat permission only + Then user "participant1" sets permissions for "participant2" in room "public room" to "CSJLAVP" with 200 (v4) + When user "participant2" shares rich-object "call" "R4nd0mT0k3n" '{"name":"Another room","call-type":"group"}' to room "public room" with 403 (v1) + Then user "participant1" sees the following messages in room "public room" with 200 + | room | actorType | actorId | actorDisplayName | message | messageParameters | + Scenario: Delete a rich object from a chat Given user "participant1" creates room "public room" (v4) | roomType | 3 | @@ -21,6 +33,19 @@ Feature: chat/public | room | actorType | actorId | actorDisplayName | message | messageParameters | parentMessage | | public room | users | participant1 | participant1-displayname | Message deleted by you | {"actor":{"type":"user","id":"participant1","name":"participant1-displayname"}} | | + Scenario: Can not delete without chat permission + Given user "participant1" creates room "public room" (v4) + | roomType | 3 | + | roomName | room | + And user "participant1" adds user "participant2" to room "public room" with 200 (v4) + When user "participant2" shares rich-object "call" "R4nd0mT0k3n" '{"name":"Another room","call-type":"group"}' to room "public room" with 201 (v1) + # Removing chat permission only + Then user "participant1" sets permissions for "participant2" in room "public room" to "CSJLAVP" with 200 (v4) + And user "participant2" deletes message "shared::call::R4nd0mT0k3n" from room "public room" with 403 + Then user "participant1" sees the following messages in room "public room" with 200 + | room | actorType | actorId | actorDisplayName | message | messageParameters | + | public room | users | participant2 | participant2-displayname | {object} | {"actor":{"type":"user","id":"participant2","name":"participant2-displayname"},"object":{"name":"Another room","call-type":"group","type":"call","id":"R4nd0mT0k3n"}} | + Scenario: Share an invalid rich object to a chat Given user "participant1" creates room "public room" (v4) | roomType | 3 | diff --git a/tests/integration/features/conversation-2/set-participant-permissions.feature b/tests/integration/features/conversation-2/set-participant-permissions.feature index 7f0beb3cd92..8126acbb1dc 100644 --- a/tests/integration/features/conversation-2/set-participant-permissions.feature +++ b/tests/integration/features/conversation-2/set-participant-permissions.feature @@ -16,12 +16,12 @@ Feature: set-publishing-permissions And user "moderator" sets permissions for "moderator" in room "one-to-one room" to "S" with 400 (v4) Then user "owner" sees the following attendees in room "one-to-one room" with 200 (v4) | actorType | actorId | permissions | - | users | owner | SJLAVP | - | users | moderator | SJLAVP | + | users | owner | SJLAVPM | + | users | moderator | SJLAVPM | And user "moderator" sees the following attendees in room "one-to-one room" with 200 (v4) | actorType | actorId | permissions | - | users | owner | SJLAVP | - | users | moderator | SJLAVP | + | users | owner | SJLAVPM | + | users | moderator | SJLAVPM | Scenario: owner can set permissions in group room Given user "owner" creates room "group room" (v4) @@ -36,18 +36,18 @@ Feature: set-publishing-permissions And user "owner" sets permissions for "invited user" in room "group room" to "S" with 200 (v4) Then user "owner" sees the following attendees in room "group room" with 200 (v4) | actorType | actorId | permissions | - | users | owner | SJLAVP | - | users | moderator | SJLAVP | + | users | owner | SJLAVPM | + | users | moderator | SJLAVPM | | users | invited user | CS | And user "moderator" sees the following attendees in room "group room" with 200 (v4) | actorType | actorId | permissions | - | users | owner | SJLAVP | - | users | moderator | SJLAVP | + | users | owner | SJLAVPM | + | users | moderator | SJLAVPM | | users | invited user | CS | And user "invited user" sees the following attendees in room "group room" with 200 (v4) | actorType | actorId | permissions | - | users | owner | SJLAVP | - | users | moderator | SJLAVP | + | users | owner | SJLAVPM | + | users | moderator | SJLAVPM | | users | invited user | CS | Scenario: moderator can set permissions in group room @@ -63,18 +63,18 @@ Feature: set-publishing-permissions And user "owner" sets permissions for "invited user" in room "group room" to "S" with 200 (v4) Then user "owner" sees the following attendees in room "group room" with 200 (v4) | actorType | actorId | permissions | - | users | owner | SJLAVP | - | users | moderator | SJLAVP | + | users | owner | SJLAVPM | + | users | moderator | SJLAVPM | | users | invited user | CS | And user "moderator" sees the following attendees in room "group room" with 200 (v4) | actorType | actorId | permissions | - | users | owner | SJLAVP | - | users | moderator | SJLAVP | + | users | owner | SJLAVPM | + | users | moderator | SJLAVPM | | users | invited user | CS | And user "invited user" sees the following attendees in room "group room" with 200 (v4) | actorType | actorId | permissions | - | users | owner | SJLAVP | - | users | moderator | SJLAVP | + | users | owner | SJLAVPM | + | users | moderator | SJLAVPM | | users | invited user | CS | Scenario: others can not set permissions in group room @@ -98,19 +98,19 @@ Feature: set-publishing-permissions And user "guest not joined" sets permissions for "invited user" in room "group room" to "S" with 404 (v4) Then user "owner" sees the following attendees in room "group room" with 200 (v4) | actorType | actorId | permissions | - | users | owner | SJLAVP | - | users | moderator | SJLAVP | - | users | invited user | SJAVP | + | users | owner | SJLAVPM | + | users | moderator | SJLAVPM | + | users | invited user | SJAVPM | And user "moderator" sees the following attendees in room "group room" with 200 (v4) | actorType | actorId | permissions | - | users | owner | SJLAVP | - | users | moderator | SJLAVP | - | users | invited user | SJAVP | + | users | owner | SJLAVPM | + | users | moderator | SJLAVPM | + | users | invited user | SJAVPM | And user "invited user" sees the following attendees in room "group room" with 200 (v4) | actorType | actorId | permissions | - | users | owner | SJLAVP | - | users | moderator | SJLAVP | - | users | invited user | SJAVP | + | users | owner | SJLAVPM | + | users | moderator | SJLAVPM | + | users | invited user | SJAVPM | Scenario: owner can set permissions in public room Given user "owner" creates room "public room" (v4) @@ -132,43 +132,43 @@ Feature: set-publishing-permissions And user "owner" sets permissions for "guest" in room "public room" to "S" with 200 (v4) Then user "owner" sees the following attendees in room "public room" with 200 (v4) | actorType | actorId | permissions | - | users | owner | SJLAVP | - | users | moderator | SJLAVP | + | users | owner | SJLAVPM | + | users | moderator | SJLAVPM | | users | invited user | CS | | users | not invited but joined user | CS | - | guests | "guest moderator" | SJLAVP | + | guests | "guest moderator" | SJLAVPM | | guests | "guest" | CS | And user "moderator" sees the following attendees in room "public room" with 200 (v4) | actorType | actorId | permissions | - | users | owner | SJLAVP | - | users | moderator | SJLAVP | + | users | owner | SJLAVPM | + | users | moderator | SJLAVPM | | users | invited user | CS | | users | not invited but joined user | CS | - | guests | "guest moderator" | SJLAVP | + | guests | "guest moderator" | SJLAVPM | | guests | "guest" | CS | And user "invited user" sees the following attendees in room "public room" with 200 (v4) | actorType | actorId | permissions | - | users | owner | SJLAVP | - | users | moderator | SJLAVP | + | users | owner | SJLAVPM | + | users | moderator | SJLAVPM | | users | invited user | CS | | users | not invited but joined user | CS | - | guests | "guest moderator" | SJLAVP | + | guests | "guest moderator" | SJLAVPM | | guests | "guest" | CS | And user "not invited but joined user" sees the following attendees in room "public room" with 200 (v4) | actorType | actorId | permissions | - | users | owner | SJLAVP | - | users | moderator | SJLAVP | + | users | owner | SJLAVPM | + | users | moderator | SJLAVPM | | users | invited user | CS | | users | not invited but joined user | CS | - | guests | "guest moderator" | SJLAVP | + | guests | "guest moderator" | SJLAVPM | | guests | "guest" | CS | And user "guest moderator" sees the following attendees in room "public room" with 200 (v4) | actorType | actorId | permissions | - | users | owner | SJLAVP | - | users | moderator | SJLAVP | + | users | owner | SJLAVPM | + | users | moderator | SJLAVPM | | users | invited user | CS | | users | not invited but joined user | CS | - | guests | "guest moderator" | SJLAVP | + | guests | "guest moderator" | SJLAVPM | | guests | "guest" | CS | Scenario: moderator can set permissions in public room @@ -191,43 +191,43 @@ Feature: set-publishing-permissions And user "moderator" sets permissions for "guest" in room "public room" to "S" with 200 (v4) Then user "owner" sees the following attendees in room "public room" with 200 (v4) | actorType | actorId | permissions | - | users | owner | SJLAVP | - | users | moderator | SJLAVP | + | users | owner | SJLAVPM | + | users | moderator | SJLAVPM | | users | invited user | CS | | users | not invited but joined user | CS | - | guests | "guest moderator" | SJLAVP | + | guests | "guest moderator" | SJLAVPM | | guests | "guest" | CS | And user "moderator" sees the following attendees in room "public room" with 200 (v4) | actorType | actorId | permissions | - | users | owner | SJLAVP | - | users | moderator | SJLAVP | + | users | owner | SJLAVPM | + | users | moderator | SJLAVPM | | users | invited user | CS | | users | not invited but joined user | CS | - | guests | "guest moderator" | SJLAVP | + | guests | "guest moderator" | SJLAVPM | | guests | "guest" | CS | And user "invited user" sees the following attendees in room "public room" with 200 (v4) | actorType | actorId | permissions | - | users | owner | SJLAVP | - | users | moderator | SJLAVP | + | users | owner | SJLAVPM | + | users | moderator | SJLAVPM | | users | invited user | CS | | users | not invited but joined user | CS | - | guests | "guest moderator" | SJLAVP | + | guests | "guest moderator" | SJLAVPM | | guests | "guest" | CS | And user "not invited but joined user" sees the following attendees in room "public room" with 200 (v4) | actorType | actorId | permissions | - | users | owner | SJLAVP | - | users | moderator | SJLAVP | + | users | owner | SJLAVPM | + | users | moderator | SJLAVPM | | users | invited user | CS | | users | not invited but joined user | CS | - | guests | "guest moderator" | SJLAVP | + | guests | "guest moderator" | SJLAVPM | | guests | "guest" | CS | And user "guest moderator" sees the following attendees in room "public room" with 200 (v4) | actorType | actorId | permissions | - | users | owner | SJLAVP | - | users | moderator | SJLAVP | + | users | owner | SJLAVPM | + | users | moderator | SJLAVPM | | users | invited user | CS | | users | not invited but joined user | CS | - | guests | "guest moderator" | SJLAVP | + | guests | "guest moderator" | SJLAVPM | | guests | "guest" | CS | # Guests can not fetch the participant list @@ -252,43 +252,43 @@ Feature: set-publishing-permissions And user "guest moderator" sets permissions for "guest" in room "public room" to "S" with 200 (v4) Then user "owner" sees the following attendees in room "public room" with 200 (v4) | actorType | actorId | permissions | - | users | owner | SJLAVP | - | users | moderator | SJLAVP | + | users | owner | SJLAVPM | + | users | moderator | SJLAVPM | | users | invited user | CS | | users | not invited but joined user | CS | - | guests | "guest moderator" | SJLAVP | + | guests | "guest moderator" | SJLAVPM | | guests | "guest" | CS | And user "moderator" sees the following attendees in room "public room" with 200 (v4) | actorType | actorId | permissions | - | users | owner | SJLAVP | - | users | moderator | SJLAVP | + | users | owner | SJLAVPM | + | users | moderator | SJLAVPM | | users | invited user | CS | | users | not invited but joined user | CS | - | guests | "guest moderator" | SJLAVP | + | guests | "guest moderator" | SJLAVPM | | guests | "guest" | CS | And user "invited user" sees the following attendees in room "public room" with 200 (v4) | actorType | actorId | permissions | - | users | owner | SJLAVP | - | users | moderator | SJLAVP | + | users | owner | SJLAVPM | + | users | moderator | SJLAVPM | | users | invited user | CS | | users | not invited but joined user | CS | - | guests | "guest moderator" | SJLAVP | + | guests | "guest moderator" | SJLAVPM | | guests | "guest" | CS | And user "not invited but joined user" sees the following attendees in room "public room" with 200 (v4) | actorType | actorId | permissions | - | users | owner | SJLAVP | - | users | moderator | SJLAVP | + | users | owner | SJLAVPM | + | users | moderator | SJLAVPM | | users | invited user | CS | | users | not invited but joined user | CS | - | guests | "guest moderator" | SJLAVP | + | guests | "guest moderator" | SJLAVPM | | guests | "guest" | CS | And user "guest moderator" sees the following attendees in room "public room" with 200 (v4) | actorType | actorId | permissions | - | users | owner | SJLAVP | - | users | moderator | SJLAVP | + | users | owner | SJLAVPM | + | users | moderator | SJLAVPM | | users | invited user | CS | | users | not invited but joined user | CS | - | guests | "guest moderator" | SJLAVP | + | guests | "guest moderator" | SJLAVPM | | guests | "guest" | CS | # Guests can not fetch the participant list @@ -338,44 +338,44 @@ Feature: set-publishing-permissions And user "guest not joined" sets permissions for "guest" in room "public room" to "S" with 404 (v4) Then user "owner" sees the following attendees in room "public room" with 200 (v4) | actorType | actorId | permissions | - | users | owner | SJLAVP | - | users | moderator | SJLAVP | - | users | invited user | SJAVP | - | users | not invited but joined user | SJAVP | - | guests | "guest moderator" | SJLAVP | - | guests | "guest" | SJAVP | + | users | owner | SJLAVPM | + | users | moderator | SJLAVPM | + | users | invited user | SJAVPM | + | users | not invited but joined user | SJAVPM | + | guests | "guest moderator" | SJLAVPM | + | guests | "guest" | SJAVPM | And user "moderator" sees the following attendees in room "public room" with 200 (v4) | actorType | actorId | permissions | - | users | owner | SJLAVP | - | users | moderator | SJLAVP | - | users | invited user | SJAVP | - | users | not invited but joined user | SJAVP | - | guests | "guest moderator" | SJLAVP | - | guests | "guest" | SJAVP | + | users | owner | SJLAVPM | + | users | moderator | SJLAVPM | + | users | invited user | SJAVPM | + | users | not invited but joined user | SJAVPM | + | guests | "guest moderator" | SJLAVPM | + | guests | "guest" | SJAVPM | And user "invited user" sees the following attendees in room "public room" with 200 (v4) | actorType | actorId | permissions | - | users | owner | SJLAVP | - | users | moderator | SJLAVP | - | users | invited user | SJAVP | - | users | not invited but joined user | SJAVP | - | guests | "guest moderator" | SJLAVP | - | guests | "guest" | SJAVP | + | users | owner | SJLAVPM | + | users | moderator | SJLAVPM | + | users | invited user | SJAVPM | + | users | not invited but joined user | SJAVPM | + | guests | "guest moderator" | SJLAVPM | + | guests | "guest" | SJAVPM | And user "not invited but joined user" sees the following attendees in room "public room" with 200 (v4) | actorType | actorId | permissions | - | users | owner | SJLAVP | - | users | moderator | SJLAVP | - | users | invited user | SJAVP | - | users | not invited but joined user | SJAVP | - | guests | "guest moderator" | SJLAVP | - | guests | "guest" | SJAVP | + | users | owner | SJLAVPM | + | users | moderator | SJLAVPM | + | users | invited user | SJAVPM | + | users | not invited but joined user | SJAVPM | + | guests | "guest moderator" | SJLAVPM | + | guests | "guest" | SJAVPM | And user "guest moderator" sees the following attendees in room "public room" with 200 (v4) | actorType | actorId | permissions | - | users | owner | SJLAVP | - | users | moderator | SJLAVP | - | users | invited user | SJAVP | - | users | not invited but joined user | SJAVP | - | guests | "guest moderator" | SJLAVP | - | guests | "guest" | SJAVP | + | users | owner | SJLAVPM | + | users | moderator | SJLAVPM | + | users | invited user | SJAVPM | + | users | not invited but joined user | SJAVPM | + | guests | "guest moderator" | SJLAVPM | + | guests | "guest" | SJAVPM | # Guests can not fetch the participant list Scenario: participants can not set permissions in room for a share @@ -402,14 +402,14 @@ Feature: set-publishing-permissions And user "guest" sets permissions for "guest" in room "file last share room" to "S" with 403 (v4) Then user "owner of file" sees the following attendees in room "file last share room" with 200 (v4) | actorType | actorId | permissions | - | users | owner of file | SJAVP | - | users | user with access to file | SJAVP | - | guests | "guest" | SJAVP | + | users | owner of file | SJAVPM | + | users | user with access to file | SJAVPM | + | guests | "guest" | SJAVPM | And user "user with access to file" sees the following attendees in room "file last share room" with 200 (v4) | actorType | actorId | permissions | - | users | owner of file | SJAVP | - | users | user with access to file | SJAVP | - | guests | "guest" | SJAVP | + | users | owner of file | SJAVPM | + | users | user with access to file | SJAVPM | + | guests | "guest" | SJAVPM | # This does not make much sense, but there is no real need to block it either. Scenario: owner can set permissions in a password request room @@ -427,5 +427,5 @@ Feature: set-publishing-permissions And user "owner of file" sets permissions for "guest" in room "password request for last share room" to "S" with 200 (v4) Then user "owner of file" sees the following attendees in room "password request for last share room" with 200 (v4) | actorType | actorId | permissions | - | users | owner of file | SJLAVP | + | users | owner of file | SJLAVPM | | guests | "guest" | CS | diff --git a/tests/integration/features/conversation-2/set-permissions.feature b/tests/integration/features/conversation-2/set-permissions.feature index c04b4f02191..6cf6c491901 100644 --- a/tests/integration/features/conversation-2/set-permissions.feature +++ b/tests/integration/features/conversation-2/set-permissions.feature @@ -13,26 +13,26 @@ Feature: set-publishing-permissions And user "owner" adds user "invited user" to room "group room" with 200 (v4) Then user "owner" sees the following attendees in room "group room" with 200 (v4) | actorType | actorId | permissions | - | users | owner | SJLAVP | - | users | moderator | SJLAVP | - | users | invited user | SJAVP | + | users | owner | SJLAVPM | + | users | moderator | SJLAVPM | + | users | invited user | SJAVPM | When user "owner" sets default permissions for room "group room" to "S" with 200 (v4) Then user "owner" sees the following attendees in room "group room" with 200 (v4) | actorType | actorId | permissions | - | users | owner | SJLAVP | - | users | moderator | SJLAVP | + | users | owner | SJLAVPM | + | users | moderator | SJLAVPM | | users | invited user | CS | When user "moderator" sets default permissions for room "group room" to "AV" with 200 (v4) Then user "owner" sees the following attendees in room "group room" with 200 (v4) | actorType | actorId | permissions | - | users | owner | SJLAVP | - | users | moderator | SJLAVP | + | users | owner | SJLAVPM | + | users | moderator | SJLAVPM | | users | invited user | CAV | When user "invited user" sets default permissions for room "group room" to "D" with 403 (v4) Then user "owner" sees the following attendees in room "group room" with 200 (v4) | actorType | actorId | permissions | - | users | owner | SJLAVP | - | users | moderator | SJLAVP | + | users | owner | SJLAVPM | + | users | moderator | SJLAVPM | | users | invited user | CAV | Scenario: Owner and moderators can set call permissions users can not @@ -47,8 +47,8 @@ Feature: set-publishing-permissions When user "invited user" sets call permissions for room "group room" to "D" with 403 (v4) Then user "owner" sees the following attendees in room "group room" with 200 (v4) | actorType | actorId | permissions | - | users | owner | SJLAVP | - | users | moderator | SJLAVP | + | users | owner | SJLAVPM | + | users | moderator | SJLAVPM | | users | invited user | CAV | Scenario: User setting over call setting over conversation setting over default @@ -59,33 +59,33 @@ Feature: set-publishing-permissions When user "owner" sets default permissions for room "group room" to "S" with 200 (v4) Then user "owner" sees the following attendees in room "group room" with 200 (v4) | actorType | actorId | permissions | - | users | owner | SJLAVP | + | users | owner | SJLAVPM | | users | invited user | CS | When user "owner" sets call permissions for room "group room" to "A" with 200 (v4) Then user "owner" sees the following attendees in room "group room" with 200 (v4) | actorType | actorId | permissions | - | users | owner | SJLAVP | + | users | owner | SJLAVPM | | users | invited user | CA | And user "owner" sets permissions for "invited user" in room "group room" to "V" with 200 (v4) Then user "owner" sees the following attendees in room "group room" with 200 (v4) | actorType | actorId | permissions | - | users | owner | SJLAVP | + | users | owner | SJLAVPM | | users | invited user | CV | And user "owner" sets permissions for "invited user" in room "group room" to "D" with 200 (v4) Then user "owner" sees the following attendees in room "group room" with 200 (v4) | actorType | actorId | permissions | - | users | owner | SJLAVP | + | users | owner | SJLAVPM | | users | invited user | CA | When user "owner" sets call permissions for room "group room" to "D" with 200 (v4) Then user "owner" sees the following attendees in room "group room" with 200 (v4) | actorType | actorId | permissions | - | users | owner | SJLAVP | + | users | owner | SJLAVPM | | users | invited user | CS | When user "owner" sets default permissions for room "group room" to "D" with 200 (v4) Then user "owner" sees the following attendees in room "group room" with 200 (v4) | actorType | actorId | permissions | - | users | owner | SJLAVP | - | users | invited user | SJAVP | + | users | owner | SJLAVPM | + | users | invited user | SJAVPM | @@ -97,12 +97,12 @@ Feature: set-publishing-permissions And user "owner" sets permissions for "invited user" in room "group room" to "V" with 200 (v4) And user "owner" sees the following attendees in room "group room" with 200 (v4) | actorType | actorId | permissions | attendeePermissions | - | users | owner | SJLAVP | D | + | users | owner | SJLAVPM | D | | users | invited user | CV | CV | When user "owner" sets call permissions for room "group room" to "A" with 200 (v4) Then user "owner" sees the following attendees in room "group room" with 200 (v4) | actorType | actorId | permissions | attendeePermissions | - | users | owner | SJLAVP | D | + | users | owner | SJLAVPM | D | | users | invited user | CA | D | Scenario: setting default permissions resets participant permissions @@ -113,12 +113,12 @@ Feature: set-publishing-permissions And user "owner" sets permissions for "invited user" in room "group room" to "V" with 200 (v4) And user "owner" sees the following attendees in room "group room" with 200 (v4) | actorType | actorId | permissions | attendeePermissions | - | users | owner | SJLAVP | D | + | users | owner | SJLAVPM | D | | users | invited user | CV | CV | When user "owner" sets default permissions for room "group room" to "A" with 200 (v4) Then user "owner" sees the following attendees in room "group room" with 200 (v4) | actorType | actorId | permissions | attendeePermissions | - | users | owner | SJLAVP | D | + | users | owner | SJLAVPM | D | | users | invited user | CA | D | Scenario: setting default permissions does not reset call permissions @@ -129,10 +129,10 @@ Feature: set-publishing-permissions And user "owner" sets call permissions for room "group room" to "V" with 200 (v4) And user "owner" sees the following attendees in room "group room" with 200 (v4) | actorType | actorId | permissions | attendeePermissions | - | users | owner | SJLAVP | D | + | users | owner | SJLAVPM | D | | users | invited user | CV | D | When user "owner" sets default permissions for room "group room" to "A" with 200 (v4) Then user "owner" sees the following attendees in room "group room" with 200 (v4) | actorType | actorId | permissions | attendeePermissions | - | users | owner | SJLAVP | D | + | users | owner | SJLAVPM | D | | users | invited user | CV | D | diff --git a/tests/integration/features/reaction/react.feature b/tests/integration/features/reaction/react.feature index d53b044e03a..2bedf4ac1d7 100644 --- a/tests/integration/features/reaction/react.feature +++ b/tests/integration/features/reaction/react.feature @@ -39,6 +39,30 @@ Feature: reaction/react | room | users | participant1 | participant1-displayname | user_added | | room | users | participant1 | participant1-displayname | conversation_created | + Scenario: React to message fails without chat permission + Given user "participant1" creates room "room" (v4) + | roomType | 3 | + | roomName | room | + And user "participant1" adds user "participant2" to room "room" with 200 (v4) + And user "participant1" sends message "Message 1" to room "room" with 201 + And user "participant2" react with "πŸ‘" on message "Message 1" to room "room" with 201 + | actorType | actorId | actorDisplayName | reaction | + | users | participant2 | participant2-displayname | πŸ‘ | + Then user "participant1" sees the following system messages in room "room" with 200 + | room | actorType | actorId | actorDisplayName | systemMessage | + | room | users | participant2 | participant2-displayname | reaction | + | room | users | participant1 | participant1-displayname | user_added | + | room | users | participant1 | participant1-displayname | conversation_created | + # Removing chat permission only + Then user "participant1" sets permissions for "participant2" in room "room" to "CSJLAVP" with 200 (v4) + When user "participant2" delete react with "πŸ‘" on message "Message 1" to room "room" with 403 + And user "participant2" react with "πŸ’™" on message "Message 1" to room "room" with 403 + And user "participant1" sees the following system messages in room "room" with 200 + | room | actorType | actorId | actorDisplayName | systemMessage | + | room | users | participant2 | participant2-displayname | reaction | + | room | users | participant1 | participant1-displayname | user_added | + | room | users | participant1 | participant1-displayname | conversation_created | + Scenario: React two times to same message with the same reaction Given user "participant1" creates room "room" (v4) | roomType | 3 | From f810f17ba24f2e57289c9a06bc8346fa4450d56b Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 27 Apr 2022 17:36:48 +0200 Subject: [PATCH 2/7] Disable UI elements which can not be used anymore Signed-off-by: Joas Schilling --- .../MessagesGroup/Message/Message.spec.js | 35 ++++++++++++++++++- .../MessagesGroup/Message/Message.vue | 16 +++++++-- .../MessageButtonsBar/MessageButtonsBar.vue | 2 +- .../NewMessageForm/NewMessageForm.vue | 10 ++++-- src/constants.js | 5 +-- 5 files changed, 60 insertions(+), 8 deletions(-) diff --git a/src/components/MessagesList/MessagesGroup/Message/Message.spec.js b/src/components/MessagesList/MessagesGroup/Message/Message.spec.js index 69cd66042e6..d9ec96590fa 100644 --- a/src/components/MessagesList/MessagesGroup/Message/Message.spec.js +++ b/src/components/MessagesList/MessagesGroup/Message/Message.spec.js @@ -3,7 +3,7 @@ import { createLocalVue, mount, shallowMount } from '@vue/test-utils' import { cloneDeep } from 'lodash' import { EventBus } from '../../../../services/EventBus' import storeConfig from '../../../../store/storeConfig' -import { CONVERSATION, ATTENDEE } from '../../../../constants' +import { CONVERSATION, ATTENDEE, PARTICIPANT } from '../../../../constants' // Components import Check from 'vue-material-design-icons/Check' @@ -50,6 +50,7 @@ describe('Message.vue', () => { lastCommonReadMessage: 0, type: CONVERSATION.TYPE.GROUP, readOnly: CONVERSATION.STATE.READ_WRITE, + permissions: PARTICIPANT.PERMISSIONS.MAX_DEFAULT, } testStoreConfig = cloneDeep(storeConfig) @@ -806,6 +807,38 @@ describe('Message.vue', () => { expect(reactionButtons.wrappers[1].text()).toBe('πŸ‘ 7') }) + test('shows reaction buttons with the right emoji count but without emoji placeholder when no chat permission', () => { + const conversationProps = { + token: TOKEN, + lastCommonReadMessage: 0, + type: CONVERSATION.TYPE.GROUP, + readOnly: CONVERSATION.STATE.READ_WRITE, + permissions: PARTICIPANT.PERMISSIONS.MAX_DEFAULT - PARTICIPANT.PERMISSIONS.CHAT, + } + testStoreConfig.modules.conversationsStore.getters.conversation + = jest.fn().mockReturnValue((token) => conversationProps) + store = new Store(testStoreConfig) + + const wrapper = shallowMount(Message, { + localVue, + store, + propsData: messageProps, + }) + + const reactionsBar = wrapper.find('.message-body__reactions') + + // Array of buttons + const reactionButtons = reactionsBar.findAll('.reaction-button') + + // Number of buttons, 2 passed into the getter and 1 is the emoji + // picker + expect(reactionButtons.length).toBe(2) + + // Text of the buttons + expect(reactionButtons.wrappers[0].text()).toBe('❀️ 1') + expect(reactionButtons.wrappers[1].text()).toBe('πŸ‘ 7') + }) + test('dispatches store action upon picking an emoji from the emojipicker', () => { const addReactionToMessageAction = jest.fn() const userHasReactedGetter = jest.fn().mockReturnValue(() => false) diff --git a/src/components/MessagesList/MessagesGroup/Message/Message.vue b/src/components/MessagesList/MessagesGroup/Message/Message.vue index bc56e152419..4769068ecc8 100644 --- a/src/components/MessagesList/MessagesGroup/Message/Message.vue +++ b/src/components/MessagesList/MessagesGroup/Message/Message.vue @@ -136,7 +136,10 @@ the main body of the message as well as a quote. - + @@ -190,7 +193,7 @@ import EmojiPicker from '@nextcloud/vue/dist/Components/EmojiPicker' import EmoticonOutline from 'vue-material-design-icons/EmoticonOutline.vue' import Popover from '@nextcloud/vue/dist/Components/Popover' import { showError, showSuccess, showWarning, TOAST_DEFAULT_TIMEOUT } from '@nextcloud/dialogs' -import { ATTENDEE } from '../../../../constants' +import { ATTENDEE, CONVERSATION, PARTICIPANT } from '../../../../constants' export default { name: 'Message', @@ -572,6 +575,10 @@ export default { return this.$store.getters.hasReactions(this.token, this.id) }, + canReact() { + return this.conversation.readOnly !== CONVERSATION.STATE.READ_ONLY && (this.conversation.permissions & PARTICIPANT.PERMISSIONS.CHAT) !== 0 + }, + simpleReactions() { return this.messageObject.reactions }, @@ -679,6 +686,11 @@ export default { }, async handleReactionClick(clickedEmoji) { + if (!this.canReact) { + showError(t('spreed', 'No permission to post reactions in this conversation')) + return + } + // Check if current user has already added this reaction to the message if (!this.userHasReacted(clickedEmoji)) { this.$store.dispatch('addReactionToMessage', { diff --git a/src/components/MessagesList/MessagesGroup/Message/MessageButtonsBar/MessageButtonsBar.vue b/src/components/MessagesList/MessagesGroup/Message/MessageButtonsBar/MessageButtonsBar.vue index fb0c58d48c5..b2f58bfcf8d 100644 --- a/src/components/MessagesList/MessagesGroup/Message/MessageButtonsBar/MessageButtonsBar.vue +++ b/src/components/MessagesList/MessagesGroup/Message/MessageButtonsBar/MessageButtonsBar.vue @@ -326,7 +326,7 @@ export default { }, acceptsReactions() { - return !this.isConversationReadOnly && !this.isDeletedMessage + return !this.isConversationReadOnly && !this.isDeletedMessage && (this.conversation.permissions & PARTICIPANT.PERMISSIONS.CHAT) !== 0 }, messageActions() { diff --git a/src/components/NewMessageForm/NewMessageForm.vue b/src/components/NewMessageForm/NewMessageForm.vue index 0942e316e9d..cbf96fc9a96 100644 --- a/src/components/NewMessageForm/NewMessageForm.vue +++ b/src/components/NewMessageForm/NewMessageForm.vue @@ -132,7 +132,7 @@ import ActionButton from '@nextcloud/vue/dist/Components/ActionButton' import EmojiPicker from '@nextcloud/vue/dist/Components/EmojiPicker' import { EventBus } from '../../services/EventBus' import { shareFile } from '../../services/filesSharingServices' -import { CONVERSATION } from '../../constants' +import { CONVERSATION, PARTICIPANT } from '../../constants' import Paperclip from 'vue-material-design-icons/Paperclip' import EmoticonOutline from 'vue-material-design-icons/EmoticonOutline' import Send from 'vue-material-design-icons/Send' @@ -197,13 +197,19 @@ export default { return this.conversation.readOnly === CONVERSATION.STATE.READ_ONLY }, + noChatPermission() { + return (this.conversation.permissions & PARTICIPANT.PERMISSIONS.CHAT) === 0 + }, + disabled() { - return this.isReadOnly || !this.currentConversationIsJoined || this.isRecordingAudio + return this.isReadOnly || this.noChatPermission || this.isReadOnly || !this.currentConversationIsJoined || this.isRecordingAudio }, placeholderText() { if (this.isReadOnly) { return t('spreed', 'This conversation has been locked') + } else if (this.noChatPermission) { + return t('spreed', 'No permission to post messages in this conversation') } else if (!this.currentConversationIsJoined) { return t('spreed', 'Joining conversation …') } else { diff --git a/src/constants.js b/src/constants.js index f9d72765fcf..c54a6a5a44c 100644 --- a/src/constants.js +++ b/src/constants.js @@ -100,8 +100,9 @@ export const PARTICIPANT = { PUBLISH_AUDIO: 16, PUBLISH_VIDEO: 32, PUBLISH_SCREEN: 64, - MAX_DEFAULT: 126, - MAX_CUSTOM: 127, + CHAT: 128, + MAX_DEFAULT: 254, + MAX_CUSTOM: 255, }, } export const SHARED_ITEM = { From 7f0dbc0c6497225090cab15fb416e7352edd9323 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 27 Apr 2022 17:43:22 +0200 Subject: [PATCH 3/7] Add an option to grant/remove chat permissions Signed-off-by: Joas Schilling --- src/components/PermissionsEditor/PermissionsEditor.vue | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/components/PermissionsEditor/PermissionsEditor.vue b/src/components/PermissionsEditor/PermissionsEditor.vue index c3fe2e9be91..137d3690943 100644 --- a/src/components/PermissionsEditor/PermissionsEditor.vue +++ b/src/components/PermissionsEditor/PermissionsEditor.vue @@ -37,6 +37,11 @@ class="checkbox"> {{ t('spreed', 'Skip the lobby') }} + + {{ t('spreed', 'Can post messages and reactions') }} + @@ -125,6 +130,8 @@ export default { callStart: false, // Permission to bypass the lobby lobbyIgnore: false, + // Permission to post messages and reactions + chatMessagesAndReactions: false, // Permission to enable the microphone publishAudio: false, // Permission to enable the camera @@ -164,6 +171,7 @@ export default { formPermissions() { return (this.callStart ? PERMISSIONS.CALL_START : 0) | (this.lobbyIgnore ? PERMISSIONS.LOBBY_IGNORE : 0) + | (this.chatMessagesAndReactions ? PERMISSIONS.CHAT : 0) | (this.publishAudio ? PERMISSIONS.PUBLISH_AUDIO : 0) | (this.publishVideo ? PERMISSIONS.PUBLISH_VIDEO : 0) | (this.publishScreen ? PERMISSIONS.PUBLISH_SCREEN : 0) @@ -177,6 +185,7 @@ export default { submitButtonDisabled() { return (!!(this.permissionsWithDefault & PERMISSIONS.CALL_START)) === this.callStart && !!(this.permissionsWithDefault & PERMISSIONS.LOBBY_IGNORE) === this.lobbyIgnore + && !!(this.permissionsWithDefault & PERMISSIONS.CHAT) === this.chatMessagesAndReactions && !!(this.permissionsWithDefault & PERMISSIONS.PUBLISH_AUDIO) === this.publishAudio && !!(this.permissionsWithDefault & PERMISSIONS.PUBLISH_VIDEO) === this.publishVideo && !!(this.permissionsWithDefault & PERMISSIONS.PUBLISH_SCREEN) === this.publishScreen @@ -197,6 +206,7 @@ export default { writePermissionsToComponent(permissions) { permissions & PERMISSIONS.CALL_START ? this.callStart = true : this.callStart = false permissions & PERMISSIONS.LOBBY_IGNORE ? this.lobbyIgnore = true : this.lobbyIgnore = false + permissions & PERMISSIONS.CHAT ? this.chatMessagesAndReactions = true : this.chatMessagesAndReactions = false permissions & PERMISSIONS.PUBLISH_AUDIO ? this.publishAudio = true : this.publishAudio = false permissions & PERMISSIONS.PUBLISH_VIDEO ? this.publishVideo = true : this.publishVideo = false permissions & PERMISSIONS.PUBLISH_SCREEN ? this.publishScreen = true : this.publishScreen = false From 5874253f25e5d780680d2dbdc7c71540d6a95aee Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 27 Apr 2022 19:13:40 +0200 Subject: [PATCH 4/7] Disallow sharing without chat permissions Signed-off-by: Joas Schilling --- .../Collaborators/RoomPlugin.php | 8 +++++ lib/Share/RoomShareProvider.php | 8 ++++- .../features/chat/file-share.feature | 29 +++++++++++++++++++ .../Collaborators/RoomPluginTest.php | 18 +++++++++++- 4 files changed, 61 insertions(+), 2 deletions(-) diff --git a/lib/Collaboration/Collaborators/RoomPlugin.php b/lib/Collaboration/Collaborators/RoomPlugin.php index ea9e92d8e93..cc03429fe7f 100644 --- a/lib/Collaboration/Collaborators/RoomPlugin.php +++ b/lib/Collaboration/Collaborators/RoomPlugin.php @@ -25,6 +25,8 @@ namespace OCA\Talk\Collaboration\Collaborators; use OCA\Talk\Manager; +use OCA\Talk\Model\Attendee; +use OCA\Talk\Participant; use OCA\Talk\Room; use OCP\Collaboration\Collaborators\ISearchPlugin; use OCP\Collaboration\Collaborators\ISearchResult; @@ -62,6 +64,12 @@ public function search($search, $limit, $offset, ISearchResult $searchResult): b continue; } + $participant = $room->getParticipant($userId, false); + if (!$participant instanceof Participant || !($participant->getPermissions() & Attendee::PERMISSIONS_CHAT)) { + // No chat permissions is like read-only + continue; + } + if (stripos($room->getDisplayName($userId), $search) !== false) { $item = $this->roomToSearchResultItem($room, $userId); diff --git a/lib/Share/RoomShareProvider.php b/lib/Share/RoomShareProvider.php index 9e3c4ad9045..24b1aa82d01 100644 --- a/lib/Share/RoomShareProvider.php +++ b/lib/Share/RoomShareProvider.php @@ -34,6 +34,7 @@ use OCA\Talk\Exceptions\ParticipantNotFoundException; use OCA\Talk\Exceptions\RoomNotFoundException; use OCA\Talk\Manager; +use OCA\Talk\Model\Attendee; use OCA\Talk\Room; use OCA\Talk\Service\ParticipantService; use OCP\AppFramework\Utility\ITimeFactory; @@ -142,13 +143,18 @@ public function create(IShare $share): IShare { } try { - $room->getParticipant($share->getSharedBy(), false); + $participant = $room->getParticipant($share->getSharedBy(), false); } catch (ParticipantNotFoundException $e) { // If the sharer is not a participant of the room even if the room // exists the error is still "Room not found". throw new GenericShareException('Room not found', $this->l->t('Conversation not found'), 404); } + if (!($participant->getPermissions() & Attendee::PERMISSIONS_CHAT)) { + // No chat permissions is like read-only + throw new GenericShareException('Room not found', $this->l->t('Conversation not found'), 404); + } + $existingShares = $this->getSharesByPath($share->getNode()); foreach ($existingShares as $existingShare) { if ($existingShare->getSharedWith() === $share->getSharedWith()) { diff --git a/tests/integration/features/chat/file-share.feature b/tests/integration/features/chat/file-share.feature index cb35a4d1655..caae8ffaed6 100644 --- a/tests/integration/features/chat/file-share.feature +++ b/tests/integration/features/chat/file-share.feature @@ -1,6 +1,7 @@ Feature: chat/public Background: Given user "participant1" exists + Given user "participant2" exists Scenario: Share a file to a chat Given user "participant1" creates room "public room" (v4) @@ -11,6 +12,18 @@ Feature: chat/public | room | actorType | actorId | actorDisplayName | message | messageParameters | | public room | users | participant1 | participant1-displayname | {file} | "IGNORE" | + Scenario: Can not share a file without chat permission + Given user "participant1" creates room "public room" (v4) + | roomType | 3 | + | roomName | room | + And user "participant1" adds user "participant2" to room "public room" with 200 (v4) + # Removing chat permission only + Then user "participant1" sets permissions for "participant2" in room "public room" to "CSJLAVP" with 200 (v4) + When user "participant2" shares "welcome.txt" with room "public room" + And the OCS status code should be 404 + Then user "participant1" sees the following messages in room "public room" with 200 + | room | actorType | actorId | actorDisplayName | message | messageParameters | + Scenario: Delete share a file message from a chat Given user "participant1" creates room "public room" (v4) | roomType | 3 | @@ -23,3 +36,19 @@ Feature: chat/public Then user "participant1" sees the following messages in room "public room" with 200 | room | actorType | actorId | actorDisplayName | message | messageParameters | | public room | users | participant1 | participant1-displayname | Message deleted by you | {"actor":{"type":"user","id":"participant1","name":"participant1-displayname"}} | + + Scenario: Can not delete a share file message without chat permission + Given user "participant1" creates room "public room" (v4) + | roomType | 3 | + | roomName | room | + And user "participant1" adds user "participant2" to room "public room" with 200 (v4) + When user "participant2" shares "welcome.txt" with room "public room" + Then user "participant1" sees the following messages in room "public room" with 200 + | room | actorType | actorId | actorDisplayName | message | messageParameters | + | public room | users | participant2 | participant2-displayname | {file} | "IGNORE" | + # Removing chat permission only + Then user "participant1" sets permissions for "participant2" in room "public room" to "CSJLAVP" with 200 (v4) + And user "participant2" deletes message "shared::file::welcome.txt" from room "public room" with 403 + Then user "participant1" sees the following messages in room "public room" with 200 + | room | actorType | actorId | actorDisplayName | message | messageParameters | + | public room | users | participant2 | participant2-displayname | {file} | "IGNORE" | diff --git a/tests/php/Collaboration/Collaborators/RoomPluginTest.php b/tests/php/Collaboration/Collaborators/RoomPluginTest.php index f96d235692a..bb116ffd720 100644 --- a/tests/php/Collaboration/Collaborators/RoomPluginTest.php +++ b/tests/php/Collaboration/Collaborators/RoomPluginTest.php @@ -27,6 +27,8 @@ use OCA\Talk\Collaboration\Collaborators\RoomPlugin; use OCA\Talk\Manager; +use OCA\Talk\Model\Attendee; +use OCA\Talk\Participant; use OCA\Talk\Room; use OCP\Collaboration\Collaborators\ISearchResult; use OCP\Collaboration\Collaborators\SearchResultType; @@ -65,8 +67,9 @@ public function setUp(): void { $this->plugin = new RoomPlugin($this->manager, $this->userSession); } - private function newRoom(int $type, string $token, string $name): Room { + private function newRoom(int $type, string $token, string $name, int $permissions = Attendee::PERMISSIONS_MAX_DEFAULT): Room { $room = $this->createMock(Room::class); + $participant = $this->createMock(Participant::class); $room->expects($this->any()) ->method('getType') @@ -80,6 +83,14 @@ private function newRoom(int $type, string $token, string $name): Room { ->method('getDisplayName') ->willReturn($name); + $room->expects($this->any()) + ->method('getParticipant') + ->willReturn($participant); + + $participant->expects($this->any()) + ->method('getPermissions') + ->willReturn($permissions); + return $room; } @@ -116,6 +127,11 @@ public function searchProvider(): array { $this->newResult('Room name', 'roomToken') ], false], + // Chats without chat permission are not returned + ['room', 2, 0, [ + $this->newRoom(Room::TYPE_GROUP, 'roomToken', 'Room name', Attendee::PERMISSIONS_MAX_DEFAULT ^ Attendee::PERMISSIONS_CHAT), + ], [], [], false], + // Search term with single exact match ['room name', 2, 0, [ $this->newRoom(Room::TYPE_GROUP, 'roomToken', 'Unmatched name'), From 951378c43505f0ae90792413a2d981fa1eefcd8d Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 27 Apr 2022 20:28:15 +0200 Subject: [PATCH 5/7] Add a capability for the permission Signed-off-by: Joas Schilling --- docs/capabilities.md | 3 +++ lib/Capabilities.php | 1 + tests/php/CapabilitiesTest.php | 1 + 3 files changed, 5 insertions(+) diff --git a/docs/capabilities.md b/docs/capabilities.md index f9e98942774..3c029b86088 100644 --- a/docs/capabilities.md +++ b/docs/capabilities.md @@ -92,3 +92,6 @@ title: Capabilities * `reactions` - Api reactions to chat message * `rich-object-list-media` - When the API to get the chat messages for shared media is available * `rich-object-delete` - When the API allows to delete chat messages which are file or rich object shares + +## 15 +* `chat-permission` - When permission 128 is required to post chat messages, reaction or share items to the conversation diff --git a/lib/Capabilities.php b/lib/Capabilities.php index 50dad5816bc..aa613833d8c 100644 --- a/lib/Capabilities.php +++ b/lib/Capabilities.php @@ -101,6 +101,7 @@ public function getCapabilities(): array { 'conversation-permissions', 'rich-object-list-media', 'rich-object-delete', + 'chat-permission', ], 'config' => [ 'attachments' => [ diff --git a/tests/php/CapabilitiesTest.php b/tests/php/CapabilitiesTest.php index 2ebf6b006de..b9ade2e0ce6 100644 --- a/tests/php/CapabilitiesTest.php +++ b/tests/php/CapabilitiesTest.php @@ -104,6 +104,7 @@ public function setUp(): void { 'conversation-permissions', 'rich-object-list-media', 'rich-object-delete', + 'chat-permission', 'reactions', ]; } From aa5d7646ffe032d758d63d4e96c4be87749b5b9f Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 27 Apr 2022 20:52:28 +0200 Subject: [PATCH 6/7] Add a migration to update existing permissions Signed-off-by: Joas Schilling --- appinfo/info.xml | 2 +- .../Version15000Date20220427183026.php | 113 ++++++++++++++++++ 2 files changed, 114 insertions(+), 1 deletion(-) create mode 100644 lib/Migration/Version15000Date20220427183026.php diff --git a/appinfo/info.xml b/appinfo/info.xml index 6856562379c..b5a25ea37a9 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -16,7 +16,7 @@ And in the works for the [coming versions](https://github.com/nextcloud/spreed/m ]]> - 15.0.0-dev.2 + 15.0.0-dev.3 agpl Aleksandra LazareviΔ‡ diff --git a/lib/Migration/Version15000Date20220427183026.php b/lib/Migration/Version15000Date20220427183026.php new file mode 100644 index 00000000000..37c0d3a5052 --- /dev/null +++ b/lib/Migration/Version15000Date20220427183026.php @@ -0,0 +1,113 @@ + + * + * @author Joas Schilling + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCA\Talk\Migration; + +use Closure; +use OCP\DB\QueryBuilder\IQueryBuilder; +use OCP\IDBConnection; +use OCP\Migration\IOutput; +use OCP\Migration\SimpleMigrationStep; + +class Version15000Date20220427183026 extends SimpleMigrationStep { + protected IDBConnection $connection; + + public function __construct(IDBConnection $connection) { + $this->connection = $connection; + } + + /** + * Update existing permissions by adding the chat permissions when set to none default + * + * @param IOutput $output + * @param Closure $schemaClosure The `\Closure` returns a `\OCP\DB\ISchemaWrapper` + * @param array $options + */ + public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options): void { + $update = $this->connection->getQueryBuilder(); + $update->update('talk_rooms') + ->set('default_permissions', $update->func()->add( + 'default_permissions', + $update->createNamedParameter(128, IQueryBuilder::PARAM_INT) // Attendee::PERMISSION_CHAT + )) + ->where($update->expr()->neq('default_permissions', $update->createNamedParameter(0, IQueryBuilder::PARAM_INT))) // Attendee::PERMISSIONS_DEFAULT + ->andWhere( + $update->expr()->neq( + $update->expr()->castColumn( + $update->expr()->bitwiseAnd( + 'default_permissions', + 128 // Attendee::PERMISSION_CHAT + ), + IQueryBuilder::PARAM_INT + ), + $update->createNamedParameter(128, IQueryBuilder::PARAM_INT) // Attendee::PERMISSION_CHAT + ) + ); + $update->executeStatement(); + + $update = $this->connection->getQueryBuilder(); + $update->update('talk_rooms') + ->set('call_permissions', $update->func()->add( + 'call_permissions', + $update->createNamedParameter(128, IQueryBuilder::PARAM_INT) // Attendee::PERMISSION_CHAT + )) + ->where($update->expr()->neq('call_permissions', $update->createNamedParameter(0, IQueryBuilder::PARAM_INT))) // Attendee::PERMISSIONS_DEFAULT + ->andWhere( + $update->expr()->neq( + $update->expr()->castColumn( + $update->expr()->bitwiseAnd( + 'call_permissions', + 128 // Attendee::PERMISSION_CHAT + ), + IQueryBuilder::PARAM_INT + ), + $update->createNamedParameter(128, IQueryBuilder::PARAM_INT) // Attendee::PERMISSION_CHAT + ) + ); + $update->executeStatement(); + + $update = $this->connection->getQueryBuilder(); + $update->update('talk_attendees') + ->set('permissions', $update->func()->add( + 'permissions', + $update->createNamedParameter(128, IQueryBuilder::PARAM_INT) // Attendee::PERMISSION_CHAT + )) + ->where($update->expr()->neq('permissions', $update->createNamedParameter(0, IQueryBuilder::PARAM_INT))) // Attendee::PERMISSIONS_DEFAULT + ->andWhere( + $update->expr()->neq( + $update->expr()->castColumn( + $update->expr()->bitwiseAnd( + 'permissions', + 128 // Attendee::PERMISSION_CHAT + ), + IQueryBuilder::PARAM_INT + ), + $update->createNamedParameter(128, IQueryBuilder::PARAM_INT) // Attendee::PERMISSION_CHAT + ) + ); + $update->executeStatement(); + } +} From 3135b47b9336ba54344caee0b349232d1ee11504 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 27 Apr 2022 21:01:00 +0200 Subject: [PATCH 7/7] Update baseline as the interface will die soon anyway it makes little sense to add the method Signed-off-by: Joas Schilling --- tests/psalm-baseline.xml | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/tests/psalm-baseline.xml b/tests/psalm-baseline.xml index e6434b1062b..ea600086d65 100644 --- a/tests/psalm-baseline.xml +++ b/tests/psalm-baseline.xml @@ -169,17 +169,19 @@ + + SharedStorage::class + $this->rootFolder IRootFolder IRootFolder - - SharedStorage::class - - + + ConnectException + @@ -197,6 +199,9 @@ + + SharedStorage::class + $this->rootFolder $this->rootFolder @@ -204,16 +209,23 @@ IRootFolder IRootFolder - - SharedStorage::class - + $this->tokenProvider + $this->tokenProvider + $this->tokenProvider + IAuthTokenProvider IAuthTokenProvider IToken + IToken + + + getAnnotationParameter + + $return['num_rooms']