feat: Add app config to allow groups of users to be lock breakers

apps/dav/lib/Connector/Sabre/LockPlugin.php

@@ -26,6 +26,9 @@
 
 use OC\Lock\Persistent\LockMapper;
 use OCA\DAV\Connector\Sabre\Exception\FileLocked;
+use OCP\IConfig;
+use OCP\IGroupManager;
+use OCP\IUser;
 use OCP\Lock\ILockingProvider;
 use OCP\Lock\LockedException;
 use Sabre\DAV\Exception\Forbidden;
@@ -42,6 +45,19 @@ class LockPlugin extends ServerPlugin {
 	 * @var \Sabre\DAV\Server
 	 */
 	private $server;
+	/**
+	 * @var IConfig
+	 */
+	private $config;
+	/**
+	 * @var IGroupManager
+	 */
+	private $groupManager;
+
+	public function __construct(IConfig $config, IGroupManager $groupManager) {
+		$this->config = $config;
+		$this->groupManager = $groupManager;
+	}
 
 	private $missedLocks = [];
 
@@ -125,8 +141,28 @@ public function beforeUnlock($uri, LockInfo $lock) {
 			return;
 		}
 		$currentUser = \OC::$server->getUserSession()->getUser();
-		if ($currentUser === null || $lock->getOwnerAccountId() !== $currentUser->getAccountId()) {
+		if ($currentUser === null) {
 			throw new Forbidden();
 		}
+
+		if ($lock->getOwnerAccountId() === $currentUser->getAccountId()) {
+			return;
+		}
+
+		if (!$this->userIsALockBreaker($currentUser)) {
+			throw new Forbidden();
+		}
+	}
+
+	private function userIsALockBreaker(IUser $currentUser): bool {
+		$lockBreakerGroups = $this->config->getAppValue('core', 'lock-breaker-groups', '[]');
+		$lockBreakerGroups = \json_decode($lockBreakerGroups) ?? [];
+
+		foreach ($lockBreakerGroups as $lockBreakerGroup) {
+			if ($this->groupManager->isInGroup($currentUser->getUID(), $lockBreakerGroup)) {
+				return true;
+			}
+		}
+		return false;
 	}
 }

apps/dav/lib/Connector/Sabre/ServerFactory.php

