From 77bdad84c21f2d836b778a2741eb8e8512ee19eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 22 Mar 2022 12:31:44 +0100 Subject: [PATCH 1/4] Add ldap:reset-group command to unmap groups from LDAP MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- apps/user_ldap/appinfo/info.xml | 1 + .../composer/composer/autoload_classmap.php | 1 + .../composer/composer/autoload_static.php | 1 + apps/user_ldap/lib/GroupPluginManager.php | 22 +++++++++++++-- apps/user_ldap/lib/Group_LDAP.php | 27 ++++++++++++++----- apps/user_ldap/lib/Group_Proxy.php | 10 +++---- apps/user_ldap/lib/UserPluginManager.php | 7 +++-- core/Command/Group/Delete.php | 2 +- 8 files changed, 52 insertions(+), 19 deletions(-) diff --git a/apps/user_ldap/appinfo/info.xml b/apps/user_ldap/appinfo/info.xml index 2dae484524121..d654d34d66ee6 100644 --- a/apps/user_ldap/appinfo/info.xml +++ b/apps/user_ldap/appinfo/info.xml @@ -50,6 +50,7 @@ A user logs into Nextcloud with their LDAP or AD credentials, and is granted acc OCA\User_LDAP\Command\CheckUser OCA\User_LDAP\Command\CreateEmptyConfig OCA\User_LDAP\Command\DeleteConfig + OCA\User_LDAP\Command\ResetGroup OCA\User_LDAP\Command\ResetUser OCA\User_LDAP\Command\Search OCA\User_LDAP\Command\SetConfig diff --git a/apps/user_ldap/composer/composer/autoload_classmap.php b/apps/user_ldap/composer/composer/autoload_classmap.php index 12ede37a941d0..1fca642fc9fbf 100644 --- a/apps/user_ldap/composer/composer/autoload_classmap.php +++ b/apps/user_ldap/composer/composer/autoload_classmap.php @@ -14,6 +14,7 @@ 'OCA\\User_LDAP\\Command\\CheckUser' => $baseDir . '/../lib/Command/CheckUser.php', 'OCA\\User_LDAP\\Command\\CreateEmptyConfig' => $baseDir . '/../lib/Command/CreateEmptyConfig.php', 'OCA\\User_LDAP\\Command\\DeleteConfig' => $baseDir . '/../lib/Command/DeleteConfig.php', + 'OCA\\User_LDAP\\Command\\ResetGroup' => $baseDir . '/../lib/Command/ResetGroup.php', 'OCA\\User_LDAP\\Command\\ResetUser' => $baseDir . '/../lib/Command/ResetUser.php', 'OCA\\User_LDAP\\Command\\Search' => $baseDir . '/../lib/Command/Search.php', 'OCA\\User_LDAP\\Command\\SetConfig' => $baseDir . '/../lib/Command/SetConfig.php', diff --git a/apps/user_ldap/composer/composer/autoload_static.php b/apps/user_ldap/composer/composer/autoload_static.php index ecf5e4167f615..9d8c33406e88f 100644 --- a/apps/user_ldap/composer/composer/autoload_static.php +++ b/apps/user_ldap/composer/composer/autoload_static.php @@ -29,6 +29,7 @@ class ComposerStaticInitUser_LDAP 'OCA\\User_LDAP\\Command\\CheckUser' => __DIR__ . '/..' . '/../lib/Command/CheckUser.php', 'OCA\\User_LDAP\\Command\\CreateEmptyConfig' => __DIR__ . '/..' . '/../lib/Command/CreateEmptyConfig.php', 'OCA\\User_LDAP\\Command\\DeleteConfig' => __DIR__ . '/..' . '/../lib/Command/DeleteConfig.php', + 'OCA\\User_LDAP\\Command\\ResetGroup' => __DIR__ . '/..' . '/../lib/Command/ResetGroup.php', 'OCA\\User_LDAP\\Command\\ResetUser' => __DIR__ . '/..' . '/../lib/Command/ResetUser.php', 'OCA\\User_LDAP\\Command\\Search' => __DIR__ . '/..' . '/../lib/Command/Search.php', 'OCA\\User_LDAP\\Command\\SetConfig' => __DIR__ . '/..' . '/../lib/Command/SetConfig.php', diff --git a/apps/user_ldap/lib/GroupPluginManager.php b/apps/user_ldap/lib/GroupPluginManager.php index d23e9d4d443a7..a25665e4691bf 100644 --- a/apps/user_ldap/lib/GroupPluginManager.php +++ b/apps/user_ldap/lib/GroupPluginManager.php @@ -26,9 +26,9 @@ use OCP\GroupInterface; class GroupPluginManager { - private $respondToActions = 0; + private int $respondToActions = 0; - private $which = [ + private array $which = [ GroupInterface::CREATE_GROUP => null, GroupInterface::DELETE_GROUP => null, GroupInterface::ADD_TO_GROUP => null, @@ -37,6 +37,8 @@ class GroupPluginManager { GroupInterface::GROUP_DETAILS => null ]; + private bool $suppressDeletion = false; + /** * @return int All implemented actions */ @@ -84,6 +86,19 @@ public function createGroup($gid) { throw new \Exception('No plugin implements createGroup in this LDAP Backend.'); } + public function canDeleteGroup(): bool { + return !$this->suppressDeletion && ($this->which[GroupInterface::DELETE_GROUP] !== null); + } + + /** + * @return bool – the value before the change + */ + public function setSuppressDeletion(bool $value): bool { + $old = $this->suppressDeletion; + $this->suppressDeletion = $value; + return $old; + } + /** * Delete a group * @param string $gid Group Id of the group to delete @@ -94,6 +109,9 @@ public function deleteGroup($gid) { $plugin = $this->which[GroupInterface::DELETE_GROUP]; if ($plugin) { + if ($this->suppressDeletion) { + return false; + } return $plugin->deleteGroup($gid); } throw new \Exception('No plugin implements deleteGroup in this LDAP Backend.'); diff --git a/apps/user_ldap/lib/Group_LDAP.php b/apps/user_ldap/lib/Group_LDAP.php index 766b77bf52199..f9d9b06174303 100644 --- a/apps/user_ldap/lib/Group_LDAP.php +++ b/apps/user_ldap/lib/Group_LDAP.php @@ -48,10 +48,11 @@ use OC\Cache\CappedMemoryCache; use OC\ServerNotAvailableException; use OCP\Group\Backend\IGetDisplayNameBackend; +use OCP\Group\Backend\IDeleteGroupBackend; use OCP\GroupInterface; use Psr\Log\LoggerInterface; -class Group_LDAP extends BackendUtility implements GroupInterface, IGroupLDAP, IGetDisplayNameBackend { +class Group_LDAP extends BackendUtility implements GroupInterface, IGroupLDAP, IGetDisplayNameBackend, IDeleteGroupBackend { protected $enabled = false; /** @var string[][] $cachedGroupMembers array of users with gid as key */ @@ -1204,6 +1205,7 @@ protected function filterValidGroups(array $listOfGroups): array { */ public function implementsActions($actions) { return (bool)((GroupInterface::COUNT_USERS | + GroupInterface::DELETE_GROUP | $this->groupPluginManager->getImplementedActions()) & $actions); } @@ -1249,19 +1251,32 @@ public function createGroup($gid) { * delete a group * * @param string $gid gid of the group to delete - * @return bool * @throws Exception */ - public function deleteGroup($gid) { - if ($this->groupPluginManager->implementsActions(GroupInterface::DELETE_GROUP)) { + public function deleteGroup(string $gid): bool { + if ($this->groupPluginManager->canDeleteGroup()) { if ($ret = $this->groupPluginManager->deleteGroup($gid)) { - #delete group in nextcloud internal db + // Delete group in nextcloud internal db $this->access->getGroupMapper()->unmap($gid); $this->access->connection->writeToCache("groupExists" . $gid, false); } return $ret; } - throw new Exception('Could not delete group in LDAP backend.'); + + // Getting dn, if false the group is not mapped + $dn = $this->access->groupname2dn($gid); + if (!$dn) { + throw new Exception('Could not delete unknown group '.$gid.' in LDAP backend.'); + } + + if (!$this->groupExists($gid)) { + // The group does not exist in the LDAP, remove the mapping + $this->access->getGroupMapper()->unmap($gid); + $this->access->connection->writeToCache("groupExists" . $gid, false); + return true; + } + + throw new Exception('Could not delete existing group '.$gid.' in LDAP backend.'); } /** diff --git a/apps/user_ldap/lib/Group_Proxy.php b/apps/user_ldap/lib/Group_Proxy.php index 92a9041949e2c..ea2fcce679c4a 100644 --- a/apps/user_ldap/lib/Group_Proxy.php +++ b/apps/user_ldap/lib/Group_Proxy.php @@ -28,10 +28,11 @@ */ namespace OCA\User_LDAP; -use OCP\Group\Backend\INamedBackend; +use OCP\Group\Backend\IDeleteGroupBackend; use OCP\Group\Backend\IGetDisplayNameBackend; +use OCP\Group\Backend\INamedBackend; -class Group_Proxy extends Proxy implements \OCP\GroupInterface, IGroupLDAP, IGetDisplayNameBackend, INamedBackend { +class Group_Proxy extends Proxy implements \OCP\GroupInterface, IGroupLDAP, IGetDisplayNameBackend, INamedBackend, IDeleteGroupBackend { private $backends = []; private $refBackend = null; @@ -171,11 +172,8 @@ public function createGroup($gid) { /** * delete a group - * - * @param string $gid gid of the group to delete - * @return bool */ - public function deleteGroup($gid) { + public function deleteGroup(string $gid): bool { return $this->handleRequest( $gid, 'deleteGroup', [$gid]); } diff --git a/apps/user_ldap/lib/UserPluginManager.php b/apps/user_ldap/lib/UserPluginManager.php index 035b7952dce65..748a210cf60ad 100644 --- a/apps/user_ldap/lib/UserPluginManager.php +++ b/apps/user_ldap/lib/UserPluginManager.php @@ -28,9 +28,9 @@ use OC\User\Backend; class UserPluginManager { - private $respondToActions = 0; + private int $respondToActions = 0; - private $which = [ + private array $which = [ Backend::CREATE_USER => null, Backend::SET_PASSWORD => null, Backend::GET_HOME => null, @@ -41,8 +41,7 @@ class UserPluginManager { 'deleteUser' => null ]; - /** @var bool */ - private $suppressDeletion = false; + private bool $suppressDeletion = false; /** * @return int All implemented actions, except for 'deleteUser' diff --git a/core/Command/Group/Delete.php b/core/Command/Group/Delete.php index 75202308944a9..be97be83407cc 100644 --- a/core/Command/Group/Delete.php +++ b/core/Command/Group/Delete.php @@ -61,7 +61,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int $output->writeln('Group "' . $gid . '" could not be deleted.'); return 1; } - if (! $this->groupManager->groupExists($gid)) { + if (!$this->groupManager->groupExists($gid)) { $output->writeln('Group "' . $gid . '" does not exist.'); return 1; } From a2c030ffea0d64a4bd23331a1b78efa2a9bd112f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 22 Mar 2022 15:15:43 +0100 Subject: [PATCH 2/4] Add type information to fix psalm error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- apps/user_ldap/lib/GroupPluginManager.php | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/user_ldap/lib/GroupPluginManager.php b/apps/user_ldap/lib/GroupPluginManager.php index a25665e4691bf..8403ad4e4be29 100644 --- a/apps/user_ldap/lib/GroupPluginManager.php +++ b/apps/user_ldap/lib/GroupPluginManager.php @@ -28,6 +28,7 @@ class GroupPluginManager { private int $respondToActions = 0; + /** @var array */ private array $which = [ GroupInterface::CREATE_GROUP => null, GroupInterface::DELETE_GROUP => null, From 91d6e88c2fb3f59163979f1e00a35364ec24cb1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Wed, 30 Mar 2022 11:02:49 +0200 Subject: [PATCH 3/4] Add missing file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- apps/user_ldap/lib/Command/ResetGroup.php | 111 ++++++++++++++++++++++ 1 file changed, 111 insertions(+) create mode 100644 apps/user_ldap/lib/Command/ResetGroup.php diff --git a/apps/user_ldap/lib/Command/ResetGroup.php b/apps/user_ldap/lib/Command/ResetGroup.php new file mode 100644 index 0000000000000..f3c3019f919d0 --- /dev/null +++ b/apps/user_ldap/lib/Command/ResetGroup.php @@ -0,0 +1,111 @@ + + * + * @author Arthur Schiwon + * @author Côme Chilliet + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ +namespace OCA\User_LDAP\Command; + +use OCA\User_LDAP\Group_Proxy; +use OCA\User_LDAP\GroupPluginManager; +use OCP\IGroup; +use OCP\IGroupManager; +use Symfony\Component\Console\Command\Command; +use Symfony\Component\Console\Helper\QuestionHelper; +use Symfony\Component\Console\Input\InputArgument; +use Symfony\Component\Console\Input\InputInterface; +use Symfony\Component\Console\Input\InputOption; +use Symfony\Component\Console\Output\OutputInterface; +use Symfony\Component\Console\Question\Question; + +class ResetGroup extends Command { + private IGroupManager $groupManager; + private GroupPluginManager $pluginManager; + private Group_Proxy $backend; + + public function __construct( + IGroupManager $groupManager, + GroupPluginManager $pluginManager, + Group_Proxy $backend + ) { + $this->groupManager = $groupManager; + $this->pluginManager = $pluginManager; + $this->backend = $backend; + parent::__construct(); + } + + protected function configure(): void { + $this + ->setName('ldap:reset-group') + ->setDescription('deletes an LDAP group independent of the group state in the LDAP') + ->addArgument( + 'gid', + InputArgument::REQUIRED, + 'the group name as used in Nextcloud' + ) + ->addOption( + 'yes', + 'y', + InputOption::VALUE_NONE, + 'do not ask for confirmation' + ); + } + + protected function execute(InputInterface $input, OutputInterface $output): int { + try { + $gid = $input->getArgument('gid'); + $group = $this->groupManager->get($gid); + if (!$group instanceof IGroup) { + throw new \Exception('Group not found'); + } + $backends = $group->getBackendNames(); + if (!in_array('LDAP', $backends)) { + throw new \Exception('The given group is not a recognized LDAP group.'); + } + if ($input->getOption('yes') === false) { + /** @var QuestionHelper $helper */ + $helper = $this->getHelper('question'); + $q = new Question('Delete all local data of this group (y|N)? '); + $input->setOption('yes', $helper->ask($input, $output, $q) === 'y'); + } + if ($input->getOption('yes') !== true) { + throw new \Exception('Reset cancelled by operator'); + } + + // Disable real deletion if a plugin supports it + $pluginManagerSuppressed = $this->pluginManager->setSuppressDeletion(true); + // Bypass groupExists test to force mapping deletion + $this->backend->getLDAPAccess($gid)->connection->writeToCache('groupExists' . $gid, false); + echo "calling delete $gid\n"; + if ($group->delete()) { + $this->pluginManager->setSuppressDeletion($pluginManagerSuppressed); + return 0; + } + } catch (\Throwable $e) { + if (isset($pluginManagerSuppressed)) { + $this->pluginManager->setSuppressDeletion($pluginManagerSuppressed); + } + $output->writeln('' . $e->getMessage() . ''); + return 1; + } + $output->writeln('Error while resetting group'); + return 2; + } +} From d7a291039dd5d58a020ef69c83f00dc15473d962 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Wed, 30 Mar 2022 12:50:02 +0200 Subject: [PATCH 4/4] Fix user_ldap unit tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- apps/user_ldap/lib/GroupPluginManager.php | 7 +++---- apps/user_ldap/tests/GroupLDAPPluginTest.php | 16 ++++++++-------- apps/user_ldap/tests/Group_LDAPTest.php | 4 ++-- 3 files changed, 13 insertions(+), 14 deletions(-) diff --git a/apps/user_ldap/lib/GroupPluginManager.php b/apps/user_ldap/lib/GroupPluginManager.php index 8403ad4e4be29..5999409cdba34 100644 --- a/apps/user_ldap/lib/GroupPluginManager.php +++ b/apps/user_ldap/lib/GroupPluginManager.php @@ -88,7 +88,7 @@ public function createGroup($gid) { } public function canDeleteGroup(): bool { - return !$this->suppressDeletion && ($this->which[GroupInterface::DELETE_GROUP] !== null); + return !$this->suppressDeletion && $this->implementsActions(GroupInterface::DELETE_GROUP); } /** @@ -102,11 +102,10 @@ public function setSuppressDeletion(bool $value): bool { /** * Delete a group - * @param string $gid Group Id of the group to delete - * @return bool + * * @throws \Exception */ - public function deleteGroup($gid) { + public function deleteGroup(string $gid): bool { $plugin = $this->which[GroupInterface::DELETE_GROUP]; if ($plugin) { diff --git a/apps/user_ldap/tests/GroupLDAPPluginTest.php b/apps/user_ldap/tests/GroupLDAPPluginTest.php index b55f57990a00c..660608d6b1fbb 100644 --- a/apps/user_ldap/tests/GroupLDAPPluginTest.php +++ b/apps/user_ldap/tests/GroupLDAPPluginTest.php @@ -84,7 +84,7 @@ public function testCreateGroup() { $pluginManager->createGroup('group'); } - + public function testCreateGroupNotRegistered() { $this->expectException(\Exception::class); $this->expectExceptionMessage('No plugin implements createGroup in this LDAP Backend.'); @@ -108,13 +108,13 @@ public function testDeleteGroup() { ->method('deleteGroup') ->with( $this->equalTo('group') - ); + )->willReturn(true); $pluginManager->register($plugin); - $pluginManager->deleteGroup('group'); + $this->assertTrue($pluginManager->deleteGroup('group')); } - + public function testDeleteGroupNotRegistered() { $this->expectException(\Exception::class); $this->expectExceptionMessage('No plugin implements deleteGroup in this LDAP Backend.'); @@ -145,7 +145,7 @@ public function testAddToGroup() { $pluginManager->addToGroup('uid', 'gid'); } - + public function testAddToGroupNotRegistered() { $this->expectException(\Exception::class); $this->expectExceptionMessage('No plugin implements addToGroup in this LDAP Backend.'); @@ -176,7 +176,7 @@ public function testRemoveFromGroup() { $pluginManager->removeFromGroup('uid', 'gid'); } - + public function testRemoveFromGroupNotRegistered() { $this->expectException(\Exception::class); $this->expectExceptionMessage('No plugin implements removeFromGroup in this LDAP Backend.'); @@ -207,7 +207,7 @@ public function testCountUsersInGroup() { $pluginManager->countUsersInGroup('gid', 'search'); } - + public function testCountUsersInGroupNotRegistered() { $this->expectException(\Exception::class); $this->expectExceptionMessage('No plugin implements countUsersInGroup in this LDAP Backend.'); @@ -237,7 +237,7 @@ public function testgetGroupDetails() { $pluginManager->getGroupDetails('gid'); } - + public function testgetGroupDetailsNotRegistered() { $this->expectException(\Exception::class); $this->expectExceptionMessage('No plugin implements getGroupDetails in this LDAP Backend.'); diff --git a/apps/user_ldap/tests/Group_LDAPTest.php b/apps/user_ldap/tests/Group_LDAPTest.php index f8327c0776c33..6204c22cb9e90 100644 --- a/apps/user_ldap/tests/Group_LDAPTest.php +++ b/apps/user_ldap/tests/Group_LDAPTest.php @@ -1092,7 +1092,7 @@ public function testDeleteGroupWithPlugin() { $pluginManager->expects($this->once()) ->method('deleteGroup') ->with('gid') - ->willReturn('result'); + ->willReturn(true); $mapper = $this->getMockBuilder(GroupMapping::class) ->setMethods(['unmap']) @@ -1108,7 +1108,7 @@ public function testDeleteGroupWithPlugin() { $ldap = new GroupLDAP($access, $pluginManager); - $this->assertEquals($ldap->deleteGroup('gid'), 'result'); + $this->assertTrue($ldap->deleteGroup('gid')); }