Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
feat(settings): Cache app discover images to ensure privacy of users
Co-authored-by: Benjamin Gaussorgues <[email protected]>
Co-authored-by: Ferdinand Thiessen <[email protected]>

Signed-off-by: Ferdinand Thiessen <[email protected]>
  • Loading branch information
susnux authored and Altahrim committed Mar 14, 2024
commit df502c114da97a6a9cfbb61c55212ab342875189
1 change: 1 addition & 0 deletions apps/settings/appinfo/routes.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
['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' => ''],
Expand Down
108 changes: 104 additions & 4 deletions apps/settings/lib/Controller/AppSettingsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,25 @@
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;
Expand All @@ -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,
Expand All @@ -80,8 +94,10 @@ public function __construct(
private LoggerInterface $logger,
private IInitialState $initialState,
private AppDiscoverFetcher $discoverFetcher,
private IClientService $clientService,
) {
parent::__construct($appName, $request);
$this->appData = $appDataFactory->get('appstore');
}

/**
Expand Down Expand Up @@ -119,6 +135,83 @@ public function getAppDiscoverJSON(): JSONResponse {
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();
Expand Down Expand Up @@ -305,7 +398,14 @@ private function getAppsForCategory($requestedCategory = ''): array {
$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();
Expand All @@ -324,7 +424,7 @@ private function getAppsForCategory($requestedCategory = ''): array {
}
}

$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') {
Expand Down Expand Up @@ -411,7 +511,7 @@ public function enableApps(array $appIds, array $groups = []): JSONResponse {

// 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) {
Expand All @@ -430,7 +530,7 @@ public function enableApps(array $appIds, array $groups = []): JSONResponse {
}
}
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);
}
Expand Down
19 changes: 14 additions & 5 deletions apps/settings/src/components/AppStoreDiscover/PostType.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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">
Expand All @@ -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'

Expand Down Expand Up @@ -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(() => {
Expand Down Expand Up @@ -174,6 +181,8 @@ export default defineComponent({

isFullWidth,
isImage,

generatePrivacyUrl,
}
},
})
Expand Down
15 changes: 15 additions & 0 deletions apps/settings/tests/Controller/AppSettingsControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -38,6 +39,8 @@
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;
Expand Down Expand Up @@ -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')
Expand All @@ -104,10 +114,13 @@ protected function setUp(): void {
$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,
Expand All @@ -120,6 +133,8 @@ protected function setUp(): void {
$this->urlGenerator,
$this->logger,
$this->initialState,
$this->discoverFetcher,
$this->clientService,
);
}

Expand Down
16 changes: 16 additions & 0 deletions lib/private/App/AppStore/Fetcher/AppDiscoverFetcher.php
Original file line number Diff line number Diff line change
Expand Up @@ -97,4 +97,20 @@ public function get($allowUnstable = false) {

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;
}
}