Check for minimum password length upon self registration.

Fixes DOMjudge#2458.
meisterT · Nov 23, 2024 · 228b1fa · 228b1fa
1 parent 1bb4255
commit 228b1fa
Show file tree

Hide file tree

Showing 7 changed files with 50 additions and 15 deletions.
diff --git a/webapp/config/services.yaml b/webapp/config/services.yaml
@@ -11,6 +11,8 @@ parameters:
     # Enable this to support removing time intervals from the contest.
     # This code is rarely tested and we discourage using it.
     removed_intervals: false
+    # Minimum password length for users
+    min_password_length: 10
 
 services:
     # default configuration for services in *this* file

diff --git a/webapp/src/Controller/Jury/UserController.php b/webapp/src/Controller/Jury/UserController.php
@@ -16,6 +16,7 @@
 use App\Service\SubmissionService;
 use App\Utils\Utils;
 use Doctrine\ORM\EntityManagerInterface;
+use Symfony\Component\DependencyInjection\Attribute\Autowire;
 use Symfony\Component\Form\FormInterface;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
@@ -42,6 +43,8 @@ public function __construct(
         KernelInterface $kernel,
         protected readonly EventLogService $eventLogService,
         protected readonly TokenStorageInterface $tokenStorage,
+        #[Autowire(param: 'min_password_length')]
+        private readonly int $minimumPasswordLength,
     ) {
         parent::__construct($em, $eventLogService, $dj, $kernel);
     }
