From 2ddccba578cbc266c548c002a3a7c96a15bdbc01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= <1005065+DeepDiver1975@users.noreply.github.com> Date: Wed, 16 Dec 2020 15:56:24 +0100 Subject: [PATCH] Add app config to allow groups of users to be lock breakers --- apps/dav/lib/Connector/Sabre/LockPlugin.php | 38 ++++++++++++++++++- .../dav/lib/Connector/Sabre/ServerFactory.php | 2 +- apps/dav/lib/Server.php | 2 +- apps/dav/tests/unit/DAV/LockPluginTest.php | 38 +++++++++++++++++-- changelog/unreleased/38222 | 6 +++ settings/Panels/Admin/PersistentLocking.php | 5 ++- settings/js/panels/persistentlocking.js | 6 +++ .../panels/admin/persistentlocking.php | 18 ++++++++- 8 files changed, 107 insertions(+), 8 deletions(-) create mode 100644 changelog/unreleased/38222 diff --git a/apps/dav/lib/Connector/Sabre/LockPlugin.php b/apps/dav/lib/Connector/Sabre/LockPlugin.php index 245f177cded5..9ef45e1b159c 100644 --- a/apps/dav/lib/Connector/Sabre/LockPlugin.php +++ b/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; } } diff --git a/apps/dav/lib/Connector/Sabre/ServerFactory.php b/apps/dav/lib/Connector/Sabre/ServerFactory.php index 9f3666091a5c..03fe337ce921 100644 --- a/apps/dav/lib/Connector/Sabre/ServerFactory.php +++ b/apps/dav/lib/Connector/Sabre/ServerFactory.php @@ -120,7 +120,7 @@ public function createServer($baseUri, // 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) { diff --git a/apps/dav/lib/Server.php b/apps/dav/lib/Server.php index 626c0b6720a6..4027a305c186 100644 --- a/apps/dav/lib/Server.php +++ b/apps/dav/lib/Server.php @@ -157,7 +157,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()); $this->server->addPlugin(new \OCA\DAV\Connector\Sabre\PublicDavLocksPlugin($fileLocksBackend, function ($uri) { diff --git a/apps/dav/tests/unit/DAV/LockPluginTest.php b/apps/dav/tests/unit/DAV/LockPluginTest.php index 91ab31ebd17b..56a4facc0da5 100644 --- a/apps/dav/tests/unit/DAV/LockPluginTest.php +++ b/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'])], + ]; + } } diff --git a/changelog/unreleased/38222 b/changelog/unreleased/38222 new file mode 100644 index 000000000000..266e9bee003b --- /dev/null +++ b/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 diff --git a/settings/Panels/Admin/PersistentLocking.php b/settings/Panels/Admin/PersistentLocking.php index f67c6710a9b5..94cfeeee90b7 100644 --- a/settings/Panels/Admin/PersistentLocking.php +++ b/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; } diff --git a/settings/js/panels/persistentlocking.js b/settings/js/panels/persistentlocking.js index f0d9335f13c5..8952c0cfcb55 100644 --- a/settings/js/panels/persistentlocking.js +++ b/settings/js/panels/persistentlocking.js @@ -1,5 +1,11 @@ $(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'); diff --git a/settings/templates/panels/admin/persistentlocking.php b/settings/templates/panels/admin/persistentlocking.php index 31bc25748264..f23d535ec22e 100644 --- a/settings/templates/panels/admin/persistentlocking.php +++ b/settings/templates/panels/admin/persistentlocking.php @@ -1,5 +1,10 @@

t('Manual File Locking')); ?>

@@ -21,6 +26,17 @@ value="1" /> - +
+

+
+ t('Allow users in the following groups to unlock files they have access to:')); ?> +
+ + +
+ t('Users in these groups can unlock files even if they are not the owner of the lock.')); ?> + +

+