Skip to content

Commit 737b5c7

Browse files
vitormattosvitormattos
authored
Merge pull request #7541 from LibreSign/backport/7538/stable32
[stable32] fix: restore FolderService userId in throwIfFileNotFound finally block
2 parents 9cb183e + 673d83d commit 737b5c7

11 files changed

Lines changed: 231 additions & 8 deletions

File tree

‎lib/Db/FileMapper.php‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -150,9 +150,8 @@ public function getByUuid(string $uuid): File {
150150
public function getStorageUserIdByUuid(string $uuid): ?string {
151151
$qb = $this->db->getQueryBuilder();
152152

153-
$qb->select('f.user_id', 'id.user_id AS id_docs_user_id', 'id.id AS id_docs_id')
153+
$qb->select('f.user_id')
154154
->from($this->getTableName(), 'f')
155-
->leftJoin('f', 'libresign_id_docs', 'id', $qb->expr()->eq('f.id', 'id.file_id'))
156155
->where($qb->expr()->eq('f.uuid', $qb->createNamedParameter($uuid)));
157156

158157
$result = $qb->executeQuery();
@@ -163,10 +162,6 @@ public function getStorageUserIdByUuid(string $uuid): ?string {
163162
throw new DoesNotExistException('File not found');
164163
}
165164

166-
if ($row['id_docs_id'] !== null && $row['id_docs_user_id'] !== null) {
167-
return $row['id_docs_user_id'];
168-
}
169-
170165
return $row['user_id'];
171166
}
172167

‎lib/ResponseDefinitions.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
*
3434
* @psalm-type LibresignFolderSettings = array{
3535
* folderName?: string,
36+
* path?: string,
3637
* separator?: string,
3738
* folderPatterns?: array{
3839
* name: string,

‎lib/Service/IdentifyMethod/AbstractIdentifyMethod.php‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
use DateTime;
1212
use InvalidArgumentException;
13+
use OC\AppFramework\Http as AppFrameworkHttp;
1314
use OCA\Libresign\AppInfo\Application;
1415
use OCA\Libresign\Db\IdentifyMethod;
1516
use OCA\Libresign\Enum\FileStatus;
@@ -189,19 +190,27 @@ protected function throwIfFileNotFound(): void {
189190
}
190191

191192
foreach ($filesToCheck as $fileInfo) {
193+
if (!is_int($fileInfo['nodeId'])) {
194+
throw new LibresignException(json_encode([
195+
'action' => JSActions::ACTION_DO_NOTHING,
196+
'errors' => [['message' => $this->identifyService->getL10n()->t('File not found')]],
197+
]), AppFrameworkHttp::STATUS_NOT_FOUND);
198+
}
199+
192200
$storageUserId = $this->identifyService->getFileMapper()
193201
->getStorageUserIdByUuid($fileInfo['uuid']);
194202
$folderService = $this->identifyService->getFolderService();
195203
$previousUserId = $folderService->getUserId();
196204
$folderService->setUserId($storageUserId);
197205
try {
198206
$folderService->getFileByNodeId($fileInfo['nodeId']);
199-
$folderService->setUserId($previousUserId);
200207
} catch (NotFoundException) {
201208
throw new LibresignException(json_encode([
202209
'action' => JSActions::ACTION_DO_NOTHING,
203210
'errors' => [['message' => $this->identifyService->getL10n()->t('File not found')]],
204-
]));
211+
]), AppFrameworkHttp::STATUS_NOT_FOUND);
212+
} finally {
213+
$folderService->setUserId($previousUserId);
205214
}
206215
}
207216
}

‎openapi-full.json‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1399,6 +1399,9 @@
13991399
"folderName": {
14001400
"type": "string"
14011401
},
1402+
"path": {
1403+
"type": "string"
1404+
},
14021405
"separator": {
14031406
"type": "string"
14041407
},

‎openapi.json‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1000,6 +1000,9 @@
10001000
"folderName": {
10011001
"type": "string"
10021002
},
1003+
"path": {
1004+
"type": "string"
1005+
},
10031006
"separator": {
10041007
"type": "string"
10051008
},

