Skip to content

Commit 3b879d3

Browse files
authored
fix(ldap-select): Restrict AuthLdap directories to only those that are active
1 parent 66a8887 commit 3b879d3

File tree

4 files changed

+51
-10
lines changed

4 files changed

+51
-10
lines changed

src/Model/Mapper/FormcreatorLdapSelectTypeMapper.php

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333

3434
namespace GlpiPlugin\Advancedforms\Model\Mapper;
3535

36+
use AuthLDAP;
3637
use Glpi\Form\Migration\FormQuestionDataConverterInterface;
3738
use GlpiPlugin\Advancedforms\Model\QuestionType\LdapQuestion;
3839
use GlpiPlugin\Advancedforms\Model\QuestionType\LdapQuestionConfig;
@@ -63,8 +64,16 @@ public function convertExtraData(array $rawData): array
6364

6465
/** @var array{ldap_auth: int, ldap_filter: string, ldap_attribute: int} $data */
6566
$data = json_decode($rawData['values'], associative: true);
67+
68+
// Ensure LDAP auth exists and is active
69+
$authLdap = new AuthLDAP();
70+
$authLdapId = 0;
71+
if ($authLdap->getFromDB($data['ldap_auth']) && $authLdap->fields['is_active']) {
72+
$authLdapId = $authLdap->getId();
73+
}
74+
6675
return [
67-
LdapQuestionConfig::AUTHLDAP_ID => $data['ldap_auth'],
76+
LdapQuestionConfig::AUTHLDAP_ID => $authLdapId,
6877
LdapQuestionConfig::LDAP_FILTER => $data['ldap_filter'],
6978
LdapQuestionConfig::LDAP_ATTRIBUTE_ID => $data['ldap_attribute'],
7079
];

templates/editor/question_types/ldap_select_config.html.twig

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@
6767
'is_horizontal' : false,
6868
'init' : question is not null ? true : false,
6969
'on_change' : 'plugin_advancedforms_on_ldap_change(this)',
70+
'condition' : {'is_active': 1},
7071
}
7172
) }}
7273

tests/Model/Mapper/FormcreatorLdapSelectTypeMapperTest.php

Lines changed: 39 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -33,31 +33,58 @@
3333

3434
namespace GlpiPlugin\Advancedforms\Tests\Model\Mapper;
3535

36+
use AuthLDAP;
3637
use Glpi\Form\AccessControl\FormAccessControlManager;
3738
use Glpi\Form\Migration\FormMigration;
3839
use Glpi\Form\Question;
39-
use GlpiPlugin\Advancedforms\Model\QuestionType\IpAddressQuestion;
4040
use GlpiPlugin\Advancedforms\Model\QuestionType\LdapQuestion;
4141
use GlpiPlugin\Advancedforms\Model\QuestionType\LdapQuestionConfig;
4242
use LogicException;
4343
use RuntimeException;
4444

4545
final class FormcreatorLdapSelectTypeMapperTest extends MapperTestCase
4646
{
47-
public function testIpTypeMigrationWhenEnabled(): void
47+
public function testLdapSelectTypeMigrationWhenEnabledWithValidAuthLDAP(): void
4848
{
4949
/** @var \DBmysql $DB */
5050
global $DB;
5151

52-
// Arrange: enable ip question type and add some fomrcreator data
52+
// Arrange: create a valid AuthLDAP
53+
$authldap = $this->createItem(AuthLDAP::class, [
54+
'name' => 'My LDAP server',
55+
'is_active' => 1,
56+
]);
57+
58+
// Act & Assert: test migration with the valid AuthLDAP
59+
$this->testLdapSelectTypeMigrationWhenEnabled(
60+
authldap_id: $authldap->getId(),
61+
expected_authldap_id: null,
62+
);
63+
}
64+
65+
public function testLdapSelectTypeMigrationWhenEnabledWithInvalidAuthLDAP(): void
66+
{
67+
// Act & Assert: test migration with an invalid AuthLDAP
68+
$this->testLdapSelectTypeMigrationWhenEnabled(
69+
authldap_id: 123,
70+
expected_authldap_id: 0,
71+
);
72+
}
73+
74+
private function testLdapSelectTypeMigrationWhenEnabled(int $authldap_id, ?int $expected_authldap_id): void
75+
{
76+
/** @var \DBmysql $DB */
77+
global $DB;
78+
79+
// Arrange: enable ldap select question type and add some formcreator data
5380
$this->enableConfigurableItem(LdapQuestion::class);
5481
$this->createSimpleFormcreatorForm(
5582
name: "My form",
5683
questions: [
5784
[
5885
'name' => 'My LDAP question',
5986
'fieldtype' => 'ldapselect',
60-
'values' => '{"ldap_auth":"123","ldap_attribute":"456","ldap_filter":"(& (uid=*) (objectClass=inetOrgPerson))"}',
87+
'values' => '{"ldap_auth":"' . $authldap_id . '","ldap_attribute":"456","ldap_filter":"(& (uid=*) (objectClass=inetOrgPerson))"}',
6188
],
6289
],
6390
);
@@ -80,27 +107,31 @@ public function testIpTypeMigrationWhenEnabled(): void
80107
if (!$config instanceof LdapQuestionConfig) {
81108
throw new LogicException();
82109
}
83-
$this->assertEquals(123, $config->getAuthLdapId());
110+
$this->assertEquals($expected_authldap_id ?? $authldap_id, $config->getAuthLdapId());
84111
$this->assertEquals(456, $config->getLdapAttributeId());
85112
$this->assertEquals(
86113
"(& (uid=*) (objectClass=inetOrgPerson))",
87114
$config->getLdapFilter(),
88115
);
89116
}
90117

91-
public function testIpTypeMigrationWhenDisabled(): void
118+
public function testLdapSelectTypeMigrationWhenDisabled(): void
92119
{
93120
/** @var \DBmysql $DB */
94121
global $DB;
95122

96-
// Arrange: add some fomrcreator data
123+
// Arrange: add some formcreator data
124+
$authldap = $this->createItem(AuthLDAP::class, [
125+
'name' => 'My LDAP server',
126+
'is_active' => 1,
127+
]);
97128
$this->createSimpleFormcreatorForm(
98129
name: "My form",
99130
questions: [
100131
[
101132
'name' => 'My LDAP question',
102133
'fieldtype' => 'ldapselect',
103-
'values' => '{"ldap_auth":"123","ldap_attribute":"456","ldap_filter":"(& (uid=*) (objectClass=inetOrgPerson))"}',
134+
'values' => '{"ldap_auth":"' . $authldap->getId() . '","ldap_attribute":"456","ldap_filter":"(& (uid=*) (objectClass=inetOrgPerson))"}',
104135
],
105136
],
106137
);

tests/Model/QuestionType/QuestionTypeTestCase.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ final public function testQuestionIsAvailableInTypeDropdownWhenEnabled(): void
101101
$this->assertContains($item->getName(), $options);
102102
}
103103

104-
final public function testIpAddressIsNotAvailableInTypeDropdownWhenDisabled(): void
104+
final public function testQuestionIsNotAvailableInTypeDropdownWhenDisabled(): void
105105
{
106106
$item = $this->getTestedQuestionType();
107107

0 commit comments

Comments
 (0)