Skip to content

Commit 79d9f2e

Browse files
provokateurinprovokateurin
committed
refactor(theming): Replace security annotations with respective attributes
Signed-off-by: provokateurin <[email protected]>
1 parent 212a621 commit 79d9f2e

File tree

3 files changed

+33
-33
lines changed

3 files changed

+33
-33
lines changed

‎apps/theming/lib/Controller/IconController.php‎

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
use OCP\App\IAppManager;
1313
use OCP\AppFramework\Controller;
1414
use OCP\AppFramework\Http;
15+
use OCP\AppFramework\Http\Attribute\NoCSRFRequired;
16+
use OCP\AppFramework\Http\Attribute\PublicPage;
1517
use OCP\AppFramework\Http\DataDisplayResponse;
1618
use OCP\AppFramework\Http\FileDisplayResponse;
1719
use OCP\AppFramework\Http\NotFoundResponse;
@@ -50,9 +52,6 @@ public function __construct(
5052
}
5153

5254
/**
53-
* @PublicPage
54-
* @NoCSRFRequired
55-
*
5655
* Get a themed icon
5756
*
5857
* @param string $app ID of the app
@@ -63,6 +62,8 @@ public function __construct(
6362
* 200: Themed icon returned
6463
* 404: Themed icon not found
6564
*/
65+
#[PublicPage]
66+
#[NoCSRFRequired]
6667
public function getThemedIcon(string $app, string $image): Response {
6768
if ($app !== 'core' && !$this->appManager->isEnabledForUser($app)) {
6869
$app = 'core';
@@ -87,16 +88,15 @@ public function getThemedIcon(string $app, string $image): Response {
8788
/**
8889
* Return a 32x32 favicon as png
8990
*
90-
* @PublicPage
91-
* @NoCSRFRequired
92-
*
9391
* @param string $app ID of the app
9492
* @return DataDisplayResponse<Http::STATUS_OK, array{Content-Type: 'image/x-icon'}>|FileDisplayResponse<Http::STATUS_OK, array{Content-Type: 'image/x-icon'}>|NotFoundResponse<Http::STATUS_NOT_FOUND, array{}>
9593
* @throws \Exception
9694
*
9795
* 200: Favicon returned
9896
* 404: Favicon not found
9997
*/
98+
#[PublicPage]
99+
#[NoCSRFRequired]
100100
public function getFavicon(string $app = 'core'): Response {
101101
if ($app !== 'core' && !$this->appManager->isEnabledForUser($app)) {
102102
$app = 'core';
@@ -133,16 +133,15 @@ public function getFavicon(string $app = 'core'): Response {
133133
/**
134134
* Return a 512x512 icon for touch devices
135135
*
136-
* @PublicPage
137-
* @NoCSRFRequired
138-
*
139136
* @param string $app ID of the app
140137
* @return DataDisplayResponse<Http::STATUS_OK, array{Content-Type: 'image/png'}>|FileDisplayResponse<Http::STATUS_OK, array{Content-Type: 'image/x-icon'|'image/png'}>|NotFoundResponse<Http::STATUS_NOT_FOUND, array{}>
141138
* @throws \Exception
142139
*
143140
* 200: Touch icon returned
144141
* 404: Touch icon not found
145142
*/
143+
#[PublicPage]
144+
#[NoCSRFRequired]
146145
public function getTouchIcon(string $app = 'core'): Response {
147146
if ($app !== 'core' && !$this->appManager->isEnabledForUser($app)) {
148147
$app = 'core';

‎apps/theming/lib/Controller/ThemingController.php‎

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,15 @@
88
use InvalidArgumentException;
99
use OCA\Theming\ImageManager;
1010
use OCA\Theming\Service\ThemesService;
11+
use OCA\Theming\Settings\Admin;
1112
use OCA\Theming\ThemingDefaults;
1213
use OCP\App\IAppManager;
1314
use OCP\AppFramework\Controller;
1415
use OCP\AppFramework\Http;
16+
use OCP\AppFramework\Http\Attribute\AuthorizedAdminSetting;
17+
use OCP\AppFramework\Http\Attribute\BruteForceProtection;
18+
use OCP\AppFramework\Http\Attribute\NoCSRFRequired;
19+
use OCP\AppFramework\Http\Attribute\PublicPage;
1520
use OCP\AppFramework\Http\DataDisplayResponse;
1621
use OCP\AppFramework\Http\DataResponse;
1722
use OCP\AppFramework\Http\FileDisplayResponse;
@@ -66,12 +71,12 @@ public function __construct(
6671
}
6772

6873
/**
69-
* @AuthorizedAdminSetting(settings=OCA\Theming\Settings\Admin)
7074
* @param string $setting
7175
* @param string $value
7276
* @return DataResponse
7377
* @throws NotPermittedException
7478
*/
79+
#[AuthorizedAdminSetting(settings: Admin::class)]
7580
public function updateStylesheet($setting, $value) {
7681
$value = trim($value);
7782
$error = null;
@@ -146,12 +151,12 @@ public function updateStylesheet($setting, $value) {
146151
}
147152

148153
/**
149-
* @AuthorizedAdminSetting(settings=OCA\Theming\Settings\Admin)
150154
* @param string $setting
151155
* @param mixed $value
152156
* @return DataResponse
153157
* @throws NotPermittedException
154158
*/
159+
#[AuthorizedAdminSetting(settings: Admin::class)]
155160
public function updateAppMenu($setting, $value) {
156161
$error = null;
157162
switch ($setting) {
@@ -195,10 +200,10 @@ private function isValidUrl(string $url): bool {
195200
}
196201

197202
/**
198-
* @AuthorizedAdminSetting(settings=OCA\Theming\Settings\Admin)
199203
* @return DataResponse
200204
* @throws NotPermittedException
201205
*/
206+
#[AuthorizedAdminSetting(settings: Admin::class)]
202207
public function uploadImage(): DataResponse {
203208
$key = $this->request->getParam('key');
204209
if (!in_array($key, self::VALID_UPLOAD_KEYS, true)) {
@@ -275,12 +280,12 @@ public function uploadImage(): DataResponse {
275280

276281
/**
277282
* Revert setting to default value
278-
* @AuthorizedAdminSetting(settings=OCA\Theming\Settings\Admin)
279283
*
280284
* @param string $setting setting which should be reverted
281285
* @return DataResponse
282286
* @throws NotPermittedException
283287
*/
288+
#[AuthorizedAdminSetting(settings: Admin::class)]
284289
public function undo(string $setting): DataResponse {
285290
$value = $this->themingDefaults->undo($setting);
286291

@@ -298,11 +303,11 @@ public function undo(string $setting): DataResponse {
298303

299304
/**
300305
* Revert all theming settings to their default values
301-
* @AuthorizedAdminSetting(settings=OCA\Theming\Settings\Admin)
302306
*
303307
* @return DataResponse
304308
* @throws NotPermittedException
305309
*/
310+
#[AuthorizedAdminSetting(settings: Admin::class)]
306311
public function undoAll(): DataResponse {
307312
$this->themingDefaults->undoAll();
308313
$this->appManager->setDefaultApps([]);
@@ -319,8 +324,6 @@ public function undoAll(): DataResponse {
319324
}
320325

321326
/**
322-
* @PublicPage
323-
* @NoCSRFRequired
324327
* @NoSameSiteCookieRequired
325328
*
326329
* Get an image
@@ -333,6 +336,8 @@ public function undoAll(): DataResponse {
333336
* 200: Image returned
334337
* 404: Image not found
335338
*/
339+
#[PublicPage]
340+
#[NoCSRFRequired]
336341
public function getImage(string $key, bool $useSvg = true) {
337342
try {
338343
$file = $this->imageManager->getImage($key, $useSvg);
@@ -356,8 +361,6 @@ public function getImage(string $key, bool $useSvg = true) {
356361
}
357362

358363
/**
359-
* @NoCSRFRequired
360-
* @PublicPage
361364
* @NoSameSiteCookieRequired
362365
* @NoTwoFactorRequired
363366
*
@@ -371,6 +374,8 @@ public function getImage(string $key, bool $useSvg = true) {
371374
* 200: Stylesheet returned
372375
* 404: Theme not found
373376
*/
377+
#[PublicPage]
378+
#[NoCSRFRequired]
374379
public function getThemeStylesheet(string $themeId, bool $plain = false, bool $withCustomCss = false) {
375380
$themes = $this->themesService->getThemes();
376381
if (!in_array($themeId, array_keys($themes))) {
@@ -407,10 +412,6 @@ public function getThemeStylesheet(string $themeId, bool $plain = false, bool $w
407412
}
408413

409414
/**
410-
* @NoCSRFRequired
411-
* @PublicPage
412-
* @BruteForceProtection(action=manifest)
413-
*
414415
* Get the manifest for an app
415416
*
416417
* @param string $app ID of the app
@@ -420,6 +421,9 @@ public function getThemeStylesheet(string $themeId, bool $plain = false, bool $w
420421
* 200: Manifest returned
421422
* 404: App not found
422423
*/
424+
#[PublicPage]
425+
#[NoCSRFRequired]
426+
#[BruteForceProtection('manifest')]
423427
public function getManifest(string $app): JSONResponse {
424428
$cacheBusterValue = $this->config->getAppValue('theming', 'cachebuster', '0');
425429
if ($app === 'core' || $app === 'settings') {

‎apps/theming/lib/Controller/UserThemeController.php‎

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
use OCA\Theming\Service\ThemesService;
1616
use OCA\Theming\ThemingDefaults;
1717
use OCP\AppFramework\Http;
18+
use OCP\AppFramework\Http\Attribute\NoAdminRequired;
19+
use OCP\AppFramework\Http\Attribute\NoCSRFRequired;
1820
use OCP\AppFramework\Http\DataResponse;
1921
use OCP\AppFramework\Http\FileDisplayResponse;
2022
use OCP\AppFramework\Http\JSONResponse;
@@ -59,8 +61,6 @@ public function __construct(string $appName,
5961
}
6062

6163
/**
62-
* @NoAdminRequired
63-
*
6464
* Enable theme
6565
*
6666
* @param string $themeId the theme ID
@@ -70,6 +70,7 @@ public function __construct(string $appName,
7070
*
7171
* 200: Theme enabled successfully
7272
*/
73+
#[NoAdminRequired]
7374
public function enableTheme(string $themeId): DataResponse {
7475
$theme = $this->validateTheme($themeId);
7576

@@ -79,8 +80,6 @@ public function enableTheme(string $themeId): DataResponse {
7980
}
8081

8182
/**
82-
* @NoAdminRequired
83-
*
8483
* Disable theme
8584
*
8685
* @param string $themeId the theme ID
@@ -90,6 +89,7 @@ public function enableTheme(string $themeId): DataResponse {
9089
*
9190
* 200: Theme disabled successfully
9291
*/
92+
#[NoAdminRequired]
9393
public function disableTheme(string $themeId): DataResponse {
9494
$theme = $this->validateTheme($themeId);
9595

@@ -128,15 +128,14 @@ private function validateTheme(string $themeId): ITheme {
128128
}
129129

130130
/**
131-
* @NoAdminRequired
132-
* @NoCSRFRequired
133-
*
134131
* Get the background image
135132
* @return FileDisplayResponse<Http::STATUS_OK, array{Content-Type: string}>|NotFoundResponse<Http::STATUS_NOT_FOUND, array{}>
136133
*
137134
* 200: Background image returned
138135
* 404: Background image not found
139136
*/
137+
#[NoAdminRequired]
138+
#[NoCSRFRequired]
140139
public function getBackground(): Http\Response {
141140
$file = $this->backgroundService->getBackground();
142141
if ($file !== null) {
@@ -148,14 +147,13 @@ public function getBackground(): Http\Response {
148147
}
149148

150149
/**
151-
* @NoAdminRequired
152-
*
153150
* Delete the background
154151
*
155152
* @return JSONResponse<Http::STATUS_OK, ThemingBackground, array{}>
156153
*
157154
* 200: Background deleted successfully
158155
*/
156+
#[NoAdminRequired]
159157
public function deleteBackground(): JSONResponse {
160158
$currentVersion = (int)$this->config->getUserValue($this->userId, Application::APP_ID, 'userCacheBuster', '0');
161159
$this->backgroundService->deleteBackgroundImage();
@@ -168,8 +166,6 @@ public function deleteBackground(): JSONResponse {
168166
}
169167

170168
/**
171-
* @NoAdminRequired
172-
*
173169
* Set the background
174170
*
175171
* @param string $type Type of background
@@ -180,6 +176,7 @@ public function deleteBackground(): JSONResponse {
180176
* 200: Background set successfully
181177
* 400: Setting background is not possible
182178
*/
179+
#[NoAdminRequired]
183180
public function setBackground(string $type = BackgroundService::BACKGROUND_DEFAULT, string $value = '', ?string $color = null): JSONResponse {
184181
$currentVersion = (int)$this->config->getUserValue($this->userId, Application::APP_ID, 'userCacheBuster', '0');
185182

0 commit comments

Comments
 (0)