From 4ecf9f66fdb73334d562b01281a06a5578f9c489 Mon Sep 17 00:00:00 2001 From: Jonas Date: Thu, 15 Oct 2026 09:46:45 +0200 Subject: [PATCH] feat(notifications): notify mentioned users about mentions by guests Store the guest name as a separate subject parameter and render it as a guest rich object labelled "(guest)", so it cannot be mistaken for an instance user. Notifications without a valid source are marked as processed, which removes broken rows from earlier mentions. Mentions are now rate limited for anonymous requests as well. Fixes #8743 Signed-off-by: Jonas Assisted-by: ClaudeCode:claude-opus-5-5 --- lib/Controller/SessionController.php | 9 +- lib/Notification/Notifier.php | 38 ++++-- lib/Service/NotificationService.php | 13 +- tests/unit/Notification/NotifierTest.php | 112 ++++++++++++++++++ .../unit/Service/NotificationServiceTest.php | 61 ++++++++++ 5 files changed, 218 insertions(+), 15 deletions(-) create mode 100644 tests/unit/Notification/NotifierTest.php create mode 100644 tests/unit/Service/NotificationServiceTest.php diff --git a/lib/Controller/SessionController.php b/lib/Controller/SessionController.php index c18ccb3ba1c..bfd0d7d8723 100644 --- a/lib/Controller/SessionController.php +++ b/lib/Controller/SessionController.php @@ -18,6 +18,7 @@ use OCA\Text\Service\SessionService; use OCP\AppFramework\ApiController; use OCP\AppFramework\Http; +use OCP\AppFramework\Http\Attribute\AnonRateLimit; use OCP\AppFramework\Http\Attribute\NoAdminRequired; use OCP\AppFramework\Http\Attribute\PublicPage; use OCP\AppFramework\Http\Attribute\UserRateLimit; @@ -126,6 +127,7 @@ public function save(int $version, string $autosaveContent, string $documentStat #[PublicPage] #[RequireDocumentSession] #[UserRateLimit(limit: 5, period: 120)] + #[AnonRateLimit(limit: 5, period: 120)] public function mention(string $mention): DataResponse { if ($this->getSession()->isGuest() && !$this->sessionService->isUserInDocument($this->getDocumentId(), $mention)) { return new DataResponse([], 403); @@ -136,7 +138,12 @@ public function mention(string $mention): DataResponse { return new DataResponse(false); } - return new DataResponse($this->notificationService->mention($document->getContextId(), $mention)); + $session = $this->getSession(); + return new DataResponse($this->notificationService->mention( + $document->getContextId(), + $mention, + $session->isGuest() ? ($session->getGuestName() ?? '') : null, + )); } private function loginSessionUser(): void { diff --git a/lib/Notification/Notifier.php b/lib/Notification/Notifier.php index 2fdb7f48308..a8c979adeb4 100644 --- a/lib/Notification/Notifier.php +++ b/lib/Notification/Notifier.php @@ -26,6 +26,7 @@ class Notifier implements INotifier { public const string TYPE_MENTIONED = 'mentioned'; public const string SUBJECT_MENTIONED_SOURCE_USER = 'sourceUser'; + public const string SUBJECT_MENTIONED_SOURCE_GUEST = 'sourceGuest'; public const string SUBJECT_MENTIONED_TARGET_USER = 'targetUser'; public function __construct( @@ -55,15 +56,34 @@ public function prepare(INotification $notification, string $languageCode): INot switch ($notification->getSubject()) { case self::TYPE_MENTIONED: $parameters = $notification->getSubjectParameters(); - $sourceUser = $parameters[self::SUBJECT_MENTIONED_SOURCE_USER]; - $sourceUserDisplayName = $this->userManager->getDisplayName($sourceUser); + $sourceUser = $parameters[self::SUBJECT_MENTIONED_SOURCE_USER] ?? null; + $sourceGuest = $parameters[self::SUBJECT_MENTIONED_SOURCE_GUEST] ?? null; + + if (is_string($sourceUser)) { + $sourceUserDisplayName = $this->userManager->getDisplayName($sourceUser); + if ($sourceUserDisplayName === null) { + throw new UnknownNotificationException(); + } + $sourceParameter = [ + 'type' => 'user', + 'id' => $sourceUser, + 'name' => $sourceUserDisplayName, + ]; + } elseif (is_string($sourceGuest)) { + $sourceParameter = [ + 'type' => 'guest', + 'id' => 'guest', + 'name' => $sourceGuest !== '' + ? $l->t('%s (guest)', [$sourceGuest]) + : $l->t('A guest'), + ]; + } else { + throw new AlreadyProcessedException(); + } + $targetUser = $notification->getUser(); $fileId = (int)$notification->getObjectId(); - if ($sourceUserDisplayName === null) { - throw new UnknownNotificationException(); - } - try { $userFolder = $this->rootFolder->getUserFolder($targetUser); } catch (NotPermittedException|NoUserException) { @@ -79,11 +99,7 @@ public function prepare(INotification $notification, string $languageCode): INot $notification->setLink($fileLink); $notification->setRichSubject($l->t('{user} has mentioned you in the text document {node}'), [ - 'user' => [ - 'type' => 'user', - 'id' => $sourceUser, - 'name' => $sourceUserDisplayName, - ], + 'user' => $sourceParameter, 'node' => [ 'type' => 'file', 'id' => (string)$node->getId(), diff --git a/lib/Service/NotificationService.php b/lib/Service/NotificationService.php index 0f934b37dc5..bdad7c702a0 100644 --- a/lib/Service/NotificationService.php +++ b/lib/Service/NotificationService.php @@ -19,17 +19,24 @@ public function __construct( ) { } - public function mention(int $fileId, string $userId): bool { + public function mention(int $fileId, string $userId, ?string $guestName = null): bool { + if ($this->userId === null && $guestName === null) { + return false; + } + if ($userId === $this->userId) { // We allow self mentions in documents but do not notify about it return false; } + $source = $this->userId !== null + ? [Notifier::SUBJECT_MENTIONED_SOURCE_USER => $this->userId] + : [Notifier::SUBJECT_MENTIONED_SOURCE_GUEST => $guestName]; + $notification = $this->manager->createNotification(); $notification->setUser($userId) ->setApp('text') - ->setSubject(Notifier::TYPE_MENTIONED, [ - Notifier::SUBJECT_MENTIONED_SOURCE_USER => $this->userId, + ->setSubject(Notifier::TYPE_MENTIONED, $source + [ Notifier::SUBJECT_MENTIONED_TARGET_USER => $userId, ]) ->setObject('file', (string)$fileId); diff --git a/tests/unit/Notification/NotifierTest.php b/tests/unit/Notification/NotifierTest.php new file mode 100644 index 00000000000..7082ad857e9 --- /dev/null +++ b/tests/unit/Notification/NotifierTest.php @@ -0,0 +1,112 @@ +createMock(IL10N::class); + $l10n->method('t')->willReturnCallback(fn (string $text, array $parameters = []) => vsprintf($text, $parameters)); + $factory = $this->createMock(IFactory::class); + $factory->method('get')->willReturn($l10n); + + $urlGenerator = $this->createMock(IURLGenerator::class); + $urlGenerator->method('getAbsoluteURL')->willReturn('http://localhost/apps/text/img/app-dark.svg'); + $urlGenerator->method('linkToRouteAbsolute')->willReturn('http://localhost/f/42'); + + $file = $this->createMock(File::class); + $file->method('getId')->willReturn(42); + $file->method('getName')->willReturn('doc.md'); + $file->method('getPath')->willReturn('/bob/files/doc.md'); + $userFolder = $this->createMock(Folder::class); + $userFolder->method('getFirstNodeById')->with(42)->willReturn($file); + $userFolder->method('getRelativePath')->willReturn('/doc.md'); + $rootFolder = $this->createMock(IRootFolder::class); + $rootFolder->method('getUserFolder')->with('bob')->willReturn($userFolder); + + $this->userManager = $this->createMock(IUserManager::class); + + $this->notifier = new Notifier( + $factory, + $this->userManager, + $urlGenerator, + $rootFolder, + $this->createMock(IEventDispatcher::class), + ); + } + + private function createMention(array $parameters): INotification { + return Server::get(IManager::class)->createNotification() + ->setApp('text') + ->setUser('bob') + ->setSubject(Notifier::TYPE_MENTIONED, $parameters + ['targetUser' => 'bob']) + ->setObject('file', '42'); + } + + public function testPrepareUserSource(): void { + $this->userManager->method('getDisplayName')->with('alice')->willReturn('Alice'); + + $notification = $this->notifier->prepare($this->createMention(['sourceUser' => 'alice']), 'en'); + + $this->assertSame( + ['type' => 'user', 'id' => 'alice', 'name' => 'Alice'], + $notification->getRichSubjectParameters()['user'], + ); + $this->assertSame('Alice has mentioned you in the text document /doc.md', $notification->getParsedSubject()); + } + + public static function dataPrepareGuestSource(): array { + return [ + 'named' => ['Gustav', 'Gustav (guest)'], + 'unnamed' => ['', 'A guest'], + ]; + } + + /** + * @dataProvider dataPrepareGuestSource + */ + public function testPrepareGuestSource(string $guestName, string $expectedName): void { + $this->userManager->expects($this->never())->method('getDisplayName'); + + $notification = $this->notifier->prepare($this->createMention(['sourceGuest' => $guestName]), 'en'); + + $this->assertSame( + ['type' => 'guest', 'id' => 'guest', 'name' => $expectedName], + $notification->getRichSubjectParameters()['user'], + ); + } + + public function testPrepareWithoutSourceIsProcessed(): void { + $this->userManager->expects($this->never())->method('getDisplayName'); + $this->expectException(AlreadyProcessedException::class); + + $this->notifier->prepare($this->createMention(['sourceUser' => null]), 'en'); + } +} diff --git a/tests/unit/Service/NotificationServiceTest.php b/tests/unit/Service/NotificationServiceTest.php new file mode 100644 index 00000000000..1f5ede7e4a8 --- /dev/null +++ b/tests/unit/Service/NotificationServiceTest.php @@ -0,0 +1,61 @@ +manager = $this->createMock(IManager::class); + $this->manager->method('createNotification') + ->willReturnCallback(fn () => Server::get(IManager::class)->createNotification()); + $this->manager->method('getCount')->willReturn(0); + + $this->timeFactory = $this->createMock(ITimeFactory::class); + $this->timeFactory->method('now')->willReturn(new \DateTimeImmutable()); + } + + public static function dataMention(): array { + return [ + 'user' => ['alice', null, ['sourceUser' => 'alice', 'targetUser' => 'bob']], + 'guest' => [null, 'Gustav', ['sourceGuest' => 'Gustav', 'targetUser' => 'bob']], + ]; + } + + /** + * @dataProvider dataMention + */ + public function testMention(?string $userId, ?string $guestName, array $expectedParameters): void { + $this->manager->expects($this->once()) + ->method('notify') + ->with($this->callback(fn (INotification $n) => $n->getSubjectParameters() === $expectedParameters)); + + $service = new NotificationService($this->manager, $this->timeFactory, $userId); + $this->assertTrue($service->mention(42, 'bob', $guestName)); + } + + public function testMentionWithoutSource(): void { + $this->manager->expects($this->never())->method('notify'); + + $service = new NotificationService($this->manager, $this->timeFactory, null); + $this->assertFalse($service->mention(42, 'bob')); + } +}