From f31f9b2272018d2b919ceca382e28c0dd74c2e64 Mon Sep 17 00:00:00 2001 From: Oleksander Piskun Date: Tue, 8 Sep 2026 08:20:06 +0000 Subject: [PATCH] fix: answer 404 from the notification endpoint when the notifications app is not enabled Signed-off-by: Oleksander Piskun --- CHANGELOG.md | 6 ++ lib/Controller/NotificationsController.php | 13 +++ openapi-full.json | 28 +++++++ openapi.json | 28 +++++++ .../NotificationsControllerTest.php | 82 +++++++++++++++++++ 5 files changed, 157 insertions(+) create mode 100644 tests/php/Controller/NotificationsControllerTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 4c6591395..2d750fd5d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,12 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/). +## [Unreleased] + +### Changed + +- `POST /ocs/v2.php/apps/app_api/api/v1/notification` now answers `404` when the `notifications` app is not enabled, instead of `200` for a notification that nobody delivers. + ## [34.0.0] ### Deprecated diff --git a/lib/Controller/NotificationsController.php b/lib/Controller/NotificationsController.php index b130f122f..1572b07f8 100644 --- a/lib/Controller/NotificationsController.php +++ b/lib/Controller/NotificationsController.php @@ -13,14 +13,17 @@ use OCA\AppAPI\Attribute\AppAPIAuth; use OCA\AppAPI\Notifications\ExNotificationsManager; use OCA\AppAPI\ResponseDefinitions; +use OCP\App\IAppManager; use OCP\AppFramework\Http; use OCP\AppFramework\Http\Attribute\NoCSRFRequired; use OCP\AppFramework\Http\Attribute\PublicPage; use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\Http\Response; +use OCP\AppFramework\OCS\OCSNotFoundException; use OCP\AppFramework\OCSController; use OCP\IRequest; use OCP\Notification\INotification; +use Psr\Log\LoggerInterface; /** * @psalm-import-type AppAPINotification from ResponseDefinitions @@ -31,6 +34,8 @@ class NotificationsController extends OCSController { public function __construct( IRequest $request, private ExNotificationsManager $exNotificationsManager, + private IAppManager $appManager, + private LoggerInterface $logger, ) { parent::__construct(Application::APP_ID, $request); @@ -43,14 +48,22 @@ public function __construct( * @param array $params Notification parameters * * @return DataResponse + * @throws OCSNotFoundException The notifications app is not enabled, so nothing could deliver the notification * * 200: Notification sent + * 404: The notifications app is not enabled on this instance */ #[AppAPIAuth] #[PublicPage] #[NoCSRFRequired] public function sendNotification(array $params): Response { $appId = $this->request->getHeader('ex-app-id'); + // The core notification manager delivers to registered notifier apps only; with the notifications app + // absent it delivers to nobody and reports nothing, so the ExApp would see a 200 for a lost notification. + if (!$this->appManager->isEnabledForAnyone('notifications')) { + $this->logger->warning('ExApp "{appId}" sent a notification, but the notifications app is not enabled', ['appId' => $appId]); + throw new OCSNotFoundException('The notifications app is not enabled, the notification cannot be delivered'); + } $userId = explode(':', base64_decode($this->request->getHeader('authorization-app-api')), 2)[0]; $notification = $this->exNotificationsManager->sendNotification($appId, $userId, $params); return new DataResponse($this->notificationToArray($notification), Http::STATUS_OK); diff --git a/openapi-full.json b/openapi-full.json index 39787bbf4..e18b1cfd2 100644 --- a/openapi-full.json +++ b/openapi-full.json @@ -5766,6 +5766,34 @@ } } } + }, + "404": { + "description": "The notifications app is not enabled, so nothing could deliver the notification", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": {} + } + } + } + } + } + } } } } diff --git a/openapi.json b/openapi.json index a520c71a2..9f281206b 100644 --- a/openapi.json +++ b/openapi.json @@ -1917,6 +1917,34 @@ } } } + }, + "404": { + "description": "The notifications app is not enabled, so nothing could deliver the notification", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": {} + } + } + } + } + } + } } } } diff --git a/tests/php/Controller/NotificationsControllerTest.php b/tests/php/Controller/NotificationsControllerTest.php new file mode 100644 index 000000000..ce3eac419 --- /dev/null +++ b/tests/php/Controller/NotificationsControllerTest.php @@ -0,0 +1,82 @@ + 'file', 'object_id' => '42', 'subject_type' => 'done', 'subject_params' => []]; + + private NotificationsController $controller; + private ExNotificationsManager&MockObject $exNotificationsManager; + private IAppManager&MockObject $appManager; + private LoggerInterface&MockObject $logger; + + protected function setUp(): void { + parent::setUp(); + $request = $this->createMock(IRequest::class); + $request->method('getHeader')->willReturnMap([ + ['ex-app-id', self::APP_ID], + ['authorization-app-api', base64_encode('alice:secret')], + ]); + $this->exNotificationsManager = $this->createMock(ExNotificationsManager::class); + $this->appManager = $this->createMock(IAppManager::class); + $this->logger = $this->createMock(LoggerInterface::class); + $this->controller = new NotificationsController( + $request, $this->exNotificationsManager, $this->appManager, $this->logger, + ); + } + + public function testSendNotificationFailsLoudlyWhenNotificationsAppIsNotEnabled(): void { + $this->appManager->method('isEnabledForAnyone')->with('notifications')->willReturn(false); + $this->exNotificationsManager->expects($this->never())->method('sendNotification'); + $this->logger->expects($this->once())->method('warning'); + + $this->expectException(OCSNotFoundException::class); + $this->controller->sendNotification(self::PARAMS); + } + + public function testSendNotificationDeliversWhenNotificationsAppIsEnabled(): void { + $this->appManager->method('isEnabledForAnyone')->with('notifications')->willReturn(true); + $notification = $this->createMock(INotification::class); + $notification->method('getApp')->willReturn(self::APP_ID); + $notification->method('getUser')->willReturn('alice'); + $notification->method('getDateTime')->willReturn(new DateTime('2026-01-01T00:00:00+00:00')); + $notification->method('getObjectType')->willReturn('file'); + $notification->method('getObjectId')->willReturn('42'); + $notification->method('getParsedSubject')->willReturn(''); + $notification->method('getParsedMessage')->willReturn(''); + $notification->method('getLink')->willReturn(''); + $notification->method('getIcon')->willReturn(''); + $this->exNotificationsManager->expects($this->once()) + ->method('sendNotification') + ->with(self::APP_ID, 'alice', self::PARAMS) + ->willReturn($notification); + $this->logger->expects($this->never())->method('warning'); + + $response = $this->controller->sendNotification(self::PARAMS); + + $this->assertSame(Http::STATUS_OK, $response->getStatus()); + $this->assertSame(self::APP_ID, $response->getData()['app']); + $this->assertSame('alice', $response->getData()['user']); + } +}