From 93db7212c86f6dcf1000f906eb52459d00c7a8b6 Mon Sep 17 00:00:00 2001
From: Rahul Shinde <shinde.r.a@gmail.com>
Date: Tue, 17 Sep 2024 16:57:09 +0530
Subject: [PATCH] 13730: Fixing "Error when creating a user without password"
 (#14107)

* 13730: Fixing "Error when creating a user without password"

* 13730-Added more coverage
---
 app/bundles/UserBundle/Entity/User.php        |  15 ++-
 .../UserControllerFunctionalTest.php          | 106 ++++++++++++++++++
 2 files changed, 116 insertions(+), 5 deletions(-)

diff --git a/app/bundles/UserBundle/Entity/User.php b/app/bundles/UserBundle/Entity/User.php
index 29780b744f8..1d7b948257a 100644
--- a/app/bundles/UserBundle/Entity/User.php
+++ b/app/bundles/UserBundle/Entity/User.php
@@ -236,7 +236,7 @@ public static function loadValidatorMetadata(ClassMetadata $metadata): void
         $metadata->addPropertyConstraint('plainPassword', new Assert\NotBlank(
             [
                 'message' => 'mautic.user.user.password.notblank',
-                'groups'  => ['CheckPassword'],
+                'groups'  => ['CheckPasswordNotBlank'],
             ]
         ));
 
@@ -262,10 +262,15 @@ public static function determineValidationGroups(Form $form): array
     {
         $data   = $form->getData();
         $groups = ['User', 'SecondPass'];
-
-        // check if creating a new user or editing an existing user and the password has been updated
-        if ($data instanceof User && (!$data->getId() || ($data->getId() && $data->getPlainPassword()))) {
-            $groups[] = 'CheckPassword';
+        if ($data instanceof User) {
+            $isNewUser        = !$data->getId();
+            $hasPlainPassword = !empty($data->getPlainPassword());
+
+            if ($isNewUser) {
+                $groups[] = $hasPlainPassword ? 'CheckPassword' : 'CheckPasswordNotBlank';
+            } elseif ($hasPlainPassword) {
+                $groups[] = 'CheckPassword';
+            }
         }
 
         return $groups;
diff --git a/app/bundles/UserBundle/Tests/Functional/Controller/UserControllerFunctionalTest.php b/app/bundles/UserBundle/Tests/Functional/Controller/UserControllerFunctionalTest.php
index a1b0c5c6191..cd95aede4d3 100644
--- a/app/bundles/UserBundle/Tests/Functional/Controller/UserControllerFunctionalTest.php
+++ b/app/bundles/UserBundle/Tests/Functional/Controller/UserControllerFunctionalTest.php
@@ -56,6 +56,112 @@ public function testEditActionFormSubmissionInvalid(): void
         $this->assertStringContainsString('The email entered is invalid.', $this->client->getResponse()->getContent());
     }
 
+    /**
+     * @param array<string, string> $data
+     *
+     * @dataProvider dataNewUserForPasswordField
+     */
+    public function testNewUserForPasswordField(array $data, string $message): void
+    {
+        $crawler = $this->client->request('GET', '/s/users/new');
+
+        $formData = [
+            'user[firstName]' => 'John',
+            'user[lastName]'  => 'Doe',
+            'user[email]'     => 'john.doe@example.com',
+        ];
+
+        $form = $crawler->selectButton('Save')->form($formData + $data);
+
+        $this->client->submit($form);
+
+        $this->assertSame(Response::HTTP_OK, $this->client->getResponse()->getStatusCode());
+        $this->assertStringContainsString($message, $this->client->getResponse()->getContent());
+    }
+
+    /**
+     * @return iterable<string, array<int, string|array<string, string>>>
+     */
+    public function dataNewUserForPasswordField(): iterable
+    {
+        yield 'Blank' => [
+            [
+                'user[plainPassword][password]' => '',
+                'user[plainPassword][confirm]'  => '',
+            ],
+            'Password cannot be blank.',
+        ];
+
+        yield 'Do not match with confirm' => [
+            [
+                'user[plainPassword][password]' => 'same',
+            ],
+            'Passwords do not match.',
+        ];
+
+        yield 'Minimum length' => [
+            [
+                'user[plainPassword][password]' => 'same',
+                'user[plainPassword][confirm]'  => 'same',
+            ],
+            'Password must be at least 6 characters.',
+        ];
+
+        yield 'No stronger' => [
+            [
+                'user[plainPassword][password]' => 'same123',
+                'user[plainPassword][confirm]'  => 'same123',
+            ],
+            'Please enter a stronger password. Your password must use a combination of upper and lower case, special characters and numbers.',
+        ];
+    }
+
+    /**
+     * @param array<string, string> $data
+     *
+     * @dataProvider dataForEditUserForPasswordField
+     */
+    public function testEditUserForPasswordField(array $data, string $message): void
+    {
+        $crawler = $this->client->request('GET', '/s/users/edit/1');
+
+        $form = $crawler->selectButton('Save')->form($data);
+
+        $this->client->submit($form);
+
+        $this->assertSame(Response::HTTP_OK, $this->client->getResponse()->getStatusCode());
+        $this->assertStringContainsString($message, $this->client->getResponse()->getContent());
+    }
+
+    /**
+     * @return iterable<string, array<int, string|array<string, string>>>
+     */
+    public function dataForEditUserForPasswordField(): iterable
+    {
+        yield 'Do not match with confirm' => [
+            [
+                'user[plainPassword][password]' => 'same',
+            ],
+            'Passwords do not match.',
+        ];
+
+        yield 'Minimum length' => [
+            [
+                'user[plainPassword][password]' => 'same',
+                'user[plainPassword][confirm]'  => 'same',
+            ],
+            'Password must be at least 6 characters.',
+        ];
+
+        yield 'No stronger' => [
+            [
+                'user[plainPassword][password]' => 'same123',
+                'user[plainPassword][confirm]'  => 'same123',
+            ],
+            'Please enter a stronger password. Your password must use a combination of upper and lower case, special characters and numbers.',
+        ];
+    }
+
     /**
      * @param array<mixed> $details
      */