imp: Allow admin to change the LDAP identifiers

Probesys · Aug 14, 2023 · 75e4ce7 · 75e4ce7
1 parent baa12e6
commit 75e4ce7
Show file tree

Hide file tree

Showing 6 changed files with 159 additions and 32 deletions.
diff --git a/src/Controller/UsersController.php b/src/Controller/UsersController.php
@@ -13,6 +13,7 @@
 use App\Service\Sorter\UserSorter;
 use App\Utils\ConstraintErrorsFormatter;
 use App\Utils\Time;
+use Symfony\Component\DependencyInjection\Attribute\Autowire;
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\PasswordHasher\Hasher\UserPasswordHasherInterface;
@@ -133,6 +134,8 @@ public function edit(
         User $user,
         OrganizationRepository $organizationRepository,
         OrganizationSorter $organizationSorter,
+        #[Autowire(env: 'bool:LDAP_ENABLED')]
+        bool $ldapEnabled,
     ): Response {
         $this->denyAccessUnlessGranted('admin:manage:users');
 
@@ -147,6 +150,9 @@ public function edit(
             'email' => $user->getEmail(),
             'name' => $user->getName(),
             'organizationUid' => $userOrganization ? $userOrganization->getUid() : '',
+            'ldapIdentifier' => $user->getLdapIdentifier(),
+            'ldapEnabled' => $ldapEnabled,
+            'managedByLdap' => $ldapEnabled && $user->getAuthType() === 'ldap',
         ]);
     }
 