‎src/types/openapi/openapi-full.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1906,6 +1906,7 @@ export type components = {
19061906
};
19071907
FolderSettings: {
19081908
folderName?: string;
1909+
path?: string;
19091910
separator?: string;
19101911
folderPatterns?: {
19111912
name: string;

‎src/types/openapi/openapi.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1320,6 +1320,7 @@ export type components = {
13201320
};
13211321
FolderSettings: {
13221322
folderName?: string;
1323+
path?: string;
13231324
separator?: string;
13241325
folderPatterns?: {
13251326
name: string;

‎tests/integration/features/bootstrap/FeatureContext.php‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,14 @@ public function userGetsWebDavPropertiesFor(string $user, string $path): void {
187187
$this->davRequest($user, 'PROPFIND', $path, $body, ['Depth' => '0']);
188188
}
189189

190+
#[Given('user :user sends WebDAV :method to :path')]
191+
public function userSendsWebDavRequest(string $user, string $method, string $path): void {
192+
$this->setCurrentUser($user);
193+
$normalizedMethod = strtoupper(trim($method));
194+
Assert::assertContains($normalizedMethod, ['DELETE', 'PUT', 'PROPFIND', 'GET', 'HEAD', 'MOVE', 'COPY', 'MKCOL'], 'Unsupported WebDAV method');
195+
$this->davRequest($user, $normalizedMethod, $path);
196+
}
197+
190198
#[Given('the WebDAV response should contain property :property with value :value')]
191199
public function theWebDavResponseShouldContainPropertyWithValue(string $property, string $value): void {
192200
$result = $this->parseXml()->xpath("//nc:$property");

‎tests/integration/features/page/validate.feature‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,3 +44,54 @@ Feature: page/validate
4444
| /apps/libresign/p/validation |
4545
| /apps/libresign/pdf/fakeuuid-6037-47be-9d9e-3d90b9d0a3ea |
4646
| /apps/libresign/p/pdf/fakeuuid-6037-47be-9d9e-3d90b9d0a3ea |
47+
48+
Scenario: Authenticated signer can fetch PDF using sign request UUID
49+
Given user "validate-signer" exists
50+
And as user "admin"
51+
And sending "post" to ocs "/apps/libresign/api/v1/request-signature"
52+
| file | {"base64":"data:application/pdf;base64,JVBERi0xLjYKJcOkw7zDtsOfCjIgMCBvYmoKPDwvTGVuZ3RoIDMgMCBSL0ZpbHRlci9GbGF0ZURlY29kZT4+CnN0cmVhbQp4nDPQM1Qo5ypUMFAw0DMwslAwtTTVMzIxV7AwMdSzMDNUKErlCtdSyOMyVADBonQuA4iUhaVCLheKYqBIDlw7xLAcuLEgFlwVVwZXmhZXoAIAI+sZGAplbmRzdHJlYW0KZW5kb2JqCgozIDAgb2JqCjg2CmVuZG9iagoKNSAwIG9iago8PAo+PgplbmRvYmoKCjYgMCBvYmoKPDwvRm9udCA1IDAgUgovUHJvY1NldFsvUERGL1RleHRdCj4+CmVuZG9iagoKMSAwIG9iago8PC9UeXBlL1BhZ2UvUGFyZW50IDQgMCBSL1Jlc291cmNlcyA2IDAgUi9NZWRpYUJveFswIDAgNTk1LjI3NTU5MDU1MTE4MSA4NDEuODg5NzYzNzc5NTI4XS9Hcm91cDw8L1MvVHJhbnNwYXJlbmN5L0NTL0RldmljZVJHQi9JIHRydWU+Pi9Db250ZW50cyAyIDAgUj4+CmVuZG9iagoKNCAwIG9iago8PC9UeXBlL1BhZ2VzCi9SZXNvdXJjZXMgNiAwIFIKL01lZGlhQm94WyAwIDAgNTk1IDg0MSBdCi9LaWRzWyAxIDAgUiBdCi9Db3VudCAxPj4KZW5kb2JqCgo3IDAgb2JqCjw8L1R5cGUvQ2F0YWxvZy9QYWdlcyA0IDAgUgovT3BlbkFjdGlvblsxIDAgUiAvWFlaIG51bGwgbnVsbCAwXQo+PgplbmRvYmoKCjggMCBvYmoKPDwvQ3JlYXRvcjxGRUZGMDA0NDAwNzIwMDYxMDA3Nz4KL1Byb2R1Y2VyPEZFRkYwMDRDMDA2OTAwNjIwMDcyMDA2NTAwNEYwMDY2MDA2NjAwNjkwMDYzMDA2NTAwMjAwMDM3MDAyRTAwMzA+Ci9DcmVhdGlvbkRhdGUoRDoyMDIxMDIyMzExMDgwOS0wMycwMCcpPj4KZW5kb2JqCgp4cmVmCjAgOQowMDAwMDAwMDAwIDY1NTM1IGYgCjAwMDAwMDAyNzAgMDAwMDAgbiAKMDAwMDAwMDAxOSAwMDAwMCBuIAowMDAwMDAwMTc2IDAwMDAwIG4gCjAwMDAwMDA0MzggMDAwMDAgbiAKMDAwMDAwMDE5NSAwMDAwMCBuIAowMDAwMDAwMjE3IDAwMDAwIG4gCjAwMDAwMDA1MzYgMDAwMDAgbiAKMDAwMDAwMDYxOSAwMDAwMCBuIAp0cmFpbGVyCjw8L1NpemUgOS9Sb290IDcgMCBSCi9JbmZvIDggMCBSCi9JRCBbIDw1RkQ4MDlEMTdFODMwQUU5OTRDODkxNDVBMTMwNUQyQz4KPDVGRDgwOUQxN0U4MzBBRTk5NEM4OTE0NUExMzA1RDJDPiBdCi9Eb2NDaGVja3N1bSAvRDZBQThGQTBBQjMwODg2QkQ5ODU0QzYyMTg5QjI2NDQKPj4Kc3RhcnR4cmVmCjc4NQolJUVPRgo="} |
53+
| signers | [{"identifyMethods":[{"method":"account","value":"validate-signer"}]}] |
54+
| name | signer-pdf |
55+
And the response should have a status code 200
56+
And as user "validate-signer"
57+
And sending "get" to ocs "/apps/libresign/api/v1/file/list?details=1"
58+
And fetch field "(SIGN_REQUEST_UUID)ocs.data.data.0.signers.0.sign_request_uuid" from previous JSON response
59+
When sending "get" to "/apps/libresign/pdf/<SIGN_REQUEST_UUID>"
60+
Then the response should have a status code 200
61+
62+
Scenario: Missing sign request UUID returns controlled File not found response
63+
Given user "validate-signer-2" exists
64+
And as user "validate-signer-2"
65+
And sending "get" to "/apps/libresign/pdf/fakeuuid-6037-47be-9d9e-3d90b9d0a3ea"
66+
Then the response should have a status code 404
67+
And the response should be a JSON array with the following mandatory values
68+
| key | value |
69+
| action | 2000 |
70+
| errors | [{"message":"Invalid UUID"}] |
71+
72+
Scenario: Unauthenticated email signer can fetch PDF and gets controlled error after source deletion
73+
Given as user "admin"
74+
And sending "post" to ocs "/apps/provisioning_api/api/v1/config/apps/libresign/identify_methods"
75+
| value | (string)[{"name":"email","enabled":true,"mandatory":true,"signatureMethods":{"clickToSign":{"enabled":true}},"can_create_account":false}] |
76+
And my inbox is empty
77+
When sending "post" to ocs "/apps/libresign/api/v1/request-signature"
78+
| file | {"url":"<BASE_URL>/apps/libresign/develop/pdf"} |
79+
| signers | [{"displayName":"External Signer","identifyMethods":[{"method":"email","value":"external@domain.test"}]}] |
80+
| name | external-email-pdf |
81+
| settings | {"folderName":"rm-target-folder"} |
82+
Then the response should have a status code 200
83+
And I open the latest email to "external@domain.test" with subject "LibreSign: There is a file for you to sign"
84+
And I fetch the signer UUID from opened email
85+
And as user ""
86+
When sending "get" to "/apps/libresign/pdf/<SIGN_REQUEST_UUID>"
87+
Then the response should have a status code 200
88+
And as user "admin"
89+
And user "admin" sends WebDAV "DELETE" to "LibreSign/rm-target-folder/external-email-pdf.pdf"
90+
And the response should have a status code 204
91+
And as user ""
92+
When sending "get" to "/apps/libresign/pdf/<SIGN_REQUEST_UUID>"
93+
Then the response should have a status code 404
94+
And the response should be a JSON array with the following mandatory values
95+
| key | value |
96+
| action | 2000 |
97+
| errors | [{"message":"File not found"}] |

‎tests/php/Unit/Db/FileMapperTest.php‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
/**
5+
* SPDX-FileCopyrightText: 2026 LibreCode coop and contributors
6+
* SPDX-License-Identifier: AGPL-3.0-or-later
7+
*/
8+
9+
namespace OCA\Libresign\Tests\Unit\Db;
10+
11+
use OCA\Libresign\Db\File;
12+
use OCA\Libresign\Db\FileMapper;
13+
use OCA\Libresign\Db\IdDocsMapper;
14+
use OCA\Libresign\Enum\FileStatus;
15+
use OCP\Server;
16+
17+
/**
18+
* @group DB
19+
*/
20+
final class FileMapperTest extends \OCA\Libresign\Tests\Unit\TestCase {
21+
private FileMapper $fileMapper;
22+
private IdDocsMapper $idDocsMapper;
23+
24+
public function setUp(): void {
25+
parent::setUp();
26+
$this->fileMapper = Server::get(FileMapper::class);
27+
$this->idDocsMapper = Server::get(IdDocsMapper::class);
28+
}
29+
30+
public function testGetStorageUserIdByUuidReturnsFileOwnerWhenNoIdDocsExists(): void {
31+
$file = $this->createFileEntity(
32+
uuid: 'a1111111-1111-4111-8111-111111111111',
33+
nodeId: 10101,
34+
userId: 'owner-user'
35+
);
36+
37+
$inserted = $this->fileMapper->insert($file);
38+
39+
$storageUserId = $this->fileMapper->getStorageUserIdByUuid($inserted->getUuid());
40+
$this->assertSame('owner-user', $storageUserId);
41+
}
42+
43+
public function testGetStorageUserIdByUuidPrefersFileOwnerWhenIdDocsExists(): void {
44+
$file = $this->createFileEntity(
45+
uuid: 'b2222222-2222-4222-8222-222222222222',
46+
nodeId: 20202,
47+
userId: 'owner-user'
48+
);
49+
50+
$inserted = $this->fileMapper->insert($file);
51+
$this->idDocsMapper->save(
52+
$inserted->getId(),
53+
null,
54+
'external-signer',
55+
'proofOfIdentity'
56+
);
57+
58+
$storageUserId = $this->fileMapper->getStorageUserIdByUuid($inserted->getUuid());
59+
$this->assertSame('owner-user', $storageUserId);
60+
}
61+
62+
private function createFileEntity(string $uuid, int $nodeId, ?string $userId): File {
63+
$file = new File();
64+
$file->setNodeId($nodeId);
65+
$file->setUserId($userId);
66+
$file->setUuid($uuid);
67+
$file->setCreatedAt(new \DateTime('now', new \DateTimeZone('UTC')));
68+
$file->setName('storage-user-resolution.pdf');
69+
$file->setStatus(FileStatus::DRAFT->value);
70+
return $file;
71+
}
72+
}

0 commit comments

Comments
 (0)