Merge pull request #3028 from nextcloud/fix/permissionChecks

fix: Update permission checks for archiving forms and transfer ownership

Author: Chartman123

lib/Controller/ApiController.php

@@ -1786,25 +1786,31 @@ private function checkAccessUpdate(array $keyValuePairs): void {
 	 * Checks if the current user is allowed to archive/unarchive the form
 	 */
 	private function checkArchivePermission(Form $form, string $currentUserId, array $keyValuePairs): void {
+		// Only check if the request is trying to change the archived state
+		if (!array_key_exists('state', $keyValuePairs)) {
+			return;
+		}
+
 		$isArchived = $this->formsService->isFormArchived($form);
-		$owner = $currentUserId === $form->getOwnerId();
-		$onlyState = sizeof($keyValuePairs) === 1 && key_exists('state', $keyValuePairs);
+		$isOwner = $currentUserId === $form->getOwnerId();
 
-		// Only check if the request is trying to change the archived state
-		if ($onlyState && $keyValuePairs['state'] === Constants::FORM_STATE_ARCHIVED) {
+		// If the request contains 'state' it must be the only key
+		if (sizeof($keyValuePairs) !== 1) {
+			$this->logger->debug('State may only be changed on its own');
+			throw new OCSForbiddenException('State may only be changed on its own');
+		}
+
+		$state = $keyValuePairs['state'];
+
+		if ($state === Constants::FORM_STATE_ARCHIVED && !$isArchived && !$isOwner) {
 			// Trying to archive
-			if (!$owner || $isArchived) {
-				$this->logger->debug('Only the form owner can archive the form, and only if it is not already archived');
-				throw new OCSForbiddenException('Only the form owner can archive the form, and only if it is not already archived');
-			}
-		} elseif ($onlyState && $keyValuePairs['state'] === Constants::FORM_STATE_CLOSED) {
+			$this->logger->debug('Only the form owner can archive the form, and only if it is not already archived');
+			throw new OCSForbiddenException('Only the form owner can archive the form, and only if it is not already archived');
+		} elseif ($state === Constants::FORM_STATE_CLOSED && $isArchived && !$isOwner) {
 			// Trying to unarchive
-			if (!$owner || !$isArchived) {
-				$this->logger->debug('Only the form owner can unarchive the form, and only if it is currently archived');
-				throw new OCSForbiddenException('Only the form owner can unarchive the form, and only if it is currently archived');
-			}
+			$this->logger->debug('Only the form owner can unarchive the form, and only if it is currently archived');
+			throw new OCSForbiddenException('Only the form owner can unarchive the form, and only if it is currently archived');
 		}
-		// All other updates are allowed (including updates that do not touch the state)
 	}
 
 	private function isLockingRequest(array $keyValuePairs): bool {

src/components/SidebarTabs/SettingsSidebarTab.vue

@@ -103,7 +103,7 @@
 		<NcCheckboxRadioSwitch
 			:model-value="formArchived"
 			aria-describedby="forms-settings__archive-form"
-			:disabled="locked"
+			:disabled="locked || !isCurrentUserOwner"
 			type="switch"
 			@update:model-value="onFormArchivedChange">
 			{{ t('forms', 'Archive form') }}
@@ -168,7 +168,10 @@
 			</div>
 		</div>
 
-		<TransferOwnership :locked="locked" :form="form" />
+		<TransferOwnership
+			:locked="locked"
+			:is-owner="isCurrentUserOwner"
+			:form="form" />
 	</div>
 </template>
 

src/components/SidebarTabs/TransferOwnership.vue

@@ -10,7 +10,7 @@
 			alignment="start"
 			variant="tertiary"
 			wide
-			:disabled="locked"
+			:disabled="locked || !isOwner"
 			@click="openModal">
 			<span class="transfer-button__text">{{
 				t('forms', 'Transfer ownership')
@@ -119,6 +119,11 @@ export default {
 			required: true,
 		},
 
+		isOwner: {
+			type: Boolean,
+			required: true,
+		},
+
 		locked: {
 			type: Boolean,
 			required: true,