@@ -160,6 +166,8 @@ public function update(
         ValidatorInterface $validator,
         TranslatorInterface $translator,
         UserPasswordHasherInterface $passwordHasher,
+        #[Autowire(env: 'bool:LDAP_ENABLED')]
+        bool $ldapEnabled,
     ): Response {
         $this->denyAccessUnlessGranted('admin:manage:users');
 
@@ -172,6 +180,13 @@ public function update(
         /** @var string $password */
         $password = $request->request->get('password', '');
 
+        /** @var string $ldapIdentifier */
+        $ldapIdentifier = $request->request->get('ldapIdentifier', '');
+
+        if ($ldapIdentifier === '') {
+            $ldapIdentifier = null;
+        }
+
         /** @var string $organizationUid */
         $organizationUid = $request->request->get('organization', '');
 
@@ -181,13 +196,25 @@ public function update(
         $organizations = $organizationRepository->findAll();
         $organizations = $organizationSorter->asTree($organizations);
 
+        $managedByLdap = $ldapEnabled && $user->getAuthType() === 'ldap';
+
+        if ($managedByLdap) {
+            // If the user is managed by LDAP, these fields cannot be changed.
+            $email = $user->getEmail();
+            $name = $user->getName();
+            $password = '';
+        }
+
         if (!$this->isCsrfTokenValid('update user', $csrfToken)) {
             return $this->renderBadRequest('users/edit.html.twig', [
                 'user' => $user,
                 'organizations' => $organizations,
                 'email' => $email,
                 'name' => $name,
                 'organizationUid' => $organizationUid,
+                'ldapIdentifier' => $ldapIdentifier,
+                'ldapEnabled' => $ldapEnabled,
+                'managedByLdap' => $managedByLdap,
                 'error' => $translator->trans('csrf.invalid', [], 'errors'),
             ]);
         }
@@ -198,6 +225,10 @@ public function update(
         $user->setName($name);
         $user->setOrganization($organization);
 
+        if ($ldapEnabled) {
+            $user->setLdapIdentifier($ldapIdentifier);
+        }
+
         if ($password !== '') {
             $hashedPassword = $passwordHasher->hashPassword($user, $password);
             $user->setPassword($hashedPassword);
@@ -211,6 +242,9 @@ public function update(
                 'email' => $email,
                 'name' => $name,
                 'organizationUid' => $organizationUid,
+                'ldapIdentifier' => $ldapIdentifier,
+                'ldapEnabled' => $ldapEnabled,
+                'managedByLdap' => $managedByLdap,
                 'errors' => ConstraintErrorsFormatter::format($errors),
             ]);
         }

diff --git a/src/Entity/User.php b/src/Entity/User.php
@@ -256,7 +256,7 @@ public function getLdapIdentifier(): ?string
         return $this->ldapIdentifier;
     }
 
-    public function setLdapIdentifier(string $ldapIdentifier): static
+    public function setLdapIdentifier(?string $ldapIdentifier): static
     {
         $this->ldapIdentifier = $ldapIdentifier;
 

diff --git a/templates/users/edit.html.twig b/templates/users/edit.html.twig
@@ -28,6 +28,14 @@
                 {{ include('alerts/_error.html.twig', { message: error }, with_context = false) }}
             {% endif %}
 
+            {% if managedByLdap %}
+                {{ include('alerts/_alert.html.twig', {
+                    type: 'info',
+                    title: 'users.edit.ldap.information' | trans,
+                    message: 'users.edit.ldap.managed' | trans,
+                }, with_context = false) }}
+            {% endif %}
+
             <div class="flow flow--small">
                 <label for="email">
                     {{ 'users.email' | trans }}
@@ -51,15 +59,19 @@
                         aria-invalid="true"
                         aria-errormessage="email-error"
                     {% endif %}
+                    {{ managedByLdap ? 'disabled' }}
                 />
             </div>
 
             <div class="flow flow--small">
                 <label for="name">
                     {{ 'users.name' | trans }}
-                    <span class="text--secondary">
-                        {{ 'forms.optional_max_chars' | trans({ number: 100 }) }}
-                    </span>
+
+                    {% if not managedByLdap %}
+                        <span class="text--secondary">
+                            {{ 'forms.optional_max_chars' | trans({ number: 100 }) }}
+                        </span>
+                    {% endif %}
                 </label>
 
                 {% if errors.name is defined %}
@@ -80,45 +92,76 @@
                         aria-invalid="true"
                         aria-errormessage="name-error"
                     {% endif %}
+                    {{ managedByLdap ? 'disabled' }}
                 />
             </div>
 
-            <div class="flow flow--small">
-                <label for="password">
-                    {{ 'users.password' | trans }}
+            {% if ldapEnabled %}
+                <label for="ldap-identifier">
+                    {{ 'users.ldap_identifier' | trans }}
+
                     <span class="text--secondary">
                         {{ 'forms.optional' | trans }}
                     </span>
                 </label>
 
-                <p class="form__caption" id="password-caption">
-                    {{ 'users.edit.leave_password_empty' | trans }}
-                </p>
+                {% if errors.ldapIdentifier is defined %}
+                    <p class="form__error" role="alert" id="ldap-identifier-error">
+                        <span class="sr-only">{{ 'forms.error' | trans }}</span>
+                        {{ errors.ldapIdentifier }}
+                    </p>
+                {% endif %}
 
-                <div class="input-container" data-controller="password">
-                    <input
-                        type="password"
-                        id="password"
-                        name="password"
-                        autocomplete="new-password"
-                        data-password-target="input"
-                        aria-describedby="password-caption"
-                    />
-
-                    <button
-                        type="button"
-                        role="switch"
-                        data-action="password#toggle"
-                        data-password-target="button"
-                    >
-                        {{ icon('eye') }}
-                        {{ icon('eye-slash') }}
-                        <span class="sr-only">
-                            {{ 'forms.show_password' | trans }}
+                <input
+                    type="text"
+                    id="ldap-identifier"
+                    name="ldapIdentifier"
+                    value="{{ ldapIdentifier }}"
+                    {% if errors.ldapIdentifier is defined %}
+                        aria-invalid="true"
+                        aria-errormessage="ldap-identifier-error"
+                    {% endif %}
+                />
+            {% endif %}
+
+            {% if not managedByLdap %}
+                <div class="flow flow--small">
+                    <label for="password">
+                        {{ 'users.password' | trans }}
+                        <span class="text--secondary">
+                            {{ 'forms.optional' | trans }}
                         </span>
-                    </button>
+                    </label>
+
+                    <p class="form__caption" id="password-caption">
+                        {{ 'users.edit.leave_password_empty' | trans }}
+                    </p>
+
+                    <div class="input-container" data-controller="password">
+                        <input
+                            type="password"
+                            id="password"
+                            name="password"
+                            autocomplete="new-password"
+                            data-password-target="input"
+                            aria-describedby="password-caption"
+                        />
+
+                        <button
+                            type="button"
+                            role="switch"
+                            data-action="password#toggle"
+                            data-password-target="button"
+                        >
+                            {{ icon('eye') }}
+                            {{ icon('eye-slash') }}
+                            <span class="sr-only">
+                                {{ 'forms.show_password' | trans }}
+                            </span>
+                        </button>
+                    </div>
                 </div>
-            </div>
+            {% endif %}
 
             <div class="flow flow--small">
                 <label for="organization">

diff --git a/tests/Controller/UsersControllerTest.php b/tests/Controller/UsersControllerTest.php
@@ -321,6 +321,50 @@ public function testPostUpdateDoesNotChangePasswordIfEmpty(): void
         $this->assertSame($newOrganization->getId(), $otherUser->getOrganization()->getId());
     }
 
+    public function testPostUpdateDoesNotChangeEmailNameOrPasswordIfLdapIsEnabled(): void
+    {
+        $client = static::createClient();
+        /** @var \Symfony\Component\PasswordHasher\Hasher\UserPasswordHasherInterface */
+        $passwordHasher = self::getContainer()->get('security.user_password_hasher');
+        $user = UserFactory::createOne();
+        $client->loginUser($user->object());
+        $this->grantAdmin($user->object(), ['admin:manage:users']);
+        $oldEmail = '[email protected]';
+        $newEmail = '[email protected]';
+        $oldName = 'Alix Pataquès';
+        $newName = 'Benedict Aphone';
+        $oldPassword = 'secret';
+        $newPassword = 'super secret';
+        $oldLdapIdentifier = 'alix';
+        $newLdapIdentifier = 'benedict';
+        $oldOrganization = OrganizationFactory::createOne();
+        $newOrganization = OrganizationFactory::createOne();
+        $otherUser = UserFactory::createOne([
+            'email' => $oldEmail,
+            'name' => $oldName,
+            'password' => $oldPassword,
+            'organization' => $oldOrganization,
+            'ldapIdentifier' => $oldLdapIdentifier,
+        ]);
+
+        $client->request('POST', "/users/{$otherUser->getUid()}/edit", [
+            '_csrf_token' => $this->generateCsrfToken($client, 'update user'),
+            'email' => $newEmail,
+            'name' => $newName,
+            'password' => $newPassword,
+            'organization' => $newOrganization->getUid(),
+            'ldapIdentifier' => $newLdapIdentifier,
+        ]);
+
+        $this->assertResponseRedirects('/users', 302);
+        $otherUser->refresh();
+        $this->assertSame($oldEmail, $otherUser->getEmail());
+        $this->assertSame($oldName, $otherUser->getName());
+        $this->assertTrue($passwordHasher->isPasswordValid($otherUser->object(), $oldPassword));
+        $this->assertSame($newOrganization->getId(), $otherUser->getOrganization()->getId());
+        $this->assertSame($newLdapIdentifier, $otherUser->getLdapIdentifier());
+    }
+
     public function testPostUpdateAcceptsEmptyOrganization(): void
     {
         $client = static::createClient();

diff --git a/translations/messages+intl-icu.en_GB.yaml b/translations/messages+intl-icu.en_GB.yaml
@@ -321,6 +321,8 @@ users.color_scheme: 'Color scheme'
 users.color_scheme.auto: Auto
 users.color_scheme.dark: Dark
 users.color_scheme.light: Light
+users.edit.ldap.information: Information
+users.edit.ldap.managed: 'You can’t change some information because this account is managed by LDAP.'
 users.edit.leave_password_empty: 'Leave blank to keep the current password.'
 users.edit.organization_caption: 'The tickets opened by email by this user will be attached to this organization.'
 users.edit.title: 'Edit a user'
@@ -332,6 +334,7 @@ users.index.new_user: 'New user'
 users.index.number: '{count, plural, =0 {No users} one {1 user} other {# users}}'
 users.index.title: Users
 users.language: Language
+users.ldap_identifier: 'LDAP identifier'
 users.n_others: '{number, plural, =0 {no other} one {1 other} other {# other}}'
 users.name: Name
 users.new.leave_password_empty: '(leave empty to forbid login)'

diff --git a/translations/messages+intl-icu.fr_FR.yaml b/translations/messages+intl-icu.fr_FR.yaml
@@ -321,6 +321,8 @@ users.color_scheme: 'Schéma de couleurs'
 users.color_scheme.auto: Auto
 users.color_scheme.dark: Sombre
 users.color_scheme.light: Clair
+users.edit.ldap.information: Information
+users.edit.ldap.managed: 'Certaines informations ne sont pas modifiables car ce compte est géré avec LDAP.'
 users.edit.leave_password_empty: 'Laissez vide pour conserver le mot de passe actuel.'
 users.edit.organization_caption: 'Les tickets ouverts par email par cet utilisateur seront attachés à cette organisation.'
 users.edit.title: 'Modifier un utilisateur'
@@ -332,6 +334,7 @@ users.index.new_user: 'Nouvel utilisateur'
 users.index.number: '{count, plural, =0 {Aucun utilisateur} one {1 utilisateur} other {# utilisateurs}}'
 users.index.title: Utilisateurs
 users.language: Langue
+users.ldap_identifier: 'Identifiant LDAP'
 users.n_others: '{number, plural, =0 {aucun autre} one {1 autre} other {# autres}}'
 users.name: Nom
 users.new.leave_password_empty: '(laissez vide pour empêcher la connexion)'