Skip to content

Commit 228b1fa

Browse files
nickygerritsennickygerritsen
committed
Check for minimum password length upon self registration.
Fixes #2458.
1 parent 1bb4255 commit 228b1fa

File tree

7 files changed

+50
-15
lines changed

7 files changed

+50
-15
lines changed

‎webapp/config/services.yaml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ parameters:
1111
# Enable this to support removing time intervals from the contest.
1212
# This code is rarely tested and we discourage using it.
1313
removed_intervals: false
14+
# Minimum password length for users
15+
min_password_length: 10
1416

1517
services:
1618
# default configuration for services in *this* file

‎webapp/src/Controller/Jury/UserController.php

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use App\Service\SubmissionService;
1717
use App\Utils\Utils;
1818
use Doctrine\ORM\EntityManagerInterface;
19+
use Symfony\Component\DependencyInjection\Attribute\Autowire;
1920
use Symfony\Component\Form\FormInterface;
2021
use Symfony\Component\HttpFoundation\Request;
2122
use Symfony\Component\HttpFoundation\Response;
@@ -42,6 +43,8 @@ public function __construct(
4243
KernelInterface $kernel,
4344
protected readonly EventLogService $eventLogService,
4445
protected readonly TokenStorageInterface $tokenStorage,
46+
#[Autowire(param: 'min_password_length')]
47+
private readonly int $minimumPasswordLength,
4548
) {
4649
parent::__construct($em, $eventLogService, $dj, $kernel);
4750
}
@@ -197,12 +200,12 @@ public function viewAction(int $userId, SubmissionService $submissionService): R
197200

