Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions lib/Controller/FileController.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
use OCA\Libresign\Service\Policy\ValidationEffectivePolicyService;
use OCA\Libresign\Service\RequestSignatureService;
use OCA\Libresign\Service\SessionService;
use OCA\Libresign\Service\Validation\FileInputValidator;
use OCP\AppFramework\Db\DoesNotExistException;
use OCP\AppFramework\Http;
use OCP\AppFramework\Http\Attribute\ApiRoute;
Expand Down Expand Up @@ -84,6 +85,7 @@ public function __construct(
private ValidateHelper $validateHelper,
private SettingsLoader $settingsLoader,
private IURLGenerator $urlGenerator,
private FileInputValidator $fileInputValidator,
) {
parent::__construct(Application::APP_ID, $request);
}
Expand Down Expand Up @@ -742,12 +744,18 @@ private function prepareFilesForSaving(array $file, array $files, array $setting
}

if (!empty($files)) {
/** @var list<array{fileNode?: Node, name?: string}> $files */
return $files;
/** @var list<array{fileNode?: Node, name?: string}> $normalizedFiles */
$normalizedFiles = array_map(
fn (mixed $each): mixed => is_array($each) ? $this->fileInputValidator->normalizeNodeId($each) : $each,
$files,
);
return $normalizedFiles;
}

if (!empty($file)) {
return [$file];
/** @var array{fileNode?: Node, name?: string} $normalizedFile */
$normalizedFile = $this->fileInputValidator->normalizeNodeId($file);
return [$normalizedFile];
}

