feat(preference): Allow apps to prevent admins from corrupting preferences via command line

core/Command/User/Setting.php

@@ -9,6 +9,9 @@
 namespace OC\Core\Command\User;
 
 use OC\Core\Command\Base;
+use OCP\Config\BeforePreferenceDeletedEvent;
+use OCP\Config\BeforePreferenceSetEvent;
+use OCP\EventDispatcher\IEventDispatcher;
 use OCP\IConfig;
 use OCP\IUser;
 use OCP\IUserManager;
@@ -22,6 +25,7 @@ class Setting extends Base {
 	public function __construct(
 		protected IUserManager $userManager,
 		protected IConfig $config,
+		protected IEventDispatcher $eventDispatcher,
 	) {
 		parent::__construct();
 	}
@@ -91,6 +95,12 @@ protected function configure() {
 				InputOption::VALUE_NONE,
 				'Checks whether the setting exists before deleting it'
 			)
+			->addOption(
+				'force',
+				null,
+				InputOption::VALUE_NONE,
+				'Set or delete the setting even if the app rejects it'
+			)
 		;
 	}
 
@@ -131,6 +141,9 @@ protected function checkInput(InputInterface $input) {
 		if ($input->getOption('error-if-not-exists') && !$input->getOption('delete')) {
 			throw new \InvalidArgumentException('The "error-if-not-exists" option can only be used together with "delete".');
 		}
+		if ($input->getOption('force') && !$input->getOption('delete') && $input->getArgument('value') === null) {
+			throw new \InvalidArgumentException('The "force" option can only be used together with "value" or "delete".');
+		}
 	}
 
 	#[\Override]
@@ -154,6 +167,16 @@ protected function execute(InputInterface $input, OutputInterface $output): int
 					return 1;
 				}
 
