Skip to content

Commit c870b6a

Browse files
rullzerrullzer
committed
Fix new routing in settings etc
Also prefix resources Unify the prefix handling Handle urls with and without slash Signed-off-by: Joas Schilling <[email protected]> Signed-off-by: Roeland Jago Douma <[email protected]>
1 parent ac57bbc commit c870b6a

3 files changed

Lines changed: 98 additions & 110 deletions

File tree

‎apps/settings/appinfo/routes.php‎

Lines changed: 52 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
11
<?php
2+
3+
declare(strict_types=1);
4+
25
/**
36
* @copyright Copyright (c) 2016, ownCloud, Inc.
47
*
@@ -31,68 +34,61 @@
3134
*
3235
*/
3336

34-
namespace OCA\Settings;
35-
36-
use OCA\Settings\AppInfo\Application;
37-
38-
/** @var Application $application */
39-
$application = \OC::$server->query(Application::class);
40-
$this->useCollection('root');
41-
$application->registerRoutes($this, [
37+
return [
4238
'resources' => [
43-
'AuthSettings' => ['url' => '/settings/personal/authtokens'],
39+
'AuthSettings' => ['url' => '/settings/personal/authtokens' , 'root' => ''],
4440
],
4541
'routes' => [
46-
['name' => 'AuthSettings#wipe', 'url' => '/settings/personal/authtokens/wipe/{id}', 'verb' => 'POST'],
42+
['name' => 'AuthSettings#wipe', 'url' => '/settings/personal/authtokens/wipe/{id}', 'verb' => 'POST' , 'root' => ''],
4743

48-
['name' => 'MailSettings#setMailSettings', 'url' => '/settings/admin/mailsettings', 'verb' => 'POST'],
49-
['name' => 'MailSettings#storeCredentials', 'url' => '/settings/admin/mailsettings/credentials', 'verb' => 'POST'],
50-
['name' => 'MailSettings#sendTestMail', 'url' => '/settings/admin/mailtest', 'verb' => 'POST'],
51-
['name' => 'Encryption#startMigration', 'url' => '/settings/admin/startmigration', 'verb' => 'POST'],
44+
['name' => 'MailSettings#setMailSettings', 'url' => '/settings/admin/mailsettings', 'verb' => 'POST' , 'root' => ''],
45+
['name' => 'MailSettings#storeCredentials', 'url' => '/settings/admin/mailsettings/credentials', 'verb' => 'POST' , 'root' => ''],
46+
['name' => 'MailSettings#sendTestMail', 'url' => '/settings/admin/mailtest', 'verb' => 'POST' , 'root' => ''],
47+
['name' => 'Encryption#startMigration', 'url' => '/settings/admin/startmigration', 'verb' => 'POST' , 'root' => ''],
5248

53-
['name' => 'AppSettings#listCategories', 'url' => '/settings/apps/categories', 'verb' => 'GET'],
54-
['name' => 'AppSettings#viewApps', 'url' => '/settings/apps', 'verb' => 'GET'],
55-
['name' => 'AppSettings#listApps', 'url' => '/settings/apps/list', 'verb' => 'GET'],
56-
['name' => 'AppSettings#enableApp', 'url' => '/settings/apps/enable/{appId}', 'verb' => 'GET'],
57-
['name' => 'AppSettings#enableApp', 'url' => '/settings/apps/enable/{appId}', 'verb' => 'POST'],
58-
['name' => 'AppSettings#enableApps', 'url' => '/settings/apps/enable', 'verb' => 'POST'],
59-
['name' => 'AppSettings#disableApp', 'url' => '/settings/apps/disable/{appId}', 'verb' => 'GET'],
60-
['name' => 'AppSettings#disableApps', 'url' => '/settings/apps/disable', 'verb' => 'POST'],
61-
['name' => 'AppSettings#updateApp', 'url' => '/settings/apps/update/{appId}', 'verb' => 'GET'],
62-
['name' => 'AppSettings#uninstallApp', 'url' => '/settings/apps/uninstall/{appId}', 'verb' => 'GET'],
63-
['name' => 'AppSettings#viewApps', 'url' => '/settings/apps/{category}', 'verb' => 'GET', 'defaults' => ['category' => '']],
64-
['name' => 'AppSettings#viewApps', 'url' => '/settings/apps/{category}/{id}', 'verb' => 'GET', 'defaults' => ['category' => '', 'id' => '']],
65-
['name' => 'AppSettings#force', 'url' => '/settings/apps/force', 'verb' => 'POST'],
49+
['name' => 'AppSettings#listCategories', 'url' => '/settings/apps/categories', 'verb' => 'GET' , 'root' => ''],
50+
['name' => 'AppSettings#viewApps', 'url' => '/settings/apps', 'verb' => 'GET' , 'root' => ''],
51+
['name' => 'AppSettings#listApps', 'url' => '/settings/apps/list', 'verb' => 'GET' , 'root' => ''],
52+
['name' => 'AppSettings#enableApp', 'url' => '/settings/apps/enable/{appId}', 'verb' => 'GET' , 'root' => ''],
53+
['name' => 'AppSettings#enableApp', 'url' => '/settings/apps/enable/{appId}', 'verb' => 'POST' , 'root' => ''],
54+
['name' => 'AppSettings#enableApps', 'url' => '/settings/apps/enable', 'verb' => 'POST' , 'root' => ''],
55+
['name' => 'AppSettings#disableApp', 'url' => '/settings/apps/disable/{appId}', 'verb' => 'GET' , 'root' => ''],
56+
['name' => 'AppSettings#disableApps', 'url' => '/settings/apps/disable', 'verb' => 'POST' , 'root' => ''],
57+
['name' => 'AppSettings#updateApp', 'url' => '/settings/apps/update/{appId}', 'verb' => 'GET' , 'root' => ''],
58+
['name' => 'AppSettings#uninstallApp', 'url' => '/settings/apps/uninstall/{appId}', 'verb' => 'GET' , 'root' => ''],
59+
['name' => 'AppSettings#viewApps', 'url' => '/settings/apps/{category}', 'verb' => 'GET', 'defaults' => ['category' => ''] , 'root' => ''],
60+
['name' => 'AppSettings#viewApps', 'url' => '/settings/apps/{category}/{id}', 'verb' => 'GET', 'defaults' => ['category' => '', 'id' => ''] , 'root' => ''],
61+
['name' => 'AppSettings#force', 'url' => '/settings/apps/force', 'verb' => 'POST' , 'root' => ''],
6662

67-
['name' => 'Users#setDisplayName', 'url' => '/settings/users/{username}/displayName', 'verb' => 'POST'],
68-
['name' => 'Users#setEMailAddress', 'url' => '/settings/users/{id}/mailAddress', 'verb' => 'PUT'],
69-
['name' => 'Users#setUserSettings', 'url' => '/settings/users/{username}/settings', 'verb' => 'PUT'],
70-
['name' => 'Users#getVerificationCode', 'url' => '/settings/users/{account}/verify', 'verb' => 'GET'],
71-
['name' => 'Users#usersList', 'url' => '/settings/users', 'verb' => 'GET'],
72-
['name' => 'Users#usersListByGroup', 'url' => '/settings/users/{group}', 'verb' => 'GET', 'requirements' => ['group' => '.+']],
73-
['name' => 'Users#setPreference', 'url' => '/settings/users/preferences/{key}', 'verb' => 'POST'],
74-
['name' => 'LogSettings#setLogLevel', 'url' => '/settings/admin/log/level', 'verb' => 'POST'],
75-
['name' => 'LogSettings#getEntries', 'url' => '/settings/admin/log/entries', 'verb' => 'GET'],
76-
['name' => 'LogSettings#download', 'url' => '/settings/admin/log/download', 'verb' => 'GET'],
77-
['name' => 'CheckSetup#check', 'url' => '/settings/ajax/checksetup', 'verb' => 'GET'],
78-
['name' => 'CheckSetup#getFailedIntegrityCheckFiles', 'url' => '/settings/integrity/failed', 'verb' => 'GET'],
79-
['name' => 'CheckSetup#rescanFailedIntegrityCheck', 'url' => '/settings/integrity/rescan', 'verb' => 'GET'],
80-
['name' => 'Certificate#addPersonalRootCertificate', 'url' => '/settings/personal/certificate', 'verb' => 'POST'],
81-
['name' => 'Certificate#removePersonalRootCertificate', 'url' => '/settings/personal/certificate/{certificateIdentifier}', 'verb' => 'DELETE'],
82-
['name' => 'Certificate#addSystemRootCertificate', 'url' => '/settings/admin/certificate', 'verb' => 'POST'],
83-
['name' => 'Certificate#removeSystemRootCertificate', 'url' => '/settings/admin/certificate/{certificateIdentifier}', 'verb' => 'DELETE'],
84-
['name' => 'PersonalSettings#index', 'url' => '/settings/user/{section}', 'verb' => 'GET', 'defaults' => ['section' => 'personal-info']],
85-
['name' => 'AdminSettings#index', 'url' => '/settings/admin/{section}', 'verb' => 'GET', 'defaults' => ['section' => 'server']],
86-
['name' => 'AdminSettings#form', 'url' => '/settings/admin/{section}', 'verb' => 'GET'],
87-
['name' => 'ChangePassword#changePersonalPassword', 'url' => '/settings/personal/changepassword', 'verb' => 'POST'],
88-
['name' => 'ChangePassword#changeUserPassword', 'url' => '/settings/users/changepassword', 'verb' => 'POST'],
89-
['name' => 'TwoFactorSettings#index', 'url' => '/settings/api/admin/twofactorauth', 'verb' => 'GET'],
90-
['name' => 'TwoFactorSettings#update', 'url' => '/settings/api/admin/twofactorauth', 'verb' => 'PUT'],
63+
['name' => 'Users#setDisplayName', 'url' => '/settings/users/{username}/displayName', 'verb' => 'POST' , 'root' => ''],
64+
['name' => 'Users#setEMailAddress', 'url' => '/settings/users/{id}/mailAddress', 'verb' => 'PUT' , 'root' => ''],
65+
['name' => 'Users#setUserSettings', 'url' => '/settings/users/{username}/settings', 'verb' => 'PUT' , 'root' => ''],
66+
['name' => 'Users#getVerificationCode', 'url' => '/settings/users/{account}/verify', 'verb' => 'GET' , 'root' => ''],
67+
['name' => 'Users#usersList', 'url' => '/settings/users', 'verb' => 'GET' , 'root' => ''],
68+
['name' => 'Users#usersListByGroup', 'url' => '/settings/users/{group}', 'verb' => 'GET', 'requirements' => ['group' => '.+'] , 'root' => ''],
69+
['name' => 'Users#setPreference', 'url' => '/settings/users/preferences/{key}', 'verb' => 'POST' , 'root' => ''],
70+
['name' => 'LogSettings#setLogLevel', 'url' => '/settings/admin/log/level', 'verb' => 'POST' , 'root' => ''],
71+
['name' => 'LogSettings#getEntries', 'url' => '/settings/admin/log/entries', 'verb' => 'GET' , 'root' => ''],
72+
['name' => 'LogSettings#download', 'url' => '/settings/admin/log/download', 'verb' => 'GET' , 'root' => ''],
73+
['name' => 'CheckSetup#check', 'url' => '/settings/ajax/checksetup', 'verb' => 'GET' , 'root' => ''],
74+
['name' => 'CheckSetup#getFailedIntegrityCheckFiles', 'url' => '/settings/integrity/failed', 'verb' => 'GET' , 'root' => ''],
75+
['name' => 'CheckSetup#rescanFailedIntegrityCheck', 'url' => '/settings/integrity/rescan', 'verb' => 'GET' , 'root' => ''],
76+
['name' => 'Certificate#addPersonalRootCertificate', 'url' => '/settings/personal/certificate', 'verb' => 'POST' , 'root' => ''],
77+
['name' => 'Certificate#removePersonalRootCertificate', 'url' => '/settings/personal/certificate/{certificateIdentifier}', 'verb' => 'DELETE' , 'root' => ''],
78+
['name' => 'Certificate#addSystemRootCertificate', 'url' => '/settings/admin/certificate', 'verb' => 'POST' , 'root' => ''],
79+
['name' => 'Certificate#removeSystemRootCertificate', 'url' => '/settings/admin/certificate/{certificateIdentifier}', 'verb' => 'DELETE' , 'root' => ''],
80+
['name' => 'PersonalSettings#index', 'url' => '/settings/user/{section}', 'verb' => 'GET', 'defaults' => ['section' => 'personal-info'] , 'root' => ''],
81+
['name' => 'AdminSettings#index', 'url' => '/settings/admin/{section}', 'verb' => 'GET', 'defaults' => ['section' => 'server'] , 'root' => ''],
82+
['name' => 'AdminSettings#form', 'url' => '/settings/admin/{section}', 'verb' => 'GET' , 'root' => ''],
83+
['name' => 'ChangePassword#changePersonalPassword', 'url' => '/settings/personal/changepassword', 'verb' => 'POST' , 'root' => ''],
84+
['name' => 'ChangePassword#changeUserPassword', 'url' => '/settings/users/changepassword', 'verb' => 'POST' , 'root' => ''],
85+
['name' => 'TwoFactorSettings#index', 'url' => '/settings/api/admin/twofactorauth', 'verb' => 'GET' , 'root' => ''],
86+
['name' => 'TwoFactorSettings#update', 'url' => '/settings/api/admin/twofactorauth', 'verb' => 'PUT' , 'root' => ''],
9187

92-
['name' => 'Help#help', 'url' => '/settings/help/{mode}', 'verb' => 'GET', 'defaults' => ['mode' => '']],
88+
['name' => 'Help#help', 'url' => '/settings/help/{mode}', 'verb' => 'GET', 'defaults' => ['mode' => ''] , 'root' => ''],
9389

94-
['name' => 'WebAuthn#startRegistration', 'url' => '/settings/api/personal/webauthn/registration', 'verb' => 'GET'],
95-
['name' => 'WebAuthn#finishRegistration', 'url' => '/settings/api/personal/webauthn/registration', 'verb' => 'POST'],
96-
['name' => 'WebAuthn#deleteRegistration', 'url' => '/settings/api/personal/webauthn/registration/{id}', 'verb' => 'DELETE'],
90+
['name' => 'WebAuthn#startRegistration', 'url' => '/settings/api/personal/webauthn/registration', 'verb' => 'GET' , 'root' => ''],
91+
['name' => 'WebAuthn#finishRegistration', 'url' => '/settings/api/personal/webauthn/registration', 'verb' => 'POST' , 'root' => ''],
92+
['name' => 'WebAuthn#deleteRegistration', 'url' => '/settings/api/personal/webauthn/registration/{id}', 'verb' => 'DELETE' , 'root' => ''],
9793
]
98-
]);
94+
];

