[stable33] fix: secure view deleted tags

cypress/e2e/settings.spec.js

@@ -78,3 +78,146 @@ describe('Office admin settings', function() {
 			.should('be.visible')
 	})
 })
+
+const serverUrl = Cypress.config('baseUrl').replace(/\/index.php\/?$/g, '')
+const davUrl = serverUrl + '/remote.php/dav/systemtags/'
+const watermarkUrl = serverUrl + '/index.php/apps/richdocuments/settings/watermark'
+
+/**
+ * Create a system tag and resolve its id
+ *
+ * @param {string} name the tag name
+ * @return {Cypress.Chainable<number>} the id of the created tag
+ */
+function createSystemTag(name) {
+	return cy.request('/csrftoken')
+		.then(({ body }) => body.token)
+		.then((requesttoken) => cy.request({
+			method: 'POST',
+			url: davUrl,
+			headers: { requesttoken, 'Content-Type': 'application/json' },
+			body: { name, userVisible: true, userAssignable: true },
+		}))
+		.then(({ headers }) => Number.parseInt(headers['content-location'].split('/').pop(), 10))
+}
+
+/**
+ * Delete a system tag
+ *
+ * @param {number} tagId the id of the tag to delete
+ */
+function deleteSystemTag(tagId) {
+	cy.request('/csrftoken')
+		.then(({ body }) => body.token)
+		.then((requesttoken) => cy.request({
+			method: 'DELETE',
+			url: davUrl + tagId,
+			headers: { requesttoken },
+			failOnStatusCode: false,
+		}))
+}
+
+describe('Secure view tag enforcement', function() {
+	let tagId
+
+	beforeEach(function() {
+		cy.login(new User('admin', 'admin'))
+		cy.intercept({
+			method: 'POST',
+			url: '/index.php/apps/richdocuments/ajax/admin.php',
+		}).as('updateSettings')
+		cy.intercept({
+			method: 'POST',
+			url: '/index.php/apps/richdocuments/settings/watermark',
+		}).as('updateWatermark')
+
+		createSystemTag('secured').then((id) => {
+			tagId = id
+		})
+
+		cy.visit('/settings/admin/richdocuments')
+		cy.get('#wopi_url')
+			.clear()
+			.type(collaboraUrl + '{enter}')
+		cy.wait('@updateSettings').its('response.statusCode').should('equal', 200)
+		cy.contains('.notecard', 'Collabora Online server is reachable.')
+		cy.get('#secure-view-settings').scrollIntoView().should('be.visible')
+	})
+
+	afterEach(function() {
+		deleteSystemTag(tagId)
+		resetWatermarkSettings()
+	})
+
+	/**
+	 * Restore the secure view settings to their defaults
+	 */
+	function resetWatermarkSettings() {
+		cy.request('/csrftoken')
+			.then(({ body }) => body.token)
+			.then((requesttoken) => cy.request({
+				method: 'POST',
+				url: watermarkUrl,
+				headers: { requesttoken },
+				body: { settings: { watermark: { enabled: false, allTags: false, allTagsList: [] } } },
+			}))
+	}
+
+	/**
+	 * The tag dropdown, only rendered once secure view is enforced on tagged files
+	 *
+	 * @return {Cypress.Chainable} the dropdown element
+	 */
+	function tagSelect() {
+		return cy.get('#secure-view-settings .checkbox-details .v-select').first()
+	}
+
+	/**
+	 * Enable secure view and enforce it on the tag created for this test
+	 */
+	function enforceSecureViewOnTag() {
+		cy.contains('.settings-entry', 'Enable secure view')
+			.find('input[type=checkbox]')
+			.check({ force: true })
+		cy.wait('@updateWatermark')
+
+		cy.contains('.settings-entry', 'Enforce secure view on tagged files')
+			.find('input[type=checkbox]')
+			.check({ force: true })
+		cy.wait('@updateWatermark')
+
+		tagSelect().click()
+		cy.contains('.vs__dropdown-menu li', 'secured').click()
+		cy.wait('@updateWatermark')
+		tagSelect().find('.vs__selected').should('contain', 'secured')
+	}
+
+	it('Keeps the tag dropdown usable after an enforced tag is deleted', function() {
+		enforceSecureViewOnTag()
+
+		deleteSystemTag(tagId)
+		cy.reload()
+
+		tagSelect().find('.vs__selected').should('not.exist')
+
+		tagSelect().click()
+		cy.get('.vs__dropdown-menu').should('be.visible')
+	})
+
+	it('Allows enforcing another tag after the enforced tag is deleted', function() {
+		enforceSecureViewOnTag()
+
+		deleteSystemTag(tagId)
+		createSystemTag('replacement').then((id) => {
+			tagId = id
+		})
+		cy.reload()
+
+		tagSelect().click()
+		cy.contains('.vs__dropdown-menu li', 'replacement').click()
+		cy.wait('@updateWatermark').its('response.statusCode').should('equal', 200)
+
+		cy.reload()
+		tagSelect().find('.vs__selected').should('contain', 'replacement')
+	})
+})

