From 5a54f5242367c6ba3e5b1ea3d8a159b04b58451d Mon Sep 17 00:00:00 2001 From: Alexander Rebello Date: Fri, 15 May 2026 18:41:34 +0200 Subject: [PATCH 1/5] Add Share API, sharing sidebar UI, docs, and tests Signed-off-by: Alexander Rebello --- docs/API_v3.md | 7 +- lib/Controller/ShareApiController.php | 84 +++- openapi.json | 6 +- src/FormsSettings.vue | 17 +- .../SidebarTabs/SharingSidebarTab.vue | 155 ++++++- .../Controller/ShareApiControllerTest.php | 379 ++++++++++++++++++ 6 files changed, 628 insertions(+), 20 deletions(-) diff --git a/docs/API_v3.md b/docs/API_v3.md index 72656055d..34230fd40 100644 --- a/docs/API_v3.md +++ b/docs/API_v3.md @@ -647,7 +647,12 @@ Update a single or all properties of an option-object | Parameter | Type | Description | |------------------|----------|-------------| | _keyValuePairs_ | Array | Array of key-value pairs to update | -- Restrictions: Currently only the _permissions_ can be updated. +- Restrictions: + - Allowed keys are _permissions_ and _token_. + - _token_ updates are only available when the admin setting _allowCustomPublicShareTokens_ is enabled. + - _token_ can only be updated on link shares. + - _token_ must be unique among link shares and only contain alphanumeric characters. + - _token_ length must be between 8 and 256 characters. - Response: **Status-Code OK**, as well as the id of the share object. ``` diff --git a/lib/Controller/ShareApiController.php b/lib/Controller/ShareApiController.php index 544d55fd7..a4981de95 100644 --- a/lib/Controller/ShareApiController.php +++ b/lib/Controller/ShareApiController.php @@ -204,7 +204,7 @@ public function newShare(int $formId, int $shareType, string $shareWith = '', ar } /** - * Update permissions of a share + * Update properties of a share * * @param int $formId of the form * @param int $shareId of the share to update @@ -212,9 +212,13 @@ public function newShare(int $formId, int $shareType, string $shareWith = '', ar * @return DataResponse * @throws OCSBadRequestException Share doesn't belong to given Form * @throws OCSBadRequestException Invalid permission given + * @throws OCSBadRequestException Invalid share token + * @throws OCSBadRequestException Share hash exists, please retry + * @throws OCSForbiddenException Custom public share tokens are not allowed * @throws OCSForbiddenException This form is not owned by the current user * @throws OCSForbiddenException Empty keyValuePairs, will not update - * @throws OCSForbiddenException Not allowed to update other properties than permissions + * @throws OCSForbiddenException Not allowed to update token on non-link share + * @throws OCSForbiddenException Not allowed to update unknown properties * @throws OCSNotFoundException Could not find share * * 200: the id of the updated share @@ -223,7 +227,7 @@ public function newShare(int $formId, int $shareType, string $shareWith = '', ar #[NoAdminRequired()] #[ApiRoute(verb: 'PATCH', url: '/api/v3/forms/{formId}/shares/{shareId}')] public function updateShare(int $formId, int $shareId, array $keyValuePairs): DataResponse { - $this->logger->debug('Updating share: {shareId} of form {formId}, permissions: {permissions}', [ + $this->logger->debug('Updating share: {shareId} of form {formId}, values: {keyValuePairs}', [ 'formId' => $formId, 'shareId' => $shareId, 'keyValuePairs' => $keyValuePairs @@ -253,22 +257,64 @@ public function updateShare(int $formId, int $shareId, array $keyValuePairs): Da throw new OCSForbiddenException('Empty keyValuePairs, will not update'); } - //Don't allow to change other properties than permissions - if (count($keyValuePairs) > 1 || !array_key_exists('permissions', $keyValuePairs)) { - $this->logger->debug('Not allowed to update other properties than permissions'); - throw new OCSForbiddenException('Not allowed to update other properties than permissions'); + $allowedKeys = ['permissions', 'token']; + foreach (array_keys($keyValuePairs) as $key) { + if (!in_array($key, $allowedKeys, true)) { + $this->logger->debug('Not allowed to update other properties than permissions or token'); + throw new OCSForbiddenException('Not allowed to update other properties than permissions or token'); + } } - if (!$this->validatePermissions($keyValuePairs['permissions'], $formShare->getShareType())) { + if (array_key_exists('permissions', $keyValuePairs) && !$this->validatePermissions($keyValuePairs['permissions'], $formShare->getShareType())) { throw new OCSBadRequestException('Invalid permission given'); } + if (array_key_exists('token', $keyValuePairs)) { + if (!$this->configService->getAllowCustomPublicToken()) { + $this->logger->debug('Custom public share tokens are not allowed.'); + throw new OCSForbiddenException('Custom public share tokens are not allowed.'); + } + + if ($formShare->getShareType() !== IShare::TYPE_LINK) { + $this->logger->debug('Not allowed to update token on non-link share'); + throw new OCSForbiddenException('Not allowed to update token on non-link share'); + } + + if (!is_string($keyValuePairs['token'])) { + throw new OCSBadRequestException('Invalid share token'); + } + + $token = $keyValuePairs['token']; + if (!array_key_exists('permissions', $keyValuePairs) && $token === $formShare->getShareWith()) { + return new DataResponse($formShare->getId()); + } + + if ($token !== $formShare->getShareWith()) { + $this->validatePublicShareToken($token); + + try { + $existingShare = $this->shareMapper->findPublicShareByHash($token); + if ($existingShare->getId() !== $formShare->getId()) { + $this->logger->debug('Share hash already exists.'); + throw new OCSBadRequestException('Share hash exists, please retry.'); + } + } catch (DoesNotExistException $e) { + // Just continue, this is what we expect to happen (share hash not existing yet). + } + + $formShare->setShareWith($token); + } + } + $this->formsService->obtainFormLock($form); - $formShare->setPermissions($keyValuePairs['permissions']); + if (array_key_exists('permissions', $keyValuePairs)) { + $formShare->setPermissions($keyValuePairs['permissions']); + } $formShare = $this->shareMapper->update($formShare); - if (in_array($formShare->getShareType(), [IShare::TYPE_USER, IShare::TYPE_GROUP, IShare::TYPE_USERGROUP, IShare::TYPE_CIRCLE], true)) { + if (array_key_exists('permissions', $keyValuePairs) + && in_array($formShare->getShareType(), [IShare::TYPE_USER, IShare::TYPE_GROUP, IShare::TYPE_USERGROUP, IShare::TYPE_CIRCLE], true)) { if (in_array(Constants::PERMISSION_RESULTS, $keyValuePairs['permissions'], true)) { $userFolder = $this->rootFolder->getUserFolder($form->getOwnerId()); $uploadedFilesFolderPath = $this->filePathHelper->getFormUploadedFilesFolderPath($form); @@ -418,4 +464,22 @@ private function validatePermissions(array $permissions, int $shareType): bool { } return true; } + + /** + * @throws OCSBadRequestException If token does not satisfy basic safety checks + */ + private function validatePublicShareToken(string $token): void { + if ($token !== trim($token)) { + throw new OCSBadRequestException('Invalid share token'); + } + + $tokenLength = strlen($token); + if ($tokenLength < Constants::PUBLIC_SHARE_TOKEN_MIN_LENGTH || $tokenLength > Constants::PUBLIC_SHARE_TOKEN_MAX_LENGTH) { + throw new OCSBadRequestException('Invalid share token'); + } + + if (preg_match('/^[a-zA-Z0-9]+$/', $token) !== 1) { + throw new OCSBadRequestException('Invalid share token'); + } + } } diff --git a/openapi.json b/openapi.json index 3ddf90c46..cbbda9ada 100644 --- a/openapi.json +++ b/openapi.json @@ -5196,7 +5196,7 @@ "/ocs/v2.php/apps/forms/api/v3/forms/{formId}/shares/{shareId}": { "patch": { "operationId": "share_api-update-share", - "summary": "Update permissions of a share", + "summary": "Update properties of a share", "description": "This endpoint allows CORS requests", "tags": [ "share_api" @@ -5293,7 +5293,7 @@ } }, "400": { - "description": "Invalid permission given", + "description": "Share hash exists, please retry", "content": { "application/json": { "schema": { @@ -5321,7 +5321,7 @@ } }, "403": { - "description": "Not allowed to update other properties than permissions", + "description": "Not allowed to update unknown properties", "content": { "application/json": { "schema": { diff --git a/src/FormsSettings.vue b/src/FormsSettings.vue index 479e76b1e..4c8080c3d 100644 --- a/src/FormsSettings.vue +++ b/src/FormsSettings.vue @@ -73,6 +73,14 @@ {{ t('forms', 'Allow sharing by link') }} + {{ t('forms', 'Allow custom public share tokens') }} + + - + {{ t('forms', 'Show QR code') }} + + + {{ t('forms', 'Save token') }} + @@ -223,6 +249,17 @@ import NcActions from '@nextcloud/vue/components/NcActions' import NcCheckboxRadioSwitch from '@nextcloud/vue/components/NcCheckboxRadioSwitch' import NcIconSvgWrapper from '@nextcloud/vue/components/NcIconSvgWrapper' import NcNoteCard from '@nextcloud/vue/components/NcNoteCard' +import NcTextField from '@nextcloud/vue/components/NcTextField' +import IconAccountMultiple from 'vue-material-design-icons/AccountMultipleOutline.vue' +import IconCheck from 'vue-material-design-icons/Check.vue' +import IconCodeBrackets from 'vue-material-design-icons/CodeBrackets.vue' +import IconLinkVariant from 'vue-material-design-icons/Link.vue' +import IconLinkBoxVariantOutline from 'vue-material-design-icons/LinkBoxOutline.vue' +import IconPlus from 'vue-material-design-icons/Plus.vue' +import IconQr from 'vue-material-design-icons/Qrcode.vue' +import IconDelete from 'vue-material-design-icons/TrashCanOutline.vue' +import FormsIcon from '../Icons/FormsIcon.vue' +import IconCopyAll from '../Icons/IconCopyAll.vue' import QRDialog from '../QRDialog.vue' import SharingSearchDiv from './SharingSearchDiv.vue' import SharingShareDiv from './SharingShareDiv.vue' @@ -236,11 +273,22 @@ import OcsResponse2Data from '../../utils/OcsResponse2Data.js' export default { components: { NcIconSvgWrapper, + FormsIcon, + IconAccountMultiple, + IconCheck, + IconCodeBrackets, + IconCopyAll, + IconDelete, + IconLinkBoxVariantOutline, + IconLinkVariant, + IconPlus, + IconQr, NcActions, NcActionButton, NcActionLink, NcCheckboxRadioSwitch, NcNoteCard, + NcTextField, QRDialog, SharingSearchDiv, SharingShareDiv, @@ -285,10 +333,27 @@ export default { return { isLoading: false, appConfig: loadState(appName, 'appConfig'), + shareTokens: {}, + savingShareTokens: {}, qrDialogText: '', } }, + watch: { + publicLinkShares: { + immediate: true, + handler(shares) { + const nextShareTokens = {} + for (const share of shares) { + nextShareTokens[share.id] = + this.shareTokens[share.id] ?? share.shareWith + } + + this.shareTokens = nextShareTokens + }, + }, + }, + computed: { isCurrentUserOwner() { return getCurrentUser().uid === this.form.ownerId @@ -489,6 +554,75 @@ export default { this.$emit('update:formProp', 'access', newAccess) }, + getShareTokenInput(share) { + return this.shareTokens[share.id] ?? share.shareWith + }, + + setShareTokenInput(share, value) { + this.shareTokens = { + ...this.shareTokens, + [share.id]: value, + } + }, + + isShareTokenSaving(share) { + return !!this.savingShareTokens[share.id] + }, + + isShareTokenDirty(share) { + return this.getShareTokenInput(share).trim() !== share.shareWith + }, + + async updateShareToken(share) { + const token = this.getShareTokenInput(share).trim() + if (token === share.shareWith) { + return + } + + this.isLoading = true + this.savingShareTokens = { + ...this.savingShareTokens, + [share.id]: true, + } + + try { + const response = await axios.patch( + generateOcsUrl('apps/forms/api/v3/forms/{id}/shares/{shareId}', { + id: this.form.id, + shareId: share.id, + }), + { + keyValuePairs: { + token, + }, + }, + ) + + this.$emit('updateShare', { + ...share, + id: OcsResponse2Data(response), + shareWith: token, + }) + + this.setShareTokenInput(share, token) + } catch (error) { + logger.error('Error while updating share token', { + error, + share, + token, + }) + showError( + t('forms', 'There was an error while updating the link token'), + ) + } finally { + this.savingShareTokens = { + ...this.savingShareTokens, + [share.id]: false, + } + this.isLoading = false + } + }, + openQrDialog(share) { this.qrDialogText = this.getPublicShareLink(share) }, @@ -543,6 +677,13 @@ export default { padding: 0px 8px; flex-grow: 1; + &--tokenized { + display: flex; + flex-direction: column; + justify-content: center; + padding-block: 8px; + } + &--twoline { span { display: block; @@ -554,5 +695,9 @@ export default { } } } + + :deep(.share-div__desc--tokenized .input-field__main-wrapper) { + min-inline-size: 220px; + } } diff --git a/tests/Unit/Controller/ShareApiControllerTest.php b/tests/Unit/Controller/ShareApiControllerTest.php index 500dc3de2..3e82078b6 100644 --- a/tests/Unit/Controller/ShareApiControllerTest.php +++ b/tests/Unit/Controller/ShareApiControllerTest.php @@ -913,4 +913,383 @@ public function testUpdateShare_NotExistingForm() { $this->expectException(NoSuchFormException::class); $this->shareApiController->updateShare(7331, 1337, [Constants::PERMISSION_SUBMIT]); } + + public function testUpdateShareToken() { + $form = new Form(); + $form->setId('5'); + $form->setOwnerId('currentUser'); + + $this->configService->expects($this->once()) + ->method('getAllowCustomPublicToken') + ->willReturn(true); + + $this->formsService->expects($this->once()) + ->method('getFormIfAllowed') + ->with(5) + ->willReturn($form); + + $share = new Share(); + $share->setId(8); + $share->setFormId(5); + $share->setShareType(IShare::TYPE_LINK); + $share->setShareWith('abcdefgh'); + $share->setPermissions([Constants::PERMISSION_SUBMIT]); + + $this->shareMapper->expects($this->once()) + ->method('findById') + ->with(8) + ->willReturn($share); + + $this->shareMapper->expects($this->once()) + ->method('findPublicShareByHash') + ->with('tokenabcd') + ->willThrowException(new DoesNotExistException('Not found')); + + $this->shareMapper->expects($this->once()) + ->method('update') + ->willReturnCallback(function (Share $updatedShare) { + $this->assertSame('tokenabcd', $updatedShare->getShareWith()); + return $updatedShare; + }); + + $this->assertEquals(new DataResponse(8), $this->shareApiController->updateShare(5, 8, ['token' => 'tokenabcd'])); + } + + public function testUpdateShareToken_CustomTokensDisabled() { + $form = new Form(); + $form->setId('5'); + $form->setOwnerId('currentUser'); + + $this->formsService->expects($this->once()) + ->method('getFormIfAllowed') + ->with(5) + ->willReturn($form); + + $share = new Share(); + $share->setId(8); + $share->setFormId(5); + $share->setShareType(IShare::TYPE_LINK); + $share->setShareWith('abcdefgh'); + + $this->shareMapper->expects($this->once()) + ->method('findById') + ->with(8) + ->willReturn($share); + + $this->configService->expects($this->once()) + ->method('getAllowCustomPublicToken') + ->willReturn(false); + + $this->expectException(OCSForbiddenException::class); + $this->shareApiController->updateShare(5, 8, ['token' => 'tokenabcd']); + } + + public function testUpdateShareToken_NonStringToken() { + $form = new Form(); + $form->setId('5'); + $form->setOwnerId('currentUser'); + + $this->configService->expects($this->once()) + ->method('getAllowCustomPublicToken') + ->willReturn(true); + + $this->formsService->expects($this->once()) + ->method('getFormIfAllowed') + ->with(5) + ->willReturn($form); + + $share = new Share(); + $share->setId(8); + $share->setFormId(5); + $share->setShareType(IShare::TYPE_LINK); + $share->setShareWith('abcdefgh'); + + $this->shareMapper->expects($this->once()) + ->method('findById') + ->with(8) + ->willReturn($share); + + $this->shareMapper->expects($this->never()) + ->method('findPublicShareByHash'); + + $this->expectException(OCSBadRequestException::class); + $this->shareApiController->updateShare(5, 8, ['token' => 123]); + } + + public function testUpdateShareToken_ForbiddenForNonLinkShare() { + $form = new Form(); + $form->setId('5'); + $form->setOwnerId('currentUser'); + + $this->configService->expects($this->once()) + ->method('getAllowCustomPublicToken') + ->willReturn(true); + + $this->formsService->expects($this->once()) + ->method('getFormIfAllowed') + ->with(5) + ->willReturn($form); + + $share = new Share(); + $share->setId(8); + $share->setFormId(5); + $share->setShareType(IShare::TYPE_USER); + $share->setShareWith('user1'); + + $this->shareMapper->expects($this->once()) + ->method('findById') + ->with(8) + ->willReturn($share); + + $this->expectException(OCSForbiddenException::class); + $this->shareApiController->updateShare(5, 8, ['token' => 'tokenabcd']); + } + + public function testUpdateShareToken_DuplicateHash() { + $form = new Form(); + $form->setId('5'); + $form->setOwnerId('currentUser'); + + $this->configService->expects($this->once()) + ->method('getAllowCustomPublicToken') + ->willReturn(true); + + $this->formsService->expects($this->once()) + ->method('getFormIfAllowed') + ->with(5) + ->willReturn($form); + + $currentShare = new Share(); + $currentShare->setId(8); + $currentShare->setFormId(5); + $currentShare->setShareType(IShare::TYPE_LINK); + $currentShare->setShareWith('abcdefgh'); + + $existingShare = new Share(); + $existingShare->setId(9); + $existingShare->setFormId(5); + $existingShare->setShareType(IShare::TYPE_LINK); + $existingShare->setShareWith('tokenabcd'); + + $this->shareMapper->expects($this->once()) + ->method('findById') + ->with(8) + ->willReturn($currentShare); + + $this->shareMapper->expects($this->once()) + ->method('findPublicShareByHash') + ->with('tokenabcd') + ->willReturn($existingShare); + + $this->expectException(OCSBadRequestException::class); + $this->shareApiController->updateShare(5, 8, ['token' => 'tokenabcd']); + } + + public function testUpdateShareToken_InvalidToken() { + $form = new Form(); + $form->setId('5'); + $form->setOwnerId('currentUser'); + + $this->configService->expects($this->once()) + ->method('getAllowCustomPublicToken') + ->willReturn(true); + + $this->formsService->expects($this->once()) + ->method('getFormIfAllowed') + ->with(5) + ->willReturn($form); + + $share = new Share(); + $share->setId(8); + $share->setFormId(5); + $share->setShareType(IShare::TYPE_LINK); + $share->setShareWith('abcdefgh'); + + $this->shareMapper->expects($this->once()) + ->method('findById') + ->with(8) + ->willReturn($share); + + $this->shareMapper->expects($this->never()) + ->method('update'); + + $this->expectException(OCSBadRequestException::class); + $this->shareApiController->updateShare(5, 8, ['token' => 'invalid-token']); + } + + public function testUpdateShareToken_WhitespaceToken() { + $form = new Form(); + $form->setId('5'); + $form->setOwnerId('currentUser'); + + $this->configService->expects($this->once()) + ->method('getAllowCustomPublicToken') + ->willReturn(true); + + $this->formsService->expects($this->once()) + ->method('getFormIfAllowed') + ->with(5) + ->willReturn($form); + + $share = new Share(); + $share->setId(8); + $share->setFormId(5); + $share->setShareType(IShare::TYPE_LINK); + $share->setShareWith('abcdefgh'); + + $this->shareMapper->expects($this->once()) + ->method('findById') + ->with(8) + ->willReturn($share); + + $this->shareMapper->expects($this->never()) + ->method('findPublicShareByHash'); + + $this->expectException(OCSBadRequestException::class); + $this->shareApiController->updateShare(5, 8, ['token' => ' customtoken ']); + } + + public function testUpdateShareToken_TooShortToken() { + $form = new Form(); + $form->setId('5'); + $form->setOwnerId('currentUser'); + + $this->configService->expects($this->once()) + ->method('getAllowCustomPublicToken') + ->willReturn(true); + + $this->formsService->expects($this->once()) + ->method('getFormIfAllowed') + ->with(5) + ->willReturn($form); + + $share = new Share(); + $share->setId(8); + $share->setFormId(5); + $share->setShareType(IShare::TYPE_LINK); + $share->setShareWith('abcdefgh'); + + $this->shareMapper->expects($this->once()) + ->method('findById') + ->with(8) + ->willReturn($share); + + $this->shareMapper->expects($this->never()) + ->method('findPublicShareByHash'); + + $this->expectException(OCSBadRequestException::class); + $this->shareApiController->updateShare(5, 8, ['token' => 'abc']); + } + + public function testUpdateShareToken_SameTokenReturnsEarly() { + $form = new Form(); + $form->setId('5'); + $form->setOwnerId('currentUser'); + + $this->configService->expects($this->once()) + ->method('getAllowCustomPublicToken') + ->willReturn(true); + + $this->formsService->expects($this->once()) + ->method('getFormIfAllowed') + ->with(5) + ->willReturn($form); + + $share = new Share(); + $share->setId(8); + $share->setFormId(5); + $share->setShareType(IShare::TYPE_LINK); + $share->setShareWith('sameToken123'); + + $this->shareMapper->expects($this->once()) + ->method('findById') + ->with(8) + ->willReturn($share); + + $this->shareMapper->expects($this->never()) + ->method('findPublicShareByHash'); + $this->shareMapper->expects($this->never()) + ->method('update'); + $this->formsService->expects($this->never()) + ->method('obtainFormLock'); + + $this->assertEquals(new DataResponse(8), $this->shareApiController->updateShare(5, 8, ['token' => 'sameToken123'])); + } + + public function testUpdateShareToken_ForeignShare() { + $form = new Form(); + $form->setId('5'); + $form->setOwnerId('currentUser'); + + $this->configService->expects($this->never()) + ->method('getAllowCustomPublicToken') + ->willReturn(true); + + $this->formsService->expects($this->once()) + ->method('getFormIfAllowed') + ->with(5) + ->willReturn($form); + + $share = new Share(); + $share->setId(8); + $share->setFormId(6); + $share->setShareType(IShare::TYPE_LINK); + $share->setShareWith('abcdefgh'); + + $this->shareMapper->expects($this->once()) + ->method('findById') + ->with(8) + ->willReturn($share); + + $this->expectException(OCSBadRequestException::class); + $this->shareApiController->updateShare(5, 8, ['token' => 'customtoken123']); + } + + public function testUpdateShareToken_ShareNotFound() { + $form = new Form(); + $form->setId('5'); + $form->setOwnerId('currentUser'); + + $this->configService->expects($this->never()) + ->method('getAllowCustomPublicToken') + ->willReturn(true); + + $this->formsService->expects($this->once()) + ->method('getFormIfAllowed') + ->with(5) + ->willReturn($form); + + $this->shareMapper->expects($this->once()) + ->method('findById') + ->with(8) + ->willThrowException(new DoesNotExistException('missing')); + + $this->expectException(OCSNotFoundException::class); + $this->shareApiController->updateShare(5, 8, ['token' => 'customtoken123']); + } + + public function testUpdateShareToken_ArchivedForm() { + $form = new Form(); + $form->setId('5'); + $form->setOwnerId('currentUser'); + + $this->configService->expects($this->never()) + ->method('getAllowCustomPublicToken') + ->willReturn(true); + + $this->formsService->expects($this->once()) + ->method('getFormIfAllowed') + ->with(5) + ->willReturn($form); + $this->formsService->expects($this->once()) + ->method('isFormArchived') + ->with($form) + ->willReturn(true); + + $this->shareMapper->expects($this->never()) + ->method('findById'); + + $this->expectException(OCSForbiddenException::class); + $this->shareApiController->updateShare(5, 8, ['token' => 'customtoken123']); + } } From 0740c93dd90a0b77455ee4121ef8620606b0d9cc Mon Sep 17 00:00:00 2001 From: Alexander Rebello Date: Fri, 15 May 2026 18:41:50 +0200 Subject: [PATCH 2/5] Update ConfigService and add/update related tests Signed-off-by: Alexander Rebello --- lib/Constants.php | 6 ++++++ lib/Service/ConfigService.php | 6 +++++- tests/Unit/Controller/ConfigControllerTest.php | 5 +++++ tests/Unit/Service/ConfigServiceTest.php | 3 +++ 4 files changed, 19 insertions(+), 1 deletion(-) diff --git a/lib/Constants.php b/lib/Constants.php index 684278976..bf0aabea5 100644 --- a/lib/Constants.php +++ b/lib/Constants.php @@ -15,6 +15,7 @@ class Constants { */ public const CONFIG_KEY_ALLOWPERMITALL = 'allowPermitAll'; public const CONFIG_KEY_ALLOWPUBLICLINK = 'allowPublicLink'; + public const CONFIG_KEY_ALLOWCUSTOMPUBLICTOKEN = 'allowCustomPublicShareTokens'; public const CONFIG_KEY_ALLOWSHOWTOALL = 'allowShowToAll'; public const CONFIG_KEY_CREATIONALLOWEDGROUPS = 'creationAllowedGroups'; public const CONFIG_KEY_RESTRICTCREATION = 'restrictCreation'; @@ -24,6 +25,7 @@ class Constants { public const CONFIG_KEYS = [ self::CONFIG_KEY_ALLOWPERMITALL, self::CONFIG_KEY_ALLOWPUBLICLINK, + self::CONFIG_KEY_ALLOWCUSTOMPUBLICTOKEN, self::CONFIG_KEY_ALLOWSHOWTOALL, self::CONFIG_KEY_CREATIONALLOWEDGROUPS, self::CONFIG_KEY_RESTRICTCREATION, @@ -41,6 +43,10 @@ class Constants { self::CONFIG_KEY_CONFIRMATIONEMAILRATELIMIT => 'int', ]; + public const PUBLIC_SHARE_TOKEN_MIN_LENGTH = 8; + public const PUBLIC_SHARE_TOKEN_MAX_LENGTH = 256; + public const PUBLIC_SHARE_HASH_REQUIREMENT = '[a-zA-Z0-9]{' . self::PUBLIC_SHARE_TOKEN_MIN_LENGTH . ',' . self::PUBLIC_SHARE_TOKEN_MAX_LENGTH . '}'; + /** * Maximum String lengths, the database is set to store. */ diff --git a/lib/Service/ConfigService.php b/lib/Service/ConfigService.php index ad73d953e..40d379679 100644 --- a/lib/Service/ConfigService.php +++ b/lib/Service/ConfigService.php @@ -38,7 +38,10 @@ public function getAllowPermitAll(): bool { public function getAllowPublicLink(): bool { return $this->appConfig->getAppValueBool(Constants::CONFIG_KEY_ALLOWPUBLICLINK, true); } - public function getAllowShowToAll() : bool { + public function getAllowCustomPublicToken(): bool { + return json_decode($this->config->getAppValue($this->appName, Constants::CONFIG_KEY_ALLOWCUSTOMPUBLICTOKEN, 'false')); + } + public function getAllowShowToAll(): bool { return $this->appConfig->getAppValueBool(Constants::CONFIG_KEY_ALLOWSHOWTOALL, true); } private function getUnformattedCreationAllowedGroups(): array { @@ -74,6 +77,7 @@ public function getAppConfig(): array { return [ Constants::CONFIG_KEY_ALLOWPERMITALL => $this->getAllowPermitAll(), Constants::CONFIG_KEY_ALLOWPUBLICLINK => $this->getAllowPublicLink(), + Constants::CONFIG_KEY_ALLOWCUSTOMPUBLICTOKEN => $this->getAllowCustomPublicToken(), Constants::CONFIG_KEY_ALLOWSHOWTOALL => $this->getAllowShowToAll(), Constants::CONFIG_KEY_CREATIONALLOWEDGROUPS => $this->getCreationAllowedGroups(), Constants::CONFIG_KEY_RESTRICTCREATION => $this->getRestrictCreation(), diff --git a/tests/Unit/Controller/ConfigControllerTest.php b/tests/Unit/Controller/ConfigControllerTest.php index 6528d9544..6380c5833 100644 --- a/tests/Unit/Controller/ConfigControllerTest.php +++ b/tests/Unit/Controller/ConfigControllerTest.php @@ -68,6 +68,11 @@ public static function dataUpdateAppConfig() { 'configValue' => true, 'strConfig' => 'true' ], + 'booleanConfigAllowCustomPublicShareTokens' => [ + 'configKey' => 'allowCustomPublicShareTokens', + 'configValue' => true, + 'strConfig' => 'true' + ], 'arrayCreationAllowedGroups' => [ 'configKey' => 'creationAllowedGroups', 'configValue' => [ diff --git a/tests/Unit/Service/ConfigServiceTest.php b/tests/Unit/Service/ConfigServiceTest.php index 5f058bfbc..3f84e4175 100644 --- a/tests/Unit/Service/ConfigServiceTest.php +++ b/tests/Unit/Service/ConfigServiceTest.php @@ -57,6 +57,7 @@ public static function dataGetAppConfig() { 'strConfig' => [ 'allowPermitAll' => 'false', 'allowPublicLink' => 'false', + 'allowCustomPublicShareTokens' => 'true', 'allowShowToAll' => 'false', 'creationAllowedGroups' => '["group1", "group2", "nonExisting"]', 'restrictCreation' => 'true', @@ -71,6 +72,7 @@ public static function dataGetAppConfig() { 'expected' => [ 'allowPermitAll' => false, 'allowPublicLink' => false, + 'allowCustomPublicShareTokens' => true, 'allowShowToAll' => false, 'creationAllowedGroups' => [ [ @@ -160,6 +162,7 @@ public static function dataGetAppConfig_Default() { 'expected' => [ 'allowPermitAll' => true, 'allowPublicLink' => true, + 'allowCustomPublicShareTokens' => false, 'allowShowToAll' => true, 'creationAllowedGroups' => [], 'restrictCreation' => false, From 92160cb5e368bb342550951ff5564fa75a9fb1ff Mon Sep 17 00:00:00 2001 From: Alexander Rebello Date: Fri, 15 May 2026 18:41:58 +0200 Subject: [PATCH 3/5] Fix minor PageController behavior Signed-off-by: Alexander Rebello --- lib/Controller/PageController.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Controller/PageController.php b/lib/Controller/PageController.php index 64f85f4a9..0ba6a6b8f 100644 --- a/lib/Controller/PageController.php +++ b/lib/Controller/PageController.php @@ -148,7 +148,7 @@ public function internalLinkView(string $hash): Response { #[NoAdminRequired()] #[NoCSRFRequired()] #[PublicPage()] - #[FrontpageRoute(verb: 'GET', url: '/s/{hash}', requirements: ['hash' => '[a-zA-Z0-9]{24,}'])] + #[FrontpageRoute(verb: 'GET', url: '/s/{hash}', requirements: ['hash' => Constants::PUBLIC_SHARE_HASH_REQUIREMENT])] public function publicLinkView(string $hash): Response { try { $share = $this->shareMapper->findPublicShareByHash($hash); From da647a3e4e8b6f37734585f2c1a065e3f64fe42e Mon Sep 17 00:00:00 2001 From: Christian Hartmann Date: Fri, 19 Jun 2026 21:15:18 +0200 Subject: [PATCH 4/5] fix review comments Signed-off-by: Christian Hartmann --- lib/Constants.php | 3 +- lib/Controller/ShareApiController.php | 3 +- lib/Service/ConfigService.php | 2 +- openapi.json | 2 +- src/FormsSettings.vue | 10 ++-- .../SidebarTabs/SharingSidebarTab.vue | 54 +++++++------------ .../Unit/Controller/ConfigControllerTest.php | 9 +--- .../Controller/ShareApiControllerTest.php | 32 ----------- 8 files changed, 29 insertions(+), 86 deletions(-) diff --git a/lib/Constants.php b/lib/Constants.php index bf0aabea5..e25ea7473 100644 --- a/lib/Constants.php +++ b/lib/Constants.php @@ -36,6 +36,7 @@ class Constants { public const CONFIG_KEY_TYPES = [ self::CONFIG_KEY_ALLOWPERMITALL => 'bool', self::CONFIG_KEY_ALLOWPUBLICLINK => 'bool', + self::CONFIG_KEY_ALLOWCUSTOMPUBLICTOKEN => 'bool', self::CONFIG_KEY_ALLOWSHOWTOALL => 'bool', self::CONFIG_KEY_RESTRICTCREATION => 'bool', self::CONFIG_KEY_ALLOWCONFIRMATIONEMAIL => 'bool', @@ -43,7 +44,7 @@ class Constants { self::CONFIG_KEY_CONFIRMATIONEMAILRATELIMIT => 'int', ]; - public const PUBLIC_SHARE_TOKEN_MIN_LENGTH = 8; + public const PUBLIC_SHARE_TOKEN_MIN_LENGTH = 1; public const PUBLIC_SHARE_TOKEN_MAX_LENGTH = 256; public const PUBLIC_SHARE_HASH_REQUIREMENT = '[a-zA-Z0-9]{' . self::PUBLIC_SHARE_TOKEN_MIN_LENGTH . ',' . self::PUBLIC_SHARE_TOKEN_MAX_LENGTH . '}'; diff --git a/lib/Controller/ShareApiController.php b/lib/Controller/ShareApiController.php index a4981de95..693aaa2ab 100644 --- a/lib/Controller/ShareApiController.php +++ b/lib/Controller/ShareApiController.php @@ -213,7 +213,6 @@ public function newShare(int $formId, int $shareType, string $shareWith = '', ar * @throws OCSBadRequestException Share doesn't belong to given Form * @throws OCSBadRequestException Invalid permission given * @throws OCSBadRequestException Invalid share token - * @throws OCSBadRequestException Share hash exists, please retry * @throws OCSForbiddenException Custom public share tokens are not allowed * @throws OCSForbiddenException This form is not owned by the current user * @throws OCSForbiddenException Empty keyValuePairs, will not update @@ -296,7 +295,7 @@ public function updateShare(int $formId, int $shareId, array $keyValuePairs): Da $existingShare = $this->shareMapper->findPublicShareByHash($token); if ($existingShare->getId() !== $formShare->getId()) { $this->logger->debug('Share hash already exists.'); - throw new OCSBadRequestException('Share hash exists, please retry.'); + throw new OCSBadRequestException('Invalid share token'); } } catch (DoesNotExistException $e) { // Just continue, this is what we expect to happen (share hash not existing yet). diff --git a/lib/Service/ConfigService.php b/lib/Service/ConfigService.php index 40d379679..5a31ae2c9 100644 --- a/lib/Service/ConfigService.php +++ b/lib/Service/ConfigService.php @@ -39,7 +39,7 @@ public function getAllowPublicLink(): bool { return $this->appConfig->getAppValueBool(Constants::CONFIG_KEY_ALLOWPUBLICLINK, true); } public function getAllowCustomPublicToken(): bool { - return json_decode($this->config->getAppValue($this->appName, Constants::CONFIG_KEY_ALLOWCUSTOMPUBLICTOKEN, 'false')); + return $this->appConfig->getAppValueBool(Constants::CONFIG_KEY_ALLOWCUSTOMPUBLICTOKEN, false); } public function getAllowShowToAll(): bool { return $this->appConfig->getAppValueBool(Constants::CONFIG_KEY_ALLOWSHOWTOALL, true); diff --git a/openapi.json b/openapi.json index cbbda9ada..efc7af40d 100644 --- a/openapi.json +++ b/openapi.json @@ -5293,7 +5293,7 @@ } }, "400": { - "description": "Share hash exists, please retry", + "description": "Invalid share token", "content": { "application/json": { "schema": { diff --git a/src/FormsSettings.vue b/src/FormsSettings.vue index 4c8080c3d..84a9df06b 100644 --- a/src/FormsSettings.vue +++ b/src/FormsSettings.vue @@ -73,14 +73,13 @@ {{ t('forms', 'Allow sharing by link') }} {{ t('forms', 'Allow custom public share tokens') }} {{ t('forms', 'Save token') }} @@ -231,6 +231,7 @@