From d8142b6a5a01df43a6aa4586ae7cdcf372f30877 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 20 Jul 2023 12:55:24 +0200 Subject: [PATCH 01/17] Refactor user_ldap group membership to use flat DB MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move away from serialized arrays. Also use a QBMapper class for the new table. Signed-off-by: Côme Chilliet --- apps/user_ldap/appinfo/info.xml | 2 +- apps/user_ldap/lib/Db/GroupMembership.php | 48 +++++++ .../lib/Db/GroupMembershipMapper.php | 77 +++++++++++ apps/user_ldap/lib/Jobs/UpdateGroups.php | 120 +++++------------- .../Version1190Date20230706134108.php | 107 ++++++++++++++++ .../Version1190Date20230706134109.php | 46 +++++++ 6 files changed, 309 insertions(+), 91 deletions(-) create mode 100644 apps/user_ldap/lib/Db/GroupMembership.php create mode 100644 apps/user_ldap/lib/Db/GroupMembershipMapper.php create mode 100644 apps/user_ldap/lib/Migration/Version1190Date20230706134108.php create mode 100644 apps/user_ldap/lib/Migration/Version1190Date20230706134109.php diff --git a/apps/user_ldap/appinfo/info.xml b/apps/user_ldap/appinfo/info.xml index 20f439148e185..425fefe38e84d 100644 --- a/apps/user_ldap/appinfo/info.xml +++ b/apps/user_ldap/appinfo/info.xml @@ -9,7 +9,7 @@ A user logs into Nextcloud with their LDAP or AD credentials, and is granted access based on an authentication request handled by the LDAP or AD server. Nextcloud does not store LDAP or AD passwords, rather these credentials are used to authenticate a user and then Nextcloud uses a session for the user ID. More information is available in the LDAP User and Group Backend documentation. - 1.18.0 + 1.19.0 agpl Dominik Schmidt Arthur Schiwon diff --git a/apps/user_ldap/lib/Db/GroupMembership.php b/apps/user_ldap/lib/Db/GroupMembership.php new file mode 100644 index 0000000000000..25be5296d52e5 --- /dev/null +++ b/apps/user_ldap/lib/Db/GroupMembership.php @@ -0,0 +1,48 @@ + + * + * @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\Db; + +use OCP\AppFramework\Db\Entity; + +/** + * @method void setUserid(string $userid) + * @method string getUserid() + * @method void setGroupid(string $groupid) + * @method string getGroupid() + */ +class GroupMembership extends Entity { + /** @var string */ + protected $groupid; + + /** @var string */ + protected $userid; + + public function __construct() { + $this->addType('groupid', 'string'); + $this->addType('userid', 'string'); + } +} diff --git a/apps/user_ldap/lib/Db/GroupMembershipMapper.php b/apps/user_ldap/lib/Db/GroupMembershipMapper.php new file mode 100644 index 0000000000000..2c30e947cb06f --- /dev/null +++ b/apps/user_ldap/lib/Db/GroupMembershipMapper.php @@ -0,0 +1,77 @@ + + * + * @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\Db; + +use OCP\AppFramework\Db\QBMapper; +use OCP\DB\QueryBuilder\IQueryBuilder; +use OCP\IDBConnection; + +/** + * @template-extends QBMapper + */ +class GroupMembershipMapper extends QBMapper { + public function __construct(IDBConnection $db) { + parent::__construct($db, 'ldap_group_membership', GroupMembership::class); + } + + /** + * @return string[] + */ + public function getKnownGroups(): array { + $query = $this->db->getQueryBuilder(); + $result = $query->selectDistinct('groupid') + ->from($this->getTableName()) + ->executeQuery(); + + $groups = $result->fetchAll(); + $result->closeCursor(); + return $groups; + } + + /** + * @return GroupMembership[] + */ + public function findGroupMemberships(string $groupid): array { + $qb = $this->db->getQueryBuilder(); + $select = $qb->select('*') + ->from($this->getTableName()) + ->where($qb->expr()->eq('groupid', $qb->createNamedParameter($groupid))); + + return $this->findEntities($select); + } + + public function deleteGroups(array $removedGroups): void { + $query = $this->db->getQueryBuilder(); + $query->delete($this->getTableName()) + ->where($query->expr()->in('groupid', $query->createParameter('groupids'))); + + foreach (array_chunk($removedGroups, 1000) as $removedGroupsChunk) { + $query->setParameter('groupids', $removedGroupsChunk, IQueryBuilder::PARAM_STR_ARRAY); + $query->executeStatement(); + } + } +} diff --git a/apps/user_ldap/lib/Jobs/UpdateGroups.php b/apps/user_ldap/lib/Jobs/UpdateGroups.php index 48687494a6105..2c3ce457cffe3 100644 --- a/apps/user_ldap/lib/Jobs/UpdateGroups.php +++ b/apps/user_ldap/lib/Jobs/UpdateGroups.php @@ -30,9 +30,10 @@ use OCP\AppFramework\Utility\ITimeFactory; use OCP\BackgroundJob\TimedJob; +use OCP\User_LDAP\Db\GroupMembership; +use OCP\User_LDAP\Db\GroupMembershipMapper; use OCA\User_LDAP\Group_Proxy; use OCP\DB\Exception; -use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\EventDispatcher\IEventDispatcher; use OCP\Group\Events\UserAddedEvent; use OCP\Group\Events\UserRemovedEvent; @@ -46,31 +47,20 @@ class UpdateGroups extends TimedJob { /** @var ?array */ private ?array $groupsFromDB = null; - private Group_Proxy $groupBackend; - private IEventDispatcher $dispatcher; - private IGroupManager $groupManager; - private IUserManager $userManager; - private LoggerInterface $logger; - private IDBConnection $dbc; public function __construct( - Group_Proxy $groupBackend, - IEventDispatcher $dispatcher, - IGroupManager $groupManager, - IUserManager $userManager, - LoggerInterface $logger, - IDBConnection $dbc, + private Group_Proxy $groupBackend, + private IEventDispatcher $dispatcher, + private IGroupManager $groupManager, + private IUserManager $userManager, + private LoggerInterface $logger, + private IDBConnection $dbc, + private GroupMembershipMapper $groupMembershipMapper, IConfig $config, - ITimeFactory $timeFactory + ITimeFactory $timeFactory, ) { parent::__construct($timeFactory); $this->interval = (int)$config->getAppValue('user_ldap', 'bgjRefreshInterval', '3600'); - $this->groupBackend = $groupBackend; - $this->dispatcher = $dispatcher; - $this->groupManager = $groupManager; - $this->userManager = $userManager; - $this->logger = $logger; - $this->dbc = $dbc; } /** @@ -90,8 +80,7 @@ public function updateGroups(): void { ['app' => 'user_ldap'] ); - /** @var string[] $knownGroups */ - $knownGroups = array_keys($this->getKnownGroups()); + $knownGroups = $this->groupMembershipMapper->getKnownGroups(); $actualGroups = $this->groupBackend->getGroups(); if (empty($actualGroups) && empty($knownGroups)) { @@ -112,30 +101,6 @@ public function updateGroups(): void { ); } - /** - * @return array - * @throws Exception - */ - private function getKnownGroups(): array { - if (is_array($this->groupsFromDB)) { - return $this->groupsFromDB; - } - $qb = $this->dbc->getQueryBuilder(); - $qb->select(['owncloudname', 'owncloudusers']) - ->from('ldap_group_members'); - - $qResult = $qb->executeQuery(); - $result = $qResult->fetchAll(); - $qResult->closeCursor(); - - $this->groupsFromDB = []; - foreach ($result as $dataset) { - $this->groupsFromDB[$dataset['owncloudname']] = $dataset; - } - - return $this->groupsFromDB; - } - /** * @param string[] $groups * @throws Exception @@ -145,16 +110,15 @@ private function handleKnownGroups(array $groups): void { 'bgJ "updateGroups" – Dealing with known Groups.', ['app' => 'user_ldap'] ); - $qb = $this->dbc->getQueryBuilder(); - $qb->update('ldap_group_members') - ->set('owncloudusers', $qb->createParameter('members')) - ->where($qb->expr()->eq('owncloudname', $qb->createParameter('groupId'))); - $groupsFromDB = $this->getKnownGroups(); foreach ($groups as $group) { - $knownUsers = unserialize($groupsFromDB[$group]['owncloudusers']); + $groupMemberships = $this->groupMembershipMapper->findGroupMemberships($group); + $knownUsers = array_map( + fn (GroupMembership $groupMembership): string => $groupMembership->getUserid(), + $groupMemberships + ); + $groupMemberships = array_combine($knownUsers, $groupMemberships); $actualUsers = $this->groupBackend->usersInGroup($group); - $hasChanged = false; $groupObject = $this->groupManager->get($group); if ($groupObject === null) { @@ -169,6 +133,7 @@ private function handleKnownGroups(array $groups): void { continue; } foreach (array_diff($knownUsers, $actualUsers) as $removedUser) { + $this->groupMembershipMapper->delete($groupMemberships[$removedUser]); $userObject = $this->userManager->get($removedUser); if ($userObject instanceof IUser) { $this->dispatcher->dispatchTyped(new UserRemovedEvent($groupObject, $userObject)); @@ -181,9 +146,9 @@ private function handleKnownGroups(array $groups): void { 'group' => $group ] ); - $hasChanged = true; } foreach (array_diff($actualUsers, $knownUsers) as $addedUser) { + $this->groupMembershipMapper->insert(GroupMembership::fromParams(['groupid' => $group,'userid' => $addedUser])); $userObject = $this->userManager->get($addedUser); if ($userObject instanceof IUser) { $this->dispatcher->dispatchTyped(new UserAddedEvent($groupObject, $userObject)); @@ -196,14 +161,6 @@ private function handleKnownGroups(array $groups): void { 'group' => $group ] ); - $hasChanged = true; - } - if ($hasChanged) { - $qb->setParameters([ - 'members' => serialize($actualUsers), - 'groupId' => $group - ]); - $qb->executeStatement(); } } $this->logger->debug( @@ -222,21 +179,16 @@ private function handleCreatedGroups(array $createdGroups): void { ['app' => 'user_ldap'] ); - $query = $this->dbc->getQueryBuilder(); - $query->insert('ldap_group_members') - ->setValue('owncloudname', $query->createParameter('owncloudname')) - ->setValue('owncloudusers', $query->createParameter('owncloudusers')); - foreach ($createdGroups as $createdGroup) { $this->logger->info( 'bgJ "updateGroups" – new group "' . $createdGroup . '" found.', ['app' => 'user_ldap'] ); - $users = serialize($this->groupBackend->usersInGroup($createdGroup)); - $query->setParameter('owncloudname', $createdGroup) - ->setParameter('owncloudusers', $users); - $query->executeStatement(); + $users = $this->groupBackend->usersInGroup($createdGroup); + foreach ($users as $user) { + $this->groupMembershipMapper->insert(GroupMembership::fromParams(['groupid' => $createdGroup,'userid' => $user])); + } } $this->logger->debug( 'bgJ "updateGroups" – FINISHED dealing with created Groups.', @@ -254,25 +206,13 @@ private function handleRemovedGroups(array $removedGroups): void { ['app' => 'user_ldap'] ); - $query = $this->dbc->getQueryBuilder(); - $query->delete('ldap_group_members') - ->where($query->expr()->in('owncloudname', $query->createParameter('owncloudnames'))); - - foreach (array_chunk($removedGroups, 1000) as $removedGroupsChunk) { - $this->logger->info( - 'bgJ "updateGroups" – groups {removedGroups} were removed.', - [ - 'app' => 'user_ldap', - 'removedGroups' => $removedGroupsChunk - ] - ); - $query->setParameter('owncloudnames', $removedGroupsChunk, IQueryBuilder::PARAM_STR_ARRAY); - $query->executeStatement(); - } - - $this->logger->debug( - 'bgJ "updateGroups" – FINISHED dealing with removed groups.', - ['app' => 'user_ldap'] + $this->groupMembershipMapper->deleteGroups($removedGroups); + $this->logger->info( + 'bgJ "updateGroups" – groups {removedGroups} were removed.', + [ + 'app' => 'user_ldap', + 'removedGroups' => $removedGroups + ] ); } } diff --git a/apps/user_ldap/lib/Migration/Version1190Date20230706134108.php b/apps/user_ldap/lib/Migration/Version1190Date20230706134108.php new file mode 100644 index 0000000000000..cf41ba1ec93b7 --- /dev/null +++ b/apps/user_ldap/lib/Migration/Version1190Date20230706134108.php @@ -0,0 +1,107 @@ + + * + * @author Your name + * + * @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\Migration; + +use Closure; +use OCP\DB\ISchemaWrapper; +use OCP\DB\Types; +use OCP\IDBConnection; +use OCP\Migration\IOutput; +use OCP\Migration\SimpleMigrationStep; + +class Version1190Date20230706134108 extends SimpleMigrationStep { + public function __construct( + private IDBConnection $dbc, + ) { + } + + public function preSchemaChange(IOutput $output, Closure $schemaClosure, array $options): void { + } + + public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper { + /** @var ISchemaWrapper $schema */ + $schema = $schemaClosure(); + + if (!$schema->hasTable('ldap_group_membership')) { + $table = $schema->createTable('ldap_group_membership'); + $table->addColumn('groupid', Types::STRING, [ + 'notnull' => true, + 'length' => 255, + 'default' => '', + ]); + $table->addColumn('userid', Types::STRING, [ + 'notnull' => true, + 'length' => 64, + 'default' => '', + ]); + return $schema; + } else { + return null; + } + } + + public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options): void { + /** @var ISchemaWrapper $schema */ + $schema = $schemaClosure(); + + if (!$schema->hasTable('ldap_group_members')) { + // Old table does not exist + return; + } + + $output->startProgress(); + $this->copyGroupMembershipData(); + $output->finishProgress(); + } + + protected function copyGroupMembershipData(): void { + $insert = $this->dbc->getQueryBuilder(); + $insert->insert('ldap_group_membership') + ->values([ + 'userid' => $insert->createParameter('userid'), + 'groupid' => $insert->createParameter('groupid'), + ]); + + $query = $this->dbc->getQueryBuilder(); + $query->select('*') + ->from('ldap_group_members'); + + $result = $query->executeQuery(); + while ($row = $result->fetch()) { + $knownUsers = unserialize($row['owncloudusers']); + foreach ($knownUsers as $knownUser) { + $insert + ->setParameter('groupid', $row['owncloudname']) + ->setParameter('userid', $knownUser) + ; + + $insert->executeStatement(); + } + } + $result->closeCursor(); + } +} diff --git a/apps/user_ldap/lib/Migration/Version1190Date20230706134109.php b/apps/user_ldap/lib/Migration/Version1190Date20230706134109.php new file mode 100644 index 0000000000000..bc88dc2beb96e --- /dev/null +++ b/apps/user_ldap/lib/Migration/Version1190Date20230706134109.php @@ -0,0 +1,46 @@ + + * + * @author Your name + * + * @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\Migration; + +use Closure; +use OCP\DB\ISchemaWrapper; +use OCP\Migration\IOutput; +use OCP\Migration\SimpleMigrationStep; + +class Version1190Date20230706134109 extends SimpleMigrationStep { + public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper { + /** @var ISchemaWrapper $schema */ + $schema = $schemaClosure(); + + if ($schema->hasTable('ldap_group_members')) { + $schema->dropTable('ldap_group_members'); + return $schema; + } + + return null; + } +} From d9b368062edc07f665c1804dfb8b3507814f9fb3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 20 Jul 2023 14:25:17 +0200 Subject: [PATCH 02/17] Update autoloader mapping for user_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/composer/composer/autoload_classmap.php | 5 +++++ apps/user_ldap/composer/composer/autoload_static.php | 5 +++++ apps/user_ldap/composer/composer/installed.php | 4 ++-- 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/apps/user_ldap/composer/composer/autoload_classmap.php b/apps/user_ldap/composer/composer/autoload_classmap.php index c9001d3d59ad6..1e75c48530078 100644 --- a/apps/user_ldap/composer/composer/autoload_classmap.php +++ b/apps/user_ldap/composer/composer/autoload_classmap.php @@ -11,6 +11,7 @@ 'OCA\\User_LDAP\\AccessFactory' => $baseDir . '/../lib/AccessFactory.php', 'OCA\\User_LDAP\\AppInfo\\Application' => $baseDir . '/../lib/AppInfo/Application.php', 'OCA\\User_LDAP\\BackendUtility' => $baseDir . '/../lib/BackendUtility.php', + 'OCA\\User_LDAP\\Command\\CheckGroup' => $baseDir . '/../lib/Command/CheckGroup.php', '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', @@ -28,6 +29,8 @@ 'OCA\\User_LDAP\\Controller\\ConfigAPIController' => $baseDir . '/../lib/Controller/ConfigAPIController.php', 'OCA\\User_LDAP\\Controller\\RenewPasswordController' => $baseDir . '/../lib/Controller/RenewPasswordController.php', 'OCA\\User_LDAP\\DataCollector\\LdapDataCollector' => $baseDir . '/../lib/DataCollector/LdapDataCollector.php', + 'OCA\\User_LDAP\\Db\\GroupMembership' => $baseDir . '/../lib/Db/GroupMembership.php', + 'OCA\\User_LDAP\\Db\\GroupMembershipMapper' => $baseDir . '/../lib/Db/GroupMembershipMapper.php', 'OCA\\User_LDAP\\Events\\GroupBackendRegistered' => $baseDir . '/../lib/Events/GroupBackendRegistered.php', 'OCA\\User_LDAP\\Events\\UserBackendRegistered' => $baseDir . '/../lib/Events/UserBackendRegistered.php', 'OCA\\User_LDAP\\Exceptions\\AttributeNotSet' => $baseDir . '/../lib/Exceptions/AttributeNotSet.php', @@ -70,6 +73,8 @@ 'OCA\\User_LDAP\\Migration\\Version1130Date20220110154718' => $baseDir . '/../lib/Migration/Version1130Date20220110154718.php', 'OCA\\User_LDAP\\Migration\\Version1130Date20220110154719' => $baseDir . '/../lib/Migration/Version1130Date20220110154719.php', 'OCA\\User_LDAP\\Migration\\Version1141Date20220323143801' => $baseDir . '/../lib/Migration/Version1141Date20220323143801.php', + 'OCA\\User_LDAP\\Migration\\Version1190Date20230706134108' => $baseDir . '/../lib/Migration/Version1190Date20230706134108.php', + 'OCA\\User_LDAP\\Migration\\Version1190Date20230706134109' => $baseDir . '/../lib/Migration/Version1190Date20230706134109.php', 'OCA\\User_LDAP\\Notification\\Notifier' => $baseDir . '/../lib/Notification/Notifier.php', 'OCA\\User_LDAP\\PagedResults\\TLinkId' => $baseDir . '/../lib/PagedResults/TLinkId.php', 'OCA\\User_LDAP\\Proxy' => $baseDir . '/../lib/Proxy.php', diff --git a/apps/user_ldap/composer/composer/autoload_static.php b/apps/user_ldap/composer/composer/autoload_static.php index 53d7bcb648140..c488b2b0a60fd 100644 --- a/apps/user_ldap/composer/composer/autoload_static.php +++ b/apps/user_ldap/composer/composer/autoload_static.php @@ -26,6 +26,7 @@ class ComposerStaticInitUser_LDAP 'OCA\\User_LDAP\\AccessFactory' => __DIR__ . '/..' . '/../lib/AccessFactory.php', 'OCA\\User_LDAP\\AppInfo\\Application' => __DIR__ . '/..' . '/../lib/AppInfo/Application.php', 'OCA\\User_LDAP\\BackendUtility' => __DIR__ . '/..' . '/../lib/BackendUtility.php', + 'OCA\\User_LDAP\\Command\\CheckGroup' => __DIR__ . '/..' . '/../lib/Command/CheckGroup.php', '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', @@ -43,6 +44,8 @@ class ComposerStaticInitUser_LDAP 'OCA\\User_LDAP\\Controller\\ConfigAPIController' => __DIR__ . '/..' . '/../lib/Controller/ConfigAPIController.php', 'OCA\\User_LDAP\\Controller\\RenewPasswordController' => __DIR__ . '/..' . '/../lib/Controller/RenewPasswordController.php', 'OCA\\User_LDAP\\DataCollector\\LdapDataCollector' => __DIR__ . '/..' . '/../lib/DataCollector/LdapDataCollector.php', + 'OCA\\User_LDAP\\Db\\GroupMembership' => __DIR__ . '/..' . '/../lib/Db/GroupMembership.php', + 'OCA\\User_LDAP\\Db\\GroupMembershipMapper' => __DIR__ . '/..' . '/../lib/Db/GroupMembershipMapper.php', 'OCA\\User_LDAP\\Events\\GroupBackendRegistered' => __DIR__ . '/..' . '/../lib/Events/GroupBackendRegistered.php', 'OCA\\User_LDAP\\Events\\UserBackendRegistered' => __DIR__ . '/..' . '/../lib/Events/UserBackendRegistered.php', 'OCA\\User_LDAP\\Exceptions\\AttributeNotSet' => __DIR__ . '/..' . '/../lib/Exceptions/AttributeNotSet.php', @@ -85,6 +88,8 @@ class ComposerStaticInitUser_LDAP 'OCA\\User_LDAP\\Migration\\Version1130Date20220110154718' => __DIR__ . '/..' . '/../lib/Migration/Version1130Date20220110154718.php', 'OCA\\User_LDAP\\Migration\\Version1130Date20220110154719' => __DIR__ . '/..' . '/../lib/Migration/Version1130Date20220110154719.php', 'OCA\\User_LDAP\\Migration\\Version1141Date20220323143801' => __DIR__ . '/..' . '/../lib/Migration/Version1141Date20220323143801.php', + 'OCA\\User_LDAP\\Migration\\Version1190Date20230706134108' => __DIR__ . '/..' . '/../lib/Migration/Version1190Date20230706134108.php', + 'OCA\\User_LDAP\\Migration\\Version1190Date20230706134109' => __DIR__ . '/..' . '/../lib/Migration/Version1190Date20230706134109.php', 'OCA\\User_LDAP\\Notification\\Notifier' => __DIR__ . '/..' . '/../lib/Notification/Notifier.php', 'OCA\\User_LDAP\\PagedResults\\TLinkId' => __DIR__ . '/..' . '/../lib/PagedResults/TLinkId.php', 'OCA\\User_LDAP\\Proxy' => __DIR__ . '/..' . '/../lib/Proxy.php', diff --git a/apps/user_ldap/composer/composer/installed.php b/apps/user_ldap/composer/composer/installed.php index 1a66c7f2416b6..82b21976e1b24 100644 --- a/apps/user_ldap/composer/composer/installed.php +++ b/apps/user_ldap/composer/composer/installed.php @@ -3,7 +3,7 @@ 'name' => '__root__', 'pretty_version' => 'dev-master', 'version' => 'dev-master', - 'reference' => 'b1797842784b250fb01ed5e3bf130705eb94751b', + 'reference' => '0ecd81bfdcfcd878556de3485d292fb4ea340d9e', 'type' => 'library', 'install_path' => __DIR__ . '/../', 'aliases' => array(), @@ -13,7 +13,7 @@ '__root__' => array( 'pretty_version' => 'dev-master', 'version' => 'dev-master', - 'reference' => 'b1797842784b250fb01ed5e3bf130705eb94751b', + 'reference' => '0ecd81bfdcfcd878556de3485d292fb4ea340d9e', 'type' => 'library', 'install_path' => __DIR__ . '/../', 'aliases' => array(), From 34fa4138fb5c5f4e469d0b74b8773ce5d5f34609 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 20 Jul 2023 15:42:26 +0200 Subject: [PATCH 03/17] Add group events to UpdateGroups 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/Jobs/UpdateGroups.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/apps/user_ldap/lib/Jobs/UpdateGroups.php b/apps/user_ldap/lib/Jobs/UpdateGroups.php index 2c3ce457cffe3..1ae8e5fcd6b37 100644 --- a/apps/user_ldap/lib/Jobs/UpdateGroups.php +++ b/apps/user_ldap/lib/Jobs/UpdateGroups.php @@ -189,6 +189,10 @@ private function handleCreatedGroups(array $createdGroups): void { foreach ($users as $user) { $this->groupMembershipMapper->insert(GroupMembership::fromParams(['groupid' => $createdGroup,'userid' => $user])); } + $groupObject = $this->groupManager->get($group); + if ($groupObject instanceof IGroup) { + $this->dispatcher->dispatchTyped(new GroupCreatedEvent($groupObject)); + } } $this->logger->debug( 'bgJ "updateGroups" – FINISHED dealing with created Groups.', @@ -207,6 +211,9 @@ private function handleRemovedGroups(array $removedGroups): void { ); $this->groupMembershipMapper->deleteGroups($removedGroups); + + //TODO find a way to dispatch GroupDeletedEvent + $this->logger->info( 'bgJ "updateGroups" – groups {removedGroups} were removed.', [ From 2c19aac9e1452872400e09fadd5a0629c1681596 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 20 Jul 2023 16:43:50 +0200 Subject: [PATCH 04/17] Move UpdateGroups methods to a service MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- .../composer/composer/autoload_classmap.php | 1 + .../composer/composer/autoload_static.php | 1 + apps/user_ldap/lib/Jobs/UpdateGroups.php | 180 +----------------- .../lib/Service/UpdateGroupsService.php | 179 +++++++++++++++++ .../UpdateGroupsServiceTest.php} | 94 ++++----- 5 files changed, 222 insertions(+), 233 deletions(-) create mode 100644 apps/user_ldap/lib/Service/UpdateGroupsService.php rename apps/user_ldap/tests/{Jobs/UpdateGroupsTest.php => Service/UpdateGroupsServiceTest.php} (66%) diff --git a/apps/user_ldap/composer/composer/autoload_classmap.php b/apps/user_ldap/composer/composer/autoload_classmap.php index 1e75c48530078..74ffce2caff94 100644 --- a/apps/user_ldap/composer/composer/autoload_classmap.php +++ b/apps/user_ldap/composer/composer/autoload_classmap.php @@ -78,6 +78,7 @@ 'OCA\\User_LDAP\\Notification\\Notifier' => $baseDir . '/../lib/Notification/Notifier.php', 'OCA\\User_LDAP\\PagedResults\\TLinkId' => $baseDir . '/../lib/PagedResults/TLinkId.php', 'OCA\\User_LDAP\\Proxy' => $baseDir . '/../lib/Proxy.php', + 'OCA\\User_LDAP\\Service\\UpdateGroupsService' => $baseDir . '/../lib/Service/UpdateGroupsService.php', 'OCA\\User_LDAP\\Settings\\Admin' => $baseDir . '/../lib/Settings/Admin.php', 'OCA\\User_LDAP\\Settings\\Section' => $baseDir . '/../lib/Settings/Section.php', 'OCA\\User_LDAP\\UserPluginManager' => $baseDir . '/../lib/UserPluginManager.php', diff --git a/apps/user_ldap/composer/composer/autoload_static.php b/apps/user_ldap/composer/composer/autoload_static.php index c488b2b0a60fd..cead1740b88ee 100644 --- a/apps/user_ldap/composer/composer/autoload_static.php +++ b/apps/user_ldap/composer/composer/autoload_static.php @@ -93,6 +93,7 @@ class ComposerStaticInitUser_LDAP 'OCA\\User_LDAP\\Notification\\Notifier' => __DIR__ . '/..' . '/../lib/Notification/Notifier.php', 'OCA\\User_LDAP\\PagedResults\\TLinkId' => __DIR__ . '/..' . '/../lib/PagedResults/TLinkId.php', 'OCA\\User_LDAP\\Proxy' => __DIR__ . '/..' . '/../lib/Proxy.php', + 'OCA\\User_LDAP\\Service\\UpdateGroupsService' => __DIR__ . '/..' . '/../lib/Service/UpdateGroupsService.php', 'OCA\\User_LDAP\\Settings\\Admin' => __DIR__ . '/..' . '/../lib/Settings/Admin.php', 'OCA\\User_LDAP\\Settings\\Section' => __DIR__ . '/..' . '/../lib/Settings/Section.php', 'OCA\\User_LDAP\\UserPluginManager' => __DIR__ . '/..' . '/../lib/UserPluginManager.php', diff --git a/apps/user_ldap/lib/Jobs/UpdateGroups.php b/apps/user_ldap/lib/Jobs/UpdateGroups.php index 1ae8e5fcd6b37..487f36f4f30da 100644 --- a/apps/user_ldap/lib/Jobs/UpdateGroups.php +++ b/apps/user_ldap/lib/Jobs/UpdateGroups.php @@ -1,10 +1,14 @@ * @author Bart Visscher * @author Christoph Wurst + * @author Côme Chilliet * @author Joas Schilling * @author Lukas Reschke * @author Morris Jobke @@ -30,32 +34,15 @@ use OCP\AppFramework\Utility\ITimeFactory; use OCP\BackgroundJob\TimedJob; -use OCP\User_LDAP\Db\GroupMembership; -use OCP\User_LDAP\Db\GroupMembershipMapper; -use OCA\User_LDAP\Group_Proxy; use OCP\DB\Exception; -use OCP\EventDispatcher\IEventDispatcher; -use OCP\Group\Events\UserAddedEvent; -use OCP\Group\Events\UserRemovedEvent; use OCP\IConfig; -use OCP\IDBConnection; -use OCP\IGroupManager; -use OCP\IUser; -use OCP\IUserManager; +use OCA\User_LDAP\Service\UpdateGroupsService; use Psr\Log\LoggerInterface; class UpdateGroups extends TimedJob { - /** @var ?array */ - private ?array $groupsFromDB = null; - public function __construct( - private Group_Proxy $groupBackend, - private IEventDispatcher $dispatcher, - private IGroupManager $groupManager, - private IUserManager $userManager, + private UpdateGroupsService $service, private LoggerInterface $logger, - private IDBConnection $dbc, - private GroupMembershipMapper $groupMembershipMapper, IConfig $config, ITimeFactory $timeFactory, ) { @@ -68,158 +55,7 @@ public function __construct( * @throws Exception */ public function run($argument): void { - $this->updateGroups(); - } - - /** - * @throws Exception - */ - public function updateGroups(): void { - $this->logger->debug( - 'Run background job "updateGroups"', - ['app' => 'user_ldap'] - ); - - $knownGroups = $this->groupMembershipMapper->getKnownGroups(); - $actualGroups = $this->groupBackend->getGroups(); - - if (empty($actualGroups) && empty($knownGroups)) { - $this->logger->info( - 'bgJ "updateGroups" – groups do not seem to be configured properly, aborting.', - ['app' => 'user_ldap'] - ); - return; - } - - $this->handleKnownGroups(array_intersect($actualGroups, $knownGroups)); - $this->handleCreatedGroups(array_diff($actualGroups, $knownGroups)); - $this->handleRemovedGroups(array_diff($knownGroups, $actualGroups)); - - $this->logger->debug( - 'bgJ "updateGroups" – Finished.', - ['app' => 'user_ldap'] - ); - } - - /** - * @param string[] $groups - * @throws Exception - */ - private function handleKnownGroups(array $groups): void { - $this->logger->debug( - 'bgJ "updateGroups" – Dealing with known Groups.', - ['app' => 'user_ldap'] - ); - - foreach ($groups as $group) { - $groupMemberships = $this->groupMembershipMapper->findGroupMemberships($group); - $knownUsers = array_map( - fn (GroupMembership $groupMembership): string => $groupMembership->getUserid(), - $groupMemberships - ); - $groupMemberships = array_combine($knownUsers, $groupMemberships); - $actualUsers = $this->groupBackend->usersInGroup($group); - - $groupObject = $this->groupManager->get($group); - if ($groupObject === null) { - /* We are not expecting the group to not be found since it was returned by $this->groupBackend->getGroups() */ - $this->logger->error( - 'bgJ "updateGroups" – Failed to get group {group} for update', - [ - 'app' => 'user_ldap', - 'group' => $group - ] - ); - continue; - } - foreach (array_diff($knownUsers, $actualUsers) as $removedUser) { - $this->groupMembershipMapper->delete($groupMemberships[$removedUser]); - $userObject = $this->userManager->get($removedUser); - if ($userObject instanceof IUser) { - $this->dispatcher->dispatchTyped(new UserRemovedEvent($groupObject, $userObject)); - } - $this->logger->info( - 'bgJ "updateGroups" – {user} removed from {group}', - [ - 'app' => 'user_ldap', - 'user' => $removedUser, - 'group' => $group - ] - ); - } - foreach (array_diff($actualUsers, $knownUsers) as $addedUser) { - $this->groupMembershipMapper->insert(GroupMembership::fromParams(['groupid' => $group,'userid' => $addedUser])); - $userObject = $this->userManager->get($addedUser); - if ($userObject instanceof IUser) { - $this->dispatcher->dispatchTyped(new UserAddedEvent($groupObject, $userObject)); - } - $this->logger->info( - 'bgJ "updateGroups" – {user} added to {group}', - [ - 'app' => 'user_ldap', - 'user' => $addedUser, - 'group' => $group - ] - ); - } - } - $this->logger->debug( - 'bgJ "updateGroups" – FINISHED dealing with known Groups.', - ['app' => 'user_ldap'] - ); - } - - /** - * @param string[] $createdGroups - * @throws Exception - */ - private function handleCreatedGroups(array $createdGroups): void { - $this->logger->debug( - 'bgJ "updateGroups" – dealing with created Groups.', - ['app' => 'user_ldap'] - ); - - foreach ($createdGroups as $createdGroup) { - $this->logger->info( - 'bgJ "updateGroups" – new group "' . $createdGroup . '" found.', - ['app' => 'user_ldap'] - ); - - $users = $this->groupBackend->usersInGroup($createdGroup); - foreach ($users as $user) { - $this->groupMembershipMapper->insert(GroupMembership::fromParams(['groupid' => $createdGroup,'userid' => $user])); - } - $groupObject = $this->groupManager->get($group); - if ($groupObject instanceof IGroup) { - $this->dispatcher->dispatchTyped(new GroupCreatedEvent($groupObject)); - } - } - $this->logger->debug( - 'bgJ "updateGroups" – FINISHED dealing with created Groups.', - ['app' => 'user_ldap'] - ); - } - - /** - * @param string[] $removedGroups - * @throws Exception - */ - private function handleRemovedGroups(array $removedGroups): void { - $this->logger->debug( - 'bgJ "updateGroups" – dealing with removed groups.', - ['app' => 'user_ldap'] - ); - - $this->groupMembershipMapper->deleteGroups($removedGroups); - - //TODO find a way to dispatch GroupDeletedEvent - - $this->logger->info( - 'bgJ "updateGroups" – groups {removedGroups} were removed.', - [ - 'app' => 'user_ldap', - 'removedGroups' => $removedGroups - ] - ); + $this->logger->debug('Run background job "updateGroups"'); + $this->service->updateGroups(); } } diff --git a/apps/user_ldap/lib/Service/UpdateGroupsService.php b/apps/user_ldap/lib/Service/UpdateGroupsService.php new file mode 100644 index 0000000000000..aaa86435695f9 --- /dev/null +++ b/apps/user_ldap/lib/Service/UpdateGroupsService.php @@ -0,0 +1,179 @@ + + * @author Bart Visscher + * @author Christoph Wurst + * @author Côme Chilliet + * @author Joas Schilling + * @author Lukas Reschke + * @author Morris Jobke + * @author Robin Appelman + * @author Robin McCorkell + * + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * 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, version 3, + * along with this program. If not, see + * + */ + +namespace OCA\User_LDAP\Service; + +use OCA\User_LDAP\Db\GroupMembership; +use OCA\User_LDAP\Db\GroupMembershipMapper; +use OCA\User_LDAP\Group_Proxy; +use OCP\DB\Exception; +use OCP\EventDispatcher\IEventDispatcher; +use OCP\Group\Events\UserAddedEvent; +use OCP\Group\Events\UserRemovedEvent; +use OCP\IGroupManager; +use OCP\IUser; +use OCP\IUserManager; +use Psr\Log\LoggerInterface; + +class UpdateGroupsService { + public function __construct( + private Group_Proxy $groupBackend, + private IEventDispatcher $dispatcher, + private IGroupManager $groupManager, + private IUserManager $userManager, + private LoggerInterface $logger, + private GroupMembershipMapper $groupMembershipMapper, + ) { + } + + /** + * @throws Exception + */ + public function updateGroups(): void { + $knownGroups = $this->groupMembershipMapper->getKnownGroups(); + $actualGroups = $this->groupBackend->getGroups(); + + if (empty($actualGroups) && empty($knownGroups)) { + $this->logger->info( + 'service "updateGroups" – groups do not seem to be configured properly, aborting.', + ); + return; + } + + $this->service->handleKnownGroups(array_intersect($actualGroups, $knownGroups)); + $this->service->handleCreatedGroups(array_diff($actualGroups, $knownGroups)); + $this->service->handleRemovedGroups(array_diff($knownGroups, $actualGroups)); + + $this->logger->debug('service "updateGroups" – Finished.'); + } + + /** + * @param string[] $groups + * @throws Exception + */ + public function handleKnownGroups(array $groups): void { + $this->logger->debug('service "updateGroups" – Dealing with known Groups.'); + + foreach ($groups as $group) { + $groupMemberships = $this->groupMembershipMapper->findGroupMemberships($group); + $knownUsers = array_map( + fn (GroupMembership $groupMembership): string => $groupMembership->getUserid(), + $groupMemberships + ); + $groupMemberships = array_combine($knownUsers, $groupMemberships); + $actualUsers = $this->groupBackend->usersInGroup($group); + + $groupObject = $this->groupManager->get($group); + if ($groupObject === null) { + /* We are not expecting the group to not be found since it was returned by $this->groupBackend->getGroups() */ + $this->logger->error( + 'service "updateGroups" – Failed to get group {group} for update', + [ + 'group' => $group + ] + ); + continue; + } + foreach (array_diff($knownUsers, $actualUsers) as $removedUser) { + $this->groupMembershipMapper->delete($groupMemberships[$removedUser]); + $userObject = $this->userManager->get($removedUser); + if ($userObject instanceof IUser) { + $this->dispatcher->dispatchTyped(new UserRemovedEvent($groupObject, $userObject)); + } + $this->logger->info( + 'service "updateGroups" – {user} removed from {group}', + [ + 'user' => $removedUser, + 'group' => $group + ] + ); + } + foreach (array_diff($actualUsers, $knownUsers) as $addedUser) { + $this->groupMembershipMapper->insert(GroupMembership::fromParams(['groupid' => $group,'userid' => $addedUser])); + $userObject = $this->userManager->get($addedUser); + if ($userObject instanceof IUser) { + $this->dispatcher->dispatchTyped(new UserAddedEvent($groupObject, $userObject)); + } + $this->logger->info( + 'service "updateGroups" – {user} added to {group}', + [ + 'user' => $addedUser, + 'group' => $group + ] + ); + } + } + $this->logger->debug('service "updateGroups" – FINISHED dealing with known Groups.'); + } + + /** + * @param string[] $createdGroups + * @throws Exception + */ + public function handleCreatedGroups(array $createdGroups): void { + $this->logger->debug('service "updateGroups" – dealing with created Groups.'); + + foreach ($createdGroups as $createdGroup) { + $this->logger->info('service "updateGroups" – new group "' . $createdGroup . '" found.'); + + $users = $this->groupBackend->usersInGroup($createdGroup); + foreach ($users as $user) { + $this->groupMembershipMapper->insert(GroupMembership::fromParams(['groupid' => $createdGroup,'userid' => $user])); + } + $groupObject = $this->groupManager->get($group); + if ($groupObject instanceof IGroup) { + $this->dispatcher->dispatchTyped(new GroupCreatedEvent($groupObject)); + } + } + $this->logger->debug('service "updateGroups" – FINISHED dealing with created Groups.'); + } + + /** + * @param string[] $removedGroups + * @throws Exception + */ + public function handleRemovedGroups(array $removedGroups): void { + $this->logger->debug('service "updateGroups" – dealing with removed groups.'); + + $this->groupMembershipMapper->deleteGroups($removedGroups); + + //TODO find a way to dispatch GroupDeletedEvent + + $this->logger->info( + 'service "updateGroups" – groups {removedGroups} were removed.', + [ + 'removedGroups' => $removedGroups + ] + ); + } +} diff --git a/apps/user_ldap/tests/Jobs/UpdateGroupsTest.php b/apps/user_ldap/tests/Service/UpdateGroupsServiceTest.php similarity index 66% rename from apps/user_ldap/tests/Jobs/UpdateGroupsTest.php rename to apps/user_ldap/tests/Service/UpdateGroupsServiceTest.php index 22a02451e277c..abf9c57c9c1fe 100644 --- a/apps/user_ldap/tests/Jobs/UpdateGroupsTest.php +++ b/apps/user_ldap/tests/Service/UpdateGroupsServiceTest.php @@ -6,6 +6,7 @@ * @copyright Copyright (c) 2020 Arthur Schiwon * * @author Arthur Schiwon + * @author Côme Chilliet * @author Joas Schilling * * @license GNU AGPL version 3 or any later version @@ -24,19 +25,17 @@ * along with this program. If not, see . * */ -namespace OCA\user_ldap\tests\Jobs; +namespace OCA\user_ldap\tests\Service; +use OCA\User_LDAP\Db\GroupMembership; +use OCA\User_LDAP\Db\GroupMembershipMapper; use OCA\User_LDAP\Group_Proxy; -use OCA\User_LDAP\Jobs\UpdateGroups; +use OCA\User_LDAP\Service\UpdateGroupsService; use OCP\AppFramework\Utility\ITimeFactory; -use OCP\DB\IResult; -use OCP\DB\QueryBuilder\IExpressionBuilder; -use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\EventDispatcher\IEventDispatcher; use OCP\Group\Events\UserAddedEvent; use OCP\Group\Events\UserRemovedEvent; use OCP\IConfig; -use OCP\IDBConnection; use OCP\IGroup; use OCP\IGroupManager; use OCP\IUser; @@ -44,10 +43,8 @@ use PHPUnit\Framework\MockObject\MockObject; use Psr\Log\LoggerInterface; use Test\TestCase; -use function serialize; - -class UpdateGroupsTest extends TestCase { +class UpdateGroupsServiceTest extends TestCase { /** @var Group_Proxy|MockObject */ protected $groupBackend; /** @var IEventDispatcher|MockObject */ @@ -58,14 +55,14 @@ class UpdateGroupsTest extends TestCase { protected $userManager; /** @var LoggerInterface|MockObject */ protected $logger; - /** @var IDBConnection|MockObject */ - protected $dbc; + /** @var GroupMembershipMapper|MockObject */ + protected $groupMembershipMapper; /** @var IConfig|MockObject */ protected $config; /** @var ITimeFactory|MockObject */ protected $timeFactory; - protected UpdateGroups $updateGroupsJob; + protected UpdateGroupsService $updateGroupsService; public function setUp(): void { $this->groupBackend = $this->createMock(Group_Proxy::class); @@ -73,17 +70,17 @@ public function setUp(): void { $this->groupManager = $this->createMock(IGroupManager::class); $this->userManager = $this->createMock(IUserManager::class); $this->logger = $this->createMock(LoggerInterface::class); - $this->dbc = $this->createMock(IDBConnection::class); + $this->groupMembershipMapper = $this->createMock(GroupMembershipMapper::class); $this->config = $this->createMock(IConfig::class); $this->timeFactory = $this->createMock(ITimeFactory::class); - $this->updateGroupsJob = new UpdateGroups( + $this->updateGroupsService = new UpdateGroupsService( $this->groupBackend, $this->dispatcher, $this->groupManager, $this->userManager, $this->logger, - $this->dbc, + $this->groupMembershipMapper, $this->config, $this->timeFactory ); @@ -91,12 +88,12 @@ public function setUp(): void { public function testHandleKnownGroups(): void { $knownGroups = [ - 'emptyGroup' => serialize([]), - 'stableGroup' => serialize(['userA', 'userC', 'userE']), - 'groupWithAdditions' => serialize(['userA', 'userC', 'userE']), - 'groupWithRemovals' => serialize(['userA', 'userC', 'userDeleted', 'userE']), - 'groupWithAdditionsAndRemovals' => serialize(['userA', 'userC', 'userE']), - 'vanishedGroup' => serialize(['userB', 'userDeleted']) + 'emptyGroup' => [], + 'stableGroup' => ['userA', 'userC', 'userE'], + 'groupWithAdditions' => ['userA', 'userC', 'userE'], + 'groupWithRemovals' => ['userA', 'userC', 'userDeleted', 'userE'], + 'groupWithAdditionsAndRemovals' => ['userA', 'userC', 'userE'], + 'vanishedGroup' => ['userB', 'userDeleted'], ]; $knownGroupsDB = []; foreach ($knownGroups as $gid => $members) { @@ -115,45 +112,20 @@ public function testHandleKnownGroups(): void { ]; $groups = array_intersect(array_keys($knownGroups), array_keys($actualGroups)); - /** @var IQueryBuilder|MockObject $updateQb */ - $updateQb = $this->createMock(IQueryBuilder::class); - $updateQb->expects($this->once()) - ->method('update') - ->willReturn($updateQb); - $updateQb->expects($this->once()) - ->method('set') - ->willReturn($updateQb); - $updateQb->expects($this->once()) - ->method('where') - ->willReturn($updateQb); - // three groups need to be updated - $updateQb->expects($this->exactly(3)) - ->method('setParameters'); - $updateQb->expects($this->exactly(3)) - ->method('executeStatement'); - $updateQb->expects($this->any()) - ->method('expr') - ->willReturn($this->createMock(IExpressionBuilder::class)); - - $stmt = $this->createMock(IResult::class); - $stmt->expects($this->once()) - ->method('fetchAll') - ->willReturn($knownGroupsDB); - - $selectQb = $this->createMock(IQueryBuilder::class); - $selectQb->expects($this->once()) - ->method('select') - ->willReturn($selectQb); - $selectQb->expects($this->once()) - ->method('from') - ->willReturn($selectQb); - $selectQb->expects($this->once()) - ->method('executeQuery') - ->willReturn($stmt); - - $this->dbc->expects($this->any()) - ->method('getQueryBuilder') - ->willReturnOnConsecutiveCalls($updateQb, $selectQb); + $this->groupMembershipMapper->expects($this->never()) + ->method('getKnownGroups'); + $this->groupMembershipMapper->expects($this->exactly(5)) + ->method('findGroupMemberships') + ->willReturnCallback( + fn ($group) => array_map( + fn ($userid) => GroupMembership::fromParams(['groupid' => $group,'userid' => $userid]), + $knownGroups[$group] + ) + ); + $this->groupMembershipMapper->expects($this->exactly(3)) + ->method('delete'); + $this->groupMembershipMapper->expects($this->exactly(2)) + ->method('insert'); $this->groupBackend->expects($this->any()) ->method('usersInGroup') @@ -192,7 +164,7 @@ public function testHandleKnownGroups(): void { } }); - $this->invokePrivate($this->updateGroupsJob, 'handleKnownGroups', [$groups]); + $this->updateGroupsService->handleKnownGroups($groups); $this->assertSame(2, $removedEvents); $this->assertSame(2, $addedEvents); From f9ed48eab93884b3b76ecd254d7c79a716bbcf24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 20 Jul 2023 16:44:25 +0200 Subject: [PATCH 05/17] Add check-group command 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 + apps/user_ldap/lib/Command/CheckGroup.php | 142 ++++++++++++++++++ apps/user_ldap/lib/Group_LDAP.php | 33 +++- apps/user_ldap/lib/Group_Proxy.php | 18 +++ .../lib/Service/UpdateGroupsService.php | 15 +- 5 files changed, 199 insertions(+), 10 deletions(-) create mode 100644 apps/user_ldap/lib/Command/CheckGroup.php diff --git a/apps/user_ldap/appinfo/info.xml b/apps/user_ldap/appinfo/info.xml index 425fefe38e84d..e96d1b40a0213 100644 --- a/apps/user_ldap/appinfo/info.xml +++ b/apps/user_ldap/appinfo/info.xml @@ -48,6 +48,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\CheckGroup OCA\User_LDAP\Command\CreateEmptyConfig OCA\User_LDAP\Command\DeleteConfig OCA\User_LDAP\Command\ResetGroup diff --git a/apps/user_ldap/lib/Command/CheckGroup.php b/apps/user_ldap/lib/Command/CheckGroup.php new file mode 100644 index 0000000000000..8617b06c57f10 --- /dev/null +++ b/apps/user_ldap/lib/Command/CheckGroup.php @@ -0,0 +1,142 @@ + + * @author Christoph Wurst + * @author Côme Chilliet + * @author Joas Schilling + * @author Morris Jobke + * @author Roeland Jago Douma + * + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * 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, version 3, + * along with this program. If not, see + * + */ + +namespace OCA\User_LDAP\Command; + +use OCA\User_LDAP\Helper; +use OCA\User_LDAP\Mapping\GroupMapping; +use OCA\User_LDAP\Group_Proxy; +use OCA\User_LDAP\Service\UpdateGroupsService; +use Symfony\Component\Console\Command\Command; +use Symfony\Component\Console\Input\InputArgument; +use Symfony\Component\Console\Input\InputInterface; +use Symfony\Component\Console\Input\InputOption; +use Symfony\Component\Console\Output\OutputInterface; + +class CheckGroup extends Command { + public function __construct( + private UpdateGroupsService $service, + protected Group_Proxy $backend, + protected Helper $helper, + protected GroupMapping $mapping, + ) { + parent::__construct(); + } + + protected function configure(): void { + $this + ->setName('ldap:check-group') + ->setDescription('checks whether a group exists on LDAP.') + ->addArgument( + 'ocName', + InputArgument::REQUIRED, + 'the group name as used in Nextcloud, or the LDAP DN' + ) + ->addOption( + 'force', + null, + InputOption::VALUE_NONE, + 'ignores disabled LDAP configuration' + ) + ->addOption( + 'update', + null, + InputOption::VALUE_NONE, + 'syncs values from LDAP' + ) + ; + } + + protected function execute(InputInterface $input, OutputInterface $output): int { + try { + $this->assertAllowed($input->getOption('force')); + $gid = $input->getArgument('ocName'); + if ($this->backend->getLDAPAccess($gid)->stringResemblesDN($gid)) { + $groupname = $this->backend->dn2GroupName($gid); + if ($groupname !== false) { + $gid = $groupname; + } + } + $wasMapped = $this->groupWasMapped($gid); + $exists = $this->backend->groupExistsOnLDAP($gid, true); + if ($exists === true) { + $output->writeln('The group is still available on LDAP.'); + if ($input->getOption('update')) { + $this->updateGroup($gid, $output, $wasMapped); + } + return 0; + } elseif ($wasMapped) { + $output->writeln('The group does not exists on LDAP anymore.'); + return 0; + } else { + throw new \Exception('The given group is not a recognized LDAP group.'); + } + } catch (\Exception $e) { + $output->writeln('' . $e->getMessage(). ''); + return 1; + } + } + + /** + * checks whether a group is actually mapped + * @param string $ocName the groupname as used in Nextcloud + */ + protected function groupWasMapped(string $ocName): bool { + $dn = $this->mapping->getDNByName($ocName); + return $dn !== false; + } + + /** + * checks whether the setup allows reliable checking of LDAP group existence + * @throws \Exception + */ + protected function assertAllowed(bool $force): void { + if ($this->helper->haveDisabledConfigurations() && !$force) { + throw new \Exception('Cannot check group existence, because ' + . 'disabled LDAP configurations are present.'); + } + + // we don't check ldapUserCleanupInterval from config.php because this + // action is triggered manually, while the setting only controls the + // background job. + } + + private function updateGroup(string $gid, OutputInterface $output, bool $wasMapped): void { + try { + if ($wasMapped) { + $this->service->handleKnownGroups([$gid]); + } else { + $this->service->handleCreatedGroups([$gid]); + } + } catch (\Exception $e) { + $output->writeln('Error while trying to lookup and update attributes from LDAP'); + } + } +} diff --git a/apps/user_ldap/lib/Group_LDAP.php b/apps/user_ldap/lib/Group_LDAP.php index 84267171d3749..3a17ba70bb1a8 100644 --- a/apps/user_ldap/lib/Group_LDAP.php +++ b/apps/user_ldap/lib/Group_LDAP.php @@ -1105,31 +1105,43 @@ public function getGroups($search = '', $limit = -1, $offset = 0) { * @throws ServerNotAvailableException */ public function groupExists($gid) { - $groupExists = $this->access->connection->getFromCache('groupExists' . $gid); - if (!is_null($groupExists)) { - return (bool)$groupExists; + return $this->groupExistsOnLDAP($gid, false); + } + + /** + * Check if a group exists + * + * @throws ServerNotAvailableException + */ + public function groupExistsOnLDAP(string $gid, bool $ignoreCache = false): bool { + $cacheKey = 'groupExists' . $gid; + if (!$ignoreCache) { + $groupExists = $this->access->connection->getFromCache($cacheKey); + if (!is_null($groupExists)) { + return (bool)$groupExists; + } } //getting dn, if false the group does not exist. If dn, it may be mapped //only, requires more checking. $dn = $this->access->groupname2dn($gid); if (!$dn) { - $this->access->connection->writeToCache('groupExists' . $gid, false); + $this->access->connection->writeToCache($cacheKey, false); return false; } if (!$this->access->isDNPartOfBase($dn, $this->access->connection->ldapBaseGroups)) { - $this->access->connection->writeToCache('groupExists' . $gid, false); + $this->access->connection->writeToCache($cacheKey, false); return false; } //if group really still exists, we will be able to read its objectClass if (!is_array($this->access->readAttribute($dn, '', $this->access->connection->ldapGroupFilter))) { - $this->access->connection->writeToCache('groupExists' . $gid, false); + $this->access->connection->writeToCache($cacheKey, false); return false; } - $this->access->connection->writeToCache('groupExists' . $gid, true); + $this->access->connection->writeToCache($cacheKey, true); return true; } @@ -1336,4 +1348,11 @@ public function getDisplayName(string $gid): string { $this->access->connection->writeToCache($cacheKey, $displayName); return $displayName; } + + /** + * returns the groupname for the given LDAP DN, if available + */ + public function dn2GroupName(string $dn): string|false { + return $this->access->dn2groupname($dn); + } } diff --git a/apps/user_ldap/lib/Group_Proxy.php b/apps/user_ldap/lib/Group_Proxy.php index 5f8d0562fd918..9d996b45f4330 100644 --- a/apps/user_ldap/lib/Group_Proxy.php +++ b/apps/user_ldap/lib/Group_Proxy.php @@ -28,6 +28,7 @@ */ namespace OCA\User_LDAP; +use OC\ServerNotAvailableException; use OCP\Group\Backend\IDeleteGroupBackend; use OCP\Group\Backend\IGetDisplayNameBackend; use OCP\Group\Backend\INamedBackend; @@ -286,6 +287,23 @@ public function groupExists($gid) { return $this->handleRequest($gid, 'groupExists', [$gid]); } + /** + * Check if a group exists + * + * @throws ServerNotAvailableException + */ + public function groupExistsOnLDAP(string $gid, bool $ignoreCache = false): bool { + return $this->handleRequest($gid, 'groupExistsOnLDAP', [$gid, $ignoreCache]); + } + + /** + * returns the groupname for the given LDAP DN, if available + */ + public function dn2GroupName(string $dn): string|false { + $id = 'DN,' . $dn; + return $this->handleRequest($id, 'dn2GroupName', [$dn]); + } + /** * Check if backend implements actions * diff --git a/apps/user_ldap/lib/Service/UpdateGroupsService.php b/apps/user_ldap/lib/Service/UpdateGroupsService.php index aaa86435695f9..ba564819d01a1 100644 --- a/apps/user_ldap/lib/Service/UpdateGroupsService.php +++ b/apps/user_ldap/lib/Service/UpdateGroupsService.php @@ -38,8 +38,10 @@ use OCA\User_LDAP\Group_Proxy; use OCP\DB\Exception; use OCP\EventDispatcher\IEventDispatcher; +use OCP\Group\Events\GroupCreatedEvent; use OCP\Group\Events\UserAddedEvent; use OCP\Group\Events\UserRemovedEvent; +use OCP\IGroup; use OCP\IGroupManager; use OCP\IUser; use OCP\IUserManager; @@ -85,6 +87,7 @@ public function handleKnownGroups(array $groups): void { $this->logger->debug('service "updateGroups" – Dealing with known Groups.'); foreach ($groups as $group) { + $this->logger->debug('service "updateGroups" – Dealing with {group}.', ['group' => $group]); $groupMemberships = $this->groupMembershipMapper->findGroupMemberships($group); $knownUsers = array_map( fn (GroupMembership $groupMembership): string => $groupMembership->getUserid(), @@ -147,13 +150,19 @@ public function handleCreatedGroups(array $createdGroups): void { $this->logger->info('service "updateGroups" – new group "' . $createdGroup . '" found.'); $users = $this->groupBackend->usersInGroup($createdGroup); - foreach ($users as $user) { - $this->groupMembershipMapper->insert(GroupMembership::fromParams(['groupid' => $createdGroup,'userid' => $user])); - } $groupObject = $this->groupManager->get($group); if ($groupObject instanceof IGroup) { $this->dispatcher->dispatchTyped(new GroupCreatedEvent($groupObject)); } + foreach ($users as $user) { + $this->groupMembershipMapper->insert(GroupMembership::fromParams(['groupid' => $createdGroup,'userid' => $user])); + if ($groupObject instanceof IGroup) { + $userObject = $this->userManager->get($user); + if ($userObject instanceof IUser) { + $this->dispatcher->dispatchTyped(new UserAddedEvent($groupObject, $userObject)); + } + } + } } $this->logger->debug('service "updateGroups" – FINISHED dealing with created Groups.'); } From ec13f22ae317b65db4bf84726c90e3fc6b7a2a0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 20 Jul 2023 17:45:57 +0200 Subject: [PATCH 06/17] Add missing primary key for ldap_group_membership MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- .../lib/Migration/Version1190Date20230706134108.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/apps/user_ldap/lib/Migration/Version1190Date20230706134108.php b/apps/user_ldap/lib/Migration/Version1190Date20230706134108.php index cf41ba1ec93b7..c6bd795f3e649 100644 --- a/apps/user_ldap/lib/Migration/Version1190Date20230706134108.php +++ b/apps/user_ldap/lib/Migration/Version1190Date20230706134108.php @@ -48,6 +48,11 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt if (!$schema->hasTable('ldap_group_membership')) { $table = $schema->createTable('ldap_group_membership'); + $table->addColumn('id', 'integer', [ + 'autoincrement' => true, + 'notnull' => true, + 'length' => 4, + ]); $table->addColumn('groupid', Types::STRING, [ 'notnull' => true, 'length' => 255, @@ -58,6 +63,7 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt 'length' => 64, 'default' => '', ]); + $table->setPrimaryKey(['id']); return $schema; } else { return null; From 1b102ca9b3c0e32752599b8df373a11af6f40df0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 20 Jul 2023 17:55:22 +0200 Subject: [PATCH 07/17] Fix errors in UpdateGroupsService.php 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/Service/UpdateGroupsService.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/apps/user_ldap/lib/Service/UpdateGroupsService.php b/apps/user_ldap/lib/Service/UpdateGroupsService.php index ba564819d01a1..5e463734079ff 100644 --- a/apps/user_ldap/lib/Service/UpdateGroupsService.php +++ b/apps/user_ldap/lib/Service/UpdateGroupsService.php @@ -72,9 +72,9 @@ public function updateGroups(): void { return; } - $this->service->handleKnownGroups(array_intersect($actualGroups, $knownGroups)); - $this->service->handleCreatedGroups(array_diff($actualGroups, $knownGroups)); - $this->service->handleRemovedGroups(array_diff($knownGroups, $actualGroups)); + $this->handleKnownGroups(array_intersect($actualGroups, $knownGroups)); + $this->handleCreatedGroups(array_diff($actualGroups, $knownGroups)); + $this->handleRemovedGroups(array_diff($knownGroups, $actualGroups)); $this->logger->debug('service "updateGroups" – Finished.'); } @@ -150,7 +150,7 @@ public function handleCreatedGroups(array $createdGroups): void { $this->logger->info('service "updateGroups" – new group "' . $createdGroup . '" found.'); $users = $this->groupBackend->usersInGroup($createdGroup); - $groupObject = $this->groupManager->get($group); + $groupObject = $this->groupManager->get($createdGroup); if ($groupObject instanceof IGroup) { $this->dispatcher->dispatchTyped(new GroupCreatedEvent($groupObject)); } From ce5a4e5b6696efffa911c15cc7e2789cf504ad2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 1 Aug 2023 11:41:56 +0200 Subject: [PATCH 08/17] Always empty cache before updating a group 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/CheckGroup.php | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/user_ldap/lib/Command/CheckGroup.php b/apps/user_ldap/lib/Command/CheckGroup.php index 8617b06c57f10..91faa11888cce 100644 --- a/apps/user_ldap/lib/Command/CheckGroup.php +++ b/apps/user_ldap/lib/Command/CheckGroup.php @@ -89,6 +89,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int if ($exists === true) { $output->writeln('The group is still available on LDAP.'); if ($input->getOption('update')) { + $this->backend->getLDAPAccess($gid)->connection->clearCache(); $this->updateGroup($gid, $output, $wasMapped); } return 0; From 7a14aa748e94b49c954a55d2a5815f4eef2e3ea8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 1 Aug 2023 12:22:40 +0200 Subject: [PATCH 09/17] Add output to check-group --update command 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/CheckGroup.php | 26 ++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/apps/user_ldap/lib/Command/CheckGroup.php b/apps/user_ldap/lib/Command/CheckGroup.php index 91faa11888cce..48fe631a09bf8 100644 --- a/apps/user_ldap/lib/Command/CheckGroup.php +++ b/apps/user_ldap/lib/Command/CheckGroup.php @@ -30,10 +30,14 @@ namespace OCA\User_LDAP\Command; +use OCA\User_LDAP\Group_Proxy; use OCA\User_LDAP\Helper; use OCA\User_LDAP\Mapping\GroupMapping; -use OCA\User_LDAP\Group_Proxy; use OCA\User_LDAP\Service\UpdateGroupsService; +use OCP\EventDispatcher\IEventDispatcher; +use OCP\Group\Events\GroupCreatedEvent; +use OCP\Group\Events\UserAddedEvent; +use OCP\Group\Events\UserRemovedEvent; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; @@ -46,6 +50,7 @@ public function __construct( protected Group_Proxy $backend, protected Helper $helper, protected GroupMapping $mapping, + protected IEventDispatcher $dispatcher, ) { parent::__construct(); } @@ -75,6 +80,9 @@ protected function configure(): void { } protected function execute(InputInterface $input, OutputInterface $output): int { + $this->dispatcher->addListener(GroupCreatedEvent::class, fn ($event) => $this->onGroupCreatedEvent($event, $output)); + $this->dispatcher->addListener(UserAddedEvent::class, fn ($event) => $this->onUserAddedEvent($event, $output)); + $this->dispatcher->addListener(UserRemovedEvent::class, fn ($event) => $this->onUserRemovedEvent($event, $output)); try { $this->assertAllowed($input->getOption('force')); $gid = $input->getArgument('ocName'); @@ -105,6 +113,22 @@ protected function execute(InputInterface $input, OutputInterface $output): int } } + public function onGroupCreatedEvent(GroupChangedEvent $event, OutputInterface $output): void { + $output->writeln('The group '.$event->getGroup()->getGID().' was added to Nextcloud with '.$event->getGroup()->count().' users'); + } + + public function onUserAddedEvent(UserAddedEvent $event, OutputInterface $output): void { + $user = $event->getUser(); + $group = $event->getGroup(); + $output->writeln('The user '.$user->getUID().' was added to group '.$group->getGID().''); + } + + public function onUserRemovedEvent(UserRemovedEvent $event, OutputInterface $output): void { + $user = $event->getUser(); + $group = $event->getGroup(); + $output->writeln('The user '.$user->getUID().' was removed from group '.$group->getGID().''); + } + /** * checks whether a group is actually mapped * @param string $ocName the groupname as used in Nextcloud From 42448c0d7898412261a8fc78a26e8450a8cd6ee5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 3 Aug 2023 13:54:16 +0200 Subject: [PATCH 10/17] Fix getKnownGroups return value 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/Db/GroupMembershipMapper.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/user_ldap/lib/Db/GroupMembershipMapper.php b/apps/user_ldap/lib/Db/GroupMembershipMapper.php index 2c30e947cb06f..0868f60d2c50c 100644 --- a/apps/user_ldap/lib/Db/GroupMembershipMapper.php +++ b/apps/user_ldap/lib/Db/GroupMembershipMapper.php @@ -47,7 +47,7 @@ public function getKnownGroups(): array { ->from($this->getTableName()) ->executeQuery(); - $groups = $result->fetchAll(); + $groups = array_map(fn ($row) => $row['groupid'], $result->fetchAll()); $result->closeCursor(); return $groups; } From 1026b2131c53d074cf71e8e3ca4766b0a1b4ead3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 3 Aug 2023 13:56:14 +0200 Subject: [PATCH 11/17] Fix check-group command for new groups 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/CheckGroup.php | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/apps/user_ldap/lib/Command/CheckGroup.php b/apps/user_ldap/lib/Command/CheckGroup.php index 48fe631a09bf8..d55b846020a0a 100644 --- a/apps/user_ldap/lib/Command/CheckGroup.php +++ b/apps/user_ldap/lib/Command/CheckGroup.php @@ -86,13 +86,15 @@ protected function execute(InputInterface $input, OutputInterface $output): int try { $this->assertAllowed($input->getOption('force')); $gid = $input->getArgument('ocName'); + $wasMapped = $this->groupWasMapped($gid); if ($this->backend->getLDAPAccess($gid)->stringResemblesDN($gid)) { $groupname = $this->backend->dn2GroupName($gid); if ($groupname !== false) { $gid = $groupname; } } - $wasMapped = $this->groupWasMapped($gid); + /* Search to trigger mapping for new groups */ + $this->backend->getGroups($gid); $exists = $this->backend->groupExistsOnLDAP($gid, true); if ($exists === true) { $output->writeln('The group is still available on LDAP.'); @@ -113,7 +115,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int } } - public function onGroupCreatedEvent(GroupChangedEvent $event, OutputInterface $output): void { + public function onGroupCreatedEvent(GroupCreatedEvent $event, OutputInterface $output): void { $output->writeln('The group '.$event->getGroup()->getGID().' was added to Nextcloud with '.$event->getGroup()->count().' users'); } @@ -131,11 +133,15 @@ public function onUserRemovedEvent(UserRemovedEvent $event, OutputInterface $out /** * checks whether a group is actually mapped - * @param string $ocName the groupname as used in Nextcloud + * @param string $gid the groupname as passed to the command */ - protected function groupWasMapped(string $ocName): bool { - $dn = $this->mapping->getDNByName($ocName); - return $dn !== false; + protected function groupWasMapped(string $gid): bool { + $dn = $this->mapping->getDNByName($gid); + if ($dn !== false) { + return true; + } + $name = $this->mapping->getNameByDN($gid); + return $name !== false; } /** From c33c40f677f0d5b674a8ad4d1d4d6445ed2cf9ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Mon, 7 Aug 2023 10:56:51 +0200 Subject: [PATCH 12/17] [user_ldap] Small perf improvements suggestion from review 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/Db/GroupMembershipMapper.php | 2 +- apps/user_ldap/lib/Service/UpdateGroupsService.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/user_ldap/lib/Db/GroupMembershipMapper.php b/apps/user_ldap/lib/Db/GroupMembershipMapper.php index 0868f60d2c50c..8f6af16b267e6 100644 --- a/apps/user_ldap/lib/Db/GroupMembershipMapper.php +++ b/apps/user_ldap/lib/Db/GroupMembershipMapper.php @@ -47,7 +47,7 @@ public function getKnownGroups(): array { ->from($this->getTableName()) ->executeQuery(); - $groups = array_map(fn ($row) => $row['groupid'], $result->fetchAll()); + $groups = array_column($result->fetchAll(), 'groupid'); $result->closeCursor(); return $groups; } diff --git a/apps/user_ldap/lib/Service/UpdateGroupsService.php b/apps/user_ldap/lib/Service/UpdateGroupsService.php index 5e463734079ff..901b1e913ebd8 100644 --- a/apps/user_ldap/lib/Service/UpdateGroupsService.php +++ b/apps/user_ldap/lib/Service/UpdateGroupsService.php @@ -90,7 +90,7 @@ public function handleKnownGroups(array $groups): void { $this->logger->debug('service "updateGroups" – Dealing with {group}.', ['group' => $group]); $groupMemberships = $this->groupMembershipMapper->findGroupMemberships($group); $knownUsers = array_map( - fn (GroupMembership $groupMembership): string => $groupMembership->getUserid(), + static fn (GroupMembership $groupMembership): string => $groupMembership->getUserid(), $groupMemberships ); $groupMemberships = array_combine($knownUsers, $groupMemberships); From ad1e487f67dd300eaddbe4d998d04ccbbca0921f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Mon, 7 Aug 2023 11:20:05 +0200 Subject: [PATCH 13/17] Dispatch UserRemovedEvents for removed groups 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/Service/UpdateGroupsService.php | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/apps/user_ldap/lib/Service/UpdateGroupsService.php b/apps/user_ldap/lib/Service/UpdateGroupsService.php index 901b1e913ebd8..ea707c7c9d803 100644 --- a/apps/user_ldap/lib/Service/UpdateGroupsService.php +++ b/apps/user_ldap/lib/Service/UpdateGroupsService.php @@ -175,6 +175,18 @@ public function handleRemovedGroups(array $removedGroups): void { $this->logger->debug('service "updateGroups" – dealing with removed groups.'); $this->groupMembershipMapper->deleteGroups($removedGroups); + foreach ($removedGroups as $group) { + $groupObject = $this->groupManager->get($group); + if ($groupObject instanceof IGroup) { + $groupMemberships = $this->groupMembershipMapper->findGroupMemberships($group); + foreach ($groupMemberships as $groupMembership) { + $userObject = $this->userManager->get($groupMembership->getUserid()); + if ($userObject instanceof IUser) { + $this->dispatcher->dispatchTyped(new UserRemovedEvent($groupObject, $userObject)); + } + } + } + } //TODO find a way to dispatch GroupDeletedEvent From 7732de75eeea0f549bb65378f33a05d2aa1a38db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Mon, 7 Aug 2023 11:32:27 +0200 Subject: [PATCH 14/17] Removed GroupCreatedEvent/GroupDeletedEvent from UpdateGroupsService MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This should be handled when mapping groups, not when registering their members. An empty group may still exist. Signed-off-by: Côme Chilliet --- apps/user_ldap/lib/Service/UpdateGroupsService.php | 6 ------ 1 file changed, 6 deletions(-) diff --git a/apps/user_ldap/lib/Service/UpdateGroupsService.php b/apps/user_ldap/lib/Service/UpdateGroupsService.php index ea707c7c9d803..faee092a37256 100644 --- a/apps/user_ldap/lib/Service/UpdateGroupsService.php +++ b/apps/user_ldap/lib/Service/UpdateGroupsService.php @@ -38,7 +38,6 @@ use OCA\User_LDAP\Group_Proxy; use OCP\DB\Exception; use OCP\EventDispatcher\IEventDispatcher; -use OCP\Group\Events\GroupCreatedEvent; use OCP\Group\Events\UserAddedEvent; use OCP\Group\Events\UserRemovedEvent; use OCP\IGroup; @@ -151,9 +150,6 @@ public function handleCreatedGroups(array $createdGroups): void { $users = $this->groupBackend->usersInGroup($createdGroup); $groupObject = $this->groupManager->get($createdGroup); - if ($groupObject instanceof IGroup) { - $this->dispatcher->dispatchTyped(new GroupCreatedEvent($groupObject)); - } foreach ($users as $user) { $this->groupMembershipMapper->insert(GroupMembership::fromParams(['groupid' => $createdGroup,'userid' => $user])); if ($groupObject instanceof IGroup) { @@ -188,8 +184,6 @@ public function handleRemovedGroups(array $removedGroups): void { } } - //TODO find a way to dispatch GroupDeletedEvent - $this->logger->info( 'service "updateGroups" – groups {removedGroups} were removed.', [ From 5425f7d3bda18e02e504a1b10905e21939296be5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Mon, 7 Aug 2023 11:52:41 +0200 Subject: [PATCH 15/17] Use BIGINT for new table id field 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/Migration/Version1190Date20230706134108.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/apps/user_ldap/lib/Migration/Version1190Date20230706134108.php b/apps/user_ldap/lib/Migration/Version1190Date20230706134108.php index c6bd795f3e649..51d4510f66c0a 100644 --- a/apps/user_ldap/lib/Migration/Version1190Date20230706134108.php +++ b/apps/user_ldap/lib/Migration/Version1190Date20230706134108.php @@ -48,10 +48,9 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt if (!$schema->hasTable('ldap_group_membership')) { $table = $schema->createTable('ldap_group_membership'); - $table->addColumn('id', 'integer', [ + $table->addColumn('id', Types::BIGINT, [ 'autoincrement' => true, 'notnull' => true, - 'length' => 4, ]); $table->addColumn('groupid', Types::STRING, [ 'notnull' => true, From b8a0954f553674379983506e050abe2418b04d7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Mon, 7 Aug 2023 14:06:15 +0200 Subject: [PATCH 16/17] Fix check-group --update for deleted groups 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/CheckGroup.php | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/apps/user_ldap/lib/Command/CheckGroup.php b/apps/user_ldap/lib/Command/CheckGroup.php index d55b846020a0a..68f96512a9b5d 100644 --- a/apps/user_ldap/lib/Command/CheckGroup.php +++ b/apps/user_ldap/lib/Command/CheckGroup.php @@ -100,11 +100,19 @@ protected function execute(InputInterface $input, OutputInterface $output): int $output->writeln('The group is still available on LDAP.'); if ($input->getOption('update')) { $this->backend->getLDAPAccess($gid)->connection->clearCache(); - $this->updateGroup($gid, $output, $wasMapped); + if ($wasMapped) { + $this->service->handleKnownGroups([$gid]); + } else { + $this->service->handleCreatedGroups([$gid]); + } } return 0; } elseif ($wasMapped) { - $output->writeln('The group does not exists on LDAP anymore.'); + $output->writeln('The group does not exist on LDAP anymore.'); + if ($input->getOption('update')) { + $this->backend->getLDAPAccess($gid)->connection->clearCache(); + $this->service->handleRemovedGroups([$gid]); + } return 0; } else { throw new \Exception('The given group is not a recognized LDAP group.'); From a0808111b44bcff5b7bc797250db5b10cd7947ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 8 Aug 2023 10:02:23 +0200 Subject: [PATCH 17/17] Add unique index for user_ldap group memberships 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/Migration/Version1190Date20230706134108.php | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/user_ldap/lib/Migration/Version1190Date20230706134108.php b/apps/user_ldap/lib/Migration/Version1190Date20230706134108.php index 51d4510f66c0a..3f554f56e6436 100644 --- a/apps/user_ldap/lib/Migration/Version1190Date20230706134108.php +++ b/apps/user_ldap/lib/Migration/Version1190Date20230706134108.php @@ -63,6 +63,7 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt 'default' => '', ]); $table->setPrimaryKey(['id']); + $table->addUniqueIndex(['groupid', 'userid'], 'user_ldap_membership_unique'); return $schema; } else { return null;