From 2a200a708e2925e2ef213422c73bf1bdef49a135 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 12 Dec 2025 14:26:43 +0100 Subject: [PATCH 1/3] Revert "fix(CachingRouter): Disable cache for findMatchingRoute" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 90948f5096e5d2e3ad4b23ee05f2f4e84029eb10. It temporary disabled cache for routes until an actual fix was added, which is done in the following commits. Signed-off-by: Daniel Calviño Sánchez --- lib/private/Route/CachingRouter.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/private/Route/CachingRouter.php b/lib/private/Route/CachingRouter.php index 0381b1df1f2e1..becdb807f738b 100644 --- a/lib/private/Route/CachingRouter.php +++ b/lib/private/Route/CachingRouter.php @@ -74,8 +74,6 @@ private function serializeRouteCollection(RouteCollection $collection): array { * @return array */ public function findMatchingRoute(string $url): array { - return parent::findMatchingRoute($url); - $this->eventLogger->start('cacheroute:match', 'Match route'); $key = $this->context->getHost() . '#' . $this->context->getBaseUrl() . '#rootCollection'; $cachedRoutes = $this->cache->get($key); From 0c9488b8f000890b654aa9acee5ab5ce05caa160 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Mon, 8 Dec 2025 16:24:33 +0100 Subject: [PATCH 2/3] test: Fix recording app state when admin is not the current user MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- build/integration/features/bootstrap/Provisioning.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/build/integration/features/bootstrap/Provisioning.php b/build/integration/features/bootstrap/Provisioning.php index 16fb9e297c7a9..b13e1aa73d2c2 100644 --- a/build/integration/features/bootstrap/Provisioning.php +++ b/build/integration/features/bootstrap/Provisioning.php @@ -827,6 +827,9 @@ public function getArrayOfSubadminsResponded($resp) { * @param string $app */ public function appEnabledStateWillBeRestoredOnceTheScenarioFinishes($app) { + $previousUser = $this->currentUser; + $this->currentUser = 'admin'; + if (in_array($app, $this->getAppsWithFilter('enabled'))) { $this->appsToEnableAfterScenario[] = $app; } elseif (in_array($app, $this->getAppsWithFilter('disabled'))) { @@ -835,6 +838,8 @@ public function appEnabledStateWillBeRestoredOnceTheScenarioFinishes($app) { // Apps that were not installed before the scenario will not be // disabled nor uninstalled after the scenario. + + $this->currentUser = $previousUser; } private function getAppsWithFilter($filter) { 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 3/3] 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 862f63ef4c2fb..25b16f420dddd 100644 --- a/apps/testing/appinfo/routes.php +++ b/apps/testing/appinfo/routes.php @@ -63,5 +63,10 @@ '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 0000000000000..26cc000b096e2 --- /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 bd557c37f6bd1..9b38b1890d65f 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 0000000000000..4bb6b67dd5307 --- /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 762570547e0fb..15c47994702d7 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 954ea73bfacf3..4ef0395ec7dbc 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 becdb807f738b..8ed903501356c 100644 --- a/lib/private/Route/CachingRouter.php +++ b/lib/private/Route/CachingRouter.php @@ -78,6 +78,13 @@ public function findMatchingRoute(string $url): array { $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));