From 709a74a2acb770751d6d6077d1c22628fdea3866 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Mon, 8 Dec 2025 16:27:58 +0100 Subject: [PATCH] fix: Fix caching routes by users with an active session MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a user has an active session only the apps that are enabled for the user are initially loaded. In order to cache the routes the routes for all apps are loaded, but routes defined in routes.php are taken into account only if the app was already loaded. Therefore, when the routes were cached in a request by a user with an active session only the routes for apps enabled for that user were cached, and those routes were used by any other user, independently of which apps they had access to. To solve that now all the enabled apps are explicitly loaded before caching the routes. Note that this did not affect routes defined using annotations on the controller files; in that case the loaded routes do not depend on the previously loaded apps, as it explicitly checks all the enabled apps. Signed-off-by: Daniel Calviño Sánchez --- apps/testing/appinfo/routes.php | 5 +++ apps/testing/clean_apcu_cache.php | 7 ++++ .../composer/composer/autoload_classmap.php | 1 + .../composer/composer/autoload_static.php | 1 + .../lib/Controller/RoutesController.php | 42 +++++++++++++++++++ .../features/bootstrap/RoutingContext.php | 23 ++++++++++ .../routing_features/apps-and-routes.feature | 19 +++++++++ lib/private/Route/CachingRouter.php | 7 ++++ 8 files changed, 105 insertions(+) create mode 100644 apps/testing/clean_apcu_cache.php create mode 100644 apps/testing/lib/Controller/RoutesController.php diff --git a/apps/testing/appinfo/routes.php b/apps/testing/appinfo/routes.php index 862f63ef4c2..25b16f420dd 100644 --- a/apps/testing/appinfo/routes.php +++ b/apps/testing/appinfo/routes.php @@ -63,5 +63,10 @@ return [ 'type' => null ] ], + [ + 'name' => 'Routes#getRoutesInRoutesPhp', + 'url' => '/api/v1/routes/routesphp/{app}', + 'verb' => 'GET', + ], ], ]; diff --git a/apps/testing/clean_apcu_cache.php b/apps/testing/clean_apcu_cache.php new file mode 100644 index 00000000000..26cc000b096 --- /dev/null +++ b/apps/testing/clean_apcu_cache.php @@ -0,0 +1,7 @@ + $baseDir . '/../lib/Controller/ConfigController.php', 'OCA\\Testing\\Controller\\LockingController' => $baseDir . '/../lib/Controller/LockingController.php', 'OCA\\Testing\\Controller\\RateLimitTestController' => $baseDir . '/../lib/Controller/RateLimitTestController.php', + 'OCA\\Testing\\Controller\\RoutesController' => $baseDir . '/../lib/Controller/RoutesController.php', 'OCA\\Testing\\Conversion\\ConversionProvider' => $baseDir . '/../lib/Conversion/ConversionProvider.php', 'OCA\\Testing\\HiddenGroupBackend' => $baseDir . '/../lib/HiddenGroupBackend.php', 'OCA\\Testing\\Listener\\GetDeclarativeSettingsValueListener' => $baseDir . '/../lib/Listener/GetDeclarativeSettingsValueListener.php', diff --git a/apps/testing/composer/composer/autoload_static.php b/apps/testing/composer/composer/autoload_static.php index bd557c37f6b..9b38b1890d6 100644 --- a/apps/testing/composer/composer/autoload_static.php +++ b/apps/testing/composer/composer/autoload_static.php @@ -27,6 +27,7 @@ class ComposerStaticInitTesting 'OCA\\Testing\\Controller\\ConfigController' => __DIR__ . '/..' . '/../lib/Controller/ConfigController.php', 'OCA\\Testing\\Controller\\LockingController' => __DIR__ . '/..' . '/../lib/Controller/LockingController.php', 'OCA\\Testing\\Controller\\RateLimitTestController' => __DIR__ . '/..' . '/../lib/Controller/RateLimitTestController.php', + 'OCA\\Testing\\Controller\\RoutesController' => __DIR__ . '/..' . '/../lib/Controller/RoutesController.php', 'OCA\\Testing\\Conversion\\ConversionProvider' => __DIR__ . '/..' . '/../lib/Conversion/ConversionProvider.php', 'OCA\\Testing\\HiddenGroupBackend' => __DIR__ . '/..' . '/../lib/HiddenGroupBackend.php', 'OCA\\Testing\\Listener\\GetDeclarativeSettingsValueListener' => __DIR__ . '/..' . '/../lib/Listener/GetDeclarativeSettingsValueListener.php', diff --git a/apps/testing/lib/Controller/RoutesController.php b/apps/testing/lib/Controller/RoutesController.php new file mode 100644 index 00000000000..4bb6b67dd53 --- /dev/null +++ b/apps/testing/lib/Controller/RoutesController.php @@ -0,0 +1,42 @@ +appManager->getAppPath($app); + } catch (AppPathNotFoundException) { + return new DataResponse([], Http::STATUS_NOT_FOUND); + } + + $file = $appPath . '/appinfo/routes.php'; + if (!file_exists($file)) { + return new DataResponse(); + } + + $routes = include $file; + + return new DataResponse($routes); + } +} diff --git a/build/integration/features/bootstrap/RoutingContext.php b/build/integration/features/bootstrap/RoutingContext.php index 762570547e0..15c47994702 100644 --- a/build/integration/features/bootstrap/RoutingContext.php +++ b/build/integration/features/bootstrap/RoutingContext.php @@ -6,6 +6,7 @@ */ use Behat\Behat\Context\Context; use Behat\Behat\Context\SnippetAcceptingContext; +use PHPUnit\Framework\Assert; require __DIR__ . '/../../vendor/autoload.php'; @@ -16,4 +17,26 @@ class RoutingContext implements Context, SnippetAcceptingContext { protected function resetAppConfigs(): void { } + + /** + * @AfterScenario + */ + public function deleteMemcacheSetting(): void { + $this->invokingTheCommand('config:system:delete memcache.local'); + } + + /** + * @Given /^route "([^"]*)" of app "([^"]*)" is defined in routes.php$/ + */ + public function routeOfAppIsDefinedInRoutesPhP(string $route, string $app): void { + $previousUser = $this->currentUser; + $this->currentUser = 'admin'; + + $this->sendingTo('GET', "/apps/testing/api/v1/routes/routesphp/{$app}"); + $this->theHTTPStatusCodeShouldBe('200'); + + Assert::assertStringContainsString($route, $this->response->getBody()->getContents()); + + $this->currentUser = $previousUser; + } } diff --git a/build/integration/routing_features/apps-and-routes.feature b/build/integration/routing_features/apps-and-routes.feature index 954ea73bfac..4ef0395ec7d 100644 --- a/build/integration/routing_features/apps-and-routes.feature +++ b/build/integration/routing_features/apps-and-routes.feature @@ -50,3 +50,22 @@ Feature: appmanagement Given As an "admin" And sending "DELETE" to "/cloud/apps/weather_status" And app "weather_status" is disabled + + Scenario: Cache routes from routes.php with a user in a group without some apps enabled + Given invoking occ with "config:system:set memcache.local --value \OC\Memcache\APCu" + And the command was successful + And route "api/v1/location" of app "weather_status" is defined in routes.php + And app "weather_status" enabled state will be restored once the scenario finishes + And invoking occ with "app:enable weather_status --groups group1" + And the command was successful + When Logging in using web as "user2" + And sending "GET" with exact url to "/apps/testing/clean_apcu_cache.php" + And Sending a "GET" to "/index.php/apps/files" with requesttoken + And the HTTP status code should be "200" + Then As an "user2" + And sending "GET" to "/apps/weather_status/api/v1/location" + And the HTTP status code should be "412" + And As an "user1" + And sending "GET" to "/apps/weather_status/api/v1/location" + And the OCS status code should be "200" + And the HTTP status code should be "200" diff --git a/lib/private/Route/CachingRouter.php b/lib/private/Route/CachingRouter.php index becdb807f73..8ed90350135 100644 --- a/lib/private/Route/CachingRouter.php +++ b/lib/private/Route/CachingRouter.php @@ -78,6 +78,13 @@ class CachingRouter extends Router { $key = $this->context->getHost() . '#' . $this->context->getBaseUrl() . '#rootCollection'; $cachedRoutes = $this->cache->get($key); if (!$cachedRoutes) { + // Ensure that all apps are loaded, as for users with an active + // session only the apps that are enabled for that user might have + // been loaded. + $enabledApps = $this->appManager->getEnabledApps(); + foreach ($enabledApps as $app) { + $this->appManager->loadApp($app); + } parent::loadRoutes(); $cachedRoutes = $this->serializeRouteCollection($this->root); $this->cache->set($key, $cachedRoutes, ($this->config->getSystemValueBool('debug') ? 3 : 3600));