Skip to content

Commit

Permalink
Fix method to get avatarURL and validate the URL
Browse files Browse the repository at this point in the history
https://github.com/nextcloud/spreed/pull/9072/files#r1142350857

Signed-off-by: Vitor Mattos <vitor@php.rio>
  • Loading branch information
vitormattos committed Mar 20, 2023
1 parent e1972c5 commit 75ae7fb
Show file tree
Hide file tree
Showing 8 changed files with 53 additions and 9 deletions.
6 changes: 5 additions & 1 deletion lib/Chat/Parser/UserMention.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
use OCA\Talk\Model\Attendee;
use OCA\Talk\Model\Message;
use OCA\Talk\Room;
use OCA\Talk\Service\AvatarService;
use OCA\Talk\Service\ParticipantService;
use OCP\Comments\ICommentsManager;
use OCP\IGroup;
Expand All @@ -50,19 +51,22 @@ class UserMention {
protected IGroupManager $groupManager;
protected GuestManager $guestManager;
protected ParticipantService $participantService;
protected AvatarService $avatarService;
protected IL10N $l;

public function __construct(ICommentsManager $commentsManager,
IUserManager $userManager,
IGroupManager $groupManager,
GuestManager $guestManager,
AvatarService $avatarService,
ParticipantService $participantService,
IL10N $l) {
$this->commentsManager = $commentsManager;
$this->userManager = $userManager;
$this->groupManager = $groupManager;
$this->guestManager = $guestManager;
$this->participantService = $participantService;
$this->avatarService = $avatarService;
$this->l = $l;
}

Expand Down Expand Up @@ -138,7 +142,7 @@ public function parseMessage(Message $chatMessage): void {
'id' => $chatMessage->getRoom()->getToken(),
'name' => $chatMessage->getRoom()->getDisplayName($userId),
'call-type' => $this->getRoomType($chatMessage->getRoom()),
'icon-url' => $chatMessage->getRoom()->getAvatar(),
'icon-url' => $this->avatarService->getAvatarUrl($chatMessage->getRoom()),
];
} elseif ($mention['type'] === 'guest') {
try {
Expand Down
6 changes: 5 additions & 1 deletion lib/Controller/ChatController.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
use OCA\Talk\Participant;
use OCA\Talk\Room;
use OCA\Talk\Service\AttachmentService;
use OCA\Talk\Service\AvatarService;
use OCA\Talk\Service\ParticipantService;
use OCA\Talk\Service\SessionService;
use OCA\Talk\Share\RoomShareProvider;
Expand Down Expand Up @@ -71,6 +72,7 @@ class ChatController extends AEnvironmentAwareController {
private ParticipantService $participantService;
private SessionService $sessionService;
protected AttachmentService $attachmentService;
protected avatarService $avatarService;
private GuestManager $guestManager;
/** @var string[] */
protected array $guestNames;
Expand All @@ -97,6 +99,7 @@ public function __construct(string $appName,
ParticipantService $participantService,
SessionService $sessionService,
AttachmentService $attachmentService,
avatarService $avatarService,
GuestManager $guestManager,
MessageParser $messageParser,
RoomShareProvider $shareProvider,
Expand All @@ -120,6 +123,7 @@ public function __construct(string $appName,
$this->participantService = $participantService;
$this->sessionService = $sessionService;
$this->attachmentService = $attachmentService;
$this->avatarService = $avatarService;
$this->guestManager = $guestManager;
$this->messageParser = $messageParser;
$this->shareProvider = $shareProvider;
Expand Down Expand Up @@ -270,7 +274,7 @@ public function shareObjectToChat(string $objectType, string $objectId, string $
}
$data['type'] = $objectType;
$data['id'] = $objectId;
$data['icon-url'] = $this->room->getAvatar();
$data['icon-url'] = $this->avatarService->getAvatarUrl($this->room);

if (isset($data['link']) && !$this->trustedDomainHelper->isTrustedUrl($data['link'])) {
return new DataResponse([], Http::STATUS_BAD_REQUEST);
Expand Down
18 changes: 11 additions & 7 deletions lib/Notification/Notifier.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
use OCA\Talk\Model\Attendee;
use OCA\Talk\Participant;
use OCA\Talk\Room;
use OCA\Talk\Service\AvatarService;
use OCA\Talk\Service\ParticipantService;
use OCA\Talk\Webinary;
use OCP\AppFramework\Utility\ITimeFactory;
Expand Down Expand Up @@ -69,6 +70,7 @@ class Notifier implements INotifier {
private IShareManager $shareManager;
protected Manager $manager;
protected ParticipantService $participantService;
protected AvatarService $avatarService;
protected INotificationManager $notificationManager;
protected ICommentsManager $commentManager;
protected MessageParser $messageParser;
Expand All @@ -92,6 +94,7 @@ public function __construct(IFactory $lFactory,
IShareManager $shareManager,
Manager $manager,
ParticipantService $participantService,
AvatarService $avatarService,
INotificationManager $notificationManager,
CommentsManager $commentManager,
MessageParser $messageParser,
Expand All @@ -109,6 +112,7 @@ public function __construct(IFactory $lFactory,
$this->shareManager = $shareManager;
$this->manager = $manager;
$this->participantService = $participantService;
$this->avatarService = $avatarService;
$this->notificationManager = $notificationManager;
$this->commentManager = $commentManager;
$this->messageParser = $messageParser;
Expand Down Expand Up @@ -322,7 +326,7 @@ protected function parseStoredRecordingFail(
'id' => $room->getId(),
'name' => $room->getDisplayName($participant->getAttendee()->getActorId()),
'call-type' => $this->getRoomType($room),
'icon-url' => $room->getAvatar(),
'icon-url' => $this->avatarService->getAvatarUrl($room),
],
]
);
Expand Down Expand Up @@ -386,7 +390,7 @@ protected function parseStoredRecording(
'id' => $room->getId(),
'name' => $room->getDisplayName($participant->getAttendee()->getActorId()),
'call-type' => $this->getRoomType($room),
'icon-url' => $room->getAvatar(),
'icon-url' => $this->avatarService->getAvatarUrl($room),
],
'file' => [
'type' => 'file',
Expand Down Expand Up @@ -483,7 +487,7 @@ protected function parseChatMessage(INotification $notification, Room $room, Par
'id' => $room->getId(),
'name' => $room->getDisplayName($notification->getUser()),
'call-type' => $this->getRoomType($room),
'icon-url' => $room->getAvatar(),
'icon-url' => $this->avatarService->getAvatarUrl($room),
];

$messageParameters = $notification->getMessageParameters();
Expand Down Expand Up @@ -780,7 +784,7 @@ protected function parseInvitation(INotification $notification, Room $room, IL10
'id' => $room->getId(),
'name' => $roomName,
'call-type' => $this->getRoomType($room),
'icon-url' => $room->getAvatar(),
'icon-url' => $this->avatarService->getAvatarUrl($room),
],
]
);
Expand All @@ -806,7 +810,7 @@ protected function parseInvitation(INotification $notification, Room $room, IL10
'id' => $room->getId(),
'name' => $roomName,
'call-type' => $this->getRoomType($room),
'icon-url' => $room->getAvatar(),
'icon-url' => $this->avatarService->getAvatarUrl($room),
],
]
);
Expand Down Expand Up @@ -858,7 +862,7 @@ protected function parseCall(INotification $notification, Room $room, IL10N $l):
'id' => $room->getId(),
'name' => $roomName,
'call-type' => $this->getRoomType($room),
'icon-url' => $room->getAvatar(),
'icon-url' => $this->avatarService->getAvatarUrl($room),
],
]
);
Expand All @@ -883,7 +887,7 @@ protected function parseCall(INotification $notification, Room $room, IL10N $l):
'id' => $room->getId(),
'name' => $roomName,
'call-type' => $this->getRoomType($room),
'icon-url' => $room->getAvatar(),
'icon-url' => $this->avatarService->getAvatarUrl($room),
],
]
);
Expand Down
7 changes: 7 additions & 0 deletions tests/integration/features/bootstrap/FeatureContext.php
Original file line number Diff line number Diff line change
Expand Up @@ -2227,6 +2227,13 @@ protected function compareDataResponse(TableNode $formData = null) {
if ($result) {
$expected[$i]['messageParameters'] = str_replace($matches[0], '"' . self::$questionToPollId[$matches[1]] . '"', $expected[$i]['messageParameters']);
}
if (isset($messages[$i]['messageParameters']['object']['icon-url'])) {
$result = preg_match('/"\{VALIDATE_ICON_URL_PATTERN\}"/', $expected[$i]['messageParameters'], $matches);
if ($result) {
Assert::assertMatchesRegularExpression('/avatar\?v=\w+/', $messages[$i]['messageParameters']['object']['icon-url']);
$expected[$i]['messageParameters'] = str_replace($matches[0], json_encode($messages[$i]['messageParameters']['object']['icon-url']), $expected[$i]['messageParameters']);
}
}
}