+				$event = new BeforePreferenceSetEvent($uid, $app, $key, $input->getArgument('value'));
+				$this->eventDispatcher->dispatchTyped($event);
+				if ($event->isRejected()) {
+					if (!$input->hasParameterOption('--force')) {
+						$output->writeln('<error>The value was rejected for setting "' . $key . '" of app "' . $app . '", use --force to set it anyway.</error>');
+						return 1;
+					}
+					$output->writeln('<comment>The value was rejected for setting "' . $key . '" of app "' . $app . '", setting it anyway.</comment>');
+				}
+
 				if ($app === 'settings' && in_array($key, ['email', 'display_name'], true)) {
 					$user = $this->userManager->get($uid);
 					if ($user instanceof IUser) {
@@ -187,6 +210,16 @@ protected function execute(InputInterface $input, OutputInterface $output): int
 					return 1;
 				}
 
+				$event = new BeforePreferenceDeletedEvent($uid, $app, $key);
+				$this->eventDispatcher->dispatchTyped($event);
+				if ($event->isRejected()) {
+					if (!$input->hasParameterOption('--force')) {
+						$output->writeln('<error>Deleting setting "' . $key . '" of app "' . $app . '" was rejected, use --force to delete it anyway.</error>');
+						return 1;
+					}
+					$output->writeln('<comment>Deleting setting "' . $key . '" of app "' . $app . '" was rejected, deleting it anyway.</comment>');
+				}
+
 				if ($app === 'settings' && in_array($key, ['email', 'display_name'], true)) {
 					$user = $this->userManager->get($uid);
 					if ($user instanceof IUser) {

lib/public/Config/BeforePreferenceDeletedEvent.php

@@ -16,7 +16,7 @@ class BeforePreferenceDeletedEvent extends Event {
 	protected string $userId;
 	protected string $appId;
 	protected string $configKey;
-	protected bool $valid = false;
+	protected ?bool $valid = null;
 
 	/**
 	 * @since 25.0.0
@@ -53,7 +53,19 @@ public function getConfigKey(): string {
 	 * @since 25.0.0
 	 */
 	public function isValid(): bool {
-		return $this->valid;
+		return $this->valid === true;
+	}
+
+	/**
+	 * Whether a listener explicitly refused the change via setValid(false)
+	 *
+	 * Admin contexts (e.g. occ) only block the change when it was rejected,
+	 * while user contexts require it to be valid.
+	 *
+	 * @since 36.0.0
+	 */
+	public function isRejected(): bool {
+		return $this->valid === false;
 	}
 
 	/**

lib/public/Config/BeforePreferenceSetEvent.php

@@ -17,7 +17,7 @@ class BeforePreferenceSetEvent extends Event {
 	protected string $appId;
 	protected string $configKey;
 	protected string $configValue;
-	protected bool $valid = false;
+	protected ?bool $valid = null;
 
 	/**
 	 * @since 25.0.0
@@ -62,7 +62,19 @@ public function getConfigValue(): string {
 	 * @since 25.0.0
 	 */
 	public function isValid(): bool {
-		return $this->valid;
+		return $this->valid === true;
+	}
+
+	/**
+	 * Whether a listener explicitly refused the change via setValid(false)
+	 *
+	 * Admin contexts (e.g. occ) only block the change when it was rejected,
+	 * while user contexts require it to be valid.
+	 *
+	 * @since 36.0.0
+	 */
+	public function isRejected(): bool {
+		return $this->valid === false;
 	}
 
 	/**

tests/Core/Command/User/SettingTest.php

@@ -10,6 +10,9 @@
 
 use InvalidArgumentException;
 use OC\Core\Command\User\Setting;
+use OCP\Config\BeforePreferenceDeletedEvent;
+use OCP\Config\BeforePreferenceSetEvent;
+use OCP\EventDispatcher\IEventDispatcher;
 use OCP\IConfig;
 use OCP\IDBConnection;
 use OCP\IUserManager;
@@ -22,6 +25,7 @@ class SettingTest extends TestCase {
 	protected IUserManager&MockObject $userManager;
 	protected IConfig&MockObject $config;
 	protected IDBConnection&MockObject $connection;
+	protected IEventDispatcher&MockObject $eventDispatcher;
 	protected InputInterface&MockObject $consoleInput;
 	protected MockObject&OutputInterface $consoleOutput;
 
@@ -32,18 +36,20 @@ protected function setUp(): void {
 		$this->userManager = $this->createMock(IUserManager::class);
 		$this->config = $this->createMock(IConfig::class);
 		$this->connection = $this->createMock(IDBConnection::class);
+		$this->eventDispatcher = $this->createMock(IEventDispatcher::class);
 		$this->consoleInput = $this->createMock(InputInterface::class);
 		$this->consoleOutput = $this->createMock(OutputInterface::class);
 	}
 
 	public function getCommand(array $methods = []) {
 		if (empty($methods)) {
-			return new Setting($this->userManager, $this->config);
+			return new Setting($this->userManager, $this->config, $this->eventDispatcher);
 		} else {
 			$mock = $this->getMockBuilder(Setting::class)
 				->setConstructorArgs([
 					$this->userManager,
 					$this->config,
+					$this->eventDispatcher,
 				])
 				->onlyMethods($methods)
 				->getMock();
@@ -161,6 +167,27 @@ public static function dataCheckInput(): array {
 				false,
 				'The "error-if-not-exists" option can only be used together with "delete".',
 			],
+			[
+				[['uid', 'username'], ['key', 'configkey'], ['value', '']],
+				[['ignore-missing-user', true], ['force', true]],
+				[],
+				false,
+				false,
+			],
+			[
+				[['uid', 'username'], ['key', 'configkey']],
+				[['ignore-missing-user', true], ['delete', true], ['force', true]],
+				[],
+				false,
+				false,
+			],
+			[
+				[['uid', 'username'], ['key', 'configkey'], ['value', null]],
+				[['ignore-missing-user', true], ['force', true]],
+				[],
+				false,
+				'The "force" option can only be used together with "value" or "delete".',
+			],
 		];
 	}
 
@@ -356,6 +383,119 @@ public function testExecuteSet($value, $updateOnly, $expectedLine, $expectedRetu
 		$this->assertEquals($expectedReturn, $this->invokePrivate($command, 'execute', [$this->consoleInput, $this->consoleOutput]));
 	}
 
+	public function testExecuteSetRejected(): void {
+		$command = $this->getCommand(['checkInput']);
+
+		$this->consoleInput->method('getArgument')
+			->willReturnMap([
+				['uid', 'username'],
+				['app', 'appname'],
+				['key', 'configkey'],
+				['value', 'setValue'],
+			]);
+		$this->consoleInput->method('hasParameterOption')
+			->willReturnMap([
+				['--update-only', false, false],
+				['--force', false, false],
+			]);
+		$this->eventDispatcher->expects($this->once())
+			->method('dispatchTyped')
+			->willReturnCallback(function (BeforePreferenceSetEvent $event): void {
+				$this->assertSame('username', $event->getUserId());
+				$this->assertSame('appname', $event->getAppId());
+				$this->assertSame('configkey', $event->getConfigKey());
+				$this->assertSame('setValue', $event->getConfigValue());
+				$event->setValid(false);
+			});
+		$this->config->expects($this->never())
+			->method('setUserValue');
+
+		$this->assertEquals(1, $this->invokePrivate($command, 'execute', [$this->consoleInput, $this->consoleOutput]));
+	}
+
+	public function testExecuteDeleteRejected(): void {
+		$command = $this->getCommand(['checkInput']);
+
+		$this->consoleInput->method('getArgument')
+			->willReturnMap([
+				['uid', 'username'],
+				['app', 'appname'],
+				['key', 'configkey'],
+			]);
+		$this->consoleInput->method('hasParameterOption')
+			->willReturnMap([
+				['--delete', false, true],
+				['--error-if-not-exists', false, false],
+				['--force', false, false],
+			]);
+		$this->eventDispatcher->expects($this->once())
+			->method('dispatchTyped')
+			->willReturnCallback(function (BeforePreferenceDeletedEvent $event): void {
+				$this->assertSame('username', $event->getUserId());
+				$this->assertSame('appname', $event->getAppId());
+				$this->assertSame('configkey', $event->getConfigKey());
+				$event->setValid(false);
+			});
+		$this->config->expects($this->never())
+			->method('deleteUserValue');
+
+		$this->assertEquals(1, $this->invokePrivate($command, 'execute', [$this->consoleInput, $this->consoleOutput]));
+	}
+
+	public function testExecuteSetRejectedForced(): void {
+		$command = $this->getCommand(['checkInput']);
+
+		$this->consoleInput->method('getArgument')
+			->willReturnMap([
+				['uid', 'username'],
+				['app', 'appname'],
+				['key', 'configkey'],
+				['value', 'setValue'],
+			]);
+		$this->consoleInput->method('hasParameterOption')
+			->willReturnMap([
+				['--update-only', false, false],
+				['--force', false, true],
+			]);
+		$this->eventDispatcher->expects($this->once())
+			->method('dispatchTyped')
+			->willReturnCallback(function (BeforePreferenceSetEvent $event): void {
+				$event->setValid(false);
+			});
+		$this->config->expects($this->once())
+			->method('setUserValue')
+			->with('username', 'appname', 'configkey', 'setValue');
+
+		$this->assertEquals(0, $this->invokePrivate($command, 'execute', [$this->consoleInput, $this->consoleOutput]));
+	}
+
+	public function testExecuteDeleteRejectedForced(): void {
+		$command = $this->getCommand(['checkInput']);
+
+		$this->consoleInput->method('getArgument')
+			->willReturnMap([
+				['uid', 'username'],
+				['app', 'appname'],
+				['key', 'configkey'],
+			]);
+		$this->consoleInput->method('hasParameterOption')
+			->willReturnMap([
+				['--delete', false, true],
+				['--error-if-not-exists', false, false],
+				['--force', false, true],
+			]);
+		$this->eventDispatcher->expects($this->once())
+			->method('dispatchTyped')
+			->willReturnCallback(function (BeforePreferenceDeletedEvent $event): void {
+				$event->setValid(false);
+			});
+		$this->config->expects($this->once())
+			->method('deleteUserValue')
+			->with('username', 'appname', 'configkey');
+
+		$this->assertEquals(0, $this->invokePrivate($command, 'execute', [$this->consoleInput, $this->consoleOutput]));
+	}
+
 	public static function dataExecuteGet(): array {
 		return [
 			['config', null, 'config', 0],