@@ -197,12 +200,12 @@ public function viewAction(int $userId, SubmissionService $submissionService): R
 
     public function checkPasswordLength(User $user, FormInterface $form): ?Response
     {
-        if ($user->getPlainPassword() && strlen($user->getPlainPassword()) < static::MIN_PASSWORD_LENGTH) {
-            $this->addFlash('danger', "Password should be " . static::MIN_PASSWORD_LENGTH . "+ chars.");
+        if ($user->getPlainPassword() && strlen($user->getPlainPassword()) < $this->minimumPasswordLength) {
+            $this->addFlash('danger', "Password should be " . $this->minimumPasswordLength . "+ chars.");
             return $this->render('jury/user_edit.html.twig', [
                 'user' => $user,
                 'form' => $form,
-                'min_password_length' => static::MIN_PASSWORD_LENGTH,
+                'min_password_length' => $this->minimumPasswordLength,
             ]);
         }
 
@@ -245,7 +248,6 @@ public function editAction(Request $request, int $userId): Response
         return $this->render('jury/user_edit.html.twig', [
             'user'                => $user,
             'form'                => $form,
-            'min_password_length' => static::MIN_PASSWORD_LENGTH,
         ]);
     }
 
@@ -295,7 +297,6 @@ function () use ($user, $form) {
         return $this->render('jury/user_add.html.twig', [
             'user' => $user,
             'form' => $form,
-            'min_password_length' => static::MIN_PASSWORD_LENGTH,
         ]);
     }
 

diff --git a/webapp/src/Controller/SecurityController.php b/webapp/src/Controller/SecurityController.php
@@ -2,6 +2,7 @@
 
 namespace App\Controller;
 
+use App\Controller\Jury\UserController;
 use App\Entity\Team;
 use App\Entity\TeamAffiliation;
 use App\Entity\TeamCategory;
@@ -12,6 +13,8 @@
 use Doctrine\ORM\EntityManagerInterface;
 use Ramsey\Uuid\Uuid;
 use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
+use Symfony\Component\DependencyInjection\Attribute\Autowire;
+use Symfony\Component\Form\FormInterface;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\HttpKernel\Exception\HttpException;
@@ -25,7 +28,9 @@ class SecurityController extends AbstractController
     public function __construct(
         private readonly DOMJudgeService $dj,
         private readonly ConfigurationService $config,
-        private readonly EntityManagerInterface $em
+        private readonly EntityManagerInterface $em,
+        #[Autowire(param: 'min_password_length')]
+        private readonly int $minimumPasswordLength,
     ) {}
 
     #[Route(path: '/login', name: 'login')]
@@ -103,7 +108,12 @@ public function registerAction(
         $registration_form->handleRequest($request);
         if ($registration_form->isSubmitted() && $registration_form->isValid()) {
             $plainPass = $registration_form->get('plainPassword')->getData();
-            $password  = $passwordHasher->hashPassword($user, $plainPass);
+            if (strlen($plainPass) < $this->minimumPasswordLength) {
+                $this->addFlash('danger', "Password should be " . $this->minimumPasswordLength . "+ chars.");
+                return $this->redirectToRoute('register');
+            }
+
+            $password = $passwordHasher->hashPassword($user, $plainPass);
             $user->setPassword($password);
             if ((string)$user->getName() === '') {
                 $user->setName($user->getUsername());

diff --git a/webapp/src/Form/Type/UserRegistrationType.php b/webapp/src/Form/Type/UserRegistrationType.php
@@ -2,6 +2,7 @@
 
 namespace App\Form\Type;
 
+use App\Controller\Jury\UserController;
 use App\Entity\Role;
 use App\Entity\Team;
 use App\Entity\TeamAffiliation;
@@ -12,6 +13,7 @@
 use Doctrine\ORM\EntityManagerInterface;
 use Doctrine\ORM\EntityRepository;
 use Symfony\Bridge\Doctrine\Form\Type\EntityType;
+use Symfony\Component\DependencyInjection\Attribute\Autowire;
 use Symfony\Component\Form\AbstractType;
 use Symfony\Component\Form\Extension\Core\Type\ChoiceType;
 use Symfony\Component\Form\Extension\Core\Type\EmailType;
@@ -35,7 +37,9 @@ class UserRegistrationType extends AbstractType
     public function __construct(
         protected readonly DOMJudgeService $dj,
         protected readonly ConfigurationService $config,
-        protected readonly EntityManagerInterface $em
+        protected readonly EntityManagerInterface $em,
+        #[Autowire(param: 'min_password_length')]
+        private readonly int $minimumPasswordLength,
     ) {}
 
     public function buildForm(FormBuilderInterface $builder, array $options): void
@@ -174,14 +178,17 @@ public function buildForm(FormBuilderInterface $builder, array $options): void
                         'placeholder' => 'Password',
                         'autocomplete' => 'new-password',
                         'spellcheck' => 'false',
+                        'minlength' => $this->minimumPasswordLength,
                     ],
+                    'help' => sprintf('Minimum length: %d characters', $this->minimumPasswordLength),
                 ],
                 'second_options' => [
                     'label' => false,
                     'attr' => [
                         'placeholder' => 'Repeat Password',
                         'autocomplete' => 'new-password',
                         'spellcheck' => 'false',
+                        'minlength' => $this->minimumPasswordLength,
                     ],
                 ],
                 'mapped' => false,

diff --git a/webapp/src/Form/Type/UserType.php b/webapp/src/Form/Type/UserType.php
@@ -9,6 +9,7 @@
 use App\Service\EventLogService;
 use Doctrine\ORM\EntityManagerInterface;
 use Symfony\Bridge\Doctrine\Form\Type\EntityType;
+use Symfony\Component\DependencyInjection\Attribute\Autowire;
 use Symfony\Component\Form\Extension\Core\Type\ChoiceType;
 use Symfony\Component\Form\Extension\Core\Type\EmailType;
 use Symfony\Component\Form\Extension\Core\Type\PasswordType;
@@ -21,7 +22,12 @@
 
 class UserType extends AbstractExternalIdEntityType
 {
-    public function __construct(protected readonly EntityManagerInterface $em, EventLogService $eventLogService)
+    public function __construct(
+        protected readonly EntityManagerInterface $em,
+        EventLogService $eventLogService,
+        #[Autowire(param: 'min_password_length')]
+        private readonly int $minimumPasswordLength,
+    )
     {
         parent::__construct($eventLogService);
     }
@@ -100,10 +106,10 @@ public function buildForm(FormBuilderInterface $builder, array $options): void
             $form->add('plainPassword', PasswordType::class, [
                 'required' => false,
                 'label' => 'Password',
-                'help' => sprintf('Currently %s - fill to change. Any current login session of the user will be terminated.', $set),
+                'help' => sprintf('Currently %s - fill to change. Any current login session of the user will be terminated. Minimum length: %d characters', $set, $this->minimumPasswordLength),
                 'attr' => [
                     'autocomplete' => 'new-password',
-                    'minlength' => UserController::MIN_PASSWORD_LENGTH,
+                    'minlength' => $this->minimumPasswordLength,
                 ],
             ]);
         });

diff --git a/webapp/templates/security/register.html.twig b/webapp/templates/security/register.html.twig
@@ -13,6 +13,15 @@
 <main>
     <div style="text-align: center;">
         <img class="mb-4" src="{{ asset('images/DOMjudgelogo.svg') }}" alt="DOMjudge" width="72">
+    </div>
+    <div class="container-fluid">
+        <div class="row">
+            <div class="col-12">
+                {% block messages %}
+                    {% include 'partials/messages.html.twig' %}
+                {% endblock %}
+            </div>
+        </div>
     </div>
         {{ form_start(registration_form, { 'attr': {'class': 'form-signin'} }) }}
         <h1 class="h3 mb-3 fw-normal">Register Account</h1>

diff --git a/webapp/tests/Unit/Controller/PublicControllerTest.php b/webapp/tests/Unit/Controller/PublicControllerTest.php
@@ -210,17 +210,17 @@ public function selfRegisterProvider(): Generator
                     continue;
                 }
                 yield[['username'=>'minimaluser', 'teamName'=>'NewTeam','affiliation'=>'none'],'shirt-recognize-bar-together', $fixtures, $category];
