From 0ba2f5e5372922fbbb4ccac65a550654b746eff3 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 de7ddb6e1c4f53f79b25e8b365603d753b1c68e8 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 2203b815b0c16..75c7f812204aa 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 51ed61bb4a324835df9db0b47b48e45a56b00480 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 4a4a1d325ccde..acbeebe889217 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 e8add77a936a6..4b346932def32 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__ . '/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));