Merge pull request #23777 from nextcloud/backport/23024/stable19

apps/theming/appinfo/info.xml

@@ -25,4 +25,7 @@
 		<admin>OCA\Theming\Settings\Admin</admin>
 		<admin-section>OCA\Theming\Settings\Section</admin-section>
 	</settings>
+	<commands>
+		<command>OCA\Theming\Command\UpdateConfig</command>
+	</commands>
 </info>

apps/theming/lib/Command/UpdateConfig.php

@@ -0,0 +1,135 @@
+<?php
+/**
+ * @copyright Copyright (c) 2020 Julius Härtl <jus@bitgrid.net>
+ *
+ * @author Julius Härtl <jus@bitgrid.net>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace OCA\Theming\Command;
+
+use OCA\Theming\ImageManager;
+use OCA\Theming\ThemingDefaults;
+use OCP\IConfig;
+use Symfony\Component\Console\Command\Command;
+use Symfony\Component\Console\Input\InputArgument;
+use Symfony\Component\Console\Input\InputInterface;
+use Symfony\Component\Console\Input\InputOption;
+use Symfony\Component\Console\Output\OutputInterface;
+
+class UpdateConfig extends Command {
+	public const SUPPORTED_KEYS = [
+		'name', 'url', 'imprintUrl', 'privacyUrl', 'slogan', 'color'
+	];
+
+	public const SUPPORTED_IMAGE_KEYS = [
+		'background', 'logo', 'favicon', 'logoheader'
+	];
+
+	private $themingDefaults;
+	private $imageManager;
+	private $config;
+
+	public function __construct(ThemingDefaults $themingDefaults, ImageManager $imageManager, IConfig $config) {
+		parent::__construct();
+
+		$this->themingDefaults = $themingDefaults;
+		$this->imageManager = $imageManager;
+		$this->config = $config;
+	}
+
+	protected function configure() {
+		$this
+			->setName('theming:config')
+			->setDescription('Set theming app config values')
+			->addArgument(
+				'key',
+				InputArgument::OPTIONAL,
+				'Key to update the theming app configuration (leave empty to get a list of all configured values)' . PHP_EOL .
+				'One of: ' . implode(', ', self::SUPPORTED_KEYS)
+			)
+			->addArgument(
+				'value',
+				InputArgument::OPTIONAL,
+				'Value to set (leave empty to obtain the current value)'
+			)
+			->addOption(
+				'reset',
+				'r',
+				InputOption::VALUE_NONE,
+				'Reset the given config key to default'
+			);
+	}
+
+
+	protected function execute(InputInterface $input, OutputInterface $output): int {
+		$key = $input->getArgument('key');
+		$value = $input->getArgument('value');
+
+		if ($key === null) {
+			$output->writeln('Current theming config:');
+			foreach (self::SUPPORTED_KEYS as $key) {
+				$value = $this->config->getAppValue('theming', $key, '');
+				$output->writeln('- ' . $key . ': ' . $value . '');
+			}
+			foreach (self::SUPPORTED_IMAGE_KEYS as $key) {
+				$value = $this->config->getAppValue('theming', $key . 'Mime', '');
+				$output->writeln('- ' . $key . ': ' . $value . '');
+			}
+			return 0;
+		}
+
+		if (!in_array($key, self::SUPPORTED_KEYS, true)) {
+			$output->writeln('<error>Invalid config key provided</error>');
+			return 1;
+		}
+
+		if ($input->getOption('reset')) {
+			$defaultValue = $this->themingDefaults->undo($key);
+			$output->writeln('<info>Reset ' . $key . ' to ' . $defaultValue . '</info>');
+			return 0;
+		}
+
+		if ($value === null) {
+			$value = $this->config->getAppValue('theming', $key, '');
+			if ($value !== '') {
+				$output->writeln('<info>' . $key . ' is currently set to ' . $value . '</info>');
+			} else {
+				$output->writeln('<info>' . $key . ' is currently not set</info>');
+			}
+			return 0;
+		}
+
+		if (in_array($key, self::SUPPORTED_IMAGE_KEYS, true)) {
+			if (file_exists(__DIR__ . $value)) {
+				$value = __DIR__ . $value;
+			}
+			if (!file_exists($value)) {
+				$output->writeln('<error>File could not be found: ' . $value . '</error>');
+				return 1;
+			}
+			$value = $this->imageManager->updateImage($key, $value);
+			$key = $key . 'Mime';
+		}
+
+		$this->themingDefaults->set($key, $value);
+		$output->writeln('<info>Updated ' . $key . ' to ' . $value . '</info>');
+
+		return 0;
+	}
+}

apps/theming/lib/Controller/ThemingController.php

@@ -221,9 +221,6 @@ class ThemingController extends Controller {
 	 * @throws NotPermittedException
 	 */
 	public function uploadImage(): DataResponse {
-		// logo / background
-		// new: favicon logo-header
-		//
 		$key = $this->request->getParam('key');
 		$image = $this->request->getUploadedFile('image');
 		$error = null;
@@ -256,23 +253,14 @@ class ThemingController extends Controller {
 			);
 		}
 
-		$name = '';
 		try {
-			$folder = $this->appData->getFolder('images');
+			$mime = $this->imageManager->updateImage($key, $image['tmp_name']);
-		} catch (NotFoundException $e) {
+			$this->themingDefaults->set($key . 'Mime', $mime);
-			$folder = $this->appData->newFolder('images');
+		} catch (\Exception $e) {
-		}
-
-		$this->imageManager->delete($key);
-
-		$target = $folder->newFile($key);
-		$supportedFormats = $this->getSupportedUploadImageFormats($key);
-		$detectedMimeType = mime_content_type($image['tmp_name']);
-		if (!in_array($image['type'], $supportedFormats) || !in_array($detectedMimeType, $supportedFormats)) {
 			return new DataResponse(
 				[
 					'data' => [
-						'message' => $this->l10n->t('Unsupported image type'),
+						'message' => $e->getMessage()
 					],
 					'status' => 'failure',
 				],
@@ -280,32 +268,7 @@ class ThemingController extends Controller {
 			);
 		}
 
-		if ($key === 'background' && strpos($detectedMimeType, 'image/svg') === false) {
-			// Optimize the image since some people may upload images that will be
-			// either to big or are not progressive rendering.
-			$newImage = @imagecreatefromstring(file_get_contents($image['tmp_name'], 'r'));
-
-			// Preserve transparency
-			imagesavealpha($newImage, true);
-			imagealphablending($newImage, true);
-
-			$tmpFile = $this->tempManager->getTemporaryFile();
-			$newWidth = imagesx($newImage) < 4096 ? imagesx($newImage) : 4096;
-			$newHeight = imagesy($newImage) / (imagesx($newImage) / $newWidth);
-			$outputImage = imagescale($newImage, $newWidth, $newHeight);
-
-			imageinterlace($outputImage, 1);
-			imagepng($outputImage, $tmpFile, 8);
-			imagedestroy($outputImage);
-
-			$target->putContent(file_get_contents($tmpFile, 'r'));
-		} else {
-			$target->putContent(file_get_contents($image['tmp_name'], 'r'));
-		}
 		$name = $image['name'];
-
-		$this->themingDefaults->set($key.'Mime', $image['type']);
-
 		$cssCached = $this->scssCacher->process(\OC::$SERVERROOT, 'core/css/css-variables.scss', 'core');
 
 		return new DataResponse(
@@ -322,24 +285,6 @@ class ThemingController extends Controller {
 		);
 	}
 
-	/**
-	 * Returns a list of supported mime types for image uploads.
-	 * "favicon" images are only allowed to be SVG when imagemagick with SVG support is available.
-	 *
-	 * @param string $key The image key, e.g. "favicon"
-	 * @return array
-	 */
-	private function getSupportedUploadImageFormats(string $key): array {
-		$supportedFormats = ['image/jpeg', 'image/png', 'image/gif',];
-
-		if ($key !== 'favicon' || $this->imageManager->shouldReplaceIcons() === true) {
-			$supportedFormats[] = 'image/svg+xml';
-			$supportedFormats[] = 'image/svg';
-		}
-
-		return $supportedFormats;
-	}
-
 	/**
 	 * Revert setting to default value
 	 *
@@ -352,11 +297,6 @@ class ThemingController extends Controller {
 		// reprocess server scss for preview
 		$cssCached = $this->scssCacher->process(\OC::$SERVERROOT, 'core/css/css-variables.scss', 'core');
 
-		if (strpos($setting, 'Mime') !== -1) {
-			$imageKey = str_replace('Mime', '', $setting);
-			$this->imageManager->delete($imageKey);
-		}
-
 		return new DataResponse(
 			[
 				'data' =>

apps/theming/lib/ImageManager.php

@@ -36,6 +36,7 @@ use OCP\Files\SimpleFS\ISimpleFolder;
 use OCP\ICacheFactory;
 use OCP\IConfig;
 use OCP\ILogger;
+use OCP\ITempManager;
 use OCP\IURLGenerator;
 
 class ImageManager {
@@ -52,27 +53,22 @@ class ImageManager {
 	private $cacheFactory;
 	/** @var ILogger */
 	private $logger;
+	/** @var ITempManager */
+	private $tempManager;
 
-	/**
-	 * ImageManager constructor.
-	 *
-	 * @param IConfig $config
-	 * @param IAppData $appData
-	 * @param IURLGenerator $urlGenerator
-	 * @param ICacheFactory $cacheFactory
-	 * @param ILogger $logger
-	 */
 	public function __construct(IConfig $config,
 								IAppData $appData,
 								IURLGenerator $urlGenerator,
 								ICacheFactory $cacheFactory,
-								ILogger $logger
+								ILogger $logger,
+								ITempManager $tempManager
 	) {
 		$this->config = $config;
 		$this->appData = $appData;
 		$this->urlGenerator = $urlGenerator;
 		$this->cacheFactory = $cacheFactory;
 		$this->logger = $logger;
+		$this->tempManager = $tempManager;
 	}
 
 	public function getImageUrl(string $key, bool $useSvg = true): string {
@@ -211,6 +207,66 @@ class ImageManager {
 		}
 	}
 
+	public function updateImage(string $key, string $tmpFile) {
+		$this->delete($key);
+
+		try {
+			$folder = $this->appData->getFolder('images');
+		} catch (NotFoundException $e) {
+			$folder = $this->appData->newFolder('images');
+		}
+
+		$target = $folder->newFile($key);
+		$supportedFormats = $this->getSupportedUploadImageFormats($key);
+		$detectedMimeType = mime_content_type($tmpFile);
+		if (!in_array($detectedMimeType, $supportedFormats, true)) {
+			throw new \Exception('Unsupported image type');
+		}
+
+		if ($key === 'background' && strpos($detectedMimeType, 'image/svg') === false) {
+			// Optimize the image since some people may upload images that will be
+			// either to big or are not progressive rendering.
+			$newImage = @imagecreatefromstring(file_get_contents($tmpFile));
+
+			// Preserve transparency
+			imagesavealpha($newImage, true);
+			imagealphablending($newImage, true);
+
+			$tmpFile = $this->tempManager->getTemporaryFile();
+			$newWidth = (int)(imagesx($newImage) < 4096 ? imagesx($newImage) : 4096);
+			$newHeight = (int)(imagesy($newImage) / (imagesx($newImage) / $newWidth));
+			$outputImage = imagescale($newImage, $newWidth, $newHeight);
+
+			imageinterlace($outputImage, 1);
+			imagepng($outputImage, $tmpFile, 8);
+			imagedestroy($outputImage);
+
+			$target->putContent(file_get_contents($tmpFile, 'r'));
+		} else {
+			$target->putContent(file_get_contents($tmpFile));
+		}
+
+		return $detectedMimeType;
+	}
+
+	/**
+	 * Returns a list of supported mime types for image uploads.
+	 * "favicon" images are only allowed to be SVG when imagemagick with SVG support is available.
+	 *
+	 * @param string $key The image key, e.g. "favicon"
+	 * @return array
+	 */
+	private function getSupportedUploadImageFormats(string $key): array {
+		$supportedFormats = ['image/jpeg', 'image/png', 'image/gif'];
+
+		if ($key !== 'favicon' || $this->shouldReplaceIcons() === true) {
+			$supportedFormats[] = 'image/svg+xml';
+			$supportedFormats[] = 'image/svg';
+		}
+
+		return $supportedFormats;
+	}
+
 	/**
 	 * remove cached files that are not required any longer
 	 *

apps/theming/lib/ThemingDefaults.php

@@ -398,6 +398,7 @@ class ThemingDefaults extends \OC_Defaults {
 		$this->config->deleteAppValue('theming', $setting);
 		$this->increaseCacheBuster();
 
+		$returnValue = '';
 		switch ($setting) {
 			case 'name':
 				$returnValue = $this->getEntity();
@@ -411,8 +412,11 @@ class ThemingDefaults extends \OC_Defaults {
 			case 'color':
 				$returnValue = $this->getColorPrimary();
 				break;
-			default:
+			case 'logo':
-				$returnValue = '';
+			case 'logoheader':
+			case 'background':
+			case 'favicon':
+				$this->imageManager->delete($setting);
 				break;
 		}
 

apps/theming/tests/Controller/ThemingControllerTest.php

@@ -49,12 +49,12 @@ use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\Files\IAppData;
 use OCP\Files\NotFoundException;
 use OCP\Files\SimpleFS\ISimpleFile;
-use OCP\Files\SimpleFS\ISimpleFolder;
 use OCP\IConfig;
 use OCP\IL10N;
 use OCP\IRequest;
 use OCP\ITempManager;
 use OCP\IURLGenerator;
+use PHPUnit\Framework\MockObject\MockObject;
 use Test\TestCase;
 
 class ThemingControllerTest extends TestCase {
@@ -98,12 +98,12 @@ class ThemingControllerTest extends TestCase {
 		$this->urlGenerator = $this->createMock(IURLGenerator::class);
 		$this->imageManager = $this->createMock(ImageManager::class);
 
-		$this->timeFactory = $this->createMock(ITimeFactory::class);
+		$timeFactory = $this->createMock(ITimeFactory::class);
-		$this->timeFactory->expects($this->any())
+		$timeFactory->expects($this->any())
 			->method('getTime')
 			->willReturn(123);
 
-		$this->overwriteService(ITimeFactory::class, $this->timeFactory);
+		$this->overwriteService(ITimeFactory::class, $timeFactory);
 
 		$this->themingController = new ThemingController(
 			'theming',
@@ -293,12 +293,9 @@ class ThemingControllerTest extends TestCase {
 				return $str;
 			});
 
-		$folder = $this->createMock(ISimpleFolder::class);
+		$this->imageManager->expects($this->once())
-		$this->appData
+			->method('updateImage')
-			->expects($this->once())
+			->willThrowException(new \Exception('Unsupported image type'));
-			->method('getFolder')
-			->with('images')
-			->willReturn($folder);
 
 		$expected = new DataResponse(
 			[
@@ -337,12 +334,9 @@ class ThemingControllerTest extends TestCase {
 				return $str;
 			});
 
-		$folder = $this->createMock(ISimpleFolder::class);
+		$this->imageManager->expects($this->once())
-		$this->appData
+			->method('updateImage')
-			->expects($this->once())
+			->willThrowException(new \Exception('Unsupported image type'));
-			->method('getFolder')
-			->with('images')
-			->willReturn($folder);
 
 		$expected = new DataResponse(
 			[
@@ -398,31 +392,6 @@ class ThemingControllerTest extends TestCase {
 				return $str;
 			});
 
-
-		$file = $this->createMock(ISimpleFile::class);
-		$folder = $this->createMock(ISimpleFolder::class);
-		if ($folderExists) {
-			$this->appData
-				->expects($this->once())
-				->method('getFolder')
-				->with('images')
-				->willReturn($folder);
-		} else {
-			$this->appData
-				->expects($this->at(0))
-				->method('getFolder')
-				->with('images')
-				->willThrowException(new NotFoundException());
-			$this->appData
-				->expects($this->at(1))
-				->method('newFolder')
-				->with('images')
-				->willReturn($folder);
-		}
-		$folder->expects($this->once())
-			->method('newFile')
-			->with('logo')
-			->willReturn($file);
 		$this->urlGenerator->expects($this->once())
 			->method('linkTo')
 			->willReturn('serverCss');
@@ -430,6 +399,10 @@ class ThemingControllerTest extends TestCase {
 			->method('getImageUrl')
 			->with('logo')
 			->willReturn('imageUrl');
+
+		$this->imageManager->expects($this->once())
+			->method('updateImage');
+
 		$expected = new DataResponse(
 			[
 				'data' =>
@@ -474,30 +447,8 @@ class ThemingControllerTest extends TestCase {
 				return $str;
 			});
 
-		$file = $this->createMock(ISimpleFile::class);
+		$this->imageManager->expects($this->once())
-		$folder = $this->createMock(ISimpleFolder::class);
+			->method('updateImage');
-		if ($folderExists) {
-			$this->appData
-				->expects($this->once())
-				->method('getFolder')
-				->with('images')
-				->willReturn($folder);
-		} else {
-			$this->appData
-				->expects($this->at(0))
-				->method('getFolder')
-				->with('images')
-				->willThrowException(new NotFoundException());
-			$this->appData
-				->expects($this->at(1))
-				->method('newFolder')
-				->with('images')
-				->willReturn($folder);
-		}
-		$folder->expects($this->once())
-			->method('newFile')
-			->with('background')
-			->willReturn($file);
 
 		$this->urlGenerator->expects($this->once())
 			->method('linkTo')
@@ -548,12 +499,9 @@ class ThemingControllerTest extends TestCase {
 				return $str;
 			});
 
-		$folder = $this->createMock(ISimpleFolder::class);
+		$this->imageManager->expects($this->once())
-		$this->appData
+			->method('updateImage')
-			->expects($this->once())
+			->willThrowException(new \Exception('Unsupported image type'));
-			->method('getFolder')
-			->with('images')
-			->willReturn($folder);
 
 		$expected = new DataResponse(
 			[
@@ -723,9 +671,6 @@ class ThemingControllerTest extends TestCase {
 			->method('linkTo')
 			->with('', '/core/css/someHash-css-variables.scss')
 			->willReturn('/nextcloudWebroot/core/css/someHash-css-variables.scss');
-		$this->imageManager->expects($this->once())
-			->method('delete')
-			->with($filename);
 
 		$expected = new DataResponse(
 			[

apps/theming/tests/ImageManagerTest.php

@@ -35,7 +35,9 @@ use OCP\Files\SimpleFS\ISimpleFolder;
 use OCP\ICacheFactory;
 use OCP\IConfig;
 use OCP\ILogger;
+use OCP\ITempManager;
 use OCP\IURLGenerator;
+use PHPUnit\Framework\MockObject\MockObject;
 use Test\TestCase;
 
 class ImageManagerTest extends TestCase {
@@ -52,6 +54,8 @@ class ImageManagerTest extends TestCase {
 	private $cacheFactory;
 	/** @var ILogger|\PHPUnit_Framework_MockObject_MockObject */
 	private $logger;
+	/** @var ITempManager|MockObject */
+	private $tempManager;
 
 	protected function setUp(): void {
 		parent::setUp();
@@ -60,12 +64,14 @@ class ImageManagerTest extends TestCase {
 		$this->urlGenerator = $this->createMock(IURLGenerator::class);
 		$this->cacheFactory = $this->createMock(ICacheFactory::class);
 		$this->logger = $this->createMock(ILogger::class);
+		$this->tempManager = $this->createMock(ITempManager::class);
 		$this->imageManager = new ImageManager(
 			$this->config,
 			$this->appData,
 			$this->urlGenerator,
 			$this->cacheFactory,
-			$this->logger
+			$this->logger,
+			$this->tempManager
 		);
 	}
 
@@ -326,4 +332,56 @@ class ImageManagerTest extends TestCase {
 			->willReturn($folders[2]);
 		$this->imageManager->cleanup();
 	}
+
+
+	public function dataUpdateImage() {
+		return [
+			['background', __DIR__ . '/../../../tests/data/testimage.png', true, true],
+			['background', __DIR__ . '/../../../tests/data/testimage.png', false, true],
+			['background', __DIR__ . '/../../../tests/data/testimage.jpg', true, true],
+			['logo', __DIR__ . '/../../../tests/data/testimagelarge.svg', true, false],
+		];
+	}
+
+	/**
+	 * @dataProvider dataUpdateImage
+	 */
+	public function testUpdateImage($key, $tmpFile, $folderExists, $shouldConvert) {
+		$file = $this->createMock(ISimpleFile::class);
+		$folder = $this->createMock(ISimpleFolder::class);
+		$oldFile = $this->createMock(ISimpleFile::class);
+		$folder->expects($this->any())
+			->method('getFile')
+			->willReturn($oldFile);
+		if ($folderExists) {
+			$this->appData
+				->expects($this->any())
+				->method('getFolder')
+				->with('images')
+				->willReturn($folder);
+		} else {
+			$this->appData
+				->expects($this->any())
+				->method('getFolder')
+				->with('images')
+				->willThrowException(new NotFoundException());
+			$this->appData
+				->expects($this->any())
+				->method('newFolder')
+				->with('images')
+				->willReturn($folder);
+		}
+		$folder->expects($this->once())
+			->method('newFile')
+			->with($key)
+			->willReturn($file);
+
+		if ($shouldConvert) {
+			$this->tempManager->expects($this->once())
+				->method('getTemporaryFile')
+				->willReturn('/tmp/randomtempfile-theming');
+		}
+
+		$this->imageManager->updateImage($key, $tmpFile);
+	}
 }

lib/private/Server.php

@@ -1126,7 +1126,7 @@ class Server extends ServerContainer implements IServerContainer {
 					$c->getURLGenerator(),
 					$c->getMemCacheFactory(),
 					new Util($c->getConfig(), $this->getAppManager(), $c->getAppDataDir('theming')),
-					new ImageManager($c->getConfig(), $c->getAppDataDir('theming'), $c->getURLGenerator(), $this->getMemCacheFactory(), $this->getLogger()),
+					new ImageManager($c->getConfig(), $c->getAppDataDir('theming'), $c->getURLGenerator(), $this->getMemCacheFactory(), $this->getLogger(), $this->getTempManager()),
 					$c->getAppManager(),
 					$c->getNavigationManager()
 				);