[stable34] fix: accept string node ids from the Files sidebar when requesting a signature

lib/Controller/RequestSignatureController.php

@@ -180,6 +180,7 @@ public function updateSignatureRequest(
 				);
 			}
 
+			$file = $this->normalizeNodeId($file);
 			$data = [
 				'uuid' => $uuid,
 				'file' => $file,
@@ -221,6 +222,41 @@ public function updateSignatureRequest(
 	 * @return DataResponse<Http::STATUS_OK, LibresignDetailedFileResponse, array{}>
 	 * @throws LibresignException
 	 */
+	/**
+	 * Normalize a Nextcloud node id at the HTTP boundary.
+	 *
+	 * The Files app sends node ids as strings (`Node.id` of `@nextcloud/files`,
+	 * 64-bit since Nextcloud 33); the services expect the int of the Nextcloud
+	 * Files API. A non-negative int or its canonical decimal string becomes an
+	 * int here; anything else is rejected before it can reach a later cast.
+	 *
+	 * @param array<string, mixed> $file
+	 * @return array<string, mixed>
+	 * @throws LibresignException
+	 */
+	private function normalizeNodeId(array $file): array {
+		$nodeId = $file['nodeId'] ?? null;
+		if ($nodeId === null) {
+			return $file;
+		}
+
+		if (is_string($nodeId) && ctype_digit($nodeId)) {
+			$nodeId = filter_var($nodeId, FILTER_VALIDATE_INT);
+		}
+
+		if (is_int($nodeId) && $nodeId >= 0) {
+			$file['nodeId'] = $nodeId;
+			return $file;
+		}
+
+		throw new LibresignException(
+			$this->l10n->t('File type: %s. Invalid fileID.', [$this->l10n->t('document to sign')]),
+		);
+	}
+
+	/**
+	 * @return DataResponse<Http::STATUS_OK, LibresignDetailedFileResponse, array{}>
+	 */
 	private function createSignatureRequest(
 		$user,
 		array $file,
@@ -241,6 +277,9 @@ private function createSignatureRequest(
 			throw new LibresignException($this->l10n->t('File or files parameter is required'));
 		}
 
+		$file = $this->normalizeNodeId($file);
+		$filesToSave = $filesToSave === null ? null : array_map(fn (array $item): array => $this->normalizeNodeId($item), $filesToSave);
+
 		$data = [
 			'file' => $file,
 			'name' => $name,

src/components/RightSidebar/AppFilesTab.vue

@@ -47,7 +47,9 @@ type PendingEnvelope = {
 }
 
 type FileInfo = {
-	id: number
+	// Nextcloud node id as tab.ts sends it: the numeric `fileid` when the node
+	// has one, otherwise the string `Node.id` of `@nextcloud/files`.
+	id: number | string
 	type?: string
 	name?: string
 	path?: string

src/store/files.js

@@ -926,6 +926,24 @@ const _filesStore = defineStore('files', () => {
 			.filter((signer) => signer && signer.identifyMethods?.length)
 	}
 
+	/**
+	 * Whether a value identifies a Nextcloud node. Besides the historical
+	 * positive number, `@nextcloud/files` exposes `Node.id` as a string and
+	 * that is what the Files sidebar hands to AppFilesTab (#8363). The string
+	 * is kept as is: node ids are 64-bit and converting them with Number()
+	 * could change the value above Number.MAX_SAFE_INTEGER. The API accepts
+	 * both representations.
+	 *
+	 * @param {unknown} value
+	 * @return {value is number | string}
+	 */
+	function isNodeId(value) {
+		if (typeof value === 'number') {
+			return Number.isInteger(value) && value > 0
+		}
+		return typeof value === 'string' && /^[1-9][0-9]*$/.test(value)
+	}
+
 	/** @param {EditableFileReferenceDraft | ApiFileRecord | EditableFileDraft | string | null | undefined} file */
 	function serializeRequestFile(file, { preferNodeId = false } = {}) {
 		if (typeof file === 'string') {
@@ -937,7 +955,7 @@ const _filesStore = defineStore('files', () => {
 		if (typeof file.path === 'string' && file.path.length > 0) {
 			return { path: file.path }
 		}
-		if (preferNodeId && typeof file.nodeId === 'number' && file.nodeId > 0) {
+		if (preferNodeId && isNodeId(file.nodeId)) {
 			return { nodeId: file.nodeId }
 		}
 		if (typeof file.fileId === 'number' && file.fileId > 0) {
@@ -948,7 +966,7 @@ const _filesStore = defineStore('files', () => {
 				return { fileId: file.id }
 			}
 		}
-		if (typeof file.nodeId === 'number' && file.nodeId > 0) {
+		if (isNodeId(file.nodeId)) {
 			return { nodeId: file.nodeId }
 		}
 		if (typeof file.url === 'string' && file.url.length > 0) {

src/tests/components/RightSidebar/AppFilesTab.spec.ts

@@ -72,7 +72,7 @@ type TitleObserver = {
 }
 
 type FileInfo = {
-	id: number
+	id: number | string
 	type?: string
 	name?: string
 	path?: string
@@ -286,6 +286,28 @@ describe('AppFilesTab', () => {
 			expect(sidebarStore.activeRequestSignatureTab).toHaveBeenCalled()
 		})
 
+		it('passes a string node id through unchanged when adding the file (#8363)', async () => {
+			filesStore.selectFileByNodeId = vi.fn().mockResolvedValue(null)
+			filesStore.addFile = vi.fn()
+			filesStore.selectFile = vi.fn()
+			sidebarStore.activeRequestSignatureTab = vi.fn()
+			wrapper = createWrapper()
+
+			// tab.ts sends `Node.id` (a string) when the node has no numeric fileid
+			await wrapper.vm.update({
+				id: '9007199254740993',
+				name: 'copy of contract.pdf',
+				path: '/Documents',
+			})
+
+			expect(filesStore.selectFileByNodeId).toHaveBeenCalledWith('9007199254740993')
+			expect(filesStore.addFile).toHaveBeenCalledWith(expect.objectContaining({
+				nodeId: '9007199254740993',
+				name: 'copy of contract.pdf',
+			}))
+			expect(sidebarStore.activeRequestSignatureTab).toHaveBeenCalled()
+		})
+
 		it('returns early when pending envelope processed', async () => {
 			window.OCA = {
 				Libresign: {

src/tests/store/files.spec.ts

@@ -1354,6 +1354,79 @@ describe('files store - critical business rules', () => {
 				expect(config.data.file).toEqual({ nodeId })
 			})
 
+			/**
+			 * Regression #8363: `@nextcloud/files` exposes `Node.id` as a string and
+			 * the Files sidebar hands it to AppFilesTab as is (a file copied in the
+			 * Files app, or any node id above Number.MAX_SAFE_INTEGER). The store
+			 * keeps it as nodeId; the request must carry it unchanged instead of
+			 * dropping the whole "file" (422 "File or files parameter is required").
+			 */
+			it('includes file.nodeId as the string the Files sidebar provided', async () => {
+				const store = useFilesStore()
+				const nodeId = '9007199254740993'
+				const tempId = -Number(nodeId)
+				store.files[tempId] = {
+					id: tempId,
+					nodeId,
+					name: 'copy of contract.pdf',
+					signers: [{ email: 'signer@example.com', identifyMethods: [{ method: 'email', value: 'signer@example.com', mandatory: 0 }] }],
+					signatureFlow: 'parallel',
+				}
+				store.selectedFileId = tempId
+
+				axiosMock.mockResolvedValue({
+					data: { ocs: { data: { id: 77, nodeId: 77, signatureFlow: 'parallel', signers: [] } } },
+				})
+
+				await store.saveOrUpdateSignatureRequest({})
+
+				const config = axiosMock.mock.calls[0][0]
+				expect(config.data.file).toEqual({ nodeId: '9007199254740993' })
+			})
+
+			it('serializes envelope files with string and number node ids as they are', async () => {
+				const store = useFilesStore()
+				store.selectedFileId = -1
+				store.files[-1] = {
+					id: -1,
+					name: 'Envelope',
+					files: [
+						{ id: -7, nodeId: '9007199254740993', name: 'first.pdf' },
+						{ id: -22, nodeId: 22, name: 'second.pdf' },
+					],
+					signers: [{ email: 'signer@example.com' }],
+					signatureFlow: 'parallel',
+				}
+				axiosMock.mockResolvedValue({
+					data: { ocs: { data: { id: 12, nodeId: 'real-node', signatureFlow: 'parallel', signers: [] } } },
+				})
+
+				await store.saveOrUpdateSignatureRequest({})
+
+				const config = axiosMock.mock.calls[0][0]
+				expect(config.data.files).toEqual([{ nodeId: '9007199254740993' }, { nodeId: 22 }])
+			})
+
+			it('does not send the empty node id tab.ts falls back to when the node has none', async () => {
+				const store = useFilesStore()
+				store.files[-1] = {
+					id: -1,
+					nodeId: '',
+					name: 'unknown.pdf',
+					signers: [{ email: 'signer@example.com' }],
+					signatureFlow: 'parallel',
+				}
+				store.selectedFileId = -1
+				axiosMock.mockResolvedValue({
+					data: { ocs: { data: { id: 12, nodeId: 12, signatureFlow: 'parallel', signers: [] } } },
+				})
+
+				await store.saveOrUpdateSignatureRequest({})
+
+				const config = axiosMock.mock.calls[0][0]
+				expect(config.data.file).toBeNull()
+			})
+
 			it('serializes envelope files with nodeId-based references for creation flows', async () => {
 				const store = useFilesStore()
 				store.selectedFileId = -1

src/tests/tab.spec.ts

@@ -108,6 +108,34 @@ describe('tab.ts', () => {
 		})
 	})
 
+	it('enabled() keeps the string node id of a PDF whose node has no numeric fileid', () => {
+		mockLoadState.mockReturnValue(true)
+		window.dispatchEvent(new Event('DOMContentLoaded'))
+		const tabConfig = mockRegisterSidebarTab.mock.calls[0][0] as {
+			enabled: (context: { node: Record<string, unknown> }) => boolean
+		}
+
+		// `@nextcloud/files` Node: `id` is always a string and `fileid` is
+		// undefined when the id does not fit a JavaScript number (#8363).
+		const enabled = tabConfig.enabled({
+			node: {
+				id: '9007199254740993',
+				fileid: undefined,
+				basename: 'copy of contract.pdf',
+				dirname: '/Documents',
+				type: 'file',
+				mime: 'application/pdf',
+			},
+		})
+
+		expect(enabled).toBe(true)
+		expect(window.OCA.Libresign.fileInfo).toMatchObject({
+			id: '9007199254740993',
+			name: 'copy of contract.pdf',
+			path: '/Documents',
+		})
+	})
+
 	it('lazy mounts Vue only when custom element is connected and unmounts on disconnect', async () => {
 		window.dispatchEvent(new Event('DOMContentLoaded'))
 

tests/php/Unit/Controller/RequestSignatureControllerTest.php

@@ -62,6 +62,90 @@ protected function setUp(): void {
 		);
 	}
 
+	/**
+	 * Regression #8363: the Files sidebar sends the node id as a string. The
+	 * controller is the boundary between the HTTP payload and the services,
+	 * so everything after it must already see an int.
+	 */
+	public function testRequestNormalizesTheStringNodeIdBeforeServices(): void {
+		$file = new FileEntity();
+		$file->setId(10);
+
+		$this->requestSignatureService->expects($this->once())
+			->method('validateNewRequestToFile')
+			->with($this->callback(static fn (array $payload): bool => $payload['file']['nodeId'] === 9007199254740993));
+		$this->requestSignatureService->expects($this->once())
+			->method('save')
+			->with($this->callback(static fn (array $payload): bool => $payload['file']['nodeId'] === 9007199254740993))
+			->willReturn($file);
+		$this->fileListService->method('formatFileWithChildren')->willReturn(['ok' => true]);
+
+		$response = $this->controller->requestSignature(
+			signers: [['identifyMethods' => [['method' => 'email', 'value' => 'user@test.coop', 'mandatory' => 0]]]],
+			name: 'copy of contract.pdf',
+			settings: [],
+			file: ['nodeId' => '9007199254740993'],
+			files: [],
+			callback: null,
+			status: 1,
+			signatureFlow: null,
+		);
+
+		$this->assertSame(Http::STATUS_OK, $response->getStatus());
+	}
+
+	public function testRequestNormalizesTheNodeIdOfEachEnvelopeFile(): void {
+		$envelope = new FileEntity();
+		$envelope->setId(30);
+
+		$this->requestSignatureService->expects($this->once())
+			->method('validateNewRequestToFile')
+			->with($this->callback(static fn (array $payload): bool => $payload['files'][0]['nodeId'] === 9007199254740993
+				&& $payload['files'][1]['nodeId'] === 22));
+		$this->requestSignatureService->expects($this->once())
+			->method('saveFiles')
+			->with($this->callback(static fn (array $payload): bool => $payload['files'][0]['nodeId'] === 9007199254740993
+				&& $payload['files'][1]['nodeId'] === 22))
+			->willReturn(['file' => $envelope, 'children' => []]);
+		$this->fileListService->method('formatFileWithChildren')->willReturn(['ok' => true]);
+
+		$response = $this->controller->requestSignature(
+			signers: [['identifyMethods' => [['method' => 'email', 'value' => 'user@test.coop', 'mandatory' => 0]]]],
+			name: 'Envelope',
+			settings: [],
+			file: [],
+			files: [
+				['nodeId' => '9007199254740993', 'name' => 'part-a.pdf'],
+				['nodeId' => 22, 'name' => 'part-b.pdf'],
+			],
+			callback: null,
+			status: 0,
+			signatureFlow: null,
+		);
+
+		$this->assertSame(Http::STATUS_OK, $response->getStatus());
+	}
+
+	public function testRequestRejectsAnInvalidNodeIdBeforeAnyService(): void {
+		$this->l10n->method('t')->willReturnCallback(static fn (string $text, array $params = []): string => vsprintf($text, $params));
+		$this->requestSignatureService->expects($this->never())->method('validateNewRequestToFile');
+		$this->requestSignatureService->expects($this->never())->method('save');
+
+		$response = $this->controller->requestSignature(
+			signers: [['identifyMethods' => [['method' => 'email', 'value' => 'user@test.coop', 'mandatory' => 0]]]],
+			name: 'contract.pdf',
+			settings: [],
+			file: ['nodeId' => 'temp-node'],
+			files: [],
+			callback: null,
+			status: 1,
+			signatureFlow: null,
+		);
+
+		$this->assertSame(Http::STATUS_UNPROCESSABLE_ENTITY, $response->getStatus());
+		$this->assertSame('File type: document to sign. Invalid fileID.', $response->getData()['message']);
+	}
+
 	#[DataProvider('statusPayloadScenarios')]
 	public function testRequestStatusPropagation(?int $status, bool $expectStatusKey): void {
 		$file = new FileEntity();