Merge pull request #1668 from nextcloud/user_ldap_use_appconfig

Do not query data that is already in the appconfig

apps/user_ldap/ajax/deleteConfiguration.php

@@ -32,7 +32,7 @@ OCP\JSON::checkAppEnabled('user_ldap');
 OCP\JSON::callCheck();
 
 $prefix = (string)$_POST['ldap_serverconfig_chooser'];
-$helper = new \OCA\User_LDAP\Helper();
+$helper = new \OCA\User_LDAP\Helper(\OC::$server->getConfig());
 if($helper->deleteServerConfiguration($prefix)) {
 	OCP\JSON::success();
 } else {

apps/user_ldap/ajax/getNewServerConfigPrefix.php

@@ -27,7 +27,7 @@ OCP\JSON::checkAdminUser();
 OCP\JSON::checkAppEnabled('user_ldap');
 OCP\JSON::callCheck();
 
-$helper = new \OCA\User_LDAP\Helper();
+$helper = new \OCA\User_LDAP\Helper(\OC::$server->getConfig());
 $serverConnections = $helper->getServerConfigurationPrefixes();
 sort($serverConnections);
 $lk = array_pop($serverConnections);

apps/user_ldap/ajax/wizard.php

@@ -60,7 +60,9 @@ $userManager = new \OCA\User_LDAP\User\Manager(
 	\OC::$server->getDatabaseConnection(),
 	\OC::$server->getUserManager());
 
-$access = new \OCA\User_LDAP\Access($con, $ldapWrapper, $userManager, new \OCA\User_LDAP\Helper());
+$access = new \OCA\User_LDAP\Access($con, $ldapWrapper, $userManager, new \OCA\User_LDAP\Helper(
+	\OC::$server->getConfig()
+));
 
 $wizard = new \OCA\User_LDAP\Wizard($configuration, $ldapWrapper, $access);
 

apps/user_ldap/appinfo/app.php

@@ -27,7 +27,7 @@
  *
  */
 
-$helper = new \OCA\User_LDAP\Helper();
+$helper = new \OCA\User_LDAP\Helper(\OC::$server->getConfig());
 $configPrefixes = $helper->getServerConfigurationPrefixes(true);
 $ldapWrapper = new OCA\User_LDAP\LDAP();
 $ocConfig = \OC::$server->getConfig();

apps/user_ldap/appinfo/install.php

@@ -26,5 +26,5 @@ if($state === 'doSet') {
 	OCP\Config::setSystemValue('ldapIgnoreNamingRules', false);
 }
 
-$helper = new \OCA\User_LDAP\Helper();
+$helper = new \OCA\User_LDAP\Helper(\OC::$server->getConfig());
 $helper->setLDAPProvider();

apps/user_ldap/appinfo/register_command.php

@@ -30,7 +30,7 @@ use OCA\User_LDAP\User\DeletedUsersIndex;
 
 $dbConnection = \OC::$server->getDatabaseConnection();
 $userMapping = new UserMapping($dbConnection);
-$helper = new Helper();
+$helper = new Helper(\OC::$server->getConfig());
 $ocConfig = \OC::$server->getConfig();
 $uBackend = new User_Proxy(
 	$helper->getServerConfigurationPrefixes(true),

apps/user_ldap/appinfo/update.php

@@ -20,5 +20,5 @@
  *
  */
 
-$helper = new \OCA\User_LDAP\Helper();
+$helper = new \OCA\User_LDAP\Helper(\OC::$server->getConfig());
 $helper->setLDAPProvider();

apps/user_ldap/lib/Command/Search.php

@@ -102,7 +102,7 @@ class Search extends Command {
 	}
 
 	protected function execute(InputInterface $input, OutputInterface $output) {
-		$helper = new Helper();
+		$helper = new Helper($this->ocConfig);
 		$configPrefixes = $helper->getServerConfigurationPrefixes(true);
 		$ldapWrapper = new LDAP();
 

apps/user_ldap/lib/Command/SetConfig.php

@@ -57,7 +57,7 @@ class SetConfig extends Command {
 	}
 
 	protected function execute(InputInterface $input, OutputInterface $output) {
-		$helper = new Helper();
+		$helper = new Helper(\OC::$server->getConfig());
 		$availableConfigs = $helper->getServerConfigurationPrefixes();
 		$configID = $input->getArgument('configID');
 		if(!in_array($configID, $availableConfigs)) {

apps/user_ldap/lib/Command/TestConfig.php

@@ -47,7 +47,7 @@ class TestConfig extends Command {
 	}
 
 	protected function execute(InputInterface $input, OutputInterface $output) {
-		$helper = new Helper();
+		$helper = new Helper(\OC::$server->getConfig());
 		$availableConfigs = $helper->getServerConfigurationPrefixes();
 		$configID = $input->getArgument('configID');
 		if(!in_array($configID, $availableConfigs)) {

apps/user_ldap/lib/Connection.php

@@ -87,7 +87,7 @@ class Connection extends LDAPUtility {
 		if($memcache->isAvailable()) {
 			$this->cache = $memcache->create();
 		}
-		$helper = new Helper();
+		$helper = new Helper(\OC::$server->getConfig());
 		$this->doNotValidate = !in_array($this->configPrefix,
 			$helper->getServerConfigurationPrefixes());
 		$this->hasPagedResultSupport =

apps/user_ldap/lib/Helper.php

@@ -30,8 +30,22 @@
 
 namespace OCA\User_LDAP;
 
+use OCP\IConfig;
+
 class Helper {
 
+	/** @var IConfig */
+	private $config;
+
+	/**
+	 * Helper constructor.
+	 *
+	 * @param IConfig $config
+	 */
+	public function __construct(IConfig $config) {
+		$this->config = $config;
+	}
+
 	/**
 	 * returns prefixes for each saved LDAP/AD server configuration.
 	 * @param bool $activeConfigurations optional, whether only active configuration shall be
@@ -55,30 +69,16 @@ class Helper {
 	public function getServerConfigurationPrefixes($activeConfigurations = false) {
 		$referenceConfigkey = 'ldap_configuration_active';
 
-		$sql = '
-			SELECT DISTINCT `configkey`
-			FROM `*PREFIX*appconfig`
-			WHERE `appid` = \'user_ldap\'
-				AND `configkey` LIKE ?
-		';
+		$keys = $this->getServersConfig($referenceConfigkey);
 
-		if($activeConfigurations) {
-			if (\OC::$server->getConfig()->getSystemValue( 'dbtype', 'sqlite' ) === 'oci') {
-				//FIXME oracle hack: need to explicitly cast CLOB to CHAR for comparison
-				$sql .= ' AND to_char(`configvalue`)=\'1\'';
-			} else {
-				$sql .= ' AND `configvalue` = \'1\'';
+		$prefixes = [];
+		foreach ($keys as $key) {
+			if ($activeConfigurations && $this->config->getAppValue('user_ldap', $key, '0') !== '1') {
+				continue;
 			}
-		}
 
-		$stmt = \OCP\DB::prepare($sql);
-
-		$serverConfigs = $stmt->execute(array('%'.$referenceConfigkey))->fetchAll();
-		$prefixes = array();
-
-		foreach($serverConfigs as $serverConfig) {
-			$len = strlen($serverConfig['configkey']) - strlen($referenceConfigkey);
-			$prefixes[] = substr($serverConfig['configkey'], 0, $len);
+			$len = strlen($key) - strlen($referenceConfigkey);
+			$prefixes[] = substr($key, 0, $len);
 		}
 
 		return $prefixes;
@@ -93,20 +93,27 @@ class Helper {
 	public function getServerConfigurationHosts() {
 		$referenceConfigkey = 'ldap_host';
 
-		$query = '
-			SELECT DISTINCT `configkey`, `configvalue`
-			FROM `*PREFIX*appconfig`
-			WHERE `appid` = \'user_ldap\'
-				AND `configkey` LIKE ?
-		';
-		$query = \OCP\DB::prepare($query);
-		$configHosts = $query->execute(array('%'.$referenceConfigkey))->fetchAll();
-		$result = array();
+		$keys = $this->getServersConfig($referenceConfigkey);
 
-		foreach($configHosts as $configHost) {
-			$len = strlen($configHost['configkey']) - strlen($referenceConfigkey);
-			$prefix = substr($configHost['configkey'], 0, $len);
-			$result[$prefix] = $configHost['configvalue'];
+		$result = array();
+		foreach($keys as $key) {
+			$len = strlen($key) - strlen($referenceConfigkey);
+			$prefix = substr($key, 0, $len);
+			$result[$prefix] = $this->config->getAppValue('user_ldap', $key);
+		}
+
+		return $result;
+	}
+
+	private function getServersConfig($value) {
+		$regex = '/' . $value . '$/S';
+
+		$keys = $this->config->getAppKeys('user_ldap');
+		$result = [];
+		foreach ($keys as $key) {
+			if (preg_match($regex, $key) === 1) {
+				$result[] = $key;
+			}
 		}
 
 		return $result;
@@ -262,7 +269,7 @@ class Helper {
 		}
 
 		//ain't it ironic?
-		$helper = new Helper();
+		$helper = new Helper(\OC::$server->getConfig());
 
 		$configPrefixes = $helper->getServerConfigurationPrefixes(true);
 		$ldapWrapper = new LDAP();

apps/user_ldap/lib/Jobs/CleanUp.php

@@ -83,7 +83,7 @@ class CleanUp extends TimedJob {
 		if(isset($arguments['helper'])) {
 			$this->ldapHelper = $arguments['helper'];
 		} else {
-			$this->ldapHelper = new Helper();
+			$this->ldapHelper = new Helper(\OC::$server->getConfig());
 		}
 
 		if(isset($arguments['ocConfig'])) {

apps/user_ldap/lib/Jobs/UpdateGroups.php

@@ -172,7 +172,7 @@ class UpdateGroups extends \OC\BackgroundJob\TimedJob {
 		if(!is_null(self::$groupBE)) {
 			return self::$groupBE;
 		}
-		$helper = new Helper();
+		$helper = new Helper(\OC::$server->getConfig());
 		$configPrefixes = $helper->getServerConfigurationPrefixes(true);
 		$ldapWrapper = new LDAP();
 		if(count($configPrefixes) === 1) {

apps/user_ldap/lib/LDAPProviderFactory.php

@@ -52,7 +52,7 @@ class LDAPProviderFactory implements ILDAPProviderFactory {
 	public function getLDAPProvider() {
 		$dbConnection = $this->serverContainer->getDatabaseConnection();
 		$userMapping = new UserMapping($dbConnection);
-		return new LDAPProvider($this->serverContainer, new Helper(), 
+		return new LDAPProvider($this->serverContainer, new Helper($this->serverContainer->getConfig()),
 					new DeletedUsersIndex($this->serverContainer->getConfig(), 
 					$dbConnection, $userMapping));
 	}

apps/user_ldap/lib/Proxy.php

@@ -77,7 +77,7 @@ abstract class Proxy {
 		$userManager =
 			new Manager($ocConfig, $fs, $log, $avatarM, new \OCP\Image(), $db, $coreUserManager);
 		$connector = new Connection($this->ldap, $configPrefix);
-		$access = new Access($connector, $this->ldap, $userManager, new Helper());
+		$access = new Access($connector, $this->ldap, $userManager, new Helper(\OC::$server->getConfig()));
 		$access->setUserMapper($userMap);
 		$access->setGroupMapper($groupMap);
 		self::$accesses[$configPrefix] = $access;

apps/user_ldap/lib/Settings/Admin.php

@@ -45,7 +45,7 @@ class Admin implements ISettings {
 	 * @return TemplateResponse
 	 */
 	public function getForm() {
-		$helper = new Helper();
+		$helper = new Helper(\OC::$server->getConfig());
 		$prefixes = $helper->getServerConfigurationPrefixes();
 		$hosts = $helper->getServerConfigurationHosts();
 

apps/user_ldap/lib/Wizard.php

@@ -724,7 +724,7 @@ class Wizard extends LDAPUtility {
 		//this did not help :(
 		//Let's see whether we can parse the Host URL and convert the domain to
 		//a base DN
-		$helper = new Helper();
+		$helper = new Helper(\OC::$server->getConfig());
 		$domain = $helper->getDomainFromURL($this->configuration->ldapHost);
 		if(!$domain) {
 			return false;

apps/user_ldap/tests/AccessTest.php

@@ -67,7 +67,7 @@ class AccessTest extends \Test\TestCase {
 				$this->createMock(Image::class),
 				$this->createMock(IDBConnection::class),
 				$this->createMock(IUserManager::class)));
-		$helper = new \OCA\User_LDAP\Helper();
+		$helper = new \OCA\User_LDAP\Helper(\OC::$server->getConfig());
 
 		return array($lw, $connector, $um, $helper);
 	}

apps/user_ldap/tests/Group_LDAPTest.php

@@ -54,7 +54,7 @@ class Group_LDAPTest extends \Test\TestCase {
 		$um = $this->getMockBuilder('\OCA\User_LDAP\User\Manager')
 			->disableOriginalConstructor()
 			->getMock();
-		$helper = new \OCA\User_LDAP\Helper();
+		$helper = new \OCA\User_LDAP\Helper(\OC::$server->getConfig());
 		$access = $this->getMock('\OCA\User_LDAP\Access',
 								 $accMethods,
 								 array($connector, $lw, $um, $helper));

apps/user_ldap/tests/HelperTest.php

@@ -0,0 +1,95 @@
+<?php
+
+namespace OCA\User_LDAP\Tests;
+
+
+use OCA\User_LDAP\Helper;
+use OCP\IConfig;
+
+class HelperTest extends \Test\TestCase {
+
+	/** @var IConfig|\PHPUnit_Framework_MockObject_MockObject */
+	private $config;
+
+	/** @var Helper */
+	private $helper;
+
+	public function setUp() {
+		parent::setUp();
+
+		$this->config = $this->createMock(IConfig::class);
+		$this->helper = new Helper($this->config);
+	}
+
+	public function testGetServerConfigurationPrefixes() {
+		$this->config->method('getAppKeys')
+			->with($this->equalTo('user_ldap'))
+			->willReturn([
+				'foo',
+				'ldap_configuration_active',
+				's1ldap_configuration_active',
+			]);
+
+		$result = $this->helper->getServerConfigurationPrefixes(false);
+
+		$this->assertEquals(['', 's1'], $result);
+	}
+
+	public function testGetServerConfigurationPrefixesActive() {
+		$this->config->method('getAppKeys')
+			->with($this->equalTo('user_ldap'))
+			->willReturn([
+				'foo',
+				'ldap_configuration_active',
+				's1ldap_configuration_active',
+			]);
+
+		$this->config->method('getAppValue')
+			->will($this->returnCallback(function($app, $key, $default) {
+				if ($app !== 'user_ldap') {
+					$this->fail('wrong app');
+				}
+				if ($key === 's1ldap_configuration_active') {
+					return '1';
+				}
+				return $default;
+			}));
+
+		$result = $this->helper->getServerConfigurationPrefixes(true);
+
+		$this->assertEquals(['s1'], $result);
+	}
+
+	public function testGetServerConfigurationHost() {
+		$this->config->method('getAppKeys')
+			->with($this->equalTo('user_ldap'))
+			->willReturn([
+				'foo',
+				'ldap_host',
+				's1ldap_host',
+				's02ldap_host',
+			]);
+
+		$this->config->method('getAppValue')
+			->will($this->returnCallback(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);
+	}
+}

apps/user_ldap/tests/Integration/AbstractIntegrationTest.php

@@ -113,7 +113,7 @@ abstract class AbstractIntegrationTest {
 	 * initializes the test Helper
 	 */
 	protected function initHelper() {
-		$this->helper = new Helper();
+		$this->helper = new Helper(\OC::$server->getConfig());
 	}
 
 	/**

apps/user_ldap/tests/LDAPProviderTest.php

@@ -121,7 +121,7 @@ class LDAPProviderTest extends \Test\TestCase {
 		
 		$server = $this->getServerMock($backend);
 		
-		$helper = new \OCA\User_LDAP\Helper();
+		$helper = new \OCA\User_LDAP\Helper(\OC::$server->getConfig());
 			
 		$ldapProvider = $this->getLDAPProvider($server);
 		$this->assertEquals(
@@ -137,7 +137,7 @@ class LDAPProviderTest extends \Test\TestCase {
 		
 		$server = $this->getServerMock($backend);
 		
-		$helper = new \OCA\User_LDAP\Helper();
+		$helper = new \OCA\User_LDAP\Helper(\OC::$server->getConfig());
 			
 		$ldapProvider = $this->getLDAPProvider($server);
 		$this->assertEquals(

apps/user_ldap/tests/Settings/AdminTest.php

@@ -55,7 +55,7 @@ class AdminTest extends TestCase {
 	 */
 	public function testGetForm() {
 
-		$helper = new Helper();
+		$helper = new Helper(\OC::$server->getConfig());
 		$prefixes = $helper->getServerConfigurationPrefixes();
 		$hosts = $helper->getServerConfigurationHosts();
 

apps/user_ldap/tests/User/UserTest.php

@@ -81,7 +81,7 @@ class UserTest extends \Test\TestCase {
 			$umMethods, array($cfMock, $fsMock, $logMock, $avaMgr, $im, $dbc, $userMgr));
 		$connector = $this->getMock('\OCA\User_LDAP\Connection',
 			$conMethods, array($lw, null, null));
-		$helper = new \OCA\User_LDAP\Helper();
+		$helper = new \OCA\User_LDAP\Helper(\OC::$server->getConfig());
 		$access = $this->getMock('\OCA\User_LDAP\Access',
 			$accMethods, array($connector, $lw, $um, $helper));
 

apps/user_ldap/tests/User_LDAPTest.php

@@ -97,7 +97,7 @@ class User_LDAPTest extends TestCase {
 			->method('getDeletedUser')
 			->will($this->returnValue($offlineUser));
 
-		$helper = new Helper();
+		$helper = new Helper(\OC::$server->getConfig());
 
 		$access = $this->getMockBuilder(Access::class)
 			->setMethodsExcept(['getConnection'])

apps/user_ldap/tests/WizardTest.php

@@ -70,7 +70,7 @@ class WizardTest extends \Test\TestCase {
 		$um = $this->getMockBuilder('\OCA\User_LDAP\User\Manager')
 					->disableOriginalConstructor()
 					->getMock();
-		$helper = new \OCA\User_LDAP\Helper();
+		$helper = new \OCA\User_LDAP\Helper(\OC::$server->getConfig());
 		$access = $this->getMock('\OCA\User_LDAP\Access',
 			$accMethods, array($connector, $lw, $um, $helper));