From 6769836945ea1c6e0cdd30431d1a53808aa19c83 Mon Sep 17 00:00:00 2001 From: Oleksander Piskun Date: Tue, 16 Jun 2026 16:44:58 +0000 Subject: [PATCH] fix(maintenance): keep only HaRP and ExApp survival routes available in maintenance mode Signed-off-by: Oleksander Piskun --- lib/AppInfo/Application.php | 2 + lib/Attribute/MaintenanceModeAvailable.php | 21 +++ lib/Controller/HarpController.php | 2 + lib/Controller/OCSApiController.php | 6 + lib/Exceptions/MaintenanceModeException.php | 22 +++ lib/Middleware/MaintenanceModeMiddleware.php | 53 +++++++ .../MaintenanceModeMiddlewareTest.php | 144 ++++++++++++++++++ 7 files changed, 250 insertions(+) create mode 100644 lib/Attribute/MaintenanceModeAvailable.php create mode 100644 lib/Exceptions/MaintenanceModeException.php create mode 100644 lib/Middleware/MaintenanceModeMiddleware.php create mode 100644 tests/php/Middleware/MaintenanceModeMiddlewareTest.php diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index a47c1be0c..670bed03e 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -21,6 +21,7 @@ use OCA\AppAPI\Middleware\AppAPIAuthMiddleware; use OCA\AppAPI\Middleware\ExAppUIL10NMiddleware; use OCA\AppAPI\Middleware\ExAppUiMiddleware; +use OCA\AppAPI\Middleware\MaintenanceModeMiddleware; use OCA\AppAPI\Notifications\ExAppNotifier; use OCA\AppAPI\PublicCapabilities; use OCA\AppAPI\SetupChecks\DaemonCheck; @@ -60,6 +61,7 @@ public function register(IRegistrationContext $context): void { $context->registerEventListener(LoadAdditionalScriptsEvent::class, LoadFilesPluginListener::class); $context->registerCapability(Capabilities::class); $context->registerCapability(PublicCapabilities::class); + $context->registerMiddleware(MaintenanceModeMiddleware::class); $context->registerMiddleware(AppAPIAuthMiddleware::class); $context->registerMiddleware(ExAppUiMiddleware::class); $context->registerMiddleware(ExAppUIL10NMiddleware::class, true); diff --git a/lib/Attribute/MaintenanceModeAvailable.php b/lib/Attribute/MaintenanceModeAvailable.php new file mode 100644 index 000000000..a0f9c3158 --- /dev/null +++ b/lib/Attribute/MaintenanceModeAvailable.php @@ -0,0 +1,21 @@ +exAppService->getExApp($appId); if ($exApp === null) { diff --git a/lib/Controller/OCSApiController.php b/lib/Controller/OCSApiController.php index 154700314..70369f784 100644 --- a/lib/Controller/OCSApiController.php +++ b/lib/Controller/OCSApiController.php @@ -11,6 +11,7 @@ use OCA\AppAPI\AppInfo\Application; use OCA\AppAPI\Attribute\AppAPIAuth; +use OCA\AppAPI\Attribute\MaintenanceModeAvailable; use OCA\AppAPI\Service\AppAPIService; use OCA\AppAPI\Service\ExAppService; use OCP\AppFramework\Http; @@ -49,6 +50,7 @@ public function __construct( #[PublicPage] #[NoAdminRequired] #[NoCSRFRequired] + #[MaintenanceModeAvailable] public function log(int $level, string $message): DataResponse { try { $this->logger->log($level, $message, [ @@ -71,6 +73,7 @@ public function getNCUsersList(): DataResponse { #[AppAPIAuth] #[PublicPage] #[NoCSRFRequired] + #[MaintenanceModeAvailable] public function setAppInitProgressDeprecated(string $appId, int $progress, string $error = ''): DataResponse { $exApp = $this->exAppService->getExApp($appId); if (!$exApp) { @@ -83,6 +86,7 @@ public function setAppInitProgressDeprecated(string $appId, int $progress, strin #[AppAPIAuth] #[PublicPage] #[NoCSRFRequired] + #[MaintenanceModeAvailable] public function setAppInitProgress(int $progress, string $error = ''): DataResponse { $exApp = $this->exAppService->getExApp($this->request->getHeader('EX-APP-ID')); if (!$exApp) { @@ -101,6 +105,7 @@ public function setAppInitProgress(int $progress, string $error = ''): DataRespo #[AppAPIAuth] #[PublicPage] #[NoCSRFRequired] + #[MaintenanceModeAvailable] public function getEnabledState(): DataResponse { $exApp = $this->exAppService->getExApp($this->request->getHeader('EX-APP-ID')); if (!$exApp) { @@ -112,6 +117,7 @@ public function getEnabledState(): DataResponse { #[AppAPIAuth] #[PublicPage] #[NoCSRFRequired] + #[MaintenanceModeAvailable] public function getNextcloudAbsoluteUrl(string $url): DataResponse { return new DataResponse([ 'absolute_url' => rtrim($this->config->getSystemValueString('overwrite.cli.url'), '/') . '/' . ltrim($url, '/'), diff --git a/lib/Exceptions/MaintenanceModeException.php b/lib/Exceptions/MaintenanceModeException.php new file mode 100644 index 000000000..5e5d0c54a --- /dev/null +++ b/lib/Exceptions/MaintenanceModeException.php @@ -0,0 +1,22 @@ +config->getSystemValueBool('maintenance', false)) { + return; + } + $reflectionMethod = new ReflectionMethod($controller, $methodName); + if (!empty($reflectionMethod->getAttributes(MaintenanceModeAvailable::class))) { + return; + } + throw new MaintenanceModeException(); + } + + public function afterException($controller, $methodName, Exception $exception): Response { + if ($exception instanceof MaintenanceModeException) { + $response = new JSONResponse(['message' => $exception->getMessage()], $exception->getCode()); + $response->addHeader('X-Nextcloud-Maintenance-Mode', '1'); + $response->addHeader('Retry-After', '120'); + return $response; + } + + throw $exception; + } +} diff --git a/tests/php/Middleware/MaintenanceModeMiddlewareTest.php b/tests/php/Middleware/MaintenanceModeMiddlewareTest.php new file mode 100644 index 000000000..396f89531 --- /dev/null +++ b/tests/php/Middleware/MaintenanceModeMiddlewareTest.php @@ -0,0 +1,144 @@ +config = $this->createMock(IConfig::class); + $this->controller = new MaintenanceModeTestController('app_api', $this->createMock(IRequest::class)); + } + + private function middleware(bool $maintenance): MaintenanceModeMiddleware { + $this->config->method('getSystemValueBool')->with('maintenance', false)->willReturn($maintenance); + return new MaintenanceModeMiddleware($this->config); + } + + public function testPassesEveryRouteWhenNotInMaintenance(): void { + $this->middleware(false)->beforeController($this->controller, 'blockedRoute'); + $this->addToAssertionCount(1); + } + + public function testPassesAllowedRouteDuringMaintenance(): void { + $this->middleware(true)->beforeController($this->controller, 'allowedRoute'); + $this->addToAssertionCount(1); + } + + public function testBlocksUnmarkedRouteDuringMaintenance(): void { + $this->expectException(MaintenanceModeException::class); + $this->middleware(true)->beforeController($this->controller, 'blockedRoute'); + } + + public function testExceptionIsAnsweredWithMaintenanceResponse(): void { + $exception = new MaintenanceModeException(); + $response = $this->middleware(false)->afterException($this->controller, 'blockedRoute', $exception); + + self::assertInstanceOf(JSONResponse::class, $response); + self::assertSame(Http::STATUS_SERVICE_UNAVAILABLE, $response->getStatus()); + self::assertSame('1', $response->getHeaders()['X-Nextcloud-Maintenance-Mode']); + self::assertSame('120', $response->getHeaders()['Retry-After']); + self::assertSame(['message' => $exception->getMessage()], $response->getData()); + } + + public function testBlockedRouteInMaintenanceYields503EndToEnd(): void { + $middleware = $this->middleware(true); + + try { + $middleware->beforeController($this->controller, 'blockedRoute'); + self::fail('Expected MaintenanceModeException to be thrown'); + } catch (MaintenanceModeException $exception) { + $response = $middleware->afterException($this->controller, 'blockedRoute', $exception); + } + + self::assertSame(Http::STATUS_SERVICE_UNAVAILABLE, $response->getStatus()); + self::assertSame('1', $response->getHeaders()['X-Nextcloud-Maintenance-Mode']); + self::assertSame('120', $response->getHeaders()['Retry-After']); + } + + public function testUnrelatedExceptionIsRethrown(): void { + $exception = new \RuntimeException('boom'); + $this->expectExceptionObject($exception); + $this->middleware(false)->afterException($this->controller, 'blockedRoute', $exception); + } + + /** + * @return list + */ + public static function allowlistedRoutes(): array { + return [ + [HarpController::class, 'getExAppMetadata'], + [OCSApiController::class, 'setAppInitProgress'], + [OCSApiController::class, 'setAppInitProgressDeprecated'], + [OCSApiController::class, 'getEnabledState'], + [OCSApiController::class, 'getNextcloudAbsoluteUrl'], + [OCSApiController::class, 'log'], + ]; + } + + #[DataProvider('allowlistedRoutes')] + public function testAllowlistedRouteCarriesAttribute(string $class, string $method): void { + $attributes = (new ReflectionMethod($class, $method))->getAttributes(MaintenanceModeAvailable::class); + self::assertNotEmpty($attributes, $class . '::' . $method . ' must stay reachable during maintenance'); + } + + /** + * @return list + */ + public static function blockedRoutes(): array { + return [ + [HarpController::class, 'getUserInfo'], + [OCSApiController::class, 'getNCUsersList'], + [OCSExAppController::class, 'getExAppsList'], + [AppConfigController::class, 'setAppConfigValue'], + [AppConfigController::class, 'getAppConfigValues'], + [PreferencesController::class, 'setUserConfigValue'], + [PreferencesController::class, 'getUserConfigValues'], + [ExAppsPageController::class, 'uninstallApp'], + [TalkBotController::class, 'registerExAppTalkBot'], + ]; + } + + #[DataProvider('blockedRoutes')] + public function testNonAllowlistedRouteHasNoAttribute(string $class, string $method): void { + $attributes = (new ReflectionMethod($class, $method))->getAttributes(MaintenanceModeAvailable::class); + self::assertEmpty($attributes, $class . '::' . $method . ' must return 503 during maintenance'); + } +}