Merge pull request #7548 from LibreSign/backport/7539/stable33

vitormattos · web-flow · commit c41098eb942a · 2026-04-22T22:16:40.000-03:00
[stable33] fix: allow signer thumbnail access and prefer file_id preview URLs
diff --git a/.github/workflows/behat-mariadb.yml b/.github/workflows/behat-mariadb.yml
@@ -108,7 +108,7 @@ jobs:
           php-version: ${{ matrix.php-versions }}
           tools: phpunit
           # https://docs.nextcloud.com/server/stable/admin_manual/installation/source_installation.html#prerequisites-for-manual-installation
-          extensions: bz2, ctype, curl, dom, fileinfo, gd, iconv, intl, json, libxml, mbstring, openssl, pcntl, posix, session, simplexml, xmlreader, xmlwriter, zip, zlib, mysql, pdo_mysql
+          extensions: bz2, ctype, curl, dom, fileinfo, gd, iconv, intl, json, libxml, mbstring, openssl, pcntl, posix, session, simplexml, xmlreader, xmlwriter, zip, zlib, mysql, pdo_mysql, imagick
           coverage: none
           ini-file: development
           # Temporary workaround for missing pcntl_* in PHP 8.3
diff --git a/.github/workflows/behat-mysql.yml b/.github/workflows/behat-mysql.yml
@@ -112,7 +112,7 @@ jobs:
           php-version: ${{ matrix.php-versions }}
           tools: phpunit
           # https://docs.nextcloud.com/server/stable/admin_manual/installation/source_installation.html#prerequisites-for-manual-installation
-          extensions: bz2, ctype, curl, dom, fileinfo, gd, iconv, intl, json, libxml, mbstring, openssl, pcntl, posix, session, simplexml, xmlreader, xmlwriter, zip, zlib, sqlite, pdo_sqlite
+          extensions: bz2, ctype, curl, dom, fileinfo, gd, iconv, intl, json, libxml, mbstring, openssl, pcntl, posix, session, simplexml, xmlreader, xmlwriter, zip, zlib, sqlite, pdo_sqlite, imagick
           coverage: none
           ini-file: development
           # Temporary workaround for missing pcntl_* in PHP 8.3
diff --git a/.github/workflows/behat-pgsql.yml b/.github/workflows/behat-pgsql.yml
@@ -111,7 +111,7 @@ jobs:
           php-version: ${{ matrix.php-versions }}
           tools: phpunit
           # https://docs.nextcloud.com/server/stable/admin_manual/installation/source_installation.html#prerequisites-for-manual-installation
-          extensions: bz2, ctype, curl, dom, fileinfo, gd, iconv, intl, json, libxml, mbstring, openssl, pcntl, posix, session, simplexml, xmlreader, xmlwriter, zip, zlib, pgsql, pdo_pgsql
+          extensions: bz2, ctype, curl, dom, fileinfo, gd, iconv, intl, json, libxml, mbstring, openssl, pcntl, posix, session, simplexml, xmlreader, xmlwriter, zip, zlib, pgsql, pdo_pgsql, imagick
           coverage: none
           ini-file: development
           # Temporary workaround for missing pcntl_* in PHP 8.3
diff --git a/.github/workflows/behat-sqlite.yml b/.github/workflows/behat-sqlite.yml
@@ -102,7 +102,7 @@ jobs:
           php-version: ${{ matrix.php-versions }}
           tools: phpunit
           # https://docs.nextcloud.com/server/stable/admin_manual/installation/source_installation.html#prerequisites-for-manual-installation
-          extensions: bz2, ctype, curl, dom, fileinfo, gd, iconv, intl, json, libxml, mbstring, openssl, pcntl, posix, session, simplexml, xmlreader, xmlwriter, zip, zlib, sqlite, pdo_sqlite
+          extensions: bz2, ctype, curl, dom, fileinfo, gd, iconv, intl, json, libxml, mbstring, openssl, pcntl, posix, session, simplexml, xmlreader, xmlwriter, zip, zlib, sqlite, pdo_sqlite, imagick
           coverage: none
           ini-file: development
           # Temporary workaround for missing pcntl_* in PHP 8.3
diff --git a/.github/workflows/phpunit-mariadb.yml b/.github/workflows/phpunit-mariadb.yml
@@ -113,7 +113,7 @@ jobs:
         with:
           php-version: ${{ matrix.php-versions }}
           # https://docs.nextcloud.com/server/stable/admin_manual/installation/source_installation.html#prerequisites-for-manual-installation