lib/AppConfig.php

@@ -55,9 +55,10 @@ class AppConfig {
 		'watermark_linkTagsList' => 'array'
 	];
 
-	private const INTEGER_LIST_KEYS = [
-		'watermark_allTagsList' => true,
-		'watermark_linkTagsList' => true,
+	/** Config keys holding the system tag ids that enforce secure view */
+	public const SECURE_VIEW_TAG_KEYS = [
+		'watermark_allTagsList',
+		'watermark_linkTagsList',
 	];
 
 	public function __construct(
@@ -93,7 +94,7 @@ public function getAppValue($key, $defaultValue = null) {
 	 * @return list<string>|string
 	 */
 	public function getAppValueArray($key) {
-		$value = $this->config->getAppValue($this->getAppNamespace($key), $key, []);
+		$value = $this->config->getAppValue($this->getAppNamespace($key), $key, '');
 		if (array_key_exists($key, self::APP_SETTING_TYPES) && self::APP_SETTING_TYPES[$key] === 'array') {
 			$value = $value !== '' ? explode(',', $value) : [];
 		}
@@ -130,7 +131,7 @@ public function getAppSettings() {
 				$value = $value === 'yes' ? true : $value;
 				$result[$key] = $value === 'no' ? false : $value;
 			}
-			if (!empty(self::INTEGER_LIST_KEYS[$key])) {
+			if (in_array($key, self::SECURE_VIEW_TAG_KEYS, true)) {
 				$result[$key] = array_map('intval', $result[$key] ?? []);
 			}
 		}

lib/AppInfo/Application.php

@@ -28,6 +28,7 @@
 use OCA\Richdocuments\Listener\ReferenceListener;
 use OCA\Richdocuments\Listener\RegisterTemplateFileCreatorListener;
 use OCA\Richdocuments\Listener\ShareLinkListener;
+use OCA\Richdocuments\Listener\TagDeletedListener;
 use OCA\Richdocuments\Middleware\WOPIMiddleware;
 use OCA\Richdocuments\Notification\Notifier;
 use OCA\Richdocuments\Preview\EMF;
@@ -65,6 +66,7 @@
 use OCP\Security\CSP\AddContentSecurityPolicyEvent;
 use OCP\Security\FeaturePolicy\AddFeaturePolicyEvent;
 use OCP\Server;
+use OCP\SystemTag\ManagerEvent;
 
 class Application extends App implements IBootstrap {
 	public const APPNAME = 'richdocuments';
@@ -95,6 +97,7 @@ public function register(IRegistrationContext $context): void {
 		$context->registerEventListener(SabrePluginAddEvent::class, AddSabrePluginListener::class);
 		$context->registerEventListener(BeforeSabrePubliclyLoadedEvent::class, AddSabrePluginListener::class);
 		$context->registerEventListener(AddMissingIndicesEvent::class, AddMissingIndicesListener::class);
+		$context->registerEventListener(ManagerEvent::EVENT_DELETE, TagDeletedListener::class);
 		$context->registerReferenceProvider(OfficeTargetReferenceProvider::class);
 		$context->registerSensitiveMethods(WopiMapper::class, [
 			'getPathForToken',

lib/Listener/TagDeletedListener.php

@@ -0,0 +1,42 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OCA\Richdocuments\Listener;
+
+use OCA\Richdocuments\AppConfig;
+use OCP\EventDispatcher\Event;
+use OCP\EventDispatcher\IEventListener;
+use OCP\SystemTag\ManagerEvent;
+
+/** @template-implements IEventListener<Event|ManagerEvent> */
+final class TagDeletedListener implements IEventListener {
+
+	public function __construct(
+		private AppConfig $appConfig,
+	) {
+	}
+
+	#[\Override]
+	public function handle(Event $event): void {
+		if (!$event instanceof ManagerEvent || $event->getEvent() !== ManagerEvent::EVENT_DELETE) {
+			return;
+		}
+
+		$tagId = $event->getTag()->getId();
+
+		foreach (AppConfig::SECURE_VIEW_TAG_KEYS as $key) {
+			$tagIds = $this->appConfig->getAppValueArray($key);
+			$remaining = array_values(array_diff($tagIds, [$tagId]));
+			if ($remaining === $tagIds) {
+				continue;
+			}
+			$this->appConfig->setAppValue($key, implode(',', $remaining));
+		}
+	}
+}

lib/Settings/Admin.php

@@ -16,6 +16,7 @@
 use OCP\AppFramework\Http\TemplateResponse;
 use OCP\IConfig;
 use OCP\Settings\ISettings;
+use OCP\SystemTag\ISystemTagManager;
 
 class Admin implements ISettings {
 	public function __construct(
@@ -27,6 +28,7 @@ public function __construct(
 		private FontService $fontService,
 		private InitialStateService $initialStateService,
 		private TokenManager $tokenManager,
+		private ISystemTagManager $systemTagManager,
 	) {
 	}
 
@@ -50,7 +52,7 @@ public function getForm(): TemplateResponse {
 					'external_apps' => $this->config->getAppValue('richdocuments', 'external_apps'),
 					'canonical_webroot' => $this->config->getAppValue('richdocuments', 'canonical_webroot'),
 					'disable_certificate_verification' => $this->config->getAppValue('richdocuments', 'disable_certificate_verification', '') === 'yes',
-					'settings' => $this->appConfig->getAppSettings(),
+					'settings' => $this->getAppSettingsWithExistingTags(),
 					'demo_servers' => $this->demoService->fetchDemoServers(),
 					'web_server' => strtolower($_SERVER['SERVER_SOFTWARE']),
 					'os_family' => PHP_VERSION_ID >= 70200 ? PHP_OS_FAMILY : PHP_OS,
@@ -67,6 +69,24 @@ public function getForm(): TemplateResponse {
 		);
 	}
 
+	/**
+	 * Tags that no longer exist are dropped from the secure view settings, as
+	 * the tag picker cannot resolve them and fails to render.
+	 */
+	private function getAppSettingsWithExistingTags(): array {
+		$settings = $this->appConfig->getAppSettings();
+		$existingTagIds = array_map('intval', array_keys($this->systemTagManager->getAllTags()));
+
+		foreach (AppConfig::SECURE_VIEW_TAG_KEYS as $key) {
+			if (!isset($settings[$key])) {
+				continue;
+			}
+			$settings[$key] = array_values(array_intersect($settings[$key], $existingTagIds));
+		}
+
+		return $settings;
+	}
+
 	#[\Override]
 	public function getSection(): string {
 		return 'richdocuments';

tests/lib/AppConfigTest.php

@@ -33,7 +33,7 @@ public function setUp(): void {
 	public function testGetAppValueArrayWithValues() {
 		$this->config->expects($this->once())
 			->method('getAppValue')
-			->with('files', 'watermark_allGroupsList', [])
+			->with('files', 'watermark_allGroupsList', '')
 			->willReturn('1,2,3');
 
 		$result = $this->appConfig->getAppValueArray('watermark_allGroupsList');
@@ -44,7 +44,7 @@ public function testGetAppValueArrayWithValues() {
 	public function testGetAppValueArrayWithSingleValue() {
 		$this->config->expects($this->once())
 			->method('getAppValue')
-			->with('files', 'watermark_allTagsList', [])
+			->with('files', 'watermark_allTagsList', '')
 			->willReturn('1');
 
 		$result = $this->appConfig->getAppValueArray('watermark_allTagsList');
@@ -55,14 +55,24 @@ public function testGetAppValueArrayWithSingleValue() {
 	public function testGetAppValueArrayWithNoneValue() {
 		$this->config->expects($this->once())
 			->method('getAppValue')
-			->with('files', 'watermark_allTagsList', [])
+			->with('files', 'watermark_allTagsList', '')
 			->willReturn('');
 
 		$result = $this->appConfig->getAppValueArray('watermark_allTagsList');
 
 		$this->assertSame([], $result);
 	}
 
+	public function testGetAppValueArrayWithUnsetKey() {
+		$this->config->expects($this->once())
+			->method('getAppValue')
+			->willReturnCallback(fn ($app, $key, $default) => $default);
+
+		$result = $this->appConfig->getAppValueArray('watermark_allTagsList');
+
+		$this->assertSame([], $result);
+	}
+
 	public function testGetPreviewConversionTimeoutReturnsDefault(): void {
 		$this->config->expects($this->once())
 			->method('getAppValue')