-                yield[['username'=>'bruteforce', 'teamName'=>'Fib(4)','affiliation'=>'none'],'0112', $fixtures, $category];
-                yield[['username'=>'fullUser', 'name'=>'Full User', 'email'=>'[email protected]','teamName'=>'Trial','affiliation'=>'none'],'.', $fixtures, $category];
+                yield[['username'=>'bruteforce', 'teamName'=>'Fib(9)','affiliation'=>'none'],'01123581321', $fixtures, $category];
+                yield[['username'=>'fullUser', 'name'=>'Full User', 'email'=>'[email protected]','teamName'=>'Trial','affiliation'=>'none'],'..........', $fixtures, $category];
                 yield[['username'=>'student@', 'teamName'=>'Student@Uni',
                        'affiliation'=>'new','affiliationName'=>'NewUni','affiliationShortName'=>'nu'],'p@ssword_Is_long', $fixtures, $category];
                 yield[['username'=>'winner@', 'teamName'=>'FunnyTeamname',
                        'affiliation'=>'new','affiliationName'=>'SomeUni','affiliationShortName'=>'su','affiliationCountry'=>'SUR'],'p@ssword_Is_long', $fixtures, $category];
                 yield[['username'=>'klasse', 'teamName'=>'Klasse', 'affiliation'=>'existing','existingAffiliation'=>'1'],'p@ssword_Is_long', $fixtures, $category];
                 yield[['username'=>'newinstsamecountry', 'name'=>'CompetingDutchTeam', 'teamName'=>'SupperT3@m','affiliation'=>'new','affiliationName'=>'Vrije Universiteit',
-                       'affiliationShortName'=>'vu','affiliationCountry'=>'NLD'],'demo', $fixtures, $category];
+                       'affiliationShortName'=>'vu','affiliationCountry'=>'NLD'],'demodemodemo', $fixtures, $category];
                 if (count($fixtures)===1) {
-                    yield[['username'=>'reusevaluesofexistinguser', 'name'=>'selfregistered user for example team','email'=>'[email protected]','teamName'=>'EasyEnough','affiliation'=>'none'],'demo', [...$fixtures, SelfRegisteredUserFixture::class],''];
+                    yield[['username'=>'reusevaluesofexistinguser', 'name'=>'selfregistered user for example team','email'=>'[email protected]','teamName'=>'EasyEnough','affiliation'=>'none'],'demodemodemo', [...$fixtures, SelfRegisteredUserFixture::class],''];
                 }
             }
         }