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 0381b1df1f2..becdb807f73 100644 --- a/lib/private/Route/CachingRouter.php +++ b/lib/private/Route/CachingRouter.php @@ -74,8 +74,6 @@ class CachingRouter extends Router { * @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 2203b815b0c..75c7f812204 100644 --- a/build/integration/features/bootstrap/Provisioning.php +++ b/build/integration/features/bootstrap/Provisioning.php @@ -827,6 +827,9 @@ trait Provisioning { * @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 @@ trait Provisioning { // 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 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 4a4a1d325cc..acbeebe8892 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 e8add77a936..4b346932def 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 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));