Assert::assertEquals($expected, array_map(function ($message) use ($includeParents, $includeReferenceId, $includeReactions, $includeReactionsSelf) {
Expand Down
10 changes: 10 additions & 0 deletions tests/integration/features/conversation/avatar.feature
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,13 @@ Feature: conversation/avatar
| roomType | 1 |
| invite | participant2 |
Then user "participant1" uploads file "/img/favicon.png" as avatar of room "one2one" with 400

Scenario: User should receive the room avatar when see a rich object at media tab
Given user "participant1" creates room "public room" (v4)
| roomType | 3 |
| roomName | public room |
And user "participant1" uploads file "/img/favicon.png" as avatar of room "public room" with 200
When user "participant1" shares rich-object "call" "R4nd0mT0k3n" '{"name":"Another room","call-type":"group"}' to room "public room" with 201 (v1)
Then user "participant1" sees the following shared other in room "public room" with 200
| 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","icon-url":"{VALIDATE_ICON_URL_PATTERN}"}} |
5 changes: 5 additions & 0 deletions tests/php/Chat/Parser/UserMentionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
use OCA\Talk\Model\Message;
use OCA\Talk\Participant;
use OCA\Talk\Room;
use OCA\Talk\Service\AvatarService;
use OCA\Talk\Service\ParticipantService;
use OCP\Comments\IComment;
use OCP\Comments\ICommentsManager;
Expand All @@ -49,6 +50,8 @@ class UserMentionTest extends TestCase {
protected $groupManager;
/** @var GuestManager|MockObject */
protected $guestManager;
/** @var AvatarService|MockObject */
protected $avatarService;
/** @var ParticipantService|MockObject */
protected $participantService;
/** @var IL10N|MockObject */
Expand All @@ -63,6 +66,7 @@ public function setUp(): void {
$this->userManager = $this->createMock(IUserManager::class);
$this->groupManager = $this->createMock(IGroupManager::class);
$this->guestManager = $this->createMock(GuestManager::class);
$this->avatarService = $this->createMock(AvatarService::class);
$this->participantService = $this->createMock(ParticipantService::class);
$this->l = $this->createMock(IL10N::class);

Expand All @@ -71,6 +75,7 @@ public function setUp(): void {
$this->userManager,
$this->groupManager,
$this->guestManager,
$this->avatarService,
$this->participantService,
$this->l);
}
Expand Down
5 changes: 5 additions & 0 deletions tests/php/Controller/ChatControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
use OCA\Talk\Participant;
use OCA\Talk\Room;
use OCA\Talk\Service\AttachmentService;
use OCA\Talk\Service\AvatarService;
use OCA\Talk\Service\ParticipantService;
use OCA\Talk\Service\SessionService;
use OCA\Talk\Share\RoomShareProvider;
Expand Down Expand Up @@ -73,6 +74,8 @@ class ChatControllerTest extends TestCase {
protected $sessionService;
/** @var AttachmentService|MockObject */
protected $attachmentService;
/** @var AvatarService|MockObject */
protected $avatarServiec;
/** @var GuestManager|MockObject */
protected $guestManager;
/** @var MessageParser|MockObject */
Expand Down Expand Up @@ -119,6 +122,7 @@ public function setUp(): void {
$this->participantService = $this->createMock(ParticipantService::class);
$this->sessionService = $this->createMock(SessionService::class);
$this->attachmentService = $this->createMock(AttachmentService::class);
$this->avatarService = $this->createMock(AvatarService::class);
$this->guestManager = $this->createMock(GuestManager::class);
$this->messageParser = $this->createMock(MessageParser::class);
$this->roomShareProvider = $this->createMock(RoomShareProvider::class);
Expand Down Expand Up @@ -157,6 +161,7 @@ private function recreateChatController() {
$this->participantService,
$this->sessionService,
$this->attachmentService,
$this->avatarService,
$this->guestManager,
$this->messageParser,
$this->roomShareProvider,
Expand Down
5 changes: 5 additions & 0 deletions tests/php/Notification/NotifierTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
use OCA\Talk\Notification\Notifier;
use OCA\Talk\Participant;
use OCA\Talk\Room;
use OCA\Talk\Service\AvatarService;
use OCA\Talk\Service\ParticipantService;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\Comments\IComment;
Expand Down Expand Up @@ -71,6 +72,8 @@ class NotifierTest extends TestCase {
protected $manager;
/** @var ParticipantService|MockObject */
protected $participantService;
/** @var AvatarService|MockObject */
protected $avatarService;
/** @var INotificationManager|MockObject */
protected $notificationManager;
/** @var CommentsManager|MockObject */
Expand Down Expand Up @@ -101,6 +104,7 @@ public function setUp(): void {
$this->shareManager = $this->createMock(IShareManager::class);
$this->manager = $this->createMock(Manager::class);
$this->participantService = $this->createMock(ParticipantService::class);
$this->avatarService = $this->createMock(AvatarService::class);
$this->notificationManager = $this->createMock(INotificationManager::class);
$this->commentsManager = $this->createMock(CommentsManager::class);
$this->messageParser = $this->createMock(MessageParser::class);
Expand All @@ -120,6 +124,7 @@ public function setUp(): void {
$this->shareManager,
$this->manager,
$this->participantService,
$this->avatarService,
$this->notificationManager,
$this->commentsManager,
$this->messageParser,
Expand Down

0 comments on commit 75ae7fb

Please sign in to comment.