-          extensions: bz2, ctype, curl, dom, fileinfo, gd, iconv, intl, json, libxml, mbstring, openssl, pcntl, posix, session, simplexml, xmlreader, xmlwriter, zip, zlib, mysql, pdo_mysql
+          extensions: bz2, ctype, curl, dom, fileinfo, gd, iconv, intl, json, libxml, mbstring, openssl, pcntl, posix, session, simplexml, xmlreader, xmlwriter, zip, zlib, mysql, pdo_mysql, imagick
           coverage: xdebug
           ini-file: development
           # Temporary workaround for missing pcntl_* in PHP 8.3
diff --git a/.github/workflows/phpunit-mysql.yml b/.github/workflows/phpunit-mysql.yml
@@ -111,7 +111,7 @@ jobs:
         with:
           php-version: ${{ matrix.php-versions }}
           # https://docs.nextcloud.com/server/stable/admin_manual/installation/source_installation.html#prerequisites-for-manual-installation
-          extensions: bz2, ctype, curl, dom, fileinfo, gd, iconv, intl, json, libxml, mbstring, openssl, pcntl, posix, session, simplexml, xmlreader, xmlwriter, zip, zlib, mysql, pdo_mysql
+          extensions: bz2, ctype, curl, dom, fileinfo, gd, iconv, intl, json, libxml, mbstring, openssl, pcntl, posix, session, simplexml, xmlreader, xmlwriter, zip, zlib, mysql, pdo_mysql, imagick
           coverage: xdebug
           ini-file: development
           # Temporary workaround for missing pcntl_* in PHP 8.3
diff --git a/.github/workflows/phpunit-pgsql.yml b/.github/workflows/phpunit-pgsql.yml
@@ -114,7 +114,7 @@ jobs:
         with:
           php-version: ${{ matrix.php-versions }}
           # https://docs.nextcloud.com/server/stable/admin_manual/installation/source_installation.html#prerequisites-for-manual-installation
-          extensions: bz2, ctype, curl, dom, fileinfo, gd, iconv, intl, json, libxml, mbstring, openssl, pcntl, posix, session, simplexml, xmlreader, xmlwriter, zip, zlib, pgsql, pdo_pgsql
+          extensions: bz2, ctype, curl, dom, fileinfo, gd, iconv, intl, json, libxml, mbstring, openssl, pcntl, posix, session, simplexml, xmlreader, xmlwriter, zip, zlib, pgsql, pdo_pgsql, imagick
           coverage: xdebug
           ini-file: development
           # Temporary workaround for missing pcntl_* in PHP 8.3
diff --git a/.github/workflows/phpunit-sqlite.yml b/.github/workflows/phpunit-sqlite.yml
@@ -110,7 +110,7 @@ jobs:
         with:
           php-version: ${{ matrix.php-versions }}
           # https://docs.nextcloud.com/server/stable/admin_manual/installation/source_installation.html#prerequisites-for-manual-installation
-          extensions: bz2, ctype, curl, dom, fileinfo, gd, iconv, intl, json, libxml, mbstring, openssl, pcntl, posix, session, simplexml, xmlreader, xmlwriter, zip, zlib, sqlite, pdo_sqlite
+          extensions: bz2, ctype, curl, dom, fileinfo, gd, iconv, intl, json, libxml, mbstring, openssl, pcntl, posix, session, simplexml, xmlreader, xmlwriter, zip, zlib, sqlite, pdo_sqlite, imagick
           coverage: xdebug
           ini-file: development
           # Temporary workaround for missing pcntl_* in PHP 8.3
diff --git a/lib/Controller/FileController.php b/lib/Controller/FileController.php
@@ -18,6 +18,7 @@
 use OCA\Libresign\Helper\JSActions;
 use OCA\Libresign\Helper\ValidateHelper;
 use OCA\Libresign\Middleware\Attribute\PrivateValidation;
+use OCA\Libresign\Middleware\Attribute\RequireFileAccess;
 use OCA\Libresign\Middleware\Attribute\RequireManager;
 use OCA\Libresign\Service\AccountService;
 use OCA\Libresign\Service\File\FileListService;
