[stable35] fix(validation): open signed PDF by UUID instead of owner node ID

playwright/e2e/sign-envelope-unauthenticated-visible-signature.spec.ts

@@ -87,6 +87,9 @@ async function enableEnvelopeScenario(request: APIRequestContext) {
 
 	await setCertificateEngine(request, 'openssl')
 
+	// This flow opens signed PDFs as an anonymous signer; do not inherit a
+	// private validation policy left by another Playwright scenario.
+	await setSystemPolicy(request, 'make_validation_url_private', '0')
 	await setSystemPolicy(request, 'envelope_enabled', '1')
 	await setSystemPolicy(request, 'identification_documents', JSON.stringify({ enabled: false, approvers: ['admin'] }))
 	await setSystemPolicy(
@@ -160,7 +163,7 @@ async function waitForSignerInvitationLink(signerEmail: string) {
 	const email = await waitForEmailTo(
 		createMailpitClient(),
 		signerEmail,
-		'LibreSign: A document is ready for your signature',
+		undefined, // Notification language can differ from the test browser locale.
 		{ interval: 250 },
 	)
 	const signLink = extractSignLink(email.Text)
@@ -285,4 +288,36 @@ test('unauthenticated signer can define a visible signature for an envelope with
 	await test.step('Then the success confirmation screen is shown with the envelope name', async () => {
 		await expectEnvelopeSigned(page, scenario.envelopeName)
 	})
+
+	await test.step('The signed PDF opens by UUID without using an Office handler', async () => {
+		const context = page.context()
+		const officeRequests: string[] = []
+		context.on('request', request => {
+			if (request.url().includes('/apps/eurooffice/')) {
+				officeRequests.push(request.url())
+			}
+		})
+
+		// The legacy Viewer cannot load anonymous PDFs through authenticated
+		// Files APIs. Check the direct URL fallback in this public signing flow.
+		const popupPromise = page.waitForEvent('popup')
+		const pdfResponse = context.waitForEvent('response', response =>
+			response.request().method() === 'GET'
+			&& response.url().includes('/apps/libresign/p/pdf/'),
+		)
+		await page.getByRole('button', { name: 'Show details' }).first().click()
+		await page.getByRole('button', { name: 'View document' }).first().click()
+
+		const popup = await popupPromise
+		expect(await popup.opener()).toBe(page)
+		const response = await pdfResponse
+		expect(response.status()).toBe(200)
+		expect(response.headers()['content-type']).toContain('application/pdf')
+		expect(new URL(response.url()).pathname).toMatch(/\/apps\/libresign\/p\/pdf\/[^/]+$/)
+
+		const signedPdf = await page.request.get(response.url())
+		expect(signedPdf.status()).toBe(200)
+		expect((await signedPdf.body()).toString('latin1')).toContain('/ByteRange')
+		expect(officeRequests).toHaveLength(0)
+	})
 })

src/components/validation/DocumentValidationDetails.vue

@@ -99,14 +99,13 @@ const size = computed(() => {
 const documentStatus = computed(() => getStatusLabel(document.value.status))
 
 async function viewDocument() {
-	if (!document.value.uuid || !document.value.name || typeof document.value.nodeId !== 'number') {
+	if (!document.value.uuid || !document.value.name) {
 		return
 	}
 	const fileUrl = generateUrl('/apps/libresign/p/pdf/{uuid}', { uuid: document.value.uuid })
 	await openDocument({
 		fileUrl,
 		filename: document.value.name,
-		nodeId: document.value.nodeId,
 	})
 }
 

src/components/validation/EnvelopeValidation.vue

@@ -65,7 +65,7 @@
 						<template #subname>
 							<strong>{{ t('libresign', 'Status:') }}</strong> {{ getFileStatusText(file) }}
 						</template>
-						<template v-if="!isTouchDevice && file.nodeId" #actions>
+						<template v-if="!isTouchDevice && file.uuid" #actions>
 							<NcActionButton @click.stop="viewFile(file)">
 								<template #icon>
 									<NcIconSvgWrapper :path="mdiEye" :size="20" />
@@ -74,7 +74,7 @@
 							</NcActionButton>
 						</template>
 						<template #extra-actions>
-							<NcButton v-if="isTouchDevice && file.nodeId" variant="tertiary" :aria-label="t('libresign', 'View PDF')" @click.stop="viewFile(file)">
+							<NcButton v-if="isTouchDevice && file.uuid" variant="tertiary" :aria-label="t('libresign', 'View PDF')" @click.stop="viewFile(file)">
 								<template #icon>
 									<NcIconSvgWrapper :path="mdiEye" :size="20" />
 								</template>
@@ -278,14 +278,13 @@ function getSignerProgressText(signer: EnvelopeSigner) {
 }
 
 function viewFile(file: EnvelopeFile) {
-	if (!file.uuid || !file.name || typeof file.nodeId !== 'number') {
+	if (!file.uuid || !file.name) {
 		return
 	}
 	const fileUrl = generateUrl('/apps/libresign/p/pdf/{uuid}', { uuid: file.uuid })
-	openDocument({
+	return openDocument({
 		fileUrl,
 		filename: file.name,
-		nodeId: file.nodeId,
 	})
 }
 

src/tests/components/validation/DocumentValidationDetails.spec.ts

@@ -398,7 +398,7 @@ describe('DocumentValidationDetails', () => {
 			expect(wrapper.vm.document.uuid).toBeUndefined()
 		})
 
-		it('calls openDocument with correct parameters', async () => {
+		it('opens the signed PDF by UUID without passing the owner nodeId', async () => {
 			wrapper = createWrapper({
 				document: {
 					uuid: 'doc-uuid-123',
@@ -410,9 +410,24 @@ describe('DocumentValidationDetails', () => {
 			await wrapper.vm.viewDocument()
 
 			expect(viewer.openDocument).toHaveBeenCalledWith({
-				fileUrl: expect.stringContaining('doc-uuid-123'),
+				fileUrl: '/apps/libresign/p/pdf/doc-uuid-123',
 				filename: 'Test.pdf',
-				nodeId: 456,
+			})
+		})
+
+		it('opens the document when the response has no nodeId', async () => {
+			wrapper = createWrapper({
+				document: {
+					uuid: 'no-node-id',
+					name: 'No nodeId.pdf',
+				},
+			})
+
+			await wrapper.vm.viewDocument()
+
+			expect(viewer.openDocument).toHaveBeenCalledWith({
+				fileUrl: '/apps/libresign/p/pdf/no-node-id',
+				filename: 'No nodeId.pdf',
 			})
 		})
 
@@ -451,8 +466,8 @@ describe('DocumentValidationDetails', () => {
 		it('does not open viewer without required viewer data', async () => {
 			wrapper = createWrapper({
 				document: {
-					uuid: 'missing-node-id',
-					name: 'Missing nodeId.pdf',
+					name: 'Missing uuid.pdf',
+					nodeId: 456,
 				},
 			})
 

src/tests/components/validation/EnvelopeValidation.spec.ts

@@ -361,20 +361,33 @@ describe('EnvelopeValidation', () => {
 			})
 
 			expect(viewer.openDocument).toHaveBeenCalledWith({
-				fileUrl: expect.stringContaining('file-uuid-123'),
+				fileUrl: '/apps/libresign/p/pdf/file-uuid-123',
 				filename: 'Document.pdf',
-				nodeId: 456,
 			})
 		})
 
-		it('does not open viewer without nodeId', () => {
+		it('opens the file by UUID when it has no nodeId', () => {
 			wrapper = createWrapper()
 
 			wrapper.vm.viewFile({
 				uuid: 'file-uuid-123',
 				name: 'Document.pdf',
 			})
 
+			expect(viewer.openDocument).toHaveBeenCalledWith({
+				fileUrl: '/apps/libresign/p/pdf/file-uuid-123',
+				filename: 'Document.pdf',
+			})
+		})
+
+		it('does not open viewer without uuid', () => {
+			wrapper = createWrapper()
+
+			wrapper.vm.viewFile({
+				name: 'Document.pdf',
+				nodeId: 456,
+			})
+
 			expect(viewer.openDocument).not.toHaveBeenCalled()
 		})
 	})
@@ -512,8 +525,8 @@ describe('EnvelopeValidation', () => {
 			expect(typeof wrapper.vm.isTouchDevice).toBe('boolean')
 		})
 
-		it('renders actions slot when not touch device and file has nodeId', async () => {
-			const file: EnvelopeFile = { id: 1, nodeId: 123, status: '3', name: 'test.pdf' }
+		it('renders actions slot when not touch device and file has uuid', async () => {
+			const file: EnvelopeFile = { id: 1, uuid: 'test-uuid', nodeId: 123, status: '3', name: 'test.pdf' }
 			wrapper = createWrapper({
 				document: { files: [file] },
 			})
@@ -536,7 +549,6 @@ describe('EnvelopeValidation', () => {
 			expect(viewer.openDocument).toHaveBeenCalledWith({
 				fileUrl: '/apps/libresign/p/pdf/test-uuid',
 				filename: 'test.pdf',
-				nodeId: 123,
 			})
 		})
 
@@ -551,7 +563,6 @@ describe('EnvelopeValidation', () => {
 			expect(viewer.openDocument).toHaveBeenCalledWith({
 				fileUrl: '/apps/libresign/p/pdf/test-uuid',
 				filename: 'test.pdf',
-				nodeId: 123,
 			})
 		})
 	})

src/tests/utils/viewer.spec.ts

@@ -3,82 +3,108 @@
  * SPDX-License-Identifier: AGPL-3.0-or-later
  */
 
-import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest'
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
+
+const mocks = vi.hoisted(() => ({ getCurrentUser: vi.fn() }))
+vi.mock('@nextcloud/auth', () => ({ getCurrentUser: mocks.getCurrentUser }))
+
 import { openDocument } from '../../utils/viewer.js'
 
-type GlobalWithOCA = typeof globalThis & {
-	OCA?: {
-		Viewer?: {
-			open?: (params: unknown) => void
-		}
-		[key: string]: unknown
-	}
+type LegacyViewer = {
+	availableHandlers?: Array<{ id: string; mimes?: string[] }>
+	open?: ReturnType<typeof vi.fn>
+	openWith?: ReturnType<typeof vi.fn>
 }
+type GlobalWithOCA = typeof globalThis & { OCA?: { Viewer?: LegacyViewer } }
 
-describe('openDocument', () => {
+describe('openDocument on Nextcloud 33-35', () => {
 	let originalOCA: GlobalWithOCA['OCA']
 	let openSpy: ReturnType<typeof vi.spyOn>
+	let viewer: Required<LegacyViewer>
 
 	beforeEach(() => {
-		originalOCA = (global as GlobalWithOCA).OCA
+		originalOCA = (globalThis as GlobalWithOCA).OCA
 		openSpy = vi.spyOn(window, 'open').mockImplementation(() => null)
+		mocks.getCurrentUser.mockReturnValue({ uid: 'signer' })
+		viewer = {
+			availableHandlers: [
+				{ id: 'eurooffice', mimes: ['application/pdf'] },
+				{ id: 'pdf', mimes: ['application/pdf'] },
+			],
+			open: vi.fn(),
+			openWith: vi.fn(),
+		}
+		;(globalThis as GlobalWithOCA).OCA = { Viewer: viewer }
 	})
 
 	afterEach(() => {
 		openSpy.mockRestore()
-		;(global as GlobalWithOCA).OCA = originalOCA
+		;(globalThis as GlobalWithOCA).OCA = originalOCA
+		vi.clearAllMocks()
 	})
 
-	it('uses Nextcloud Viewer when available', () => {
-		const viewerOpen = vi.fn()
-		;(global as GlobalWithOCA).OCA = {
-			Viewer: {
-				open: viewerOpen,
-			},
-		}
-
-		const file = {
-			filename: 'doc.pdf',
-			nodeId: 123,
-			url: 'https://example.test/index.php/apps/files/?file=/doc.pdf',
-		}
-
+	it.each(['owner', 'signer without access to the original'])('uses native PDF handler without original fileid for %s', () => {
 		openDocument({
-			fileUrl: file.url,
-			filename: file.filename,
-			nodeId: file.nodeId,
+			fileUrl: '/index.php/apps/libresign/p/pdf/signed-uuid',
+			filename: 'signed.pdf',
+			nodeId: 123,
+		})
+		expect(viewer.openWith).toHaveBeenCalledOnce()
+		const [id, options] = viewer.openWith.mock.calls[0]
+		expect(id).toBe('pdf')
+		expect(options.fileInfo).toEqual({
+			source: '/index.php/apps/libresign/p/pdf/signed-uuid',
+			basename: 'signed.pdf',
+			mime: 'application/pdf',
 		})
+		expect(options.list).toEqual([options.fileInfo])
+		expect(viewer.open).not.toHaveBeenCalled()
+		expect(openSpy).not.toHaveBeenCalled()
+	})
 
-		expect(viewerOpen).toHaveBeenCalledTimes(1)
-		const payload = viewerOpen.mock.calls[0]?.[0]
-		if (payload) {
-			expect(payload.fileInfo.basename).toBe('doc.pdf')
-			expect(payload.fileInfo.fileid).toBe(123)
-			expect(payload.fileInfo.mime).toBe('application/pdf')
-			expect(payload.fileInfo.source).toContain('/index.php/apps/files/')
-		}
+	it('falls back on public pages even if the PDF handler is registered', () => {
+		mocks.getCurrentUser.mockReturnValue(null)
+		openDocument({ fileUrl: '/apps/libresign/p/pdf/signed-uuid', filename: 'signed.pdf', nodeId: 123 })
+		expect(viewer.openWith).not.toHaveBeenCalled()
+		expect(openSpy).toHaveBeenCalledOnce()
+		expect(new URL(openSpy.mock.calls[0][0] as string).pathname).toBe('/apps/libresign/p/pdf/signed-uuid')
 	})
 
-	it('opens new window when Viewer is not available', () => {
-		;(global as GlobalWithOCA).OCA = undefined
+	it('does not let Office handlers handle signed UUID PDFs', () => {
+		viewer.availableHandlers = [{ id: 'eurooffice', mimes: ['application/pdf'] }]
+		openDocument({ fileUrl: '/apps/libresign/p/pdf/signed-uuid', filename: 'signed.pdf', nodeId: 456 })
+		expect(viewer.openWith).not.toHaveBeenCalled()
+		expect(viewer.open).not.toHaveBeenCalled()
+		expect(openSpy).toHaveBeenCalledOnce()
+	})
 
-		const file = {
-			filename: 'doc.pdf',
-			nodeId: 456,
-			url: '/apps/files/?file=/doc.pdf',
-		}
+	it('falls back when the Viewer is not installed', () => {
+		;(globalThis as GlobalWithOCA).OCA = undefined
+		openDocument({ fileUrl: '/apps/libresign/p/pdf/signed-uuid?download=1', filename: 'signed.pdf' })
+		const url = new URL(openSpy.mock.calls[0][0] as string)
+		expect(url.searchParams.get('download')).toBe('1')
+		expect(url.searchParams.get('_t')).toMatch(/^\d+$/)
+	})
 
-		openDocument({
-			fileUrl: file.url,
-			filename: file.filename,
-			nodeId: file.nodeId,
+	it('keeps the original file ID for ordinary Nextcloud documents', () => {
+		openDocument({ fileUrl: '/apps/files/?file=/contract.pdf', filename: 'contract.pdf', nodeId: 123 })
+		expect(viewer.open).toHaveBeenCalledWith({
+			fileInfo: { source: '/apps/files/?file=/contract.pdf', basename: 'contract.pdf', mime: 'application/pdf', fileid: 123 },
+			list: [{ source: '/apps/files/?file=/contract.pdf', basename: 'contract.pdf', mime: 'application/pdf', fileid: 123 }],
 		})
+		expect(viewer.openWith).not.toHaveBeenCalled()
+	})
 
-		expect(openSpy).toHaveBeenCalledTimes(1)
-		const openedUrl = openSpy.mock.calls[0]?.[0]
-		if (openedUrl) {
-			expect(openedUrl).toContain('/apps/files/?file=/doc.pdf')
-			expect(openedUrl).toMatch(/_t=\d+$/)
-		}
+	it('opens external URLs directly instead of giving them to local handlers', () => {
+		openDocument({ fileUrl: 'https://example.test/file.pdf?lang=pt', filename: 'external.pdf', nodeId: 123 })
+		expect(viewer.open).not.toHaveBeenCalled()
+		expect(viewer.openWith).not.toHaveBeenCalled()
+		expect(new URL(openSpy.mock.calls[0][0] as string).hostname).toBe('example.test')
+	})
+
+	it('propagates Viewer errors rather than silently opening another window', () => {
+		viewer.openWith.mockImplementation(() => { throw new Error('Viewer failed') })
+		expect(() => openDocument({ fileUrl: '/apps/libresign/p/pdf/signed-uuid', filename: 'signed.pdf' })).toThrow('Viewer failed')
+		expect(openSpy).not.toHaveBeenCalled()
 	})
 })