From 897416b10323cce466e62a659d7acdc22cc1cf0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Maia?= Date: Fri, 11 Sep 2026 22:53:35 -0300 Subject: [PATCH 1/2] fix: store identification documents of external signers under the file owner MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When an unauthenticated signer uploads an identification document, IdDocsService::addFilesToDocumentFolder() called saveFile() without a userManager. FolderService then had no user id, so the node was written to the unauthenticated appdata folder, while the File row received the owner of the signed file as user_id — the value getPdfByUuid() later uses to look the node up. The document was intact on disk but unreachable from the validation UI (empty viewer, 404 on download). Resolve the owner of the signed file with the same lookup saveFile() already uses for user_id and pass it as userManager, mirroring addIdDocs(). Uploads for a sign request whose owner cannot be resolved keep the current behaviour. Resolves: #8364 Assisted-by: Claude Code:claude-opus-5 Signed-off-by: André Maia --- lib/Service/IdDocsService.php | 22 +++++ tests/php/Unit/Service/IdDocsServiceTest.php | 85 ++++++++++++++++++++ 2 files changed, 107 insertions(+) diff --git a/lib/Service/IdDocsService.php b/lib/Service/IdDocsService.php index dd5be169c0..66ff51087d 100644 --- a/lib/Service/IdDocsService.php +++ b/lib/Service/IdDocsService.php @@ -23,6 +23,7 @@ use OCP\IAppConfig; use OCP\IL10N; use OCP\IUser; +use OCP\IUserManager; use Sabre\DAV\UUIDUtil; class IdDocsService { @@ -38,6 +39,7 @@ public function __construct( private IdentifyMethodMapper $identifyMethodMapper, private ITimeFactory $timeFactory, private IAppConfig $appConfig, + private IUserManager $userManager, ) { } @@ -115,16 +117,36 @@ public function addFilesToDocumentFolder( foreach ($files as $fileIndex => $file) { $this->validateTypeOfFile($fileIndex, $file); } + // Store the documents under the owner of the file being signed, where + // later lookups by the stored user_id expect them; without an IUser the + // node would be written to the unauthenticated appdata folder instead. + $owner = $this->getOwnerOfSignedFile($signRequest); foreach ($files as $fileData) { $dataToSave = $fileData; $dataToSave['signRequest'] = $signRequest; $dataToSave['name'] = $fileData['name'] ?? $fileData['type']; + if ($owner instanceof IUser) { + $dataToSave['userManager'] = $owner; + } $file = $this->requestSignatureService->saveFile($dataToSave); $this->idDocsMapper->save($file->getId(), $signRequest->getId(), null, $fileData['type']); } } + private function getOwnerOfSignedFile(SignRequest $signRequest): ?IUser { + $signedFileId = $signRequest->getFileId(); + if (!$signedFileId) { + return null; + } + try { + $signedFile = $this->fileMapper->getById($signedFileId); + } catch (\Throwable) { + return null; + } + return $this->userManager->get($signedFile->getUserId()); + } + public function list(array $filter, ?int $page = null, ?int $length = null, array $sort = []): array { $page = $page ?? 1; $length = $length ?? (int)$this->appConfig->getValueInt(Application::APP_ID, 'length_of_page', 100); diff --git a/tests/php/Unit/Service/IdDocsServiceTest.php b/tests/php/Unit/Service/IdDocsServiceTest.php index 34c1138b4b..2fea4b206b 100644 --- a/tests/php/Unit/Service/IdDocsServiceTest.php +++ b/tests/php/Unit/Service/IdDocsServiceTest.php @@ -20,6 +20,8 @@ use OCA\Libresign\Service\RequestSignatureService; use OCP\IAppConfig; use OCP\IL10N; +use OCP\IUser; +use OCP\IUserManager; use PHPUnit\Framework\MockObject\MockObject; /** @@ -37,6 +39,7 @@ final class IdDocsServiceTest extends \OCA\Libresign\Tests\Unit\TestCase { private RequestSignatureService&MockObject $requestSignatureService; private TimeFactory&MockObject $timeFactory; private IAppConfig&MockObject $appConfig; + private IUserManager&MockObject $userManager; public function setUp(): void { parent::setUp(); @@ -53,6 +56,7 @@ public function setUp(): void { $this->requestSignatureService = $this->createMock(RequestSignatureService::class); $this->timeFactory = $this->createMock(TimeFactory::class); $this->appConfig = $this->createMock(IAppConfig::class); + $this->userManager = $this->createMock(IUserManager::class); } private function getIdDocsService(): IdDocsService { @@ -67,6 +71,7 @@ private function getIdDocsService(): IdDocsService { $this->identifyMethodMapper, $this->timeFactory, $this->appConfig, + $this->userManager, ); } @@ -166,4 +171,84 @@ public function testDeleteIdDocBySignRequestThrowsOnInvalidDoc(): void { $service = $this->getIdDocsService(); $service->deleteIdDocBySignRequest(123, $signRequest); } + public function testAddFilesToDocumentFolderStoresFilesUnderTheOwnerOfTheSignedFile(): void { + $signRequest = new SignRequest(); + $signRequest->setId(55); + $signRequest->setFileId(10); + + $signedFile = new \OCA\Libresign\Db\File(); + $signedFile->setUserId('owner'); + $this->fileMapper->method('getById') + ->with(10) + ->willReturn($signedFile); + + $owner = $this->createMock(IUser::class); + $this->userManager->method('get') + ->with('owner') + ->willReturn($owner); + + $this->fileTypeMapper->method('getTypes') + ->willReturn(['IDENTIFICATION' => ['type' => 'IDENTIFICATION']]); + + $savedFile = new \OCA\Libresign\Db\File(); + $savedFile->setId(77); + $this->requestSignatureService->expects($this->once()) + ->method('saveFile') + ->with($this->callback(function (array $data) use ($owner, $signRequest): bool { + $this->assertSame($owner, $data['userManager']); + $this->assertSame($signRequest, $data['signRequest']); + $this->assertSame('id-front.pdf', $data['name']); + return true; + })) + ->willReturn($savedFile); + + $this->idDocsMapper->expects($this->once()) + ->method('save') + ->with(77, 55, null, 'IDENTIFICATION'); + + $service = $this->getIdDocsService(); + $service->addFilesToDocumentFolder( + [['type' => 'IDENTIFICATION', 'name' => 'id-front.pdf', 'base64' => 'ZmFrZQ==']], + $signRequest, + ); + } + + public function testAddFilesToDocumentFolderWithoutResolvableOwnerKeepsCurrentBehaviour(): void { + $signRequest = new SignRequest(); + $signRequest->setId(55); + $signRequest->setFileId(10); + + $signedFile = new \OCA\Libresign\Db\File(); + $signedFile->setUserId('deleted-owner'); + $this->fileMapper->method('getById') + ->with(10) + ->willReturn($signedFile); + + $this->userManager->method('get') + ->with('deleted-owner') + ->willReturn(null); + + $this->fileTypeMapper->method('getTypes') + ->willReturn(['IDENTIFICATION' => ['type' => 'IDENTIFICATION']]); + + $savedFile = new \OCA\Libresign\Db\File(); + $savedFile->setId(77); + $this->requestSignatureService->expects($this->once()) + ->method('saveFile') + ->with($this->callback(function (array $data): bool { + $this->assertArrayNotHasKey('userManager', $data); + return true; + })) + ->willReturn($savedFile); + + $this->idDocsMapper->expects($this->once()) + ->method('save') + ->with(77, 55, null, 'IDENTIFICATION'); + + $service = $this->getIdDocsService(); + $service->addFilesToDocumentFolder( + [['type' => 'IDENTIFICATION', 'base64' => 'ZmFrZQ==']], + $signRequest, + ); + } } From 56a21754efe39b6425cbf22fcc0e0324e0efb7b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Maia?= Date: Sat, 12 Sep 2026 09:18:59 -0300 Subject: [PATCH 2/2] fix: only treat a missing file row as "no owner" when resolving id docs storage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit getOwnerOfSignedFile() caught \Throwable, which turned unexpected database or programming errors into the unauthenticated appdata fallback — the same inconsistent state this change is fixing. Catch only the DoesNotExistException documented by FileMapper::getById() and let other failures surface. Add one test for the missing row (falls back, no userManager) and one asserting that an unexpected exception is propagated before saveFile() is reached. Assisted-by: Claude Code:claude-opus-5 Signed-off-by: André Maia --- lib/Service/IdDocsService.php | 2 +- tests/php/Unit/Service/IdDocsServiceTest.php | 55 ++++++++++++++++++++ 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/lib/Service/IdDocsService.php b/lib/Service/IdDocsService.php index 66ff51087d..82156c492e 100644 --- a/lib/Service/IdDocsService.php +++ b/lib/Service/IdDocsService.php @@ -141,7 +141,7 @@ private function getOwnerOfSignedFile(SignRequest $signRequest): ?IUser { } try { $signedFile = $this->fileMapper->getById($signedFileId); - } catch (\Throwable) { + } catch (\OCP\AppFramework\Db\DoesNotExistException) { return null; } return $this->userManager->get($signedFile->getUserId()); diff --git a/tests/php/Unit/Service/IdDocsServiceTest.php b/tests/php/Unit/Service/IdDocsServiceTest.php index 2fea4b206b..95e0f5dcc8 100644 --- a/tests/php/Unit/Service/IdDocsServiceTest.php +++ b/tests/php/Unit/Service/IdDocsServiceTest.php @@ -251,4 +251,59 @@ public function testAddFilesToDocumentFolderWithoutResolvableOwnerKeepsCurrentBe $signRequest, ); } + + public function testAddFilesToDocumentFolderWithoutSignedFileRowKeepsCurrentBehaviour(): void { + $signRequest = new SignRequest(); + $signRequest->setId(55); + $signRequest->setFileId(10); + + $this->fileMapper->method('getById') + ->with(10) + ->willThrowException(new \OCP\AppFramework\Db\DoesNotExistException('missing')); + + $this->userManager->expects($this->never()) + ->method('get'); + + $this->fileTypeMapper->method('getTypes') + ->willReturn(['IDENTIFICATION' => ['type' => 'IDENTIFICATION']]); + + $savedFile = new \OCA\Libresign\Db\File(); + $savedFile->setId(77); + $this->requestSignatureService->expects($this->once()) + ->method('saveFile') + ->with($this->callback(function (array $data): bool { + $this->assertArrayNotHasKey('userManager', $data); + return true; + })) + ->willReturn($savedFile); + + $service = $this->getIdDocsService(); + $service->addFilesToDocumentFolder( + [['type' => 'IDENTIFICATION', 'base64' => 'ZmFrZQ==']], + $signRequest, + ); + } + + public function testAddFilesToDocumentFolderDoesNotHideUnexpectedFailuresWhenResolvingTheOwner(): void { + $signRequest = new SignRequest(); + $signRequest->setId(55); + $signRequest->setFileId(10); + + $this->fileMapper->method('getById') + ->with(10) + ->willThrowException(new \RuntimeException('database unavailable')); + + $this->fileTypeMapper->method('getTypes') + ->willReturn(['IDENTIFICATION' => ['type' => 'IDENTIFICATION']]); + + $this->requestSignatureService->expects($this->never()) + ->method('saveFile'); + + $service = $this->getIdDocsService(); + $this->expectException(\RuntimeException::class); + $service->addFilesToDocumentFolder( + [['type' => 'IDENTIFICATION', 'base64' => 'ZmFrZQ==']], + $signRequest, + ); + } }