@@ -353,6 +354,7 @@ public function list(
 	 */
 	#[NoAdminRequired]
 	#[NoCSRFRequired]
+	#[RequireFileAccess('nodeId')]
 	#[ApiRoute(verb: 'GET', url: '/api/{apiVersion}/file/thumbnail/{nodeId}', requirements: ['apiVersion' => '(v1)'])]
 	public function getThumbnail(
 		int $nodeId = -1,
@@ -369,9 +371,6 @@ public function getThumbnail(
 
 		try {
 			$libreSignFile = $this->fileMapper->getByNodeId($nodeId);
-			if ($libreSignFile->getUserId() !== $this->userSession->getUser()->getUID()) {
-				return new DataResponse([], Http::STATUS_FORBIDDEN);
-			}
 
 			if ($libreSignFile->getNodeType() === 'envelope') {
 				if ($mimeFallback) {
@@ -411,6 +410,7 @@ public function getThumbnail(
 	 */
 	#[NoAdminRequired]
 	#[NoCSRFRequired]
+	#[RequireFileAccess('fileId')]
 	#[ApiRoute(verb: 'GET', url: '/api/{apiVersion}/file/thumbnail/file_id/{fileId}', requirements: ['apiVersion' => '(v1)'])]
 	public function getThumbnailByFileId(
 		int $fileId = -1,
@@ -427,9 +427,6 @@ public function getThumbnailByFileId(
 
 		try {
 			$libreSignFile = $this->fileMapper->getById($fileId);
-			if ($libreSignFile->getUserId() !== $this->userSession->getUser()->getUID()) {
-				return new DataResponse([], Http::STATUS_FORBIDDEN);
-			}
 
 			if ($libreSignFile->getNodeType() === 'envelope') {
 				if ($mimeFallback) {
diff --git a/lib/Middleware/Attribute/RequireFileAccess.php b/lib/Middleware/Attribute/RequireFileAccess.php
@@ -0,0 +1,23 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * SPDX-FileCopyrightText: 2026 LibreCode coop and contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OCA\Libresign\Middleware\Attribute;
+
+use Attribute;
+
+#[Attribute(Attribute::TARGET_METHOD)]
+class RequireFileAccess {
+	public function __construct(
+		private string $identifier = 'fileId',
+	) {
+	}
+
+	public function getIdentifier(): string {
+		return $this->identifier;
+	}
+}
diff --git a/lib/Middleware/InjectionMiddleware.php b/lib/Middleware/InjectionMiddleware.php
@@ -22,11 +22,13 @@
 use OCA\Libresign\Helper\ValidateHelper;
 use OCA\Libresign\Middleware\Attribute\CanSignRequestUuid;
 use OCA\Libresign\Middleware\Attribute\PrivateValidation;
+use OCA\Libresign\Middleware\Attribute\RequireFileAccess;
 use OCA\Libresign\Middleware\Attribute\RequireManager;
 use OCA\Libresign\Middleware\Attribute\RequireSetupOk;
 use OCA\Libresign\Middleware\Attribute\RequireSigner;
 use OCA\Libresign\Middleware\Attribute\RequireSignerUuid;
 use OCA\Libresign\Middleware\Attribute\RequireSignRequestUuid;
+use OCA\Libresign\Service\FileAccessService;
 use OCA\Libresign\Service\SignFileService;
 use OCA\Libresign\Service\UuidResolverService;
 use OCP\AppFramework\Controller;
@@ -58,6 +60,7 @@ public function __construct(
 		private CertificateEngineFactory $certificateEngineFactory,
 		private FileMapper $fileMapper,
 		private IInitialState $initialState,
+		private FileAccessService $fileAccessService,
 		private SignFileService $signFileService,
 		private UuidResolverService $uuidResolverService,
 		private IL10N $l10n,
@@ -93,6 +96,9 @@ public function beforeController(Controller $controller, string $methodName) {
 		if (!empty($reflectionMethod->getAttributes(RequireSignerUuid::class))) {
 			$this->requireSignerUuid();
 		}
+		if (!empty($reflectionMethod->getAttributes(RequireFileAccess::class))) {
+			$this->requireFileAccess($reflectionMethod);
+		}
 
 		$this->requireSetupOk($reflectionMethod);
 
@@ -232,6 +238,26 @@ private function requireSignerUuid(): void {
 		}
 	}
 
+	private function requireFileAccess(\ReflectionMethod $reflectionMethod): void {
+		$attributes = $reflectionMethod->getAttributes(RequireFileAccess::class);
+		$attribute = current($attributes);
+		/** @var RequireFileAccess $requirement */
+		$requirement = $attribute->newInstance();
+
+		$identifier = $requirement->getIdentifier();
+		$hasAccess = match ($identifier) {
+			'nodeId' => $this->fileAccessService->userCanAccessFileByNodeId((int)$this->request->getParam('nodeId', -1)),
+			'fileId' => $this->fileAccessService->userCanAccessFileById((int)$this->request->getParam('fileId', -1)),
+			default => throw new \InvalidArgumentException('Unsupported file access identifier: ' . $identifier),
+		};
+
+		if ($hasAccess) {
+			return;
+		}
+
+		throw new LibresignException(json_encode([]), Http::STATUS_FORBIDDEN);
+	}
+
 	private function redirectSignedToValidationIfNeeded(RequireSignRequestUuid $requirement): void {
 		if (!$requirement->redirectIfSignedToValidation()) {
 			return;
diff --git a/lib/Search/FileSearchProvider.php b/lib/Search/FileSearchProvider.php
@@ -92,11 +92,12 @@ private function formatResult(File $file, IUser $user): SearchResultEntry {
 				$icon = $this->mimeTypeDetector->mimeTypeIcon($node->getMimetype());
 
 				$thumbnailUrl = $this->urlGenerator->linkToRouteAbsolute(
-					'core.Preview.getPreviewByFileId',
+					'ocs.libresign.File.getThumbnailByFileId',
 					[
+						'apiVersion' => 'v1',
 						'x' => 32,
 						'y' => 32,
-						'fileId' => $node->getId()
+						'fileId' => $file->getId()
 					]
 				);
 
diff --git a/lib/Service/FileAccessService.php b/lib/Service/FileAccessService.php
@@ -0,0 +1,62 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * SPDX-FileCopyrightText: 2024 LibreCode coop and contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OCA\Libresign\Service;
+
+use OCA\Libresign\Db\File as FileEntity;
+use OCA\Libresign\Db\FileMapper;
+use OCP\IUser;
+use OCP\IUserSession;
+
+class FileAccessService {
+	public function __construct(
+		private FileMapper $fileMapper,
+		private SignFileService $signFileService,
+		private IUserSession $userSession,
+	) {
+	}
+
+	public function userCanAccessFileById(int $fileId, ?IUser $user = null): bool {
+		$user = $this->resolveUser($user);
+		if (!$user) {
+			return false;
+		}
+
+		return $this->userCanAccessFile($this->fileMapper->getById($fileId), $user);
+	}
+
+	public function userCanAccessFileByNodeId(int $nodeId, ?IUser $user = null): bool {
+		$user = $this->resolveUser($user);
+		if (!$user) {
+			return false;
+		}
+
+		return $this->userCanAccessFile($this->fileMapper->getByNodeId($nodeId), $user);
+	}
+
+	private function resolveUser(?IUser $user): ?IUser {
+		return $user ?? $this->userSession->getUser();
+	}
+
+	private function userCanAccessFile(FileEntity $file, IUser $user): bool {
+		if ($file->getUserId() === $user->getUID()) {
+			return true;
+		}
+
+		return $this->userCanSignFile($file, $user);
+	}
+
+	private function userCanSignFile(FileEntity $file, IUser $user): bool {
+		try {
+			$this->signFileService->getSignRequestToSign($file, null, $user);
+			return true;
+		} catch (\Exception) {
+			return false;
+		}
+	}
+}
diff --git a/psalm.xml b/psalm.xml
@@ -45,5 +45,6 @@
     </issueHandlers>
     <stubs>
         <file name="tests/stubs/oc_hooks_emitter.php" />
+        <file name="tests/stubs/psr_http_client.php" />
     </stubs>
 </psalm>
diff --git a/src/components/File/File.vue b/src/components/File/File.vue
@@ -61,14 +61,14 @@ const previewUrl = computed(() => {
 	}
 
 	let filePreviewUrl = ''
-	if (currentFile.value.nodeId) {
-		filePreviewUrl = generateOcsUrl('/apps/libresign/api/v1/file/thumbnail/{nodeId}', {
-			nodeId: currentFile.value.nodeId,
-		})
-	} else if (currentFile.value.id) {
+	if (currentFile.value.id) {
 		filePreviewUrl = generateOcsUrl('/apps/libresign/api/v1/file/thumbnail/file_id/{fileId}', {
 			fileId: currentFile.value.id,
 		})
+	} else if (currentFile.value.nodeId) {
+		filePreviewUrl = generateOcsUrl('/apps/libresign/api/v1/file/thumbnail/{nodeId}', {
+			nodeId: currentFile.value.nodeId,
+		})
 	} else {
 		filePreviewUrl = window.location.origin + generateUrl('/core/preview?fileId={fileid}', {
 			fileid: currentFile.value.id,
diff --git a/src/components/RightSidebar/EnvelopeFilesList.vue b/src/components/RightSidebar/EnvelopeFilesList.vue
@@ -412,11 +412,17 @@ function validateMaxFileUploads(filesCount: number) {
 	return true
 }
 
-function getPreviewUrl(file: Partial<EnvelopeFile> & { nodeId?: number }) {
-	if (!file.nodeId) return null
-	const url = new URL(generateOcsUrl('/apps/libresign/api/v1/file/thumbnail/{nodeId}', {
-		nodeId: file.nodeId,
-	}))
+function getPreviewUrl(file: Partial<EnvelopeFile> & { id?: number; nodeId?: number }) {
+	if (!file.id && !file.nodeId) return null
+
+	const previewUrl = file.id
+		? generateOcsUrl('/apps/libresign/api/v1/file/thumbnail/file_id/{fileId}', {
+			fileId: file.id,
+		})
+		: generateOcsUrl('/apps/libresign/api/v1/file/thumbnail/{nodeId}', {
+			nodeId: file.nodeId,
+		})
+	const url = new URL(previewUrl)
 	url.searchParams.set('x', '32')
 	url.searchParams.set('y', '32')
 	url.searchParams.set('mimeFallback', 'true')
diff --git a/src/tests/components/File/File.spec.ts b/src/tests/components/File/File.spec.ts
@@ -9,7 +9,7 @@ import { mount } from '@vue/test-utils'
 import File from '../../../components/File/File.vue'
 
 type FileEntry = {
-	id: number
+	id?: number
 	nodeId?: number
 	name: string
 	status: number
@@ -83,31 +83,31 @@ describe('File.vue', () => {
 		sidebarStoreMock.activeRequestSignatureTab.mockReset()
 	})
 
-	it('renders the selected file preview using the node id thumbnail endpoint', () => {
+	it('renders the selected file preview using the file id thumbnail endpoint', () => {
 		const wrapper = createWrapper()
 
 		const image = wrapper.find('img')
 
 		expect(image.exists()).toBe(true)
 		expect(wrapper.find('h1').text()).toBe('contract.pdf')
-		expect(image.attributes('src')).toContain('/apps/libresign/api/v1/file/thumbnail/99')
+		expect(image.attributes('src')).toContain('/apps/libresign/api/v1/file/thumbnail/file_id/13')
 		expect(image.attributes('src')).toContain('x=128')
 		expect(image.attributes('src')).toContain('y=128')
 		expect(image.attributes('src')).toContain('mimeFallback=true')
 		expect(image.attributes('src')).toContain('a=0')
 	})
 
-	it('falls back to the file id thumbnail endpoint when node id is absent', () => {
+	it('falls back to the node id thumbnail endpoint when the file id is absent', () => {
 		filesStoreMock.files[7] = {
-			id: 13,
+			nodeId: 99,
 			name: 'fallback.pdf',
 			status: 1,
 			statusText: 'Pending',
 		}
 
 		const wrapper = createWrapper()
 
-		expect(wrapper.find('img').attributes('src')).toContain('/apps/libresign/api/v1/file/thumbnail/file_id/13')
+		expect(wrapper.find('img').attributes('src')).toContain('/apps/libresign/api/v1/file/thumbnail/99')
 	})
 
 	it('selects the file and opens the request signature sidebar on click', async () => {
diff --git a/src/tests/components/RightSidebar/EnvelopeFilesList.spec.ts b/src/tests/components/RightSidebar/EnvelopeFilesList.spec.ts
diff --git a/src/tests/views/FilesList/FileEntryPreview.spec.ts b/src/tests/views/FilesList/FileEntryPreview.spec.ts
diff --git a/src/views/FilesList/FileEntry/FileEntryPreview.vue b/src/views/FilesList/FileEntry/FileEntryPreview.vue
diff --git a/tests/integration/features/file/thumbnail.feature b/tests/integration/features/file/thumbnail.feature
diff --git a/tests/php/Api/Controller/FileControllerTest.php b/tests/php/Api/Controller/FileControllerTest.php
diff --git a/tests/php/Unit/Middleware/InjectionMiddlewareTest.php b/tests/php/Unit/Middleware/InjectionMiddlewareTest.php
diff --git a/tests/php/Unit/Service/FileAccessServiceTest.php b/tests/php/Unit/Service/FileAccessServiceTest.php
diff --git a/tests/stubs/psr_http_client.php b/tests/stubs/psr_http_client.php
