[stable35] fix: store identification documents of external signers under the file owner

lib/Service/IdDocsService.php

@@ -24,6 +24,7 @@
 use OCP\IAppConfig;
 use OCP\IL10N;
 use OCP\IUser;
+use OCP\IUserManager;
 use Sabre\DAV\UUIDUtil;
 
 class IdDocsService {
@@ -39,6 +40,7 @@ public function __construct(
 		private IdentifyMethodMapper $identifyMethodMapper,
 		private ITimeFactory $timeFactory,
 		private IAppConfig $appConfig,
+		private IUserManager $userManager,
 	) {
 	}
 
@@ -118,16 +120,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 (\OCP\AppFramework\Db\DoesNotExistException) {
+			return null;
+		}
+		return $this->userManager->get($signedFile->getUserId());
+	}
+
 	public function list(array $filter, ?int $page = null, ?int $length = null, array $sort = []): array {
 		$page ??= 1;
 		$length ??= (int)$this->appConfig->getValueInt(Application::APP_ID, 'length_of_page', 100);

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,139 @@ 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,
+		);
+	}
+
+	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,
+		);
+	}
 }