‎lib/private/AppFramework/Routing/RouteConfig.php‎

Lines changed: 44 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ class RouteConfig {
6060
'core',
6161
'files_sharing',
6262
'files',
63+
'settings',
6364
'spreed',
6465
];
6566

@@ -82,10 +83,10 @@ public function __construct(DIContainer $container, IRouter $router, $routes) {
8283
public function register() {
8384

8485
// parse simple
85-
$this->processSimpleRoutes($this->routes);
86+
$this->processIndexRoutes($this->routes);
8687

8788
// parse resources
88-
$this->processResources($this->routes);
89+
$this->processIndexResources($this->routes);
8990

9091
/*
9192
* OCS routes go into a different collection
@@ -114,7 +115,7 @@ private function processOCS(array $routes): void {
114115
* @param array $routes
115116
* @throws \UnexpectedValueException
116117
*/
117-
private function processSimpleRoutes(array $routes): void {
118+
private function processIndexRoutes(array $routes): void {
118119
$simpleRoutes = $routes['routes'] ?? [];
119120
foreach ($simpleRoutes as $simpleRoute) {
120121
$this->processRoute($simpleRoute);
@@ -124,14 +125,9 @@ private function processSimpleRoutes(array $routes): void {
124125
protected function processRoute(array $route, string $routeNamePrefix = ''): void {
125126
$name = $route['name'];
126127
$postfix = $route['postfix'] ?? '';
127-
$defaultRoot = $this->appName === 'core' ? '' : '/apps/' . $this->appName;
128-
$root = $route['root'] ?? $defaultRoot;
129-
if ($routeNamePrefix === '' && !\in_array($this->appName, $this->rootUrlApps, true)) {
130-
// Only allow root URLS for some apps
131-
$root = $defaultRoot;
132-
}
128+
$root = $this->buildRootPrefix($route, $routeNamePrefix);
133129

134-
$url = $root . $route['url'];
130+
$url = $root . '/' . ltrim($route['url'], '/');
135131
$verb = strtoupper($route['verb'] ?? 'GET');
136132

137133
$split = explode('#', $name, 2);
@@ -176,44 +172,7 @@ protected function processRoute(array $route, string $routeNamePrefix = ''): voi
176172
* @param array $routes
177173
*/
178174
private function processOCSResources(array $routes): void {
179-
// declaration of all restful actions
180-
$actions = [
181-
['name' => 'index', 'verb' => 'GET', 'on-collection' => true],
182-
['name' => 'show', 'verb' => 'GET'],
183-
['name' => 'create', 'verb' => 'POST', 'on-collection' => true],
184-
['name' => 'update', 'verb' => 'PUT'],
185-
['name' => 'destroy', 'verb' => 'DELETE'],
186-
];
187-
188-
$resources = $routes['ocs-resources'] ?? [];
189-
foreach ($resources as $resource => $config) {
190-
$root = $config['root'] ?? '/apps/' . $this->appName;
191-
192-
// the url parameter used as id to the resource
193-
foreach ($actions as $action) {
194-
$url = $root . $config['url'];
195-
$method = $action['name'];
196-
$verb = strtoupper($action['verb'] ?? 'GET');
197-
$collectionAction = $action['on-collection'] ?? false;
198-
if (!$collectionAction) {
199-
$url .= '/{id}';
200-
}
201-
if (isset($action['url-postfix'])) {
202-
$url .= '/' . $action['url-postfix'];
203-
}
204-
205-
$controller = $resource;
206-
207-
$controllerName = $this->buildControllerName($controller);
208-
$actionName = $this->buildActionName($method);
209-
210-
$routeName = 'ocs.' . $this->appName . '.' . strtolower($resource) . '.' . strtolower($method);
211-
212-
$this->router->create($routeName, $url)->method($verb)->action(
213-
new RouteActionHandler($this->container, $controllerName, $actionName)
214-
);
215-
}
216-
}
175+
$this->processResources($routes['ocs-resources'] ?? [], 'ocs.');
217176
}
218177

219178
/**
@@ -226,7 +185,22 @@ private function processOCSResources(array $routes): void {
226185
*
227186
* @param array $routes
228187
*/
229-
private function processResources(array $routes): void {
188+
private function processIndexResources(array $routes): void {
189+
$this->processResources($routes['resources'] ?? []);
190+
}
191+
192+
/**
193+
* For a given name and url restful routes are created:
194+
* - index
195+
* - show
196+
* - create
197+
* - update
198+
* - destroy
199+
*
200+
* @param array $resources
201+
* @param string $routeNamePrefix
202+
*/
203+
protected function processResources(array $resources, string $routeNamePrefix = ''): void {
230204
// declaration of all restful actions
231205
$actions = [
232206
['name' => 'index', 'verb' => 'GET', 'on-collection' => true],
@@ -236,13 +210,14 @@ private function processResources(array $routes): void {
236210
['name' => 'destroy', 'verb' => 'DELETE'],
237211
];
238212

239-
$resources = $routes['resources'] ?? [];
240213
foreach ($resources as $resource => $config) {
214+
$root = $this->buildRootPrefix($config, $routeNamePrefix);
241215

242216
// the url parameter used as id to the resource
243217
foreach ($actions as $action) {
244-
$url = $config['url'];
218+
$url = $root . '/' . ltrim($config['url'], '/');
245219
$method = $action['name'];
220+
246221
$verb = strtoupper($action['verb'] ?? 'GET');
247222
$collectionAction = $action['on-collection'] ?? false;
248223
if (!$collectionAction) {
@@ -257,7 +232,7 @@ private function processResources(array $routes): void {
257232
$controllerName = $this->buildControllerName($controller);
258233
$actionName = $this->buildActionName($method);
259234

260-
$routeName = $this->appName . '.' . strtolower($resource) . '.' . strtolower($method);
235+
$routeName = $routeNamePrefix . $this->appName . '.' . strtolower($resource) . '.' . strtolower($method);
261236

262237
$this->router->create($routeName, $url)->method($verb)->action(
263238
new RouteActionHandler($this->container, $controllerName, $actionName)
@@ -266,6 +241,23 @@ private function processResources(array $routes): void {
266241
}
267242
}
268243

244+
private function buildRootPrefix(array $route, string $routeNamePrefix): string {
245+
$defaultRoot = $this->appName === 'core' ? '' : '/apps/' . $this->appName;
246+
$root = $route['root'] ?? $defaultRoot;
247+
248+
if ($routeNamePrefix !== '') {
249+
// In OCS all apps are whitelisted
250+
return $root;
251+
}
252+
253+
if (!\in_array($this->appName, $this->rootUrlApps, true)) {
254+
// Only allow root URLS for some apps
255+
return $defaultRoot;
256+
}
257+
258+
return $root;
259+
}
260+
269261
/**
270262
* Based on a given route name the controller name is generated
271263
* @param string $controller

‎tests/lib/AppFramework/Routing/RoutingTest.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -194,13 +194,13 @@ public function testOCSResourceWithRoot() {
194194
public function testResource() {
195195
$routes = ['resources' => ['account' => ['url' => '/accounts']]];
196196

197-
$this->assertResource($routes, 'account', '/accounts', 'AccountController', 'id');
197+
$this->assertResource($routes, 'account', '/apps/app1/accounts', 'AccountController', 'id');
198198
}
199199

200200
public function testResourceWithUnderScoreName() {
201201
$routes = ['resources' => ['admin_accounts' => ['url' => '/admin/accounts']]];
202202

203-
$this->assertResource($routes, 'admin_accounts', '/admin/accounts', 'AdminAccountsController', 'id');
203+
$this->assertResource($routes, 'admin_accounts', '/apps/app1/admin/accounts', 'AdminAccountsController', 'id');
204204
}
205205

206206
private function assertSimpleRoute($routes, $name, $verb, $url, $controllerName, $actionName, array $requirements = [], array $defaults = [], $postfix = '', $allowRootUrl = false): void {

0 commit comments

Comments
 (0)