]> source.dussan.org Git - nextcloud-server.git/commitdiff
Refactor user_ldap group membership to use flat DB
authorCôme Chilliet <come.chilliet@nextcloud.com>
Thu, 20 Jul 2023 10:55:24 +0000 (12:55 +0200)
committerCôme Chilliet <come.chilliet@nextcloud.com>
Thu, 10 Aug 2023 08:57:25 +0000 (10:57 +0200)
Move away from serialized arrays. Also use a QBMapper class for the new table.

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
apps/user_ldap/appinfo/info.xml
apps/user_ldap/lib/Db/GroupMembership.php [new file with mode: 0644]
apps/user_ldap/lib/Db/GroupMembershipMapper.php [new file with mode: 0644]
apps/user_ldap/lib/Jobs/UpdateGroups.php
apps/user_ldap/lib/Migration/Version1190Date20230706134108.php [new file with mode: 0644]
apps/user_ldap/lib/Migration/Version1190Date20230706134109.php [new file with mode: 0644]

index 20f439148e185e73b9062358328bb45f77f18aea..425fefe38e84d29f17b79f0e17e9f42c2ea28f12 100644 (file)
@@ -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.
 
        </description>
-       <version>1.18.0</version>
+       <version>1.19.0</version>
        <licence>agpl</licence>
        <author>Dominik Schmidt</author>
        <author>Arthur Schiwon</author>
diff --git a/apps/user_ldap/lib/Db/GroupMembership.php b/apps/user_ldap/lib/Db/GroupMembership.php
new file mode 100644 (file)
index 0000000..25be529
--- /dev/null
@@ -0,0 +1,48 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * @copyright Copyright (c) 2023 Côme Chilliet <come.chilliet@nextcloud.com>
+ *
+ * @author Côme Chilliet <come.chilliet@nextcloud.com>
+ *
+ * @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 <http://www.gnu.org/licenses/>.
+ *
+ */
+
+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 (file)
index 0000000..2c30e94
--- /dev/null
@@ -0,0 +1,77 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * @copyright Copyright (c) 2023 Côme Chilliet <come.chilliet@nextcloud.com>
+ *
+ * @author Côme Chilliet <come.chilliet@nextcloud.com>
+ *
+ * @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 <http://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace OCA\User_LDAP\Db;
+
+use OCP\AppFramework\Db\QBMapper;
+use OCP\DB\QueryBuilder\IQueryBuilder;
+use OCP\IDBConnection;
+
+/**
+ * @template-extends QBMapper<GroupMembership>
+ */
+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();
+               }
+       }
+}
index 48687494a610590a33433aa0d98ba8292dfcd77a..2c3ce457cffe3509533fb2fdaba3b11d70c3276d 100644 (file)
@@ -30,9 +30,10 @@ namespace OCA\User_LDAP\Jobs;
 
 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 @@ use Psr\Log\LoggerInterface;
 class UpdateGroups extends TimedJob {
        /** @var ?array<string, array{owncloudusers: string, owncloudname: string}>  */
        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 @@ class UpdateGroups extends TimedJob {
                        ['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 @@ class UpdateGroups extends TimedJob {
                );
        }
 
-       /**
-        * @return array<string, array{owncloudusers: string, owncloudname: string}>
-        * @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 @@ class UpdateGroups extends TimedJob {
                        '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 @@ class UpdateGroups extends TimedJob {
                                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 @@ class UpdateGroups extends TimedJob {
                                                '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 @@ class UpdateGroups extends TimedJob {
                                                'group' => $group
                                        ]
                                );
-                               $hasChanged = true;
-                       }
-                       if ($hasChanged) {
-                               $qb->setParameters([
-                                       'members' => serialize($actualUsers),
-                                       'groupId' => $group
-                               ]);
-                               $qb->executeStatement();
                        }
                }
                $this->logger->debug(
@@ -222,21 +179,16 @@ class UpdateGroups extends TimedJob {
                        ['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 @@ class UpdateGroups extends TimedJob {
                        ['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 (file)
index 0000000..cf41ba1
--- /dev/null
@@ -0,0 +1,107 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * @copyright Copyright (c) 2023 Your name <your@email.com>
+ *
+ * @author Your name <your@email.com>
+ *
+ * @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 <http://www.gnu.org/licenses/>.
+ *
+ */
+
+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 (file)
index 0000000..bc88dc2
--- /dev/null
@@ -0,0 +1,46 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * @copyright Copyright (c) 2023 Your name <your@email.com>
+ *
+ * @author Your name <your@email.com>
+ *
+ * @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 <http://www.gnu.org/licenses/>.
+ *
+ */
+
+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;
+       }
+}