diff --git a/apps/user_ldap/ajax/deleteConfiguration.php b/apps/user_ldap/ajax/deleteConfiguration.php index f09295355aca2..68bce69f98257 100644 --- a/apps/user_ldap/ajax/deleteConfiguration.php +++ b/apps/user_ldap/ajax/deleteConfiguration.php @@ -1,8 +1,6 @@ deleteServerConfiguration($prefix)) { \OC_JSON::success(); } else { diff --git a/apps/user_ldap/ajax/getNewServerConfigPrefix.php b/apps/user_ldap/ajax/getNewServerConfigPrefix.php index e8746a7c1b11b..e5ba6375c7379 100644 --- a/apps/user_ldap/ajax/getNewServerConfigPrefix.php +++ b/apps/user_ldap/ajax/getNewServerConfigPrefix.php @@ -2,8 +2,6 @@ use OCA\User_LDAP\Configuration; use OCA\User_LDAP\Helper; -use OCP\IConfig; -use OCP\IDBConnection; use OCP\Server; /** @@ -16,7 +14,7 @@ \OC_JSON::checkAppEnabled('user_ldap'); \OC_JSON::callCheck(); -$helper = new Helper(Server::get(IConfig::class), Server::get(IDBConnection::class)); +$helper = Server::get(Helper::class); $serverConnections = $helper->getServerConfigurationPrefixes(); sort($serverConnections); $lk = array_pop($serverConnections); diff --git a/apps/user_ldap/lib/Command/Search.php b/apps/user_ldap/lib/Command/Search.php index ad13c94c84a56..85906b20e9a7f 100644 --- a/apps/user_ldap/lib/Command/Search.php +++ b/apps/user_ldap/lib/Command/Search.php @@ -12,7 +12,6 @@ use OCA\User_LDAP\LDAP; use OCA\User_LDAP\User_Proxy; use OCP\IConfig; -use OCP\IDBConnection; use OCP\Server; use Symfony\Component\Console\Command\Command; @@ -83,7 +82,7 @@ protected function validateOffsetAndLimit(int $offset, int $limit): void { } protected function execute(InputInterface $input, OutputInterface $output): int { - $helper = new Helper($this->ocConfig, Server::get(IDBConnection::class)); + $helper = Server::get(Helper::class); $configPrefixes = $helper->getServerConfigurationPrefixes(true); $ldapWrapper = new LDAP(); diff --git a/apps/user_ldap/lib/Command/SetConfig.php b/apps/user_ldap/lib/Command/SetConfig.php index 5a0b65a2c3e27..7e9efcf34d031 100644 --- a/apps/user_ldap/lib/Command/SetConfig.php +++ b/apps/user_ldap/lib/Command/SetConfig.php @@ -11,8 +11,6 @@ use OCA\User_LDAP\ConnectionFactory; use OCA\User_LDAP\Helper; use OCA\User_LDAP\LDAP; -use OCP\IConfig; -use OCP\IDBConnection; use OCP\Server; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputArgument; @@ -43,7 +41,7 @@ protected function configure(): void { } protected function execute(InputInterface $input, OutputInterface $output): int { - $helper = new Helper(Server::get(IConfig::class), Server::get(IDBConnection::class)); + $helper = Server::get(Helper::class); $availableConfigs = $helper->getServerConfigurationPrefixes(); $configID = $input->getArgument('configID'); if (!in_array($configID, $availableConfigs)) { diff --git a/apps/user_ldap/lib/Connection.php b/apps/user_ldap/lib/Connection.php index 78a5f93d29ed0..b25a2e72b2b07 100644 --- a/apps/user_ldap/lib/Connection.php +++ b/apps/user_ldap/lib/Connection.php @@ -11,8 +11,6 @@ use OCA\User_LDAP\Exceptions\ConfigurationIssueException; use OCP\ICache; use OCP\ICacheFactory; -use OCP\IConfig; -use OCP\IDBConnection; use OCP\IL10N; use OCP\Server; use OCP\Util; @@ -156,7 +154,7 @@ public function __construct( if ($memcache->isAvailable()) { $this->cache = $memcache->createDistributed(); } - $helper = new Helper(Server::get(IConfig::class), Server::get(IDBConnection::class)); + $helper = Server::get(Helper::class); $this->doNotValidate = !in_array($this->configPrefix, $helper->getServerConfigurationPrefixes()); $this->logger = Server::get(LoggerInterface::class); diff --git a/apps/user_ldap/lib/Helper.php b/apps/user_ldap/lib/Helper.php index 27c19e6c4f59c..d3abf04fd1e9c 100644 --- a/apps/user_ldap/lib/Helper.php +++ b/apps/user_ldap/lib/Helper.php @@ -9,7 +9,7 @@ use OCP\Cache\CappedMemoryCache; use OCP\DB\QueryBuilder\IQueryBuilder; -use OCP\IConfig; +use OCP\IAppConfig; 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 @@ public function __construct( * 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->getValueString('user_ldap', $prefix . 'ldap_configuration_active') === '1') + )); + } + + protected function getAllServerConfigurationPrefixes(): array { + $unfilled = ['UNFILLED']; + $prefixes = $this->appConfig->getValueArray('user_ldap', '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->setValueArray('user_ldap', 'configuration_prefixes', $prefixes); return $prefixes; } @@ -68,46 +84,45 @@ public function getServerConfigurationPrefixes($activeConfigurations = false): a * * determines the host for every configured connection * - * @return array an array with configprefix as keys + * @return array 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->getValueString('user_ldap', $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->setValueArray('user_ldap', 'configuration_prefixes', $prefixes); + return $prefix; } private function getServersConfig(string $value): array { $regex = '/' . $value . '$/S'; - $keys = $this->config->getAppKeys('user_ldap'); + $keys = $this->appConfig->getKeys('user_ldap'); $result = []; foreach ($keys as $key) { if (preg_match($regex, $key) === 1) { @@ -125,7 +140,9 @@ private function getServersConfig(string $value): array { * @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 @@ public function deleteServerConfiguration($prefix) { $query->andWhere($query->expr()->notLike('configkey', $query->createNamedParameter('s%'))); } - $deletedRows = $query->execute(); + $deletedRows = $query->executeStatement(); + + unset($prefixes[$index]); + $this->appConfig->setValueArray('user_ldap', 'configuration_prefixes', array_values($prefixes)); + return $deletedRows !== 0; } @@ -152,10 +173,13 @@ public function deleteServerConfiguration($prefix) { * 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->getValueString('user_ldap', $prefix . 'ldap_configuration_active') !== '1') { + return true; + } + } + return false; } /** diff --git a/apps/user_ldap/lib/Jobs/CleanUp.php b/apps/user_ldap/lib/Jobs/CleanUp.php index 7cfc473c9509a..a1309c0c46302 100644 --- a/apps/user_ldap/lib/Jobs/CleanUp.php +++ b/apps/user_ldap/lib/Jobs/CleanUp.php @@ -67,7 +67,7 @@ public function setArguments($arguments): void { if (isset($arguments['helper'])) { $this->ldapHelper = $arguments['helper']; } else { - $this->ldapHelper = new Helper(Server::get(IConfig::class), Server::get(IDBConnection::class)); + $this->ldapHelper = Server::get(Helper::class); } if (isset($arguments['ocConfig'])) { diff --git a/apps/user_ldap/lib/Settings/Admin.php b/apps/user_ldap/lib/Settings/Admin.php index 014210ca8f09a..49868eb68c7d1 100644 --- a/apps/user_ldap/lib/Settings/Admin.php +++ b/apps/user_ldap/lib/Settings/Admin.php @@ -8,8 +8,6 @@ use OCA\User_LDAP\Configuration; use OCA\User_LDAP\Helper; use OCP\AppFramework\Http\TemplateResponse; -use OCP\IConfig; -use OCP\IDBConnection; use OCP\IL10N; use OCP\Server; use OCP\Settings\IDelegatedSettings; @@ -26,7 +24,7 @@ public function __construct( * @return TemplateResponse */ public function getForm() { - $helper = new Helper(Server::get(IConfig::class), Server::get(IDBConnection::class)); + $helper = Server::get(Helper::class); $prefixes = $helper->getServerConfigurationPrefixes(); if (count($prefixes) === 0) { $newPrefix = $helper->getNextServerConfigurationPrefix(); diff --git a/apps/user_ldap/tests/AccessTest.php b/apps/user_ldap/tests/AccessTest.php index 86ce2aff8541b..eafaeb7640381 100644 --- a/apps/user_ldap/tests/AccessTest.php +++ b/apps/user_ldap/tests/AccessTest.php @@ -25,7 +25,6 @@ use OCP\IAppConfig; use OCP\IAvatarManager; use OCP\IConfig; -use OCP\IDBConnection; use OCP\Image; use OCP\IUserManager; use OCP\Notification\IManager as INotificationManager; @@ -110,7 +109,7 @@ private function getConnectorAndLdapMock() { $this->createMock(INotificationManager::class), $this->shareManager]) ->getMock(); - $helper = new Helper(Server::get(IConfig::class), Server::get(IDBConnection::class)); + $helper = Server::get(Helper::class); return [$lw, $connector, $um, $helper]; } diff --git a/apps/user_ldap/tests/HelperTest.php b/apps/user_ldap/tests/HelperTest.php index 470b67c5531bc..adea600d90062 100644 --- a/apps/user_ldap/tests/HelperTest.php +++ b/apps/user_ldap/tests/HelperTest.php @@ -8,7 +8,7 @@ namespace OCA\User_LDAP\Tests; use OCA\User_LDAP\Helper; -use OCP\IConfig; +use OCP\IAppConfig; use OCP\IDBConnection; use OCP\Server; use PHPUnit\Framework\MockObject\MockObject; @@ -17,45 +17,53 @@ * @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('getKeys') + ->with('user_ldap') ->willReturn([ 'foo', 'ldap_configuration_active', 's1ldap_configuration_active', ]); + $this->appConfig->method('getValueArray') + ->with('user_ldap', 'configuration_prefixes') + -> willReturnArgument(2); + $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('getKeys') + ->with('user_ldap') ->willReturn([ 'foo', 'ldap_configuration_active', 's1ldap_configuration_active', ]); - $this->config->method('getAppValue') + $this->appConfig->method('getValueArray') + ->with('user_ldap', 'configuration_prefixes') + -> willReturnArgument(2); + + $this->appConfig->method('getValueString') ->willReturnCallback(function ($app, $key, $default) { - if ($app !== 'user_ldap') { - $this->fail('wrong app'); - } if ($key === 's1ldap_configuration_active') { return '1'; } @@ -67,21 +75,58 @@ public function testGetServerConfigurationPrefixesActive(): void { $this->assertEquals(['s1'], $result); } - public function testGetServerConfigurationHost(): void { - $this->config->method('getAppKeys') - ->with($this->equalTo('user_ldap')) + public function testGetServerConfigurationHostFromAppKeys(): void { + $this->appConfig->method('getKeys') + ->with('user_ldap') ->willReturn([ 'foo', 'ldap_host', 's1ldap_host', 's02ldap_host', + 'ldap_configuration_active', + 's1ldap_configuration_active', + 's02ldap_configuration_active', ]); - $this->config->method('getAppValue') + $this->appConfig->method('getValueArray') + ->with('user_ldap', 'configuration_prefixes') + -> willReturnArgument(2); + + $this->appConfig->method('getValueString') ->willReturnCallback(function ($app, $key, $default) { - if ($app !== 'user_ldap') { - $this->fail('wrong app'); + 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('getKeys'); + + $this->appConfig->method('getValueArray') + ->with('user_ldap', 'configuration_prefixes') + -> willReturn([ + '', + 's1', + 's02', + ]); + + $this->appConfig->method('getValueString') + ->willReturnCallback(function ($app, $key, $default) { if ($key === 'ldap_host') { return 'example.com'; } diff --git a/apps/user_ldap/tests/Integration/AbstractIntegrationTest.php b/apps/user_ldap/tests/Integration/AbstractIntegrationTest.php index be8d7702cd164..00f8be185860c 100644 --- a/apps/user_ldap/tests/Integration/AbstractIntegrationTest.php +++ b/apps/user_ldap/tests/Integration/AbstractIntegrationTest.php @@ -16,7 +16,6 @@ use OCA\User_LDAP\UserPluginManager; use OCP\IAvatarManager; use OCP\IConfig; -use OCP\IDBConnection; use OCP\Image; use OCP\IUserManager; use OCP\Server; @@ -125,7 +124,7 @@ protected function initUserManager() { * initializes the test Helper */ protected function initHelper() { - $this->helper = new Helper(Server::get(IConfig::class), Server::get(IDBConnection::class)); + $this->helper = Server::get(Helper::class); } /** diff --git a/apps/user_ldap/tests/LDAPProviderTest.php b/apps/user_ldap/tests/LDAPProviderTest.php index a4da4a9194849..57323e374aa6b 100644 --- a/apps/user_ldap/tests/LDAPProviderTest.php +++ b/apps/user_ldap/tests/LDAPProviderTest.php @@ -21,7 +21,6 @@ use OCP\EventDispatcher\IEventDispatcher; use OCP\ICacheFactory; use OCP\IConfig; -use OCP\IDBConnection; use OCP\IServerContainer; use OCP\Server; use Psr\Log\LoggerInterface; @@ -199,7 +198,7 @@ public function testDNasBaseParameter(): void { $server = $this->getServerMock($userBackend, $this->getDefaultGroupBackendMock()); - $helper = new Helper(Server::get(IConfig::class), Server::get(IDBConnection::class)); + $helper = Server::get(Helper::class); $ldapProvider = $this->getLDAPProvider($server); $this->assertEquals( @@ -212,7 +211,7 @@ public function testSanitizeDN(): void { $server = $this->getServerMock($userBackend, $this->getDefaultGroupBackendMock()); - $helper = new Helper(Server::get(IConfig::class), Server::get(IDBConnection::class)); + $helper = Server::get(Helper::class); $ldapProvider = $this->getLDAPProvider($server); $this->assertEquals( diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml index 5e1cdb28dd354..4c189cc41a3d8 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -2597,14 +2597,6 @@ - - - - - - - -