From e66f8731afe445cf38a67d06ce858a2c56ebdb2f Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Fri, 5 Jun 2015 17:20:31 +0200 Subject: [PATCH] UserBackendConfigForm: Allow to configure user backends of type msldap fixes #9355 --- .../Config/UserBackend/LdapBackendForm.php | 33 ++++++++++++------- .../forms/Config/UserBackendConfigForm.php | 29 ++++++++++------ .../UserBackend/LdapBackendFormTest.php | 23 ++++++------- 3 files changed, 50 insertions(+), 35 deletions(-) diff --git a/application/forms/Config/UserBackend/LdapBackendForm.php b/application/forms/Config/UserBackend/LdapBackendForm.php index c520ec68f..ac2091398 100644 --- a/application/forms/Config/UserBackend/LdapBackendForm.php +++ b/application/forms/Config/UserBackend/LdapBackendForm.php @@ -8,7 +8,7 @@ use Icinga\Web\Form; use Icinga\Data\ConfigObject; use Icinga\Data\ResourceFactory; use Icinga\Exception\AuthenticationException; -use Icinga\Authentication\User\LdapUserBackend; +use Icinga\Authentication\User\UserBackend; /** * Form class for adding/modifying LDAP user backends @@ -48,6 +48,8 @@ class LdapBackendForm extends Form */ public function createElements(array $formData) { + $isAd = isset($formData['type']) ? $formData['type'] === 'msldap' : false; + $this->addElement( 'text', 'name', @@ -77,10 +79,13 @@ class LdapBackendForm extends Form 'text', 'user_class', array( - 'required' => true, - 'label' => $this->translate('LDAP User Object Class'), - 'description' => $this->translate('The object class used for storing users on the LDAP server.'), - 'value' => 'inetOrgPerson' + 'preserveDefault' => true, + 'required' => ! $isAd, + 'ignore' => $isAd, + 'disabled' => $isAd ?: null, + 'label' => $this->translate('LDAP User Object Class'), + 'description' => $this->translate('The object class used for storing users on the LDAP server.'), + 'value' => $isAd ? 'user' : 'inetOrgPerson' ) ); $this->addElement( @@ -117,12 +122,15 @@ class LdapBackendForm extends Form 'text', 'user_name_attribute', array( - 'required' => true, - 'label' => $this->translate('LDAP User Name Attribute'), - 'description' => $this->translate( + 'preserveDefault' => true, + 'required' => ! $isAd, + 'ignore' => $isAd, + 'disabled' => $isAd ?: null, + 'label' => $this->translate('LDAP User Name Attribute'), + 'description' => $this->translate( 'The attribute name used for storing the user name on the LDAP server.' ), - 'value' => 'uid' + 'value' => $isAd ? 'sAMAccountName' : 'uid' ) ); $this->addElement( @@ -130,7 +138,7 @@ class LdapBackendForm extends Form 'backend', array( 'disabled' => true, - 'value' => 'ldap' + 'value' => $isAd ? 'msldap' : 'ldap' ) ); $this->addElement( @@ -170,8 +178,7 @@ class LdapBackendForm extends Form public static function isValidUserBackend(Form $form) { try { - $ldapUserBackend = new LdapUserBackend(ResourceFactory::createResource($form->getResourceConfig())); - $ldapUserBackend->setConfig(new ConfigObject($form->getValues())); + $ldapUserBackend = UserBackend::create(null, new ConfigObject($form->getValues())); $ldapUserBackend->assertAuthenticationPossible(); } catch (AuthenticationException $e) { if (($previous = $e->getPrevious()) !== null) { @@ -193,6 +200,8 @@ class LdapBackendForm extends Form * Return the configuration for the chosen resource * * @return ConfigObject + * + * @todo Check whether it's possible to drop this (Or even all occurences!) */ public function getResourceConfig() { diff --git a/application/forms/Config/UserBackendConfigForm.php b/application/forms/Config/UserBackendConfigForm.php index 0a30dd590..62a68e70b 100644 --- a/application/forms/Config/UserBackendConfigForm.php +++ b/application/forms/Config/UserBackendConfigForm.php @@ -60,16 +60,24 @@ class UserBackendConfigForm extends ConfigForm */ public function getBackendForm($type) { - if ($type === 'db') { - $form = new DbBackendForm(); - $form->setResources(isset($this->resources['db']) ? $this->resources['db'] : array()); - } elseif ($type === 'ldap') { - $form = new LdapBackendForm(); - $form->setResources(isset($this->resources['ldap']) ? $this->resources['ldap'] : array()); - } elseif ($type === 'external') { - $form = new ExternalBackendForm(); - } else { - throw new InvalidArgumentException(sprintf($this->translate('Invalid backend type "%s" provided'), $type)); + switch ($type) + { + case 'db': + $form = new DbBackendForm(); + $form->setResources(isset($this->resources['db']) ? $this->resources['db'] : array()); + break; + case 'ldap': + case 'msldap': + $form = new LdapBackendForm(); + $form->setResources(isset($this->resources['ldap']) ? $this->resources['ldap'] : array()); + break; + case 'external': + $form = new ExternalBackendForm(); + break; + default: + throw new InvalidArgumentException( + sprintf($this->translate('Invalid backend type "%s" provided'), $type) + ); } return $form; @@ -296,6 +304,7 @@ class UserBackendConfigForm extends ConfigForm } if (isset($this->resources['ldap']) && ($backendType === 'ldap' || Platform::extensionLoaded('ldap'))) { $backendTypes['ldap'] = 'LDAP'; + $backendTypes['msldap'] = 'ActiveDirectory'; } $externalBackends = array_filter( diff --git a/test/php/application/forms/Config/UserBackend/LdapBackendFormTest.php b/test/php/application/forms/Config/UserBackend/LdapBackendFormTest.php index f7373a7ae..6fe63adf4 100644 --- a/test/php/application/forms/Config/UserBackend/LdapBackendFormTest.php +++ b/test/php/application/forms/Config/UserBackend/LdapBackendFormTest.php @@ -27,10 +27,9 @@ class LdapBackendFormTest extends BaseTestCase */ public function testValidBackendIsValid() { - $this->setUpResourceFactoryMock(); - Mockery::mock('overload:Icinga\Authentication\User\LdapUserBackend') - ->shouldReceive('assertAuthenticationPossible')->andReturnNull() - ->shouldReceive('setConfig')->andReturnNull(); + $ldapUserBackendMock = Mockery::mock('overload:Icinga\Authentication\User\LdapUserBackend'); + $ldapUserBackendMock->shouldReceive('assertAuthenticationPossible')->andReturnNull(); + $this->setUpUserBackendMock($ldapUserBackendMock); // Passing array(null) is required to make Mockery call the constructor... $form = Mockery::mock('Icinga\Forms\Config\UserBackend\LdapBackendForm[getView]', array(null)); @@ -53,9 +52,9 @@ class LdapBackendFormTest extends BaseTestCase */ public function testInvalidBackendIsNotValid() { - $this->setUpResourceFactoryMock(); - Mockery::mock('overload:Icinga\Authentication\User\LdapUserBackend') - ->shouldReceive('assertAuthenticationPossible')->andThrow(new AuthenticationException); + $ldapUserBackendMock = Mockery::mock('overload:Icinga\Authentication\User\LdapUserBackend'); + $ldapUserBackendMock->shouldReceive('assertAuthenticationPossible')->andThrow(new AuthenticationException); + $this->setUpUserBackendMock($ldapUserBackendMock); // Passing array(null) is required to make Mockery call the constructor... $form = Mockery::mock('Icinga\Forms\Config\UserBackend\LdapBackendForm[getView]', array(null)); @@ -72,12 +71,10 @@ class LdapBackendFormTest extends BaseTestCase ); } - protected function setUpResourceFactoryMock() + protected function setUpUserBackendMock($ldapUserBackendMock) { - Mockery::mock('alias:Icinga\Data\ResourceFactory') - ->shouldReceive('createResource') - ->andReturn(Mockery::mock('Icinga\Protocol\Ldap\Connection')) - ->shouldReceive('getResourceConfig') - ->andReturn(new ConfigObject()); + Mockery::mock('alias:Icinga\Authentication\User\UserBackend') + ->shouldReceive('create') + ->andReturn($ldapUserBackendMock); } }