Skip to content

Commit

Permalink
Merge pull request #46819 from nextcloud/refactor/theming/security-at…
Browse files Browse the repository at this point in the history 
…tributes
  • Loading branch information
@provokateurin
provokateurin authored Jul 29, 2024
2 parents 8f23598 + 79d9f2e commit 20e054c
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 33 deletions.
17 changes: 8 additions & 9 deletions apps/theming/lib/Controller/IconController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -50,9 +52,6 @@ public function __construct(
}

/**
* @PublicPage
* @NoCSRFRequired
*
* Get a themed icon
*
* @param string $app ID of the app
Expand All @@ -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';
Expand All @@ -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';
Expand Down Expand Up @@ -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';
Expand Down
30 changes: 17 additions & 13 deletions apps/theming/lib/Controller/ThemingController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -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);

Expand All @@ -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([]);
Expand All @@ -319,8 +324,6 @@ public function undoAll(): DataResponse {
}

/**
* @PublicPage
* @NoCSRFRequired
* @NoSameSiteCookieRequired
*
* Get an image
Expand All @@ -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);
Expand All @@ -356,8 +361,6 @@ public function getImage(string $key, bool $useSvg = true) {
}

/**
* @NoCSRFRequired
* @PublicPage
* @NoSameSiteCookieRequired
* @NoTwoFactorRequired
*
Expand All @@ -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))) {
Expand Down Expand Up @@ -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
Expand All @@ -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') {
Expand Down
19 changes: 8 additions & 11 deletions apps/theming/lib/Controller/UserThemeController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -59,8 +61,6 @@ public function __construct(string $appName,
}

/**
* @NoAdminRequired
*
* Enable theme
*
* @param string $themeId the theme ID
Expand All @@ -70,6 +70,7 @@ public function __construct(string $appName,
*
* 200: Theme enabled successfully
*/
#[NoAdminRequired]
public function enableTheme(string $themeId): DataResponse {
$theme = $this->validateTheme($themeId);

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

/**
* @NoAdminRequired
*
* Disable theme
*
* @param string $themeId the theme ID
Expand All @@ -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);

Expand Down Expand Up @@ -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) {
Expand All @@ -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();
Expand All @@ -168,8 +166,6 @@ public function deleteBackground(): JSONResponse {
}

/**
* @NoAdminRequired
*
* Set the background
*
* @param string $type Type of background
Expand All @@ -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');

Expand Down

0 comments on commit 20e054c

Please sign in to comment.