@@ -122,7 +122,7 @@ public function createServer(
 		// FIXME: The following line is a workaround for legacy components relying on being able to send a GET to /
 		$server->addPlugin(new \OCA\DAV\Connector\Sabre\DummyGetResponsePlugin());
 		$server->addPlugin(new \OCA\DAV\Connector\Sabre\ExceptionLoggerPlugin('webdav', $this->logger));
-		$server->addPlugin(new \OCA\DAV\Connector\Sabre\LockPlugin());
+		$server->addPlugin(new \OCA\DAV\Connector\Sabre\LockPlugin($this->config, \OC::$server->getGroupManager()));
 
 		$fileLocksBackend = new FileLocksBackend($server->tree, true, $this->timeFactory, $isPublicAccess);
 		$server->addPlugin(new \OCA\DAV\Connector\Sabre\PublicDavLocksPlugin($fileLocksBackend, function ($uri) use ($isPublicAccess) {

apps/dav/lib/Server.php

@@ -159,7 +159,7 @@ public function __construct(IRequest $request, $baseUri) {
 
 		$this->server->addPlugin(new ExceptionLoggerPlugin('webdav', $logger));
 		$this->server->addPlugin(new \Sabre\DAV\Sync\Plugin());
-		$this->server->addPlugin(new LockPlugin());
+		$this->server->addPlugin(new LockPlugin(\OC::$server->getConfig(), \OC::$server->getGroupManager()));
 
 		$fileLocksBackend = new FileLocksBackend($this->server->tree, false, OC::$server->getTimeFactory(), $isPublicAccess);
 		$this->server->addPlugin(new \OCA\DAV\Connector\Sabre\PublicDavLocksPlugin($fileLocksBackend, function ($uri) {

apps/dav/tests/unit/DAV/LockPluginTest.php

@@ -24,8 +24,14 @@
 use OC\Lock\Persistent\Lock;
 use OC\Lock\Persistent\LockMapper;
 use OCA\DAV\Connector\Sabre\LockPlugin;
+use OCP\AppFramework\Db\DoesNotExistException;
+use OCP\AppFramework\Db\MultipleObjectsReturnedException;
+use OCP\AppFramework\QueryException;
+use OCP\IConfig;
+use OCP\IGroupManager;
 use OCP\IUser;
 use OCP\IUserSession;
+use Sabre\DAV\Exception\Forbidden;
 use Sabre\DAV\Locks\LockInfo;
 use Sabre\DAV\Server;
 use Sabre\DAV\Tree;
@@ -50,10 +56,20 @@ class LockPluginTest extends TestCase {
 	private $lockMapper;
 	/** @var IUserSession | \PHPUnit\Framework\MockObject\MockObject */
 	private $userSession;
+	/**
+	 * @var IConfig|\PHPUnit\Framework\MockObject\MockObject
+	 */
+	private $config;
+	/**
+	 * @var IGroupManager|\PHPUnit\Framework\MockObject\MockObject
+	 */
+	private $groupManager;
 
 	public function setUp(): void {
 		parent::setUp();
-		$this->plugin = new LockPlugin();
+		$this->config = $this->createMock(IConfig::class);
+		$this->groupManager = $this->createMock(IGroupManager::class);
+		$this->plugin = new LockPlugin($this->config, $this->groupManager);
 
 		$this->server = $this->createMock(Server::class);
 		$this->tree = $this->createMock(Tree::class);
@@ -74,9 +90,15 @@ protected function tearDown(): void {
 	}
 
 	/**
+	 * @dataProvider providesConfigValues
+	 * @param string $lockBreakerGroups
+	 * @throws DoesNotExistException
+	 * @throws Forbidden
+	 * @throws MultipleObjectsReturnedException
+	 * @throws QueryException
 	 */
-	public function testBeforeUnlock() {
-		$this->expectException(\Sabre\DAV\Exception\Forbidden::class);
+	public function testBeforeUnlock(string $lockBreakerGroups): void {
+		$this->expectException(Forbidden::class);
 
 		$lock = new LockInfo();
 		$lock->token = '123-456-789';
@@ -88,6 +110,16 @@ public function testBeforeUnlock() {
 		$user->method('getAccountId')->willReturn(777);
 		$this->userSession->method('getUser')->willReturn($user);
 
+		$this->config->method('getAppValue')->willReturn($lockBreakerGroups);
+
 		$this->plugin->beforeUnlock('foo', $lock);
 	}
+
+	public function providesConfigValues(): array {
+		return [
+			'empty array' => ['[]'],
+			'not a valid json string' => ['[}'],
+			'not in any group' => [\json_encode(['group1', 'group2'])],
+		];
+	}
 }

changelog/unreleased/38222

@@ -0,0 +1,6 @@
+Enhancement: Special user groups can break persistent locks
+
+Not only the owner of a lock can unlock a resource but the lock breaker groups
+are allowed to break locks as well.
+
+https://github.com/owncloud/core/pull/38222

settings/Panels/Admin/PersistentLocking.php

@@ -39,15 +39,18 @@ public function getPriority() {
 	}
 
 	public function getSectionID() {
-		return 'additional';
+		return 'general';
 	}
 
 	public function getPanel() {
+		$lockBreakerGroups = \json_decode($this->config->getAppValue('core', 'lock-breaker-groups', '[]'), true);
+
 		// we must use the same container
 		$tmpl = new Template('settings', 'panels/admin/persistentlocking');
 		$tmpl->assign('defaultTimeout', $this->config->getAppValue('core', 'lock_timeout_default', LockManager::LOCK_TIMEOUT_DEFAULT));
 		$tmpl->assign('maximumTimeout', $this->config->getAppValue('core', 'lock_timeout_max', LockManager::LOCK_TIMEOUT_MAX));
 		$tmpl->assign('manualFileLockOnClientsEnabled', $this->config->getAppValue('files', 'enable_lock_file_action', 'no'));
+		$tmpl->assign('lock-breaker-groups', \implode('|', $lockBreakerGroups));
 
 		return $tmpl;
 	}

settings/js/panels/persistentlocking.js

@@ -1,12 +1,22 @@
 $(document).ready(function() {
 
+	var lockBreakersGroupsList = $('#lock_breakers_groups_list');
+	OC.Settings.setupGroupsSelect(lockBreakersGroupsList);
+	lockBreakersGroupsList.change(function(ev) {
+		OC.AppConfig.setValue('core', 'lock-breaker-groups', JSON.stringify(ev.val || []));
+	});
+
 	$('#persistentlocking input').change(function () {
 		var currentInput = $(this);
 		var name = currentInput.attr('name');
 		var isValid = true;
 		var app = '';
 		var value = '';
 
+		// handled above
+		if (name === 'lock_breakers_groups_list') {
+			return;
+		}
 		if (name === 'enable_lock_file_action') {
 			app = 'files';
 			if (this.checked) {

settings/templates/panels/admin/persistentlocking.php

@@ -1,5 +1,10 @@
 <?php
 script('settings', 'panels/persistentlocking');
+/**
+ * @var array $_
+ * @var \OCP\IL10N $l
+ * @var OC_Defaults $theme
+ */
 ?>
 <div class="section" id="persistentlocking">
 	<h2 class="app-name"><?php p($l->t('Manual File Locking')); ?></h2>
@@ -21,6 +26,17 @@
 		   value="1" <?php if ($_['manualFileLockOnClientsEnabled'] === 'yes') {
 	print_unescaped('checked="checked"');
 } ?> />
-	<label for="manualFileLockOnClientsEnabled"><?php p($l->t('Enable manual file locking on clients'));?></label>
+	<label for="manualFileLockOnClientsEnabled"><?php p($l->t('Enable manual file locking on the web interface'));?></label>
 	<br/>
+	<p id="lock-breakers">
+		<br />
+		<?php p($l->t('Allow users in the following groups to unlock files they have access to:')); ?>
+		<br />
+		<input name="lock_breakers_groups_list" type="hidden" id="lock_breakers_groups_list" value="<?php p($_['lock-breaker-groups']) ?>" style="width: 400px">
+		<em>
+			<br />
+			<?php p($l->t('Users in these groups can unlock files even if they are not the owner of the lock.')); ?>
+		</em>
+	</p>
+
 </div>

tests/Settings/Panels/Admin/PersistentLockingTest.php

@@ -23,38 +23,44 @@
 use OC\Settings\Panels\Admin\PersistentLocking;
 use OC\Lock\Persistent\LockManager;
 use OCP\IConfig;
+use Test\TestCase;
 
 /**
  * @package Tests\Settings\Panels\Admin
  */
-class PersistentLockingTest extends \Test\TestCase {
+class PersistentLockingTest extends TestCase {
 	/** @var IConfig */
 	private $config;
+	/**
+	 * @var PersistentLocking
+	 */
+	private $panel;
 
 	public function setUp(): void {
 		parent::setUp();
 		$this->config = $this->createMock(IConfig::class);
 		$this->panel = new PersistentLocking($this->config);
 	}
 
-	public function testGetPriority() {
-		$this->assertSame(0, $this->panel->getPriority());
+	public function testGetPriority(): void {
+		self::assertSame(0, $this->panel->getPriority());
 	}
 
-	public function testGetSection() {
-		$this->assertEquals('additional', $this->panel->getSectionID());
+	public function testGetSection(): void {
+		self::assertEquals('general', $this->panel->getSectionID());
 	}
 
-	public function testGetPanel() {
+	public function testGetPanel(): void {
 		$this->config->method('getAppValue')
-			->will($this->returnValueMap([
+			->willReturnMap([
+				['core', 'lock-breaker-groups', '[]', '[]'],
 				['core', 'lock_timeout_default', LockManager::LOCK_TIMEOUT_DEFAULT, 44],
 				['core', 'lock_timeout_max', LockManager::LOCK_TIMEOUT_MAX, 9999],
-			]));
+			]);
 
 		$templateHtml = $this->panel->getPanel()->fetchPage();
 		// applied modifiers "m" for multiline and "s" to include newlines in the dot char
-		$this->assertRegExp('/input[[:space:]].*name="lock_timeout_default"[[:space:]].*value="44"/ms', $templateHtml);
-		$this->assertRegExp('/input[[:space:]].*name="lock_timeout_max"[[:space:]].*value="9999"/ms', $templateHtml);
+		self::assertRegExp('/input[[:space:]].*name="lock_timeout_default"[[:space:]].*value="44"/ms', $templateHtml);
+		self::assertRegExp('/input[[:space:]].*name="lock_timeout_max"[[:space:]].*value="9999"/ms', $templateHtml);
 	}
 }