198201
public function checkPasswordLength(User $user, FormInterface $form): ?Response
199202
{
200-
if ($user->getPlainPassword() && strlen($user->getPlainPassword()) < static::MIN_PASSWORD_LENGTH) {
201-
$this->addFlash('danger', "Password should be " . static::MIN_PASSWORD_LENGTH . "+ chars.");
203+
if ($user->getPlainPassword() && strlen($user->getPlainPassword()) < $this->minimumPasswordLength) {
204+
$this->addFlash('danger', "Password should be " . $this->minimumPasswordLength . "+ chars.");
202205
return $this->render('jury/user_edit.html.twig', [
203206
'user' => $user,
204207
'form' => $form,
205-
'min_password_length' => static::MIN_PASSWORD_LENGTH,
208+
'min_password_length' => $this->minimumPasswordLength,
206209
]);
207210
}
208211

@@ -245,7 +248,6 @@ public function editAction(Request $request, int $userId): Response
245248
return $this->render('jury/user_edit.html.twig', [
246249
'user' => $user,
247250
'form' => $form,
248-
'min_password_length' => static::MIN_PASSWORD_LENGTH,
249251
]);
250252
}
251253

@@ -295,7 +297,6 @@ function () use ($user, $form) {
295297
return $this->render('jury/user_add.html.twig', [
296298
'user' => $user,
297299
'form' => $form,
298-
'min_password_length' => static::MIN_PASSWORD_LENGTH,
299300
]);
300301
}
301302

‎webapp/src/Controller/SecurityController.php

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
namespace App\Controller;
44

5+
use App\Controller\Jury\UserController;
56
use App\Entity\Team;
67
use App\Entity\TeamAffiliation;
78
use App\Entity\TeamCategory;
@@ -12,6 +13,8 @@
1213
use Doctrine\ORM\EntityManagerInterface;
1314
use Ramsey\Uuid\Uuid;
1415
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
16+
use Symfony\Component\DependencyInjection\Attribute\Autowire;
17+
use Symfony\Component\Form\FormInterface;
1518
use Symfony\Component\HttpFoundation\Request;
1619
use Symfony\Component\HttpFoundation\Response;
1720
use Symfony\Component\HttpKernel\Exception\HttpException;
@@ -25,7 +28,9 @@ class SecurityController extends AbstractController
2528
public function __construct(
2629
private readonly DOMJudgeService $dj,
2730
private readonly ConfigurationService $config,
28-
private readonly EntityManagerInterface $em
31+
private readonly EntityManagerInterface $em,
32+
#[Autowire(param: 'min_password_length')]
33+
private readonly int $minimumPasswordLength,
2934
) {}
3035

3136
#[Route(path: '/login', name: 'login')]
@@ -103,7 +108,12 @@ public function registerAction(
103108
$registration_form->handleRequest($request);
104109
if ($registration_form->isSubmitted() && $registration_form->isValid()) {
105110
$plainPass = $registration_form->get('plainPassword')->getData();
106-
$password = $passwordHasher->hashPassword($user, $plainPass);
111+
if (strlen($plainPass) < $this->minimumPasswordLength) {
112+
$this->addFlash('danger', "Password should be " . $this->minimumPasswordLength . "+ chars.");
113+
return $this->redirectToRoute('register');
114+
}
115+
116+
$password = $passwordHasher->hashPassword($user, $plainPass);
107117
$user->setPassword($password);
108118
if ((string)$user->getName() === '') {
109119
$user->setName($user->getUsername());

‎webapp/src/Form/Type/UserRegistrationType.php

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
namespace App\Form\Type;
44

5+
use App\Controller\Jury\UserController;
56
use App\Entity\Role;
67
use App\Entity\Team;
78
use App\Entity\TeamAffiliation;
@@ -12,6 +13,7 @@
1213
use Doctrine\ORM\EntityManagerInterface;
1314
use Doctrine\ORM\EntityRepository;
1415
use Symfony\Bridge\Doctrine\Form\Type\EntityType;
16+
use Symfony\Component\DependencyInjection\Attribute\Autowire;
1517
use Symfony\Component\Form\AbstractType;
1618
use Symfony\Component\Form\Extension\Core\Type\ChoiceType;
1719
use Symfony\Component\Form\Extension\Core\Type\EmailType;
@@ -35,7 +37,9 @@ class UserRegistrationType extends AbstractType
3537
public function __construct(
3638
protected readonly DOMJudgeService $dj,
3739
protected readonly ConfigurationService $config,
38-
protected readonly EntityManagerInterface $em
40+
protected readonly EntityManagerInterface $em,
41+
#[Autowire(param: 'min_password_length')]
42+
private readonly int $minimumPasswordLength,
3943
) {}
4044

4145
public function buildForm(FormBuilderInterface $builder, array $options): void
@@ -174,14 +178,17 @@ public function buildForm(FormBuilderInterface $builder, array $options): void
174178
'placeholder' => 'Password',
175179
'autocomplete' => 'new-password',
176180
'spellcheck' => 'false',
181+
'minlength' => $this->minimumPasswordLength,
177182
],
183+
'help' => sprintf('Minimum length: %d characters', $this->minimumPasswordLength),
178184
],
179185
'second_options' => [
180186
'label' => false,
181187
'attr' => [
182188
'placeholder' => 'Repeat Password',
183189
'autocomplete' => 'new-password',
184190
'spellcheck' => 'false',
191+
'minlength' => $this->minimumPasswordLength,
185192
],
186193
],
187194
'mapped' => false,

‎webapp/src/Form/Type/UserType.php

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
use App\Service\EventLogService;
1010
use Doctrine\ORM\EntityManagerInterface;
1111
use Symfony\Bridge\Doctrine\Form\Type\EntityType;
12+
use Symfony\Component\DependencyInjection\Attribute\Autowire;
1213
use Symfony\Component\Form\Extension\Core\Type\ChoiceType;
1314
use Symfony\Component\Form\Extension\Core\Type\EmailType;
1415
use Symfony\Component\Form\Extension\Core\Type\PasswordType;
@@ -21,7 +22,12 @@
2122

2223
class UserType extends AbstractExternalIdEntityType
2324
{
24-
public function __construct(protected readonly EntityManagerInterface $em, EventLogService $eventLogService)
25+
public function __construct(
26+
protected readonly EntityManagerInterface $em,
27+
EventLogService $eventLogService,
28+
#[Autowire(param: 'min_password_length')]
29+
private readonly int $minimumPasswordLength,
30+
)
2531
{
2632
parent::__construct($eventLogService);
2733
}
@@ -100,10 +106,10 @@ public function buildForm(FormBuilderInterface $builder, array $options): void
100106
$form->add('plainPassword', PasswordType::class, [
101107
'required' => false,
102108
'label' => 'Password',
103-
'help' => sprintf('Currently %s - fill to change. Any current login session of the user will be terminated.', $set),
109+
'help' => sprintf('Currently %s - fill to change. Any current login session of the user will be terminated. Minimum length: %d characters', $set, $this->minimumPasswordLength),
104110
'attr' => [
105111
'autocomplete' => 'new-password',
106-
'minlength' => UserController::MIN_PASSWORD_LENGTH,
112+
'minlength' => $this->minimumPasswordLength,
107113
],
108114
]);
109115
});

‎webapp/templates/security/register.html.twig

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,15 @@
1313
<main>
1414
<div style="text-align: center;">
1515
<img class="mb-4" src="{{ asset('images/DOMjudgelogo.svg') }}" alt="DOMjudge" width="72">
16+
</div>
17+
<div class="container-fluid">
18+
<div class="row">
19+
<div class="col-12">
20+
{% block messages %}
21+
{% include 'partials/messages.html.twig' %}
22+
{% endblock %}
23+
</div>
24+
</div>
1625
</div>
1726
{{ form_start(registration_form, { 'attr': {'class': 'form-signin'} }) }}
1827
<h1 class="h3 mb-3 fw-normal">Register Account</h1>

‎webapp/tests/Unit/Controller/PublicControllerTest.php

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -210,17 +210,17 @@ public function selfRegisterProvider(): Generator
210210
continue;
211211
}
212212
yield[['username'=>'minimaluser', 'teamName'=>'NewTeam','affiliation'=>'none'],'shirt-recognize-bar-together', $fixtures, $category];
213-
yield[['username'=>'bruteforce', 'teamName'=>'Fib(4)','affiliation'=>'none'],'0112', $fixtures, $category];
214-
yield[['username'=>'fullUser', 'name'=>'Full User', 'email'=>'[email protected]','teamName'=>'Trial','affiliation'=>'none'],'.', $fixtures, $category];
213+
yield[['username'=>'bruteforce', 'teamName'=>'Fib(9)','affiliation'=>'none'],'01123581321', $fixtures, $category];
214+
yield[['username'=>'fullUser', 'name'=>'Full User', 'email'=>'[email protected]','teamName'=>'Trial','affiliation'=>'none'],'..........', $fixtures, $category];
215215
yield[['username'=>'student@', 'teamName'=>'Student@Uni',
216216
'affiliation'=>'new','affiliationName'=>'NewUni','affiliationShortName'=>'nu'],'p@ssword_Is_long', $fixtures, $category];
217217
yield[['username'=>'winner@', 'teamName'=>'FunnyTeamname',
218218
'affiliation'=>'new','affiliationName'=>'SomeUni','affiliationShortName'=>'su','affiliationCountry'=>'SUR'],'p@ssword_Is_long', $fixtures, $category];
219219
yield[['username'=>'klasse', 'teamName'=>'Klasse', 'affiliation'=>'existing','existingAffiliation'=>'1'],'p@ssword_Is_long', $fixtures, $category];
220220
yield[['username'=>'newinstsamecountry', 'name'=>'CompetingDutchTeam', 'teamName'=>'SupperT3@m','affiliation'=>'new','affiliationName'=>'Vrije Universiteit',
221-
'affiliationShortName'=>'vu','affiliationCountry'=>'NLD'],'demo', $fixtures, $category];
221+
'affiliationShortName'=>'vu','affiliationCountry'=>'NLD'],'demodemodemo', $fixtures, $category];
222222
if (count($fixtures)===1) {
223-
yield[['username'=>'reusevaluesofexistinguser', 'name'=>'selfregistered user for example team','email'=>'[email protected]','teamName'=>'EasyEnough','affiliation'=>'none'],'demo', [...$fixtures, SelfRegisteredUserFixture::class],''];
223+
yield[['username'=>'reusevaluesofexistinguser', 'name'=>'selfregistered user for example team','email'=>'[email protected]','teamName'=>'EasyEnough','affiliation'=>'none'],'demodemodemo', [...$fixtures, SelfRegisteredUserFixture::class],''];
224224
}
225225
}
226226
}

0 commit comments

Comments
 (0)