From c33b1723bc0a6ef5688cfea706c3c65915949a5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Cailly?= Date: Wed, 31 Dec 2025 15:26:51 +0100 Subject: [PATCH 1/2] fix(ldap-select): Restrict AuthLdap directories to only those that are active --- .../FormcreatorLdapSelectTypeMapper.php | 11 ++++- .../ldap_select_config.html.twig | 1 + .../FormcreatorLdapSelectTypeMapperTest.php | 48 +++++++++++++++---- .../QuestionType/QuestionTypeTestCase.php | 2 +- 4 files changed, 52 insertions(+), 10 deletions(-) diff --git a/src/Model/Mapper/FormcreatorLdapSelectTypeMapper.php b/src/Model/Mapper/FormcreatorLdapSelectTypeMapper.php index ca43f14..ec75584 100644 --- a/src/Model/Mapper/FormcreatorLdapSelectTypeMapper.php +++ b/src/Model/Mapper/FormcreatorLdapSelectTypeMapper.php @@ -33,6 +33,7 @@ namespace GlpiPlugin\Advancedforms\Model\Mapper; +use AuthLDAP; use Glpi\Form\Migration\FormQuestionDataConverterInterface; use GlpiPlugin\Advancedforms\Model\QuestionType\LdapQuestion; use GlpiPlugin\Advancedforms\Model\QuestionType\LdapQuestionConfig; @@ -63,8 +64,16 @@ public function convertExtraData(array $rawData): array /** @var array{ldap_auth: int, ldap_filter: string, ldap_attribute: int} $data */ $data = json_decode($rawData['values'], associative: true); + + // Ensure LDAP auth exists and is active + $authLdap = new AuthLDAP(); + $authLdapId = 0; + if ($authLdap->getFromDB($data['ldap_auth']) && $authLdap->fields['is_active']) { + $authLdapId = $authLdap->getId(); + } + return [ - LdapQuestionConfig::AUTHLDAP_ID => $data['ldap_auth'], + LdapQuestionConfig::AUTHLDAP_ID => $authLdapId, LdapQuestionConfig::LDAP_FILTER => $data['ldap_filter'], LdapQuestionConfig::LDAP_ATTRIBUTE_ID => $data['ldap_attribute'], ]; diff --git a/templates/editor/question_types/ldap_select_config.html.twig b/templates/editor/question_types/ldap_select_config.html.twig index ed2fe6d..3d68b92 100644 --- a/templates/editor/question_types/ldap_select_config.html.twig +++ b/templates/editor/question_types/ldap_select_config.html.twig @@ -67,6 +67,7 @@ 'is_horizontal' : false, 'init' : question is not null ? true : false, 'on_change' : 'plugin_advancedforms_on_ldap_change(this)', + 'condition' : {'is_active': 1}, } ) }} diff --git a/tests/Model/Mapper/FormcreatorLdapSelectTypeMapperTest.php b/tests/Model/Mapper/FormcreatorLdapSelectTypeMapperTest.php index 56d59b0..5e3f7ea 100644 --- a/tests/Model/Mapper/FormcreatorLdapSelectTypeMapperTest.php +++ b/tests/Model/Mapper/FormcreatorLdapSelectTypeMapperTest.php @@ -33,10 +33,10 @@ namespace GlpiPlugin\Advancedforms\Tests\Model\Mapper; +use AuthLDAP; use Glpi\Form\AccessControl\FormAccessControlManager; use Glpi\Form\Migration\FormMigration; use Glpi\Form\Question; -use GlpiPlugin\Advancedforms\Model\QuestionType\IpAddressQuestion; use GlpiPlugin\Advancedforms\Model\QuestionType\LdapQuestion; use GlpiPlugin\Advancedforms\Model\QuestionType\LdapQuestionConfig; use LogicException; @@ -44,12 +44,40 @@ final class FormcreatorLdapSelectTypeMapperTest extends MapperTestCase { - public function testIpTypeMigrationWhenEnabled(): void + + public function testLdapSelectTypeMigrationWhenEnabledWithValidAuthLDAP(): void + { + /** @var \DBmysql $DB */ + global $DB; + + // Arrange: create a valid AuthLDAP + $authldap = $this->createItem(AuthLDAP::class, [ + 'name' => 'My LDAP server', + 'is_active' => 1, + ]); + + // Act & Assert: test migration with the valid AuthLDAP + $this->testLdapSelectTypeMigrationWhenEnabled( + authldap_id: $authldap->getId(), + expected_authldap_id: null, + ); + } + + public function testLdapSelectTypeMigrationWhenEnabledWithInvalidAuthLDAP(): void + { + // Act & Assert: test migration with an invalid AuthLDAP + $this->testLdapSelectTypeMigrationWhenEnabled( + authldap_id: 123, + expected_authldap_id: 0, + ); + } + + private function testLdapSelectTypeMigrationWhenEnabled(int $authldap_id, ?int $expected_authldap_id): void { /** @var \DBmysql $DB */ global $DB; - // Arrange: enable ip question type and add some fomrcreator data + // Arrange: enable ldap select question type and add some formcreator data $this->enableConfigurableItem(LdapQuestion::class); $this->createSimpleFormcreatorForm( name: "My form", @@ -57,7 +85,7 @@ public function testIpTypeMigrationWhenEnabled(): void [ 'name' => 'My LDAP question', 'fieldtype' => 'ldapselect', - 'values' => '{"ldap_auth":"123","ldap_attribute":"456","ldap_filter":"(& (uid=*) (objectClass=inetOrgPerson))"}', + 'values' => '{"ldap_auth":"'.$authldap_id.'","ldap_attribute":"456","ldap_filter":"(& (uid=*) (objectClass=inetOrgPerson))"}', ], ], ); @@ -80,7 +108,7 @@ public function testIpTypeMigrationWhenEnabled(): void if (!$config instanceof LdapQuestionConfig) { throw new LogicException(); } - $this->assertEquals(123, $config->getAuthLdapId()); + $this->assertEquals($expected_authldap_id ?? $authldap_id, $config->getAuthLdapId()); $this->assertEquals(456, $config->getLdapAttributeId()); $this->assertEquals( "(& (uid=*) (objectClass=inetOrgPerson))", @@ -88,19 +116,23 @@ public function testIpTypeMigrationWhenEnabled(): void ); } - public function testIpTypeMigrationWhenDisabled(): void + public function testLdapSelectTypeMigrationWhenDisabled(): void { /** @var \DBmysql $DB */ global $DB; - // Arrange: add some fomrcreator data + // Arrange: add some formcreator data + $authldap = $this->createItem(AuthLDAP::class, [ + 'name' => 'My LDAP server', + 'is_active' => 1, + ]); $this->createSimpleFormcreatorForm( name: "My form", questions: [ [ 'name' => 'My LDAP question', 'fieldtype' => 'ldapselect', - 'values' => '{"ldap_auth":"123","ldap_attribute":"456","ldap_filter":"(& (uid=*) (objectClass=inetOrgPerson))"}', + 'values' => '{"ldap_auth":"'.$authldap->getId().'","ldap_attribute":"456","ldap_filter":"(& (uid=*) (objectClass=inetOrgPerson))"}', ], ], ); diff --git a/tests/Model/QuestionType/QuestionTypeTestCase.php b/tests/Model/QuestionType/QuestionTypeTestCase.php index 51a9979..a4e8a14 100644 --- a/tests/Model/QuestionType/QuestionTypeTestCase.php +++ b/tests/Model/QuestionType/QuestionTypeTestCase.php @@ -101,7 +101,7 @@ final public function testQuestionIsAvailableInTypeDropdownWhenEnabled(): void $this->assertContains($item->getName(), $options); } - final public function testIpAddressIsNotAvailableInTypeDropdownWhenDisabled(): void + final public function testQuestionIsNotAvailableInTypeDropdownWhenDisabled(): void { $item = $this->getTestedQuestionType(); From 987100ca934361bdbb68105d36e5e92799f45a15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Cailly?= Date: Wed, 31 Dec 2025 15:38:17 +0100 Subject: [PATCH 2/2] fix: PHP lint --- tests/Model/Mapper/FormcreatorLdapSelectTypeMapperTest.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/Model/Mapper/FormcreatorLdapSelectTypeMapperTest.php b/tests/Model/Mapper/FormcreatorLdapSelectTypeMapperTest.php index 5e3f7ea..a14420d 100644 --- a/tests/Model/Mapper/FormcreatorLdapSelectTypeMapperTest.php +++ b/tests/Model/Mapper/FormcreatorLdapSelectTypeMapperTest.php @@ -44,7 +44,6 @@ final class FormcreatorLdapSelectTypeMapperTest extends MapperTestCase { - public function testLdapSelectTypeMigrationWhenEnabledWithValidAuthLDAP(): void { /** @var \DBmysql $DB */ @@ -85,7 +84,7 @@ private function testLdapSelectTypeMigrationWhenEnabled(int $authldap_id, ?int $ [ 'name' => 'My LDAP question', 'fieldtype' => 'ldapselect', - 'values' => '{"ldap_auth":"'.$authldap_id.'","ldap_attribute":"456","ldap_filter":"(& (uid=*) (objectClass=inetOrgPerson))"}', + 'values' => '{"ldap_auth":"' . $authldap_id . '","ldap_attribute":"456","ldap_filter":"(& (uid=*) (objectClass=inetOrgPerson))"}', ], ], ); @@ -132,7 +131,7 @@ public function testLdapSelectTypeMigrationWhenDisabled(): void [ 'name' => 'My LDAP question', 'fieldtype' => 'ldapselect', - 'values' => '{"ldap_auth":"'.$authldap->getId().'","ldap_attribute":"456","ldap_filter":"(& (uid=*) (objectClass=inetOrgPerson))"}', + 'values' => '{"ldap_auth":"' . $authldap->getId() . '","ldap_attribute":"456","ldap_filter":"(& (uid=*) (objectClass=inetOrgPerson))"}', ], ], );