Merge pull request nextcloud#46819 from nextcloud/refactor/theming/se…

…curity-attributes

apps/theming/lib/Controller/IconController.php

@@ -12,6 +12,8 @@
 use OCP\App\IAppManager;
 use OCP\AppFramework\Controller;
 use OCP\AppFramework\Http;
+use OCP\AppFramework\Http\Attribute\NoCSRFRequired;
+use OCP\AppFramework\Http\Attribute\PublicPage;
 use OCP\AppFramework\Http\DataDisplayResponse;
 use OCP\AppFramework\Http\FileDisplayResponse;
 use OCP\AppFramework\Http\NotFoundResponse;
@@ -50,9 +52,6 @@ public function __construct(
 	}
 
 	/**
-	 * @PublicPage
-	 * @NoCSRFRequired
-	 *
 	 * Get a themed icon
 	 *
 	 * @param string $app ID of the app
@@ -63,6 +62,8 @@ public function __construct(
 	 * 200: Themed icon returned
 	 * 404: Themed icon not found
 	 */
+	#[PublicPage]
+	#[NoCSRFRequired]
 	public function getThemedIcon(string $app, string $image): Response {
 		if ($app !== 'core' && !$this->appManager->isEnabledForUser($app)) {
 			$app = 'core';
@@ -87,16 +88,15 @@ public function getThemedIcon(string $app, string $image): Response {
 	/**
 	 * Return a 32x32 favicon as png
 	 *
-	 * @PublicPage
-	 * @NoCSRFRequired
-	 *
 	 * @param string $app ID of the app
 	 * @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{}>
 	 * @throws \Exception
 	 *
 	 * 200: Favicon returned
 	 * 404: Favicon not found
 	 */
+	#[PublicPage]
+	#[NoCSRFRequired]
 	public function getFavicon(string $app = 'core'): Response {
 		if ($app !== 'core' && !$this->appManager->isEnabledForUser($app)) {
 			$app = 'core';
@@ -133,16 +133,15 @@ public function getFavicon(string $app = 'core'): Response {
 	/**
 	 * Return a 512x512 icon for touch devices
 	 *
-	 * @PublicPage
-	 * @NoCSRFRequired
-	 *
 	 * @param string $app ID of the app
 	 * @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{}>
 	 * @throws \Exception
 	 *
 	 * 200: Touch icon returned
 	 * 404: Touch icon not found
 	 */
+	#[PublicPage]
+	#[NoCSRFRequired]
 	public function getTouchIcon(string $app = 'core'): Response {
 		if ($app !== 'core' && !$this->appManager->isEnabledForUser($app)) {
 			$app = 'core';

apps/theming/lib/Controller/ThemingController.php

@@ -8,10 +8,15 @@
 use InvalidArgumentException;
 use OCA\Theming\ImageManager;
 use OCA\Theming\Service\ThemesService;
+use OCA\Theming\Settings\Admin;
 use OCA\Theming\ThemingDefaults;
 use OCP\App\IAppManager;
 use OCP\AppFramework\Controller;
 use OCP\AppFramework\Http;
+use OCP\AppFramework\Http\Attribute\AuthorizedAdminSetting;
+use OCP\AppFramework\Http\Attribute\BruteForceProtection;
+use OCP\AppFramework\Http\Attribute\NoCSRFRequired;
+use OCP\AppFramework\Http\Attribute\PublicPage;
 use OCP\AppFramework\Http\DataDisplayResponse;
 use OCP\AppFramework\Http\DataResponse;
 use OCP\AppFramework\Http\FileDisplayResponse;
@@ -66,12 +71,12 @@ public function __construct(
 	}
 
 	/**
-	 * @AuthorizedAdminSetting(settings=OCA\Theming\Settings\Admin)
 	 * @param string $setting
 	 * @param string $value
 	 * @return DataResponse
 	 * @throws NotPermittedException
 	 */
+	#[AuthorizedAdminSetting(settings: Admin::class)]
 	public function updateStylesheet($setting, $value) {
 		$value = trim($value);
 		$error = null;
@@ -146,12 +151,12 @@ public function updateStylesheet($setting, $value) {
 	}
 
 	/**
-	 * @AuthorizedAdminSetting(settings=OCA\Theming\Settings\Admin)
 	 * @param string $setting
 	 * @param mixed $value
 	 * @return DataResponse
 	 * @throws NotPermittedException
 	 */
+	#[AuthorizedAdminSetting(settings: Admin::class)]
 	public function updateAppMenu($setting, $value) {
 		$error = null;
 		switch ($setting) {
@@ -195,10 +200,10 @@ private function isValidUrl(string $url): bool {
 	}
 
 	/**
-	 * @AuthorizedAdminSetting(settings=OCA\Theming\Settings\Admin)
 	 * @return DataResponse
 	 * @throws NotPermittedException
 	 */
+	#[AuthorizedAdminSetting(settings: Admin::class)]
 	public function uploadImage(): DataResponse {
 		$key = $this->request->getParam('key');
 		if (!in_array($key, self::VALID_UPLOAD_KEYS, true)) {
@@ -275,12 +280,12 @@ public function uploadImage(): DataResponse {
 
 	/**
 	 * Revert setting to default value
-	 * @AuthorizedAdminSetting(settings=OCA\Theming\Settings\Admin)
 	 *
 	 * @param string $setting setting which should be reverted
 	 * @return DataResponse
 	 * @throws NotPermittedException
 	 */
+	#[AuthorizedAdminSetting(settings: Admin::class)]
 	public function undo(string $setting): DataResponse {
 		$value = $this->themingDefaults->undo($setting);
 
@@ -298,11 +303,11 @@ public function undo(string $setting): DataResponse {
 
 	/**
 	 * Revert all theming settings to their default values
-	 * @AuthorizedAdminSetting(settings=OCA\Theming\Settings\Admin)
 	 *
 	 * @return DataResponse
 	 * @throws NotPermittedException
 	 */
+	#[AuthorizedAdminSetting(settings: Admin::class)]
 	public function undoAll(): DataResponse {
 		$this->themingDefaults->undoAll();
 		$this->appManager->setDefaultApps([]);
@@ -319,8 +324,6 @@ public function undoAll(): DataResponse {
 	}
 
 	/**
-	 * @PublicPage
-	 * @NoCSRFRequired
 	 * @NoSameSiteCookieRequired
 	 *
 	 * Get an image
@@ -333,6 +336,8 @@ public function undoAll(): DataResponse {
 	 * 200: Image returned
 	 * 404: Image not found
 	 */
+	#[PublicPage]
+	#[NoCSRFRequired]
 	public function getImage(string $key, bool $useSvg = true) {
 		try {
 			$file = $this->imageManager->getImage($key, $useSvg);
@@ -356,8 +361,6 @@ public function getImage(string $key, bool $useSvg = true) {
 	}
 
 	/**
-	 * @NoCSRFRequired
-	 * @PublicPage
 	 * @NoSameSiteCookieRequired
 	 * @NoTwoFactorRequired
 	 *
@@ -371,6 +374,8 @@ public function getImage(string $key, bool $useSvg = true) {
 	 * 200: Stylesheet returned
 	 * 404: Theme not found
 	 */
+	#[PublicPage]
+	#[NoCSRFRequired]
 	public function getThemeStylesheet(string $themeId, bool $plain = false, bool $withCustomCss = false) {
 		$themes = $this->themesService->getThemes();
 		if (!in_array($themeId, array_keys($themes))) {
@@ -407,10 +412,6 @@ public function getThemeStylesheet(string $themeId, bool $plain = false, bool $w
 	}
 
 	/**
-	 * @NoCSRFRequired
-	 * @PublicPage
-	 * @BruteForceProtection(action=manifest)
-	 *
 	 * Get the manifest for an app
 	 *
 	 * @param string $app ID of the app
@@ -420,6 +421,9 @@ public function getThemeStylesheet(string $themeId, bool $plain = false, bool $w
 	 * 200: Manifest returned
 	 * 404: App not found
 	 */
+	#[PublicPage]
+	#[NoCSRFRequired]
+	#[BruteForceProtection('manifest')]
 	public function getManifest(string $app): JSONResponse {
 		$cacheBusterValue = $this->config->getAppValue('theming', 'cachebuster', '0');
 		if ($app === 'core' || $app === 'settings') {

apps/theming/lib/Controller/UserThemeController.php

@@ -15,6 +15,8 @@
 use OCA\Theming\Service\ThemesService;
 use OCA\Theming\ThemingDefaults;
 use OCP\AppFramework\Http;
+use OCP\AppFramework\Http\Attribute\NoAdminRequired;
+use OCP\AppFramework\Http\Attribute\NoCSRFRequired;
 use OCP\AppFramework\Http\DataResponse;
 use OCP\AppFramework\Http\FileDisplayResponse;
 use OCP\AppFramework\Http\JSONResponse;
@@ -59,8 +61,6 @@ public function __construct(string $appName,
 	}
 
 	/**
-	 * @NoAdminRequired
-	 *
 	 * Enable theme
 	 *
 	 * @param string $themeId the theme ID
@@ -70,6 +70,7 @@ public function __construct(string $appName,
 	 *
 	 * 200: Theme enabled successfully
 	 */
+	#[NoAdminRequired]
 	public function enableTheme(string $themeId): DataResponse {
 		$theme = $this->validateTheme($themeId);
 
@@ -79,8 +80,6 @@ public function enableTheme(string $themeId): DataResponse {
 	}
 
 	/**
-	 * @NoAdminRequired
-	 *
 	 * Disable theme
 	 *
 	 * @param string $themeId the theme ID
@@ -90,6 +89,7 @@ public function enableTheme(string $themeId): DataResponse {
 	 *
 	 * 200: Theme disabled successfully
 	 */
+	#[NoAdminRequired]
 	public function disableTheme(string $themeId): DataResponse {
 		$theme = $this->validateTheme($themeId);
 
@@ -128,15 +128,14 @@ private function validateTheme(string $themeId): ITheme {
 	}
 
 	/**
-	 * @NoAdminRequired
-	 * @NoCSRFRequired
-	 *
 	 * Get the background image
 	 * @return FileDisplayResponse<Http::STATUS_OK, array{Content-Type: string}>|NotFoundResponse<Http::STATUS_NOT_FOUND, array{}>
 	 *
 	 * 200: Background image returned
 	 * 404: Background image not found
 	 */
+	#[NoAdminRequired]
+	#[NoCSRFRequired]
 	public function getBackground(): Http\Response {
 		$file = $this->backgroundService->getBackground();
 		if ($file !== null) {
@@ -148,14 +147,13 @@ public function getBackground(): Http\Response {
 	}
 
 	/**
-	 * @NoAdminRequired
-	 *
 	 * Delete the background
 	 *
 	 * @return JSONResponse<Http::STATUS_OK, ThemingBackground, array{}>
 	 *
 	 * 200: Background deleted successfully
 	 */
+	#[NoAdminRequired]
 	public function deleteBackground(): JSONResponse {
 		$currentVersion = (int)$this->config->getUserValue($this->userId, Application::APP_ID, 'userCacheBuster', '0');
 		$this->backgroundService->deleteBackgroundImage();
@@ -168,8 +166,6 @@ public function deleteBackground(): JSONResponse {
 	}
 
 	/**
-	 * @NoAdminRequired
-	 *
 	 * Set the background
 	 *
 	 * @param string $type Type of background
@@ -180,6 +176,7 @@ public function deleteBackground(): JSONResponse {
 	 * 200: Background set successfully
 	 * 400: Setting background is not possible
 	 */
+	#[NoAdminRequired]
 	public function setBackground(string $type = BackgroundService::BACKGROUND_DEFAULT, string $value = '', ?string $color = null): JSONResponse {
 		$currentVersion = (int)$this->config->getUserValue($this->userId, Application::APP_ID, 'userCacheBuster', '0');