Merge pull request #56921 from nextcloud/fix-caching-routes-by-users-with-an-active-session

Author: provokateurin

apps/testing/appinfo/routes.php

@@ -63,5 +63,10 @@
 				'type' => null
 			]
 		],
+		[
+			'name' => 'Routes#getRoutesInRoutesPhp',
+			'url' => '/api/v1/routes/routesphp/{app}',
+			'verb' => 'GET',
+		],
 	],
 ];

apps/testing/clean_apcu_cache.php

@@ -0,0 +1,7 @@
+<?php
+
+/**
+ * SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+apcu_clear_cache();

apps/testing/composer/composer/autoload_classmap.php

@@ -12,6 +12,7 @@
     'OCA\\Testing\\Controller\\ConfigController' => $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',

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',

apps/testing/lib/Controller/RoutesController.php

@@ -0,0 +1,42 @@
+<?php
+
+/**
+ * SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+namespace OCA\Testing\Controller;
+
+use OCP\App\AppPathNotFoundException;
+use OCP\App\IAppManager;
+use OCP\AppFramework\Http;
+use OCP\AppFramework\Http\DataResponse;
+use OCP\AppFramework\OCSController;
+use OCP\IRequest;
+
+class RoutesController extends OCSController {
+
+	public function __construct(
+		string $appName,
+		IRequest $request,
+		private IAppManager $appManager,
+	) {
+		parent::__construct($appName, $request);
+	}
+
+	public function getRoutesInRoutesPhp(string $app): DataResponse {
+		try {
+			$appPath = $this->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);
+	}
+}

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) {

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;
+	}
 }

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"

lib/private/Route/CachingRouter.php

@@ -74,12 +74,17 @@ 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);
 		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));