fix(user_ldap): Store the list of used configuration prefixed in appconfig

This avoids getting all keys from appconfig, which was triggering
 loading of lazy configuration on all requests.

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
pull/52916/head
Côme Chilliet 2025-05-16 19:13:40 +07:00
parent eb9a6214a1
commit bc7309ca1c
No known key found for this signature in database
GPG Key ID: A3E2F658B28C760A
2 changed files with 122 additions and 56 deletions

@ -7,9 +7,9 @@
*/
namespace OCA\User_LDAP;
use OCP\AppFramework\Services\IAppConfig;
use OCP\Cache\CappedMemoryCache;
use OCP\DB\QueryBuilder\IQueryBuilder;
use OCP\IConfig;
use OCP\IDBConnection;
use OCP\Server;
@ -18,7 +18,7 @@ class Helper {
protected CappedMemoryCache $sanitizeDnCache;
public function __construct(
private IConfig $config,
private IAppConfig $appConfig,
private IDBConnection $connection,
) {
$this->sanitizeDnCache = new CappedMemoryCache(10000);
@ -45,21 +45,37 @@ class Helper {
* except the default (first) server shall be connected to.
*
*/
public function getServerConfigurationPrefixes($activeConfigurations = false): array {
public function getServerConfigurationPrefixes(bool $activeConfigurations = false): array {
$all = $this->getAllServerConfigurationPrefixes();
if (!$activeConfigurations) {
return $all;
}
return array_values(array_filter(
$all,
fn (string $prefix): bool => ($this->appConfig->getAppValueString($prefix . 'ldap_configuration_active') === '1')
));
}
protected function getAllServerConfigurationPrefixes(): array {
$unfilled = ['UNFILLED'];
$prefixes = $this->appConfig->getAppValueArray('configuration_prefixes', $unfilled);
if ($prefixes !== $unfilled) {
return $prefixes;
}
/* Fallback to browsing key for migration from Nextcloud<32 */
$referenceConfigkey = 'ldap_configuration_active';
$keys = $this->getServersConfig($referenceConfigkey);
$prefixes = [];
foreach ($keys as $key) {
if ($activeConfigurations && $this->config->getAppValue('user_ldap', $key, '0') !== '1') {
continue;
}
$len = strlen($key) - strlen($referenceConfigkey);
$prefixes[] = substr($key, 0, $len);
}
asort($prefixes);
sort($prefixes);
$this->appConfig->setAppValueArray('configuration_prefixes', $prefixes);
return $prefixes;
}
@ -68,46 +84,45 @@ class Helper {
*
* determines the host for every configured connection
*
* @return array an array with configprefix as keys
* @return array<string,string> an array with configprefix as keys
*
*/
public function getServerConfigurationHosts() {
$referenceConfigkey = 'ldap_host';
$keys = $this->getServersConfig($referenceConfigkey);
public function getServerConfigurationHosts(): array {
$prefixes = $this->getServerConfigurationPrefixes();
$referenceConfigkey = 'ldap_host';
$result = [];
foreach ($keys as $key) {
$len = strlen($key) - strlen($referenceConfigkey);
$prefix = substr($key, 0, $len);
$result[$prefix] = $this->config->getAppValue('user_ldap', $key);
foreach ($prefixes as $prefix) {
$result[$prefix] = $this->appConfig->getAppValueString($prefix . $referenceConfigkey);
}
return $result;
}
/**
* return the next available configuration prefix
*
* @return string
* return the next available configuration prefix and register it as used
*/
public function getNextServerConfigurationPrefix() {
$serverConnections = $this->getServerConfigurationPrefixes();
if (count($serverConnections) === 0) {
return 's01';
public function getNextServerConfigurationPrefix(): string {
$prefixes = $this->getServerConfigurationPrefixes();
if (count($prefixes) === 0) {
$prefix = 's01';
} else {
sort($prefixes);
$lastKey = array_pop($prefixes);
$lastNumber = (int)str_replace('s', '', $lastKey);
$prefix = 's' . str_pad((string)($lastNumber + 1), 2, '0', STR_PAD_LEFT);
}
sort($serverConnections);
$lastKey = array_pop($serverConnections);
$lastNumber = (int)str_replace('s', '', $lastKey);
return 's' . str_pad((string)($lastNumber + 1), 2, '0', STR_PAD_LEFT);
$prefixes[] = $prefix;
$this->appConfig->setAppValueArray('configuration_prefixes', $prefixes);
return $prefix;
}
private function getServersConfig(string $value): array {
$regex = '/' . $value . '$/S';
$keys = $this->config->getAppKeys('user_ldap');
$keys = $this->appConfig->getAppKeys();
$result = [];
foreach ($keys as $key) {
if (preg_match($regex, $key) === 1) {
@ -125,7 +140,9 @@ class Helper {
* @return bool true on success, false otherwise
*/
public function deleteServerConfiguration($prefix) {
if (!in_array($prefix, self::getServerConfigurationPrefixes())) {
$prefixes = $this->getServerConfigurationPrefixes();
$index = array_search($prefix, $prefixes);
if ($index === false) {
return false;
}
@ -144,7 +161,11 @@ class Helper {
$query->andWhere($query->expr()->notLike('configkey', $query->createNamedParameter('s%')));
}
$deletedRows = $query->execute();
$deletedRows = $query->executeStatement();
unset($prefixes[$index]);
$this->appConfig->setAppValueArray('configuration_prefixes', array_values($prefixes));
return $deletedRows !== 0;
}
@ -152,10 +173,13 @@ class Helper {
* checks whether there is one or more disabled LDAP configurations
*/
public function haveDisabledConfigurations(): bool {
$all = $this->getServerConfigurationPrefixes(false);
$active = $this->getServerConfigurationPrefixes(true);
return count($all) !== count($active) || count($all) === 0;
$all = $this->getServerConfigurationPrefixes();
foreach ($all as $prefix) {
if ($this->appConfig->getAppValueString($prefix . 'ldap_configuration_active') !== '1') {
return true;
}
}
return false;
}
/**

@ -8,7 +8,7 @@ declare(strict_types=1);
namespace OCA\User_LDAP\Tests;
use OCA\User_LDAP\Helper;
use OCP\IConfig;
use OCP\AppFramework\Services\IAppConfig;
use OCP\IDBConnection;
use OCP\Server;
use PHPUnit\Framework\MockObject\MockObject;
@ -17,45 +17,51 @@ use PHPUnit\Framework\MockObject\MockObject;
* @group DB
*/
class HelperTest extends \Test\TestCase {
private IConfig&MockObject $config;
private IAppConfig&MockObject $appConfig;
private Helper $helper;
protected function setUp(): void {
parent::setUp();
$this->config = $this->createMock(IConfig::class);
$this->helper = new Helper($this->config, Server::get(IDBConnection::class));
$this->appConfig = $this->createMock(IAppConfig::class);
$this->helper = new Helper(
$this->appConfig,
Server::get(IDBConnection::class)
);
}
public function testGetServerConfigurationPrefixes(): void {
$this->config->method('getAppKeys')
->with($this->equalTo('user_ldap'))
$this->appConfig->method('getAppKeys')
->willReturn([
'foo',
'ldap_configuration_active',
's1ldap_configuration_active',
]);
$this->appConfig->method('getAppValueArray')
->with('configuration_prefixes')
-> willReturnArgument(1);
$result = $this->helper->getServerConfigurationPrefixes(false);
$this->assertEquals(['', 's1'], $result);
}
public function testGetServerConfigurationPrefixesActive(): void {
$this->config->method('getAppKeys')
->with($this->equalTo('user_ldap'))
$this->appConfig->method('getAppKeys')
->willReturn([
'foo',
'ldap_configuration_active',
's1ldap_configuration_active',
]);
$this->config->method('getAppValue')
->willReturnCallback(function ($app, $key, $default) {
if ($app !== 'user_ldap') {
$this->fail('wrong app');
}
$this->appConfig->method('getAppValueArray')
->with('configuration_prefixes')
-> willReturnArgument(1);
$this->appConfig->method('getAppValueString')
->willReturnCallback(function ($key, $default) {
if ($key === 's1ldap_configuration_active') {
return '1';
}
@ -67,21 +73,57 @@ class HelperTest extends \Test\TestCase {
$this->assertEquals(['s1'], $result);
}
public function testGetServerConfigurationHost(): void {
$this->config->method('getAppKeys')
->with($this->equalTo('user_ldap'))
public function testGetServerConfigurationHostFromAppKeys(): void {
$this->appConfig->method('getAppKeys')
->willReturn([
'foo',
'ldap_host',
's1ldap_host',
's02ldap_host',
'ldap_configuration_active',
's1ldap_configuration_active',
's02ldap_configuration_active',
]);
$this->config->method('getAppValue')
->willReturnCallback(function ($app, $key, $default) {
if ($app !== 'user_ldap') {
$this->fail('wrong app');
$this->appConfig->method('getAppValueArray')
->with('configuration_prefixes')
-> willReturnArgument(1);
$this->appConfig->method('getAppValueString')
->willReturnCallback(function ($key, $default) {
if ($key === 'ldap_host') {
return 'example.com';
}
if ($key === 's1ldap_host') {
return 'foo.bar.com';
}
return $default;
});
$result = $this->helper->getServerConfigurationHosts();
$this->assertEquals([
'' => 'example.com',
's1' => 'foo.bar.com',
's02' => '',
], $result);
}
public function testGetServerConfigurationHost(): void {
$this->appConfig
->expects(self::never())
->method('getAppKeys');
$this->appConfig->method('getAppValueArray')
->with('configuration_prefixes')
-> willReturn([
'',
's1',
's02',
]);
$this->appConfig->method('getAppValueString')
->willReturnCallback(function ($key, $default) {
if ($key === 'ldap_host') {
return 'example.com';
}