Skip to content

Commit 4cbea7d

Browse files
committed
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 <[email protected]>
1 parent 1095ded commit 4cbea7d

File tree

6 files changed

+99
-0
lines changed

6 files changed

+99
-0
lines changed

apps/testing/appinfo/routes.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,5 +63,10 @@
6363
'type' => null
6464
]
6565
],
66+
[
67+
'name' => 'Routes#getRoutesInRoutesPhp',
68+
'url' => '/api/v1/routes/routesphp/{app}',
69+
'verb' => 'GET',
70+
],
6671
],
6772
];

apps/testing/composer/composer/autoload_classmap.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
'OCA\\Testing\\Controller\\ConfigController' => $baseDir . '/../lib/Controller/ConfigController.php',
1313
'OCA\\Testing\\Controller\\LockingController' => $baseDir . '/../lib/Controller/LockingController.php',
1414
'OCA\\Testing\\Controller\\RateLimitTestController' => $baseDir . '/../lib/Controller/RateLimitTestController.php',
15+
'OCA\\Testing\\Controller\\RoutesController' => $baseDir . '/../lib/Controller/RoutesController.php',
1516
'OCA\\Testing\\Conversion\\ConversionProvider' => $baseDir . '/../lib/Conversion/ConversionProvider.php',
1617
'OCA\\Testing\\HiddenGroupBackend' => $baseDir . '/../lib/HiddenGroupBackend.php',
1718
'OCA\\Testing\\Listener\\GetDeclarativeSettingsValueListener' => $baseDir . '/../lib/Listener/GetDeclarativeSettingsValueListener.php',

apps/testing/composer/composer/autoload_static.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ class ComposerStaticInitTesting
2727
'OCA\\Testing\\Controller\\ConfigController' => __DIR__ . '/..' . '/../lib/Controller/ConfigController.php',
2828
'OCA\\Testing\\Controller\\LockingController' => __DIR__ . '/..' . '/../lib/Controller/LockingController.php',
2929
'OCA\\Testing\\Controller\\RateLimitTestController' => __DIR__ . '/..' . '/../lib/Controller/RateLimitTestController.php',
30+
'OCA\\Testing\\Controller\\RoutesController' => __DIR__ . '/..' . '/../lib/Controller/RoutesController.php',
3031
'OCA\\Testing\\Conversion\\ConversionProvider' => __DIR__ . '/..' . '/../lib/Conversion/ConversionProvider.php',
3132
'OCA\\Testing\\HiddenGroupBackend' => __DIR__ . '/..' . '/../lib/HiddenGroupBackend.php',
3233
'OCA\\Testing\\Listener\\GetDeclarativeSettingsValueListener' => __DIR__ . '/..' . '/../lib/Listener/GetDeclarativeSettingsValueListener.php',
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
<?php
2+
3+
/**
4+
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
5+
* SPDX-License-Identifier: AGPL-3.0-only
6+
*/
7+
namespace OCA\Testing\Controller;
8+
9+
use OCP\App\IAppManager;
10+
use OCP\AppFramework\Http;
11+
use OCP\AppFramework\Http\DataResponse;
12+
use OCP\AppFramework\OCSController;
13+
use OCP\IRequest;
14+
15+
class RoutesController extends OCSController {
16+
17+
/**
18+
* @param string $appName
19+
* @param IRequest $request
20+
* @param IAppManager $appManager
21+
*/
22+
public function __construct(
23+
$appName,
24+
IRequest $request,
25+
private IAppManager $appManager,
26+
) {
27+
parent::__construct($appName, $request);
28+
}
29+
30+
/**
31+
* @param string $app
32+
* @return DataResponse
33+
*/
34+
public function getRoutesInRoutesPhp(string $app): DataResponse {
35+
try {
36+
$appPath = $this->appManager->getAppPath($app);
37+
} catch (AppPathNotFoundException) {
38+
return new DataResponse([], Http::STATUS_NOT_FOUND);
39+
}
40+
41+
$file = $appPath . '/appinfo/routes.php';
42+
if (!file_exists($file)) {
43+
return new DataResponse();
44+
}
45+
46+
$routes = include $file;
47+
48+
return new DataResponse($routes);
49+
}
50+
}

build/integration/features/bootstrap/RoutingContext.php

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
*/
77
use Behat\Behat\Context\Context;
88
use Behat\Behat\Context\SnippetAcceptingContext;
9+
use PHPUnit\Framework\Assert;
910

1011
require __DIR__ . '/../../vendor/autoload.php';
1112

@@ -16,4 +17,26 @@ class RoutingContext implements Context, SnippetAcceptingContext {
1617

1718
protected function resetAppConfigs(): void {
1819
}
20+
21+
/**
22+
* @AfterScenario
23+
*/
24+
public function deleteMemcacheSetting() {
25+
$this->invokingTheCommand('config:system:delete memcache.local');
26+
}
27+
28+
/**
29+
* @Given /^route "([^"]*)" of app "([^"]*)" is defined in routes.php$/
30+
*/
31+
public function routeOfAppIsDefinedInRoutesPhP($route, $app) {
32+
$previousUser = $this->currentUser;
33+
$this->currentUser = 'admin';
34+
35+
$this->sendingTo('GET', "/apps/testing/api/v1/routes/routesphp/{$app}");
36+
$this->theHTTPStatusCodeShouldBe('200');
37+
38+
Assert::assertStringContainsString($route, $this->response->getBody()->getContents());
39+
40+
$this->currentUser = $previousUser;
41+
}
1942
}

build/integration/routing_features/apps-and-routes.feature

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,3 +50,22 @@ Feature: appmanagement
5050
Given As an "admin"
5151
And sending "DELETE" to "/cloud/apps/weather_status"
5252
And app "weather_status" is disabled
53+
54+
Scenario: Cache routes from routes.php with a user in a group without some apps enabled
55+
Given invoking occ with "config:system:set memcache.local --value \OC\Memcache\APCu"
56+
And the command was successful
57+
And route "api/v1/location" of app "weather_status" is defined in routes.php
58+
And app "weather_status" enabled state will be restored once the scenario finishes
59+
And invoking occ with "app:enable weather_status --groups group1"
60+
And the command was successful
61+
When Logging in using web as "user2"
62+
And sending "GET" with exact url to "/apps/testing/clean_apcu_cache.php"
63+
And Sending a "GET" to "/index.php/apps/files" with requesttoken
64+
And the HTTP status code should be "200"
65+
Then As an "user2"
66+
And sending "GET" to "/apps/weather_status/api/v1/location"
67+
And the HTTP status code should be "412"
68+
And As an "user1"
69+
And sending "GET" to "/apps/weather_status/api/v1/location"
70+
And the OCS status code should be "200"
71+
And the HTTP status code should be "200"

0 commit comments

Comments
 (0)