mirror of
https://github.com/nextcloud/server.git
synced 2026-02-03 20:41:22 -05:00
fix: Fix caching routes by users with an active session
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 <danxuliu@gmail.com>
This commit is contained in:
parent
0c9488b8f0
commit
709a74a2ac
8 changed files with 105 additions and 0 deletions
|
|
@ -63,5 +63,10 @@ return [
|
|||
'type' => null
|
||||
]
|
||||
],
|
||||
[
|
||||
'name' => 'Routes#getRoutesInRoutesPhp',
|
||||
'url' => '/api/v1/routes/routesphp/{app}',
|
||||
'verb' => 'GET',
|
||||
],
|
||||
],
|
||||
];
|
||||
|
|
|
|||
7
apps/testing/clean_apcu_cache.php
Normal file
7
apps/testing/clean_apcu_cache.php
Normal file
|
|
@ -0,0 +1,7 @@
|
|||
<?php
|
||||
|
||||
/**
|
||||
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
|
||||
* SPDX-License-Identifier: AGPL-3.0-or-later
|
||||
*/
|
||||
apcu_clear_cache();
|
||||
|
|
@ -12,6 +12,7 @@ return array(
|
|||
'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',
|
||||
|
|
|
|||
|
|
@ -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',
|
||||
|
|
|
|||
42
apps/testing/lib/Controller/RoutesController.php
Normal file
42
apps/testing/lib/Controller/RoutesController.php
Normal file
|
|
@ -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);
|
||||
}
|
||||
}
|
||||
|
|
@ -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;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
|
|
|
|||
|
|
@ -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));
|
||||
|
|
|
|||
Loading…
Reference in a new issue