// TRANSLATORS Error shown when creating or updating a signature request without a file.
Expand Down
4 changes: 2 additions & 2 deletions lib/Controller/RequestSignatureController.php
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public function __construct(
* @param LibresignNewSigner[] $signers Collection of signers who must sign the document. Use identifyMethods as the canonical format. Other supported fields: displayName, description, notify, signingOrder, status, geolocationRequired
* @param string $name The name of file to sign
* @param LibresignFolderSettings $settings Settings to define how and where the file should be stored
* @param LibresignNewFile $file File object. Supports nodeId, url, base64 or path.
* @param LibresignNewFile $file File object. Supports nodeId (a non-negative integer or its canonical decimal string, as Nextcloud node ids can exceed a JavaScript number), url, base64 or path.
* @param list<LibresignNewFile> $files Multiple files to create an envelope (optional, use either file or files). Each file supports nodeId, url, base64 or path.
* @param string|null $callback URL that will receive a POST after the document is signed
* @param integer|null $status Numeric code of status * 0 - no signers * 1 - signed * 2 - pending
Expand Down Expand Up @@ -134,7 +134,7 @@ public function requestSignature(
* @param LibresignNewSigner[]|null $signers Collection of signers who must sign the document. Use identifyMethods as the canonical format.
* @param string|null $uuid UUID of sign request. The signer UUID is what the person receives via email when asked to sign. This is not the file UUID.
* @param LibresignVisibleElement[]|null $visibleElements Visible elements on document
* @param LibresignNewFile|null $file File object. Supports nodeId, url, base64 or path when creating a new request.
* @param LibresignNewFile|null $file File object. Supports nodeId (a non-negative integer or its canonical decimal string), url, base64 or path when creating a new request.
* @param integer|null $status Numeric code of status * 0 - no signers * 1 - signed * 2 - pending
* @param array<string, mixed>|null $policy Structured policy payload with request-level overrides and active context.
* @param string|null $name The name of file to sign
Expand Down
2 changes: 1 addition & 1 deletion lib/ResponseDefinitions.php
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@
* }
* @psalm-type LibresignNewFile = array{
* base64?: string,
* nodeId?: non-negative-int,
* nodeId?: non-negative-int|numeric-string,
* path?: string,
* url?: string,
* name?: string,
Expand Down
24 changes: 24 additions & 0 deletions lib/Service/IdDocsService.php
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,28 @@ private function validateIdDoc(int $fileIndex, array $file, IUser $user): void {
}
}

/**
* `file` of each entry is the HTTP `LibresignNewFile` payload; its node
* id is normalized once here, the same boundary the signature request has.
*/
private function normalizeNodeIds(array $files): array {
foreach ($files as $fileIndex => $fileData) {
if (!is_array($fileData) || !is_array($fileData['file'] ?? null)) {
continue;
}
try {
$files[$fileIndex]['file'] = $this->fileInputValidator->normalizeNodeId($fileData['file'], FileInputValidator::TYPE_ACCOUNT_DOCUMENT);
} catch (LibresignException $e) {
throw new LibresignException(json_encode([
'type' => 'danger',
'file' => $fileIndex,
'message' => $e->getMessage(),
]));
}
}
return $files;
}

public function validateIdDocs(array $files, IUser $user): void {
foreach ($files as $fileIndex => $file) {
$this->validateTypeOfFile($fileIndex, $file);
Expand All @@ -90,6 +112,7 @@ public function validateIdDocs(array $files, IUser $user): void {
}

public function addIdDocs(array $files, IUser $user): void {
$files = $this->normalizeNodeIds($files);
$this->validateIdDocs($files, $user);
foreach ($files as $fileData) {
$dataToSave = $fileData;
Expand Down Expand Up @@ -119,6 +142,7 @@ public function addFilesToDocumentFolder(
array $files,
SignRequest $signRequest,
): void {
$files = $this->normalizeNodeIds($files);
foreach ($files as $fileIndex => $file) {
$this->validateTypeOfFile($fileIndex, $file);
}
Expand Down
10 changes: 5 additions & 5 deletions lib/Service/RequestSignatureService.php
Original file line number Diff line number Diff line change
Expand Up @@ -361,15 +361,15 @@ public function saveFile(array $data): FileEntity {
}
return $this->fileStatusService->updateFileStatusIfUpgrade($file, $data['status'] ?? 0);
}
$fileId = null;
$nodeId = null;
if (isset($data['file']['fileNode']) && $data['file']['fileNode'] instanceof Node) {
$fileId = $data['file']['fileNode']->getId();
$nodeId = $data['file']['fileNode']->getId();
} elseif (!empty($data['file']['nodeId'])) {
$fileId = $data['file']['nodeId'];
$nodeId = $data['file']['nodeId'];
}
if (!is_null($fileId)) {
if (!is_null($nodeId)) {
try {
$file = $this->fileMapper->getByNodeId($fileId);
$file = $this->fileMapper->getByNodeId($nodeId);
$this->filePolicyApplier->syncAllPolicies($file, $data);
return $this->fileStatusService->updateFileStatusIfUpgrade($file, $data['status'] ?? 0);
} catch (\Throwable) {
Expand Down
15 changes: 15 additions & 0 deletions lib/Service/RequestSignatureWorkflowService.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ public function __construct(
private SignerValidator $signerValidator,
private VisibleElementValidator $visibleElementValidator,
private FileMapper $fileMapper,
private FileInputValidator $fileInputValidator,
) {
}

Expand Down Expand Up @@ -67,6 +68,8 @@ public function createRequest(
throw new LibresignException($this->l10n->t('File or files parameter is required'));
}

$file = $this->fileInputValidator->normalizeNodeId($file);
$files = $this->normalizeNodeIds($files);
$resolvedPolicy = $this->resolvePolicyPayload($policy);
$data = [
'file' => $file,
Expand Down Expand Up @@ -128,6 +131,7 @@ public function updateExistingRequest(
?string $name = null,
array $settings = [],
): array {
$file = $this->fileInputValidator->normalizeNodeId($file);
$resolvedPolicy = $this->resolvePolicyPayload($policy);
$data = [
'uuid' => $uuid,
Expand Down Expand Up @@ -159,6 +163,17 @@ public function updateExistingRequest(
];
}

/**
* @param list<array<string, mixed>> $files
* @return list<array<string, mixed>>
*/
private function normalizeNodeIds(array $files): array {
return array_map(
fn (mixed $file): mixed => is_array($file) ? $this->fileInputValidator->normalizeNodeId($file) : $file,
$files,
);
}

/** @return list<FileEntity> */
private function loadChildFilesIfEnvelope(FileEntity $fileEntity): array {
return $fileEntity->getParentFileId() === null || $fileEntity->isEnvelope()
Expand Down
32 changes: 32 additions & 0 deletions lib/Service/Validation/FileInputValidator.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,38 @@ public function __construct(
) {
}

/**
* Normalize the node id of a file payload received over HTTP.
*
* The API accepts `nodeId` as a non-negative int or as its canonical
* decimal string (digits only, no sign, no leading zeros, within the int
* range): the Files app exposes node ids as strings (`Node.id` of
* `@nextcloud/files`) and, since Nextcloud 33, they can exceed what a
* JavaScript number holds. Call it once at the boundary: past it,
* `nodeId` is either absent or the non-negative `int` the Nextcloud Files
* API works with, and anything else is rejected here instead of reaching
* a later cast.
*
* @param array<string, mixed> $file
* @return array<string, mixed>
* @throws LibresignException when `nodeId` is present and is neither a non-negative int nor its canonical decimal string
*/
public function normalizeNodeId(array $file, int $type = self::TYPE_TO_SIGN): array {
$nodeId = $file['nodeId'] ?? null;
if ($nodeId === null) {
return $file;
}
if (is_string($nodeId) && ctype_digit($nodeId)) {
// FILTER_VALIDATE_INT also rejects leading zeros and overflow.
$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->getTypeOfFile($type)]));
}

public function validateNewFile(array $data, int $type = self::TYPE_TO_SIGN, ?IUser $user = null): void {
$this->validateFile($data, $type, $user);
if (!empty($data['file']['nodeId'])) {
Expand Down
17 changes: 12 additions & 5 deletions openapi-full.json
Original file line number Diff line number Diff line change
Expand Up @@ -2119,9 +2119,16 @@
"type": "string"
},
"nodeId": {
"type": "integer",
"format": "int64",
"minimum": 0
"oneOf": [
{
"type": "integer",
"format": "int64",
"minimum": 0
},
{
"type": "string"
}
]
},
"path": {
"type": "string"
Expand Down Expand Up @@ -10146,7 +10153,7 @@
"file": {
"$ref": "#/components/schemas/NewFile",
"default": [],
"description": "File object. Supports nodeId, url, base64 or path."
"description": "File object. Supports nodeId (a non-negative integer or its canonical decimal string, as Nextcloud node ids can exceed a JavaScript number), url, base64 or path."
},
"files": {
"type": "array",
Expand Down Expand Up @@ -10322,7 +10329,7 @@
"file": {
"$ref": "#/components/schemas/NewFile",
"nullable": true,
"description": "File object. Supports nodeId, url, base64 or path when creating a new request."
"description": "File object. Supports nodeId (a non-negative integer or its canonical decimal string), url, base64 or path when creating a new request."
},
"status": {
"type": "integer",
Expand Down
17 changes: 12 additions & 5 deletions openapi.json
Original file line number Diff line number Diff line change
Expand Up @@ -1583,9 +1583,16 @@
"type": "string"
},
"nodeId": {
"type": "integer",
"format": "int64",
"minimum": 0
"oneOf": [
{
"type": "integer",
"format": "int64",
"minimum": 0
},
{
"type": "string"
}
]
},
"path": {
"type": "string"
Expand Down Expand Up @@ -9444,7 +9451,7 @@
"file": {
"$ref": "#/components/schemas/NewFile",
"default": [],
"description": "File object. Supports nodeId, url, base64 or path."
"description": "File object. Supports nodeId (a non-negative integer or its canonical decimal string, as Nextcloud node ids can exceed a JavaScript number), url, base64 or path."
},
"files": {
"type": "array",
Expand Down Expand Up @@ -9620,7 +9627,7 @@
"file": {
"$ref": "#/components/schemas/NewFile",
"nullable": true,
"description": "File object. Supports nodeId, url, base64 or path when creating a new request."
"description": "File object. Supports nodeId (a non-negative integer or its canonical decimal string), url, base64 or path when creating a new request."
},
"status": {
"type": "integer",
Expand Down
4 changes: 3 additions & 1 deletion src/components/RightSidebar/AppFilesTab.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
22 changes: 20 additions & 2 deletions src/store/files.js
Original file line number Diff line number Diff line change
Expand Up @@ -961,6 +961,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') {
Expand All @@ -972,7 +990,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) {
Expand All @@ -983,7 +1001,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) {
Expand Down
24 changes: 23 additions & 1 deletion src/tests/components/RightSidebar/AppFilesTab.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ type TitleObserver = {
}

type FileInfo = {
id: number
id: number | string
type?: string
name?: string
path?: string
Expand Down Expand Up @@ -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: {
Expand Down
Loading
Loading