Skip to content

Commit

Permalink
Refactor user_ldap group membership to use flat DB
Browse files Browse the repository at this point in the history
Move away from serialized arrays. Also use a QBMapper class for the new table.

Signed-off-by: Côme Chilliet <[email protected]>
  • Loading branch information
come-nc committed Jul 20, 2023
1 parent 77f2497 commit c53e516
Show file tree
Hide file tree
Showing 6 changed files with 313 additions and 91 deletions.
2 changes: 1 addition & 1 deletion apps/user_ldap/appinfo/info.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
48 changes: 48 additions & 0 deletions apps/user_ldap/lib/Db/GroupMembership.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
<?php

declare(strict_types=1);

/**
* @copyright Copyright (c) 2023 Côme Chilliet <[email protected]>
*
* @author Côme Chilliet <[email protected]>
*
* @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');
}
}
76 changes: 76 additions & 0 deletions apps/user_ldap/lib/Db/GroupMembershipMapper.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
<?php

declare(strict_types=1);

/**
* @copyright Copyright (c) 2023 Côme Chilliet <[email protected]>
*
* @author Côme Chilliet <[email protected]>
*
* @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\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();
}
}
}
120 changes: 30 additions & 90 deletions apps/user_ldap/lib/Jobs/UpdateGroups.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -46,31 +47,20 @@
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;
}

/**
Expand All @@ -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)) {
Expand All @@ -112,30 +101,6 @@ public function updateGroups(): void {
);
}

/**
* @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
Expand All @@ -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) {
Expand All @@ -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));
Expand All @@ -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));
Expand All @@ -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(
Expand All @@ -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.',
Expand All @@ -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
]
);
}
}
Loading

0 comments on commit c53e516

Please sign in to comment.