From 557abb599fcd3353ca109e049e5f85d1ce0a07e9 Mon Sep 17 00:00:00 2001 From: Alexander Rebello Date: Fri, 24 Apr 2026 01:17:56 +0200 Subject: [PATCH 01/12] Add admin flag for custom public tokens Signed-off-by: Alexander Rebello --- lib/Constants.php | 5 +++++ lib/Service/ConfigService.php | 6 +++++- src/FormsSettings.vue | 14 ++++++++++++++ tests/Unit/Controller/ConfigControllerTest.php | 9 +++++++-- tests/Unit/Service/ConfigServiceTest.php | 3 +++ 5 files changed, 34 insertions(+), 3 deletions(-) diff --git a/lib/Constants.php b/lib/Constants.php index 0ff0341bd..29c83d7f5 100644 --- a/lib/Constants.php +++ b/lib/Constants.php @@ -15,17 +15,22 @@ 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'; 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 ]; + public const PUBLIC_SHARE_TOKEN_MIN_LENGTH = 8; + public const PUBLIC_SHARE_TOKEN_MAX_LENGTH = 256; + /** * Maximum String lengths, the database is set to store. */ diff --git a/lib/Service/ConfigService.php b/lib/Service/ConfigService.php index 593315c9e..1b3891b2d 100644 --- a/lib/Service/ConfigService.php +++ b/lib/Service/ConfigService.php @@ -37,7 +37,10 @@ public function getAllowPermitAll(): bool { public function getAllowPublicLink(): bool { return json_decode($this->config->getAppValue($this->appName, 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 json_decode($this->config->getAppValue($this->appName, Constants::CONFIG_KEY_ALLOWSHOWTOALL, 'true')); } private function getUnformattedCreationAllowedGroups(): array { @@ -57,6 +60,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/src/FormsSettings.vue b/src/FormsSettings.vue index 60c607c8e..bd26a8eca 100644 --- a/src/FormsSettings.vue +++ b/src/FormsSettings.vue @@ -32,6 +32,13 @@ @update:modelValue="onAllowPublicLinkChange"> {{ t('forms', 'Allow sharing by link') }} + + {{ t('forms', 'Allow custom public share tokens') }} + [ + 'booleanConfigAllowPermitAll' => [ 'configKey' => 'allowPermitAll', 'configValue' => true, 'strConfig' => 'true' ], - 'booleanConfig' => [ + 'booleanConfigAllowShowToAll' => [ 'configKey' => 'allowShowToAll', 'configValue' => true, 'strConfig' => 'true' ], + 'booleanConfigAllowCustomPublicShareTokens' => [ + 'configKey' => 'allowCustomPublicShareTokens', + 'configValue' => true, + 'strConfig' => 'true' + ], 'arrayConfig' => [ 'configKey' => 'allowPermitAll', 'configValue' => [ diff --git a/tests/Unit/Service/ConfigServiceTest.php b/tests/Unit/Service/ConfigServiceTest.php index d0ff7a420..354dc7033 100644 --- a/tests/Unit/Service/ConfigServiceTest.php +++ b/tests/Unit/Service/ConfigServiceTest.php @@ -62,6 +62,7 @@ public static function dataGetAppConfig() { 'strConfig' => [ 'allowPermitAll' => 'false', 'allowPublicLink' => 'false', + 'allowCustomPublicShareTokens' => 'true', 'allowShowToAll' => 'false', 'creationAllowedGroups' => '["group1", "group2", "nonExisting"]', 'restrictCreation' => 'true', @@ -73,6 +74,7 @@ public static function dataGetAppConfig() { 'expected' => [ 'allowPermitAll' => false, 'allowPublicLink' => false, + 'allowCustomPublicShareTokens' => true, 'allowShowToAll' => false, 'creationAllowedGroups' => [ [ @@ -136,6 +138,7 @@ public static function dataGetAppConfig_Default() { 'expected' => [ 'allowPermitAll' => true, 'allowPublicLink' => true, + 'allowCustomPublicShareTokens' => false, 'allowShowToAll' => true, 'creationAllowedGroups' => [], 'restrictCreation' => false, From be327ac697a61ac6d77e51cd9f0963e31e04b2ee Mon Sep 17 00:00:00 2001 From: Alexander Rebello Date: Fri, 24 Apr 2026 01:17:56 +0200 Subject: [PATCH 02/12] Add public share token editing Signed-off-by: Alexander Rebello --- docs/API_v3.md | 24 +++ lib/Controller/PageController.php | 3 +- lib/Controller/ShareApiController.php | 97 +++++++++++ .../Controller/ShareApiControllerTest.php | 151 ++++++++++++++++++ 4 files changed, 274 insertions(+), 1 deletion(-) diff --git a/docs/API_v3.md b/docs/API_v3.md index 40c61d54c..62c0aae6b 100644 --- a/docs/API_v3.md +++ b/docs/API_v3.md @@ -653,6 +653,30 @@ Update a single or all properties of an option-object "data": 5 ``` +### Update a Public Share Token + +- Endpoint: `/api/v3/forms/{formId}/shares/{shareId}/token` +- Method: `PATCH` +- Url-Parameters: + | Parameter | Type | Description | + |-----------|---------|-------------| + | _formId_ | Integer | ID of the form containing the share | + | _shareId_ | Integer | ID of the public link share to update | +- Parameters: + | Parameter | Type | Description | + |-----------|---------|-------------| + | _token_ | String | New token for the public share link | +- Restrictions: + - Only available when the admin setting _allowCustomPublicShareTokens_ is enabled. + - Only link shares can be updated. + - 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 updated share. + +``` +"data": 5 +``` + ## Submission Endpoints ### Get Form Submissions diff --git a/lib/Controller/PageController.php b/lib/Controller/PageController.php index 43db2366a..b530b5766 100644 --- a/lib/Controller/PageController.php +++ b/lib/Controller/PageController.php @@ -40,6 +40,7 @@ #[OpenAPI(scope: OpenAPI::SCOPE_IGNORE)] class PageController extends Controller { private const TEMPLATE_MAIN = 'main'; + private const PUBLIC_SHARE_HASH_REQUIREMENT = '[a-zA-Z0-9]{8,256}'; public function __construct( string $appName, @@ -145,7 +146,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' => self::PUBLIC_SHARE_HASH_REQUIREMENT])] public function publicLinkView(string $hash): Response { try { $share = $this->shareMapper->findPublicShareByHash($hash); diff --git a/lib/Controller/ShareApiController.php b/lib/Controller/ShareApiController.php index badf724f9..217afcf0d 100644 --- a/lib/Controller/ShareApiController.php +++ b/lib/Controller/ShareApiController.php @@ -301,6 +301,85 @@ public function updateShare(int $formId, int $shareId, array $keyValuePairs): Da return new DataResponse($formShare->getId()); } + /** + * Update token/hash of a public link share + * + * @param int $formId of the form + * @param int $shareId of the share to update + * @param string $token The new share token + * @return DataResponse + * @throws OCSBadRequestException Share doesn't belong to given Form + * @throws OCSBadRequestException Invalid share token + * @throws OCSBadRequestException Share hash exists, please retry + * @throws OCSForbiddenException Custom public share tokens are not allowed + * @throws OCSForbiddenException Not allowed to update token on non-link share + * @throws OCSForbiddenException This form is not owned by the current user + * @throws OCSNotFoundException Could not find share + * + * 200: the id of the updated share + */ + #[CORS()] + #[NoAdminRequired()] + #[ApiRoute(verb: 'PATCH', url: '/api/v3/forms/{formId}/shares/{shareId}/token')] + public function updateShareToken(int $formId, int $shareId, string $token): DataResponse { + $this->logger->debug('Updating share token: {shareId} of form {formId}', [ + 'formId' => $formId, + 'shareId' => $shareId, + ]); + + if (!$this->configService->getAllowCustomPublicToken()) { + $this->logger->debug('Custom public share tokens are not allowed.'); + throw new OCSForbiddenException('Custom public share tokens are not allowed.'); + } + + $form = $this->formsService->getFormIfAllowed($formId); + if ($this->formsService->isFormArchived($form)) { + $this->logger->debug('This form is archived and can not be modified'); + throw new OCSForbiddenException('This form is archived and can not be modified'); + } + + try { + $formShare = $this->shareMapper->findById($shareId); + } catch (IMapperException $e) { + $this->logger->debug('Could not find share', ['exception' => $e]); + throw new OCSNotFoundException('Could not find share'); + } + + if ($formId !== $formShare->getFormId()) { + $this->logger->debug('This share doesn\'t belong to the given Form'); + throw new OCSBadRequestException('Share doesn\'t belong to given Form'); + } + + 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 ($token === $formShare->getShareWith()) { + return new DataResponse($formShare->getId()); + } + + $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). + } + + $this->formsService->obtainFormLock($form); + + $formShare->setShareWith($token); + $formShare = $this->shareMapper->update($formShare); + $this->formMapper->update($form); + + return new DataResponse($formShare->getId()); + } + /** * Delete a share * @@ -421,4 +500,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/tests/Unit/Controller/ShareApiControllerTest.php b/tests/Unit/Controller/ShareApiControllerTest.php index 3f3dad8e7..de03cd845 100644 --- a/tests/Unit/Controller/ShareApiControllerTest.php +++ b/tests/Unit/Controller/ShareApiControllerTest.php @@ -952,4 +952,155 @@ 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->updateShareToken(5, 8, 'tokenabcd')); + } + + public function testUpdateShareToken_CustomTokensDisabled() { + $this->configService->expects($this->once()) + ->method('getAllowCustomPublicToken') + ->willReturn(false); + + $this->expectException(OCSForbiddenException::class); + $this->shareApiController->updateShareToken(5, 8, 'tokenabcd'); + } + + 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->updateShareToken(5, 8, '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->updateShareToken(5, 8, '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->updateShareToken(5, 8, 'invalid-token'); + } } From b92f1cc1e01da501d4260d6c3bec2ed6987092fd Mon Sep 17 00:00:00 2001 From: Alexander Rebello Date: Fri, 24 Apr 2026 01:17:57 +0200 Subject: [PATCH 03/12] Expose public token editor in sharing sidebar Signed-off-by: Alexander Rebello --- .../SidebarTabs/SharingSidebarTab.vue | 138 +++++++++++++++++- 1 file changed, 133 insertions(+), 5 deletions(-) diff --git a/src/components/SidebarTabs/SharingSidebarTab.vue b/src/components/SidebarTabs/SharingSidebarTab.vue index 07fdd32fc..b27623df7 100644 --- a/src/components/SidebarTabs/SharingSidebarTab.vue +++ b/src/components/SidebarTabs/SharingSidebarTab.vue @@ -53,11 +53,23 @@ - + {{ t('forms', 'Show QR code') }} + + + {{ t('forms', 'Save token') }} + @@ -212,7 +238,9 @@ import NcActionLink from '@nextcloud/vue/components/NcActionLink' import NcActions from '@nextcloud/vue/components/NcActions' import NcCheckboxRadioSwitch from '@nextcloud/vue/components/NcCheckboxRadioSwitch' 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' @@ -234,6 +262,7 @@ export default { components: { FormsIcon, IconAccountMultiple, + IconCheck, IconCodeBrackets, IconCopyAll, IconDelete, @@ -246,6 +275,7 @@ export default { NcActionLink, NcCheckboxRadioSwitch, NcNoteCard, + NcTextField, QRDialog, SharingSearchDiv, SharingShareDiv, @@ -276,10 +306,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 @@ -480,6 +527,76 @@ 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}/token', + { + id: this.form.id, + shareId: share.id, + }, + ), + { + 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) }, @@ -534,6 +651,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; @@ -545,5 +669,9 @@ export default { } } } + + :deep(.share-div__desc--tokenized .input-field__main-wrapper) { + min-inline-size: 220px; + } } From 174a21ca2167373d244c36dac38daef24e070dd7 Mon Sep 17 00:00:00 2001 From: Alexander Rebello Date: Fri, 24 Apr 2026 08:53:44 +0200 Subject: [PATCH 04/12] Add coverage for share token updates Signed-off-by: Alexander Rebello --- .../Controller/ShareApiControllerTest.php | 182 ++++++++++++++++++ 1 file changed, 182 insertions(+) diff --git a/tests/Unit/Controller/ShareApiControllerTest.php b/tests/Unit/Controller/ShareApiControllerTest.php index de03cd845..1853b5842 100644 --- a/tests/Unit/Controller/ShareApiControllerTest.php +++ b/tests/Unit/Controller/ShareApiControllerTest.php @@ -1103,4 +1103,186 @@ public function testUpdateShareToken_InvalidToken() { $this->expectException(OCSBadRequestException::class); $this->shareApiController->updateShareToken(5, 8, '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->updateShareToken(5, 8, ' 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->updateShareToken(5, 8, '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->updateShareToken(5, 8, 'sameToken123')); + } + + public function testUpdateShareToken_ForeignShare() + { + $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(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->updateShareToken(5, 8, 'customtoken123'); + } + + public function testUpdateShareToken_ShareNotFound() + { + $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); + + $this->shareMapper->expects($this->once()) + ->method('findById') + ->with(8) + ->willThrowException(new DoesNotExistException('missing')); + + $this->expectException(OCSNotFoundException::class); + $this->shareApiController->updateShareToken(5, 8, 'customtoken123'); + } + + public function testUpdateShareToken_ArchivedForm() + { + $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); + $this->formsService->expects($this->once()) + ->method('isFormArchived') + ->with($form) + ->willReturn(true); + + $this->shareMapper->expects($this->never()) + ->method('findById'); + + $this->expectException(OCSForbiddenException::class); + $this->shareApiController->updateShareToken(5, 8, 'customtoken123'); + } } From 82662162cd42015ceee9bbb59d6e3e69c81c0df8 Mon Sep 17 00:00:00 2001 From: Alexander Rebello Date: Fri, 24 Apr 2026 08:59:06 +0200 Subject: [PATCH 05/12] Update OpenAPI spec for share token endpoint Signed-off-by: Alexander Rebello --- openapi.json | 211 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 211 insertions(+) diff --git a/openapi.json b/openapi.json index 9d80a1151..3a1df9188 100644 --- a/openapi.json +++ b/openapi.json @@ -5557,6 +5557,217 @@ } } } + }, + "/ocs/v2.php/apps/forms/api/v3/forms/{formId}/shares/{shareId}/token": { + "patch": { + "operationId": "share_api-update-share-token", + "summary": "Update token/hash of a public link share", + "description": "This endpoint allows CORS requests", + "tags": [ + "share_api" + ], + "security": [ + { + "basic_auth": [] + } + ], + "requestBody": { + "required": true, + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "token" + ], + "properties": { + "token": { + "type": "string", + "description": "The new share token" + } + } + } + } + } + }, + "parameters": [ + { + "name": "formId", + "in": "path", + "description": "of the form", + "required": true, + "schema": { + "type": "integer", + "format": "int64" + } + }, + { + "name": "shareId", + "in": "path", + "description": "of the share to update", + "required": true, + "schema": { + "type": "integer", + "format": "int64" + } + }, + { + "name": "OCS-APIRequest", + "in": "header", + "description": "Required to be true for the API request to pass", + "required": true, + "schema": { + "type": "boolean", + "default": true + } + } + ], + "responses": { + "200": { + "description": "the id of the updated share", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": { + "type": "integer", + "format": "int64" + } + } + } + } + } + } + } + }, + "400": { + "description": "Share hash exists, please retry", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": {} + } + } + } + } + } + } + }, + "403": { + "description": "This form is not owned by the current user", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": {} + } + } + } + } + } + } + }, + "404": { + "description": "Could not find share", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": {} + } + } + } + } + } + } + }, + "401": { + "description": "Current user is not logged in", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": {} + } + } + } + } + } + } + } + } + } } }, "tags": [] From c86e612504357b4f5e87df396ac84bc502506f3a Mon Sep 17 00:00:00 2001 From: Alexander Rebello Date: Fri, 24 Apr 2026 09:04:37 +0200 Subject: [PATCH 06/12] Fix php-cs-fixer style in share token tests Signed-off-by: Alexander Rebello --- .../Unit/Controller/ShareApiControllerTest.php | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/tests/Unit/Controller/ShareApiControllerTest.php b/tests/Unit/Controller/ShareApiControllerTest.php index 1853b5842..ab2a5a459 100644 --- a/tests/Unit/Controller/ShareApiControllerTest.php +++ b/tests/Unit/Controller/ShareApiControllerTest.php @@ -1104,8 +1104,7 @@ public function testUpdateShareToken_InvalidToken() { $this->shareApiController->updateShareToken(5, 8, 'invalid-token'); } - public function testUpdateShareToken_WhitespaceToken() - { + public function testUpdateShareToken_WhitespaceToken() { $form = new Form(); $form->setId('5'); $form->setOwnerId('currentUser'); @@ -1137,8 +1136,7 @@ public function testUpdateShareToken_WhitespaceToken() $this->shareApiController->updateShareToken(5, 8, ' customtoken '); } - public function testUpdateShareToken_TooShortToken() - { + public function testUpdateShareToken_TooShortToken() { $form = new Form(); $form->setId('5'); $form->setOwnerId('currentUser'); @@ -1170,8 +1168,7 @@ public function testUpdateShareToken_TooShortToken() $this->shareApiController->updateShareToken(5, 8, 'abc'); } - public function testUpdateShareToken_SameTokenReturnsEarly() - { + public function testUpdateShareToken_SameTokenReturnsEarly() { $form = new Form(); $form->setId('5'); $form->setOwnerId('currentUser'); @@ -1206,8 +1203,7 @@ public function testUpdateShareToken_SameTokenReturnsEarly() $this->assertEquals(new DataResponse(8), $this->shareApiController->updateShareToken(5, 8, 'sameToken123')); } - public function testUpdateShareToken_ForeignShare() - { + public function testUpdateShareToken_ForeignShare() { $form = new Form(); $form->setId('5'); $form->setOwnerId('currentUser'); @@ -1236,8 +1232,7 @@ public function testUpdateShareToken_ForeignShare() $this->shareApiController->updateShareToken(5, 8, 'customtoken123'); } - public function testUpdateShareToken_ShareNotFound() - { + public function testUpdateShareToken_ShareNotFound() { $form = new Form(); $form->setId('5'); $form->setOwnerId('currentUser'); @@ -1260,8 +1255,7 @@ public function testUpdateShareToken_ShareNotFound() $this->shareApiController->updateShareToken(5, 8, 'customtoken123'); } - public function testUpdateShareToken_ArchivedForm() - { + public function testUpdateShareToken_ArchivedForm() { $form = new Form(); $form->setId('5'); $form->setOwnerId('currentUser'); From 36b859155c7403f9cd7ed792379a1ce3ca394f35 Mon Sep 17 00:00:00 2001 From: Alexander Rebello Date: Fri, 24 Apr 2026 10:23:56 +0200 Subject: [PATCH 07/12] Refactor share token updates into share patch endpoint Signed-off-by: Alexander Rebello --- docs/API_v3.md | 31 +-- lib/Constants.php | 1 + lib/Controller/PageController.php | 3 +- lib/Controller/ShareApiController.php | 145 +++++------- openapi.json | 217 +----------------- .../SidebarTabs/SharingSidebarTab.vue | 6 +- .../Controller/ShareApiControllerTest.php | 22 +- 7 files changed, 82 insertions(+), 343 deletions(-) diff --git a/docs/API_v3.md b/docs/API_v3.md index 62c0aae6b..ce4ba2e00 100644 --- a/docs/API_v3.md +++ b/docs/API_v3.md @@ -631,7 +631,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. ``` @@ -653,30 +658,6 @@ Update a single or all properties of an option-object "data": 5 ``` -### Update a Public Share Token - -- Endpoint: `/api/v3/forms/{formId}/shares/{shareId}/token` -- Method: `PATCH` -- Url-Parameters: - | Parameter | Type | Description | - |-----------|---------|-------------| - | _formId_ | Integer | ID of the form containing the share | - | _shareId_ | Integer | ID of the public link share to update | -- Parameters: - | Parameter | Type | Description | - |-----------|---------|-------------| - | _token_ | String | New token for the public share link | -- Restrictions: - - Only available when the admin setting _allowCustomPublicShareTokens_ is enabled. - - Only link shares can be updated. - - 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 updated share. - -``` -"data": 5 -``` - ## Submission Endpoints ### Get Form Submissions diff --git a/lib/Constants.php b/lib/Constants.php index 29c83d7f5..d734d6c76 100644 --- a/lib/Constants.php +++ b/lib/Constants.php @@ -30,6 +30,7 @@ class Constants { 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/Controller/PageController.php b/lib/Controller/PageController.php index b530b5766..c353d6638 100644 --- a/lib/Controller/PageController.php +++ b/lib/Controller/PageController.php @@ -40,7 +40,6 @@ #[OpenAPI(scope: OpenAPI::SCOPE_IGNORE)] class PageController extends Controller { private const TEMPLATE_MAIN = 'main'; - private const PUBLIC_SHARE_HASH_REQUIREMENT = '[a-zA-Z0-9]{8,256}'; public function __construct( string $appName, @@ -146,7 +145,7 @@ public function internalLinkView(string $hash): Response { #[NoAdminRequired()] #[NoCSRFRequired()] #[PublicPage()] - #[FrontpageRoute(verb: 'GET', url: '/s/{hash}', requirements: ['hash' => self::PUBLIC_SHARE_HASH_REQUIREMENT])] + #[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); diff --git a/lib/Controller/ShareApiController.php b/lib/Controller/ShareApiController.php index 217afcf0d..a341d4b9e 100644 --- a/lib/Controller/ShareApiController.php +++ b/lib/Controller/ShareApiController.php @@ -207,7 +207,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 @@ -215,9 +215,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 @@ -226,7 +230,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 @@ -256,22 +260,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 unknown properties'); + throw new OCSForbiddenException('Not allowed to update unknown properties'); + } } - 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->formsService->getFormUploadedFilesFolderPath($form); @@ -301,85 +347,6 @@ public function updateShare(int $formId, int $shareId, array $keyValuePairs): Da return new DataResponse($formShare->getId()); } - /** - * Update token/hash of a public link share - * - * @param int $formId of the form - * @param int $shareId of the share to update - * @param string $token The new share token - * @return DataResponse - * @throws OCSBadRequestException Share doesn't belong to given Form - * @throws OCSBadRequestException Invalid share token - * @throws OCSBadRequestException Share hash exists, please retry - * @throws OCSForbiddenException Custom public share tokens are not allowed - * @throws OCSForbiddenException Not allowed to update token on non-link share - * @throws OCSForbiddenException This form is not owned by the current user - * @throws OCSNotFoundException Could not find share - * - * 200: the id of the updated share - */ - #[CORS()] - #[NoAdminRequired()] - #[ApiRoute(verb: 'PATCH', url: '/api/v3/forms/{formId}/shares/{shareId}/token')] - public function updateShareToken(int $formId, int $shareId, string $token): DataResponse { - $this->logger->debug('Updating share token: {shareId} of form {formId}', [ - 'formId' => $formId, - 'shareId' => $shareId, - ]); - - if (!$this->configService->getAllowCustomPublicToken()) { - $this->logger->debug('Custom public share tokens are not allowed.'); - throw new OCSForbiddenException('Custom public share tokens are not allowed.'); - } - - $form = $this->formsService->getFormIfAllowed($formId); - if ($this->formsService->isFormArchived($form)) { - $this->logger->debug('This form is archived and can not be modified'); - throw new OCSForbiddenException('This form is archived and can not be modified'); - } - - try { - $formShare = $this->shareMapper->findById($shareId); - } catch (IMapperException $e) { - $this->logger->debug('Could not find share', ['exception' => $e]); - throw new OCSNotFoundException('Could not find share'); - } - - if ($formId !== $formShare->getFormId()) { - $this->logger->debug('This share doesn\'t belong to the given Form'); - throw new OCSBadRequestException('Share doesn\'t belong to given Form'); - } - - 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 ($token === $formShare->getShareWith()) { - return new DataResponse($formShare->getId()); - } - - $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). - } - - $this->formsService->obtainFormLock($form); - - $formShare->setShareWith($token); - $formShare = $this->shareMapper->update($formShare); - $this->formMapper->update($form); - - return new DataResponse($formShare->getId()); - } - /** * Delete a share * diff --git a/openapi.json b/openapi.json index 3a1df9188..b5870638a 100644 --- a/openapi.json +++ b/openapi.json @@ -5157,7 +5157,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" @@ -5254,7 +5254,7 @@ } }, "400": { - "description": "Invalid permission given", + "description": "Share hash exists, please retry", "content": { "application/json": { "schema": { @@ -5282,7 +5282,7 @@ } }, "403": { - "description": "Not allowed to update other properties than permissions", + "description": "Not allowed to update unknown properties", "content": { "application/json": { "schema": { @@ -5557,217 +5557,6 @@ } } } - }, - "/ocs/v2.php/apps/forms/api/v3/forms/{formId}/shares/{shareId}/token": { - "patch": { - "operationId": "share_api-update-share-token", - "summary": "Update token/hash of a public link share", - "description": "This endpoint allows CORS requests", - "tags": [ - "share_api" - ], - "security": [ - { - "basic_auth": [] - } - ], - "requestBody": { - "required": true, - "content": { - "application/json": { - "schema": { - "type": "object", - "required": [ - "token" - ], - "properties": { - "token": { - "type": "string", - "description": "The new share token" - } - } - } - } - } - }, - "parameters": [ - { - "name": "formId", - "in": "path", - "description": "of the form", - "required": true, - "schema": { - "type": "integer", - "format": "int64" - } - }, - { - "name": "shareId", - "in": "path", - "description": "of the share to update", - "required": true, - "schema": { - "type": "integer", - "format": "int64" - } - }, - { - "name": "OCS-APIRequest", - "in": "header", - "description": "Required to be true for the API request to pass", - "required": true, - "schema": { - "type": "boolean", - "default": true - } - } - ], - "responses": { - "200": { - "description": "the id of the updated share", - "content": { - "application/json": { - "schema": { - "type": "object", - "required": [ - "ocs" - ], - "properties": { - "ocs": { - "type": "object", - "required": [ - "meta", - "data" - ], - "properties": { - "meta": { - "$ref": "#/components/schemas/OCSMeta" - }, - "data": { - "type": "integer", - "format": "int64" - } - } - } - } - } - } - } - }, - "400": { - "description": "Share hash exists, please retry", - "content": { - "application/json": { - "schema": { - "type": "object", - "required": [ - "ocs" - ], - "properties": { - "ocs": { - "type": "object", - "required": [ - "meta", - "data" - ], - "properties": { - "meta": { - "$ref": "#/components/schemas/OCSMeta" - }, - "data": {} - } - } - } - } - } - } - }, - "403": { - "description": "This form is not owned by the current user", - "content": { - "application/json": { - "schema": { - "type": "object", - "required": [ - "ocs" - ], - "properties": { - "ocs": { - "type": "object", - "required": [ - "meta", - "data" - ], - "properties": { - "meta": { - "$ref": "#/components/schemas/OCSMeta" - }, - "data": {} - } - } - } - } - } - } - }, - "404": { - "description": "Could not find share", - "content": { - "application/json": { - "schema": { - "type": "object", - "required": [ - "ocs" - ], - "properties": { - "ocs": { - "type": "object", - "required": [ - "meta", - "data" - ], - "properties": { - "meta": { - "$ref": "#/components/schemas/OCSMeta" - }, - "data": {} - } - } - } - } - } - } - }, - "401": { - "description": "Current user is not logged in", - "content": { - "application/json": { - "schema": { - "type": "object", - "required": [ - "ocs" - ], - "properties": { - "ocs": { - "type": "object", - "required": [ - "meta", - "data" - ], - "properties": { - "meta": { - "$ref": "#/components/schemas/OCSMeta" - }, - "data": {} - } - } - } - } - } - } - } - } - } } }, "tags": [] diff --git a/src/components/SidebarTabs/SharingSidebarTab.vue b/src/components/SidebarTabs/SharingSidebarTab.vue index b27623df7..b99ecf7cc 100644 --- a/src/components/SidebarTabs/SharingSidebarTab.vue +++ b/src/components/SidebarTabs/SharingSidebarTab.vue @@ -561,14 +561,16 @@ export default { try { const response = await axios.patch( generateOcsUrl( - 'apps/forms/api/v3/forms/{id}/shares/{shareId}/token', + 'apps/forms/api/v3/forms/{id}/shares/{shareId}', { id: this.form.id, shareId: share.id, }, ), { - token, + keyValuePairs: { + token, + }, }, ) diff --git a/tests/Unit/Controller/ShareApiControllerTest.php b/tests/Unit/Controller/ShareApiControllerTest.php index ab2a5a459..0bb1e29c9 100644 --- a/tests/Unit/Controller/ShareApiControllerTest.php +++ b/tests/Unit/Controller/ShareApiControllerTest.php @@ -991,7 +991,7 @@ public function testUpdateShareToken() { return $updatedShare; }); - $this->assertEquals(new DataResponse(8), $this->shareApiController->updateShareToken(5, 8, 'tokenabcd')); + $this->assertEquals(new DataResponse(8), $this->shareApiController->updateShare(5, 8, ['token' => 'tokenabcd'])); } public function testUpdateShareToken_CustomTokensDisabled() { @@ -1000,7 +1000,7 @@ public function testUpdateShareToken_CustomTokensDisabled() { ->willReturn(false); $this->expectException(OCSForbiddenException::class); - $this->shareApiController->updateShareToken(5, 8, 'tokenabcd'); + $this->shareApiController->updateShare(5, 8, ['token' => 'tokenabcd']); } public function testUpdateShareToken_ForbiddenForNonLinkShare() { @@ -1029,7 +1029,7 @@ public function testUpdateShareToken_ForbiddenForNonLinkShare() { ->willReturn($share); $this->expectException(OCSForbiddenException::class); - $this->shareApiController->updateShareToken(5, 8, 'tokenabcd'); + $this->shareApiController->updateShare(5, 8, ['token' => 'tokenabcd']); } public function testUpdateShareToken_DuplicateHash() { @@ -1069,7 +1069,7 @@ public function testUpdateShareToken_DuplicateHash() { ->willReturn($existingShare); $this->expectException(OCSBadRequestException::class); - $this->shareApiController->updateShareToken(5, 8, 'tokenabcd'); + $this->shareApiController->updateShare(5, 8, ['token' => 'tokenabcd']); } public function testUpdateShareToken_InvalidToken() { @@ -1101,7 +1101,7 @@ public function testUpdateShareToken_InvalidToken() { ->method('update'); $this->expectException(OCSBadRequestException::class); - $this->shareApiController->updateShareToken(5, 8, 'invalid-token'); + $this->shareApiController->updateShare(5, 8, ['token' => 'invalid-token']); } public function testUpdateShareToken_WhitespaceToken() { @@ -1133,7 +1133,7 @@ public function testUpdateShareToken_WhitespaceToken() { ->method('findPublicShareByHash'); $this->expectException(OCSBadRequestException::class); - $this->shareApiController->updateShareToken(5, 8, ' customtoken '); + $this->shareApiController->updateShare(5, 8, ['token' => ' customtoken ']); } public function testUpdateShareToken_TooShortToken() { @@ -1165,7 +1165,7 @@ public function testUpdateShareToken_TooShortToken() { ->method('findPublicShareByHash'); $this->expectException(OCSBadRequestException::class); - $this->shareApiController->updateShareToken(5, 8, 'abc'); + $this->shareApiController->updateShare(5, 8, ['token' => 'abc']); } public function testUpdateShareToken_SameTokenReturnsEarly() { @@ -1200,7 +1200,7 @@ public function testUpdateShareToken_SameTokenReturnsEarly() { $this->formsService->expects($this->never()) ->method('obtainFormLock'); - $this->assertEquals(new DataResponse(8), $this->shareApiController->updateShareToken(5, 8, 'sameToken123')); + $this->assertEquals(new DataResponse(8), $this->shareApiController->updateShare(5, 8, ['token' => 'sameToken123'])); } public function testUpdateShareToken_ForeignShare() { @@ -1229,7 +1229,7 @@ public function testUpdateShareToken_ForeignShare() { ->willReturn($share); $this->expectException(OCSBadRequestException::class); - $this->shareApiController->updateShareToken(5, 8, 'customtoken123'); + $this->shareApiController->updateShare(5, 8, ['token' => 'customtoken123']); } public function testUpdateShareToken_ShareNotFound() { @@ -1252,7 +1252,7 @@ public function testUpdateShareToken_ShareNotFound() { ->willThrowException(new DoesNotExistException('missing')); $this->expectException(OCSNotFoundException::class); - $this->shareApiController->updateShareToken(5, 8, 'customtoken123'); + $this->shareApiController->updateShare(5, 8, ['token' => 'customtoken123']); } public function testUpdateShareToken_ArchivedForm() { @@ -1277,6 +1277,6 @@ public function testUpdateShareToken_ArchivedForm() { ->method('findById'); $this->expectException(OCSForbiddenException::class); - $this->shareApiController->updateShareToken(5, 8, 'customtoken123'); + $this->shareApiController->updateShare(5, 8, ['token' => 'customtoken123']); } } From 74d859b1c731b3f8a80cf6e51d1d744720551319 Mon Sep 17 00:00:00 2001 From: Alexander Rebello Date: Fri, 24 Apr 2026 10:46:54 +0200 Subject: [PATCH 08/12] Fix lint and unit tests after share update refactor Signed-off-by: Alexander Rebello --- .../SidebarTabs/SharingSidebarTab.vue | 11 +++----- .../Controller/ShareApiControllerTest.php | 26 ++++++++++++++++--- 2 files changed, 27 insertions(+), 10 deletions(-) diff --git a/src/components/SidebarTabs/SharingSidebarTab.vue b/src/components/SidebarTabs/SharingSidebarTab.vue index b99ecf7cc..527b0e526 100644 --- a/src/components/SidebarTabs/SharingSidebarTab.vue +++ b/src/components/SidebarTabs/SharingSidebarTab.vue @@ -560,13 +560,10 @@ export default { try { const response = await axios.patch( - generateOcsUrl( - 'apps/forms/api/v3/forms/{id}/shares/{shareId}', - { - id: this.form.id, - shareId: share.id, - }, - ), + generateOcsUrl('apps/forms/api/v3/forms/{id}/shares/{shareId}', { + id: this.form.id, + shareId: share.id, + }), { keyValuePairs: { token, diff --git a/tests/Unit/Controller/ShareApiControllerTest.php b/tests/Unit/Controller/ShareApiControllerTest.php index 0bb1e29c9..b7c371471 100644 --- a/tests/Unit/Controller/ShareApiControllerTest.php +++ b/tests/Unit/Controller/ShareApiControllerTest.php @@ -995,6 +995,26 @@ public function testUpdateShareToken() { } 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); @@ -1208,7 +1228,7 @@ public function testUpdateShareToken_ForeignShare() { $form->setId('5'); $form->setOwnerId('currentUser'); - $this->configService->expects($this->once()) + $this->configService->expects($this->never()) ->method('getAllowCustomPublicToken') ->willReturn(true); @@ -1237,7 +1257,7 @@ public function testUpdateShareToken_ShareNotFound() { $form->setId('5'); $form->setOwnerId('currentUser'); - $this->configService->expects($this->once()) + $this->configService->expects($this->never()) ->method('getAllowCustomPublicToken') ->willReturn(true); @@ -1260,7 +1280,7 @@ public function testUpdateShareToken_ArchivedForm() { $form->setId('5'); $form->setOwnerId('currentUser'); - $this->configService->expects($this->once()) + $this->configService->expects($this->never()) ->method('getAllowCustomPublicToken') ->willReturn(true); From c216d0adc9cac7914f125b9691ca9033249d7fef Mon Sep 17 00:00:00 2001 From: Alexander Rebello Date: Sun, 26 Apr 2026 16:40:33 +0200 Subject: [PATCH 09/12] Stabilize eslint changes detection Signed-off-by: Alexander Rebello --- .github/workflows/lint-eslint.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/workflows/lint-eslint.yml b/.github/workflows/lint-eslint.yml index af1187119..42385a14e 100644 --- a/.github/workflows/lint-eslint.yml +++ b/.github/workflows/lint-eslint.yml @@ -28,6 +28,12 @@ jobs: src: ${{ steps.changes.outputs.src}} steps: + - name: Checkout + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + persist-credentials: false + fetch-depth: 0 + - uses: dorny/paths-filter@fbd0ab8f3e69293af611ebaee6363fc25e6d187d # v4.0.1 id: changes continue-on-error: true From 8a96b095d4c60d43f1aa4ab47a1f0f8a3e746ab8 Mon Sep 17 00:00:00 2001 From: Alexander Rebello Date: Tue, 28 Apr 2026 15:35:44 +0200 Subject: [PATCH 10/12] Update lib/Controller/ShareApiController.php Co-authored-by: Christian Hartmann Signed-off-by: Alexander Rebello --- lib/Controller/ShareApiController.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Controller/ShareApiController.php b/lib/Controller/ShareApiController.php index a341d4b9e..200524867 100644 --- a/lib/Controller/ShareApiController.php +++ b/lib/Controller/ShareApiController.php @@ -263,8 +263,8 @@ public function updateShare(int $formId, int $shareId, array $keyValuePairs): Da $allowedKeys = ['permissions', 'token']; foreach (array_keys($keyValuePairs) as $key) { if (!in_array($key, $allowedKeys, true)) { - $this->logger->debug('Not allowed to update unknown properties'); - throw new OCSForbiddenException('Not allowed to update unknown properties'); + $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'); } } From 8d7708afd5a2057c016ca0c3c8c6018c5be3f176 Mon Sep 17 00:00:00 2001 From: Alexander Rebello Date: Tue, 28 Apr 2026 15:41:55 +0200 Subject: [PATCH 11/12] Revert workflow changes - use template version --- .github/workflows/lint-eslint.yml | 6 ------ 1 file changed, 6 deletions(-) diff --git a/.github/workflows/lint-eslint.yml b/.github/workflows/lint-eslint.yml index 42385a14e..af1187119 100644 --- a/.github/workflows/lint-eslint.yml +++ b/.github/workflows/lint-eslint.yml @@ -28,12 +28,6 @@ jobs: src: ${{ steps.changes.outputs.src}} steps: - - name: Checkout - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - with: - persist-credentials: false - fetch-depth: 0 - - uses: dorny/paths-filter@fbd0ab8f3e69293af611ebaee6363fc25e6d187d # v4.0.1 id: changes continue-on-error: true From 84b9f62659f30f0e5c27adc6d2b92051685b9a90 Mon Sep 17 00:00:00 2001 From: Alexander Rebello Date: Tue, 28 Apr 2026 16:02:49 +0200 Subject: [PATCH 12/12] Cover custom share token validation Signed-off-by: Alexander Rebello --- .github/workflows/lint-eslint.yml | 6 ---- .../Controller/ShareApiControllerTest.php | 32 +++++++++++++++++++ 2 files changed, 32 insertions(+), 6 deletions(-) diff --git a/.github/workflows/lint-eslint.yml b/.github/workflows/lint-eslint.yml index 42385a14e..af1187119 100644 --- a/.github/workflows/lint-eslint.yml +++ b/.github/workflows/lint-eslint.yml @@ -28,12 +28,6 @@ jobs: src: ${{ steps.changes.outputs.src}} steps: - - name: Checkout - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - with: - persist-credentials: false - fetch-depth: 0 - - uses: dorny/paths-filter@fbd0ab8f3e69293af611ebaee6363fc25e6d187d # v4.0.1 id: changes continue-on-error: true diff --git a/tests/Unit/Controller/ShareApiControllerTest.php b/tests/Unit/Controller/ShareApiControllerTest.php index b7c371471..e04c88d55 100644 --- a/tests/Unit/Controller/ShareApiControllerTest.php +++ b/tests/Unit/Controller/ShareApiControllerTest.php @@ -1023,6 +1023,38 @@ public function testUpdateShareToken_CustomTokensDisabled() { $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');