feat(settings): Cache app discover images to ensure privacy of users

Co-authored-by: Benjamin Gaussorgues <benjamin.gaussorgues@nextcloud.com>
Co-authored-by: Ferdinand Thiessen <opensource@fthiessen.de>

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>

5 files changed, 150 insertions, 9 deletions

diff --git a/apps/settings/appinfo/routes.php b/apps/settings/appinfo/routes.php
index 6eea170fe39..50c4e7f526c 100644
--- a/apps/settings/appinfo/routes.php
+++ b/apps/settings/appinfo/routes.php
@@ -40,6 +40,7 @@ return [
 		['name' => 'MailSettings#sendTestMail', 'url' => '/settings/admin/mailtest', 'verb' => 'POST' , 'root' => ''],
 
 		['name' => 'AppSettings#getAppDiscoverJSON', 'url' => '/settings/api/apps/discover', 'verb' => 'GET', 'root' => ''],
+		['name' => 'AppSettings#getAppDiscoverMedia', 'url' => '/settings/api/apps/media', 'verb' => 'GET', 'root' => ''],
 		['name' => 'AppSettings#listCategories', 'url' => '/settings/apps/categories', 'verb' => 'GET' , 'root' => ''],
 		['name' => 'AppSettings#viewApps', 'url' => '/settings/apps', 'verb' => 'GET' , 'root' => ''],
 		['name' => 'AppSettings#listApps', 'url' => '/settings/apps/list', 'verb' => 'GET' , 'root' => ''],
diff --git a/apps/settings/lib/Controller/AppSettingsController.php b/apps/settings/lib/Controller/AppSettingsController.php
index 58869537c44..5d92b02d5c1 100644
--- a/apps/settings/lib/Controller/AppSettingsController.php
+++ b/apps/settings/lib/Controller/AppSettingsController.php
@@ -42,14 +42,25 @@ use OC\App\DependencyAnalyzer;
 use OC\App\Platform;
 use OC\Installer;
 use OC_App;
+use OCP\App\AppPathNotFoundException;
 use OCP\App\IAppManager;
 use OCP\AppFramework\Controller;
 use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\Attribute\OpenAPI;
 use OCP\AppFramework\Http\ContentSecurityPolicy;
+use OCP\AppFramework\Http\FileDisplayResponse;
 use OCP\AppFramework\Http\JSONResponse;
+use OCP\AppFramework\Http\NotFoundResponse;
+use OCP\AppFramework\Http\Response;
 use OCP\AppFramework\Http\TemplateResponse;
 use OCP\AppFramework\Services\IInitialState;
+use OCP\Files\AppData\IAppDataFactory;
+use OCP\Files\IAppData;
+use OCP\Files\NotFoundException;
+use OCP\Files\NotPermittedException;
+use OCP\Files\SimpleFS\ISimpleFile;
+use OCP\Files\SimpleFS\ISimpleFolder;
+use OCP\Http\Client\IClientService;
 use OCP\IConfig;
 use OCP\IL10N;
 use OCP\INavigationManager;
@@ -64,9 +75,12 @@ class AppSettingsController extends Controller {
 	/** @var array */
 	private $allApps = [];
 
+	private IAppData $appData;
+
 	public function __construct(
 		string $appName,
 		IRequest $request,
+		IAppDataFactory $appDataFactory,
 		private IL10N $l10n,
 		private IConfig $config,
 		private INavigationManager $navigationManager,
@@ -80,8 +94,10 @@ class AppSettingsController extends Controller {
 		private LoggerInterface $logger,
 		private IInitialState $initialState,
 		private AppDiscoverFetcher $discoverFetcher,
+		private IClientService $clientService,
 	) {
 		parent::__construct($appName, $request);
+		$this->appData = $appDataFactory->get('appstore');
 	}
 
 	/**
@@ -119,6 +135,83 @@ class AppSettingsController extends Controller {
 		return new JSONResponse($data);
 	}
 
+	/**
+	 * @PublicPage
+	 * @NoCSRFRequired
+	 *
+	 * Get a image for the app discover section - this is proxied for privacy and CSP reasons
+	 *
+	 * @param string $image
+	 * @throws \Exception
+	 */
+	public function getAppDiscoverMedia(string $fileName): Response {
+		$etag = $this->discoverFetcher->getETag() ?? date('Y-m');
+		$folder = null;
+		try {
+			$folder = $this->appData->getFolder('app-discover-cache');
+			$this->cleanUpImageCache($folder, $etag);
+		} catch (\Throwable $e) {
+			$folder = $this->appData->newFolder('app-discover-cache');
+		}
+
+		// Get the current cache folder
+		try {
+			$folder = $folder->getFolder($etag);
+		} catch (NotFoundException $e) {
+			$folder = $folder->newFolder($etag);
+		}
+
+		$info = pathinfo($fileName);
+		$hashName = md5($fileName);
+		$allFiles = $folder->getDirectoryListing();
+		// Try to find the file
+		$file = array_filter($allFiles, function (ISimpleFile $file) use ($hashName) {
+			return str_starts_with($file->getName(), $hashName);
+		});
+		// Get the first entry
+		$file = reset($file);
+		// If not found request from Web
+		if ($file === false) {
+			try {
+				$client = $this->clientService->newClient();
+				$fileResponse = $client->get($fileName);
+				$contentType = $fileResponse->getHeader('Content-Type');
+				$extension = $info['extension'] ?? '';
+				$file = $folder->newFile($hashName . '.' . base64_encode($contentType) . '.' . $extension, $fileResponse->getBody());
+			} catch (\Throwable $e) {
+				$this->logger->warning('Could not load media file for app discover section', ['media_src' => $fileName, 'exception' => $e]);
+				return new NotFoundResponse();
+			}
+		} else {
+			// File was found so we can get the content type from the file name
+			$contentType = base64_decode(explode('.', $file->getName())[1] ?? '');
+		}
+
+		$response = new FileDisplayResponse($file, Http::STATUS_OK, ['Content-Type' => $contentType]);
+		// cache for 7 days
+		$response->cacheFor(604800, false, true);
+		return $response;
+	}
+
+	/**
+	 * Remove orphaned folders from the image cache that do not match the current etag
+	 * @param ISimpleFolder $folder The folder to clear
+	 * @param string $etag The etag (directory name) to keep
+	 */
+	private function cleanUpImageCache(ISimpleFolder $folder, string $etag): void {
+		// Cleanup old cache folders
+		$allFiles = $folder->getDirectoryListing();
+		foreach ($allFiles as $dir) {
+			try {
+				if ($dir->getName() !== $etag) {
+					$dir->delete();
+				}
+			} catch (NotPermittedException $e) {
+				// ignore folder for now
+			}
+		}
+	}
+
 	private function getAppsWithUpdates() {
 		$appClass = new \OC_App();
 		$apps = $appClass->listAllApps();
@@ -305,7 +398,14 @@ class AppSettingsController extends Controller {
 				$nextCloudVersionDependencies['nextcloud']['@attributes']['max-version'] = $nextCloudVersion->getMaximumVersion();
 			}
 			$phpVersion = $versionParser->getVersion($app['releases'][0]['rawPhpVersionSpec']);
-			$existsLocally = \OC_App::getAppPath($app['id']) !== false;
+
+			try {
+				$this->appManager->getAppPath($app['id']);
+				$existsLocally = true;
+			} catch (AppPathNotFoundException $e) {
+				$existsLocally = false;
+			}
+
 			$phpDependencies = [];
 			if ($phpVersion->getMinimumVersion() !== '') {
 				$phpDependencies['php']['@attributes']['min-version'] = $phpVersion->getMinimumVersion();
@@ -324,7 +424,7 @@ class AppSettingsController extends Controller {
 				}
 			}
 
-			$currentLanguage = substr(\OC::$server->getL10NFactory()->findLanguage(), 0, 2);
+			$currentLanguage = substr($this->l10nFactory->findLanguage(), 0, 2);
 			$enabledValue = $this->config->getAppValue($app['id'], 'enabled', 'no');
 			$groups = null;
 			if ($enabledValue !== 'no' && $enabledValue !== 'yes') {
@@ -411,7 +511,7 @@ class AppSettingsController extends Controller {
 
 				// Check if app is already downloaded
 				/** @var Installer $installer */
-				$installer = \OC::$server->query(Installer::class);
+				$installer = \OC::$server->get(Installer::class);
 				$isDownloaded = $installer->isDownloaded($appId);
 
 				if (!$isDownloaded) {
@@ -430,7 +530,7 @@ class AppSettingsController extends Controller {
 				}
 			}
 			return new JSONResponse(['data' => ['update_required' => $updateRequired]]);
-		} catch (\Exception $e) {
+		} catch (\Throwable $e) {
 			$this->logger->error('could not enable apps', ['exception' => $e]);
 			return new JSONResponse(['data' => ['message' => $e->getMessage()]], Http::STATUS_INTERNAL_SERVER_ERROR);
 		}
diff --git a/apps/settings/src/components/AppStoreDiscover/PostType.vue b/apps/settings/src/components/AppStoreDiscover/PostType.vue
index 9d19a1b4da4..443daa6287f 100644
--- a/apps/settings/src/components/AppStoreDiscover/PostType.vue
+++ b/apps/settings/src/components/AppStoreDiscover/PostType.vue
@@ -50,11 +50,11 @@
 				@ended="hasPlaybackEnded = true">
 				<source v-for="source of mediaSources"
 					:key="source.src"
-					:src="isImage ? undefined : source.src"
-					:srcset="isImage ? source.src : undefined"
+					:src="isImage ? undefined : generatePrivacyUrl(source.src)"
+					:srcset="isImage ? generatePrivacyUrl(source.src) : undefined"
 					:type="source.mime">
 				<img v-if="isImage"
-					:src="mediaSources[0].src"
+					:src="generatePrivacyUrl(mediaSources[0].src)"
 					:alt="mediaAlt">
 			</component>
 			<div class="app-discover-post__play-icon-wrapper">
@@ -71,11 +71,12 @@
 import type { IAppDiscoverPost } from '../../constants/AppDiscoverTypes.ts'
 import type { PropType } from 'vue'
 
+import { mdiPlayCircleOutline } from '@mdi/js'
+import { generateUrl } from '@nextcloud/router'
+import { useElementVisibility } from '@vueuse/core'
 import { computed, defineComponent, ref, watchEffect } from 'vue'
 import { commonAppDiscoverProps } from './common'
 import { useLocalizedValue } from '../../composables/useGetLocalizedValue'
-import { useElementVisibility } from '@vueuse/core'
-import { mdiPlayCircleOutline } from '@mdi/js'
 
 import NcIconSvgWrapper from '@nextcloud/vue/dist/Components/NcIconSvgWrapper.js'
 
@@ -135,6 +136,12 @@ export default defineComponent({
 		const hasPlaybackEnded = ref(false)
 		const showPlayVideo = computed(() => localizedMedia.value?.link && hasPlaybackEnded.value)
 
+		/**
+		 * Generate URL for cached media to prevent user can be tracked
+		 * @param url The URL to resolve
+		 */
+		const generatePrivacyUrl = (url: string) => url.startsWith('/') ? url : generateUrl('/settings/api/apps/media?fileName={fileName}', { fileName: url })
+
 		const mediaElement = ref<HTMLVideoElement|HTMLPictureElement>()
 		const mediaIsVisible = useElementVisibility(mediaElement, { threshold: 0.3 })
 		watchEffect(() => {
@@ -174,6 +181,8 @@ export default defineComponent({
 
 			isFullWidth,
 			isImage,
+
+			generatePrivacyUrl,
 		}
 	},
 })
diff --git a/apps/settings/tests/Controller/AppSettingsControllerTest.php b/apps/settings/tests/Controller/AppSettingsControllerTest.php
index 198e5fc4ec4..25f2431ee77 100644
--- a/apps/settings/tests/Controller/AppSettingsControllerTest.php
+++ b/apps/settings/tests/Controller/AppSettingsControllerTest.php
@@ -29,6 +29,7 @@
 namespace OCA\Settings\Tests\Controller;
 
 use OC\App\AppStore\Bundles\BundleFetcher;
+use OC\App\AppStore\Fetcher\AppDiscoverFetcher;
 use OC\App\AppStore\Fetcher\AppFetcher;
 use OC\App\AppStore\Fetcher\CategoryFetcher;
 use OC\Installer;
@@ -38,6 +39,8 @@ use OCP\AppFramework\Http\ContentSecurityPolicy;
 use OCP\AppFramework\Http\JSONResponse;
 use OCP\AppFramework\Http\TemplateResponse;
 use OCP\AppFramework\Services\IInitialState;
+use OCP\Files\AppData\IAppDataFactory;
+use OCP\Http\Client\IClientService;
 use OCP\IConfig;
 use OCP\IL10N;
 use OCP\INavigationManager;
@@ -84,11 +87,18 @@ class AppSettingsControllerTest extends TestCase {
 	private $logger;
 	/** @var IInitialState|MockObject */
 	private $initialState;
+	/** @var IAppDataFactory|MockObject */
+	private $appDataFactory;
+	/** @var AppDiscoverFetcher|MockObject */
+	private $discoverFetcher;
+	/** @var IClientService|MockObject */
+	private $clientService;
 
 	protected function setUp(): void {
 		parent::setUp();
 
 		$this->request = $this->createMock(IRequest::class);
+		$this->appDataFactory = $this->createMock(IAppDataFactory::class);
 		$this->l10n = $this->createMock(IL10N::class);
 		$this->l10n->expects($this->any())
 			->method('t')
@@ -104,10 +114,13 @@ class AppSettingsControllerTest extends TestCase {
 		$this->urlGenerator = $this->createMock(IURLGenerator::class);
 		$this->logger = $this->createMock(LoggerInterface::class);
 		$this->initialState = $this->createMock(IInitialState::class);
+		$this->discoverFetcher = $this->createMock(AppDiscoverFetcher::class);
+		$this->clientService = $this->createMock(IClientService::class);
 
 		$this->appSettingsController = new AppSettingsController(
 			'settings',
 			$this->request,
+			$this->appDataFactory,
 			$this->l10n,
 			$this->config,
 			$this->navigationManager,
@@ -120,6 +133,8 @@ class AppSettingsControllerTest extends TestCase {
 			$this->urlGenerator,
 			$this->logger,
 			$this->initialState,
+			$this->discoverFetcher,
+			$this->clientService,
 		);
 	}
 
diff --git a/lib/private/App/AppStore/Fetcher/AppDiscoverFetcher.php b/lib/private/App/AppStore/Fetcher/AppDiscoverFetcher.php
index 17ee446aaa1..48dba6d48dc 100644
--- a/lib/private/App/AppStore/Fetcher/AppDiscoverFetcher.php
+++ b/lib/private/App/AppStore/Fetcher/AppDiscoverFetcher.php
@@ -97,4 +97,20 @@ class AppDiscoverFetcher extends Fetcher {
 
 		return $entries;
 	}
+
+	public function getETag(): string|null {
+		$rootFolder = $this->appData->getFolder('/');
+
+		try {
+			$file = $rootFolder->getFile($this->fileName);
+			$jsonBlob = json_decode($file->getContent(), true);
+
+			if (is_array($jsonBlob) && isset($jsonBlob['ETag'])) {
+				return (string)$jsonBlob['ETag'];
+			}
+		} catch (\Throwable $e) {
+			// ignore
+		}
+		return null;
+	}
 }