fix: prevent empty phone numbers from being accepted when updating user information (#774)

This commit is contained in:
Maxiloud 2025-10-31 16:35:20 +01:00 committed by Paul Andrieux
parent 92e4466f4f
commit 9d98b8ffc6
4 changed files with 85 additions and 36 deletions

View file

@ -21,12 +21,15 @@ use App\MessageBus\CommandBus;
use App\MessageBus\QueryBus; use App\MessageBus\QueryBus;
use App\MessageHandler\Command\Security\AccountCreateStep1CommandHandler; use App\MessageHandler\Command\Security\AccountCreateStep1CommandHandler;
use App\Repository\ConfigurationRepository; use App\Repository\ConfigurationRepository;
use libphonenumber\PhoneNumber;
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
use Symfony\Bundle\SecurityBundle\Security; use Symfony\Bundle\SecurityBundle\Security;
use Symfony\Component\Form\FormError;
use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\Messenger\Exception\HandlerFailedException; use Symfony\Component\Messenger\Exception\HandlerFailedException;
use Symfony\Component\Routing\Annotation\Route; use Symfony\Component\Routing\Annotation\Route;
use Symfony\Contracts\Translation\TranslatorInterface;
/** /**
* @see AccountCreateActionStep1Test * @see AccountCreateActionStep1Test
@ -44,6 +47,7 @@ final class AccountCreateController extends AbstractController
private readonly CommandBus $commandBus, private readonly CommandBus $commandBus,
private readonly Security $security, private readonly Security $security,
private readonly ConfigurationRepository $configurationRepository, private readonly ConfigurationRepository $configurationRepository,
private readonly TranslatorInterface $translator,
) { ) {
$this->i18nPrefix = $this->getI18nPrefix(); $this->i18nPrefix = $this->getI18nPrefix();
} }
@ -103,7 +107,21 @@ final class AccountCreateController extends AbstractController
$configuration = $this->configurationRepository->getInstanceConfigurationOrCreate(); $configuration = $this->configurationRepository->getInstanceConfigurationOrCreate();
// nominal case: user found and token not expired // nominal case: user found and token not expired
$form = $this->createForm(AccountCreateStep2FormType::class, $user->setStep2Defaults())->handleRequest($request); $form = $this->createForm(AccountCreateStep2FormType::class, $user->setStep2Defaults())->handleRequest($request);
if ($form->isSubmitted() && $form->isValid()) { if ($form->isSubmitted()) {
$phone = $form->get('phone')->getData() ?? '';
if (!$phone instanceof PhoneNumber || $phone->getNationalNumber() === '') {
$form->get('phone')->addError(
new FormError($this->translator->trans('account_create.phone.empty.error', [], 'validators'))
);
}
if ($phone instanceof PhoneNumber && \strlen($phone->getNationalNumber() ?? '') < 8) {
$form->get('phone')->addError(
new FormError($this->translator->trans('account_create.phone.short.error', [], 'validators'))
);
}
if ($form->isValid()) {
/** @var User $user */ /** @var User $user */
$user = $form->getData(); $user = $form->getData();
$user->setType(UserType::USER); $user->setType(UserType::USER);
@ -138,6 +156,7 @@ final class AccountCreateController extends AbstractController
return $this->redirectToRoute(MyAccountAction::ROUTE); return $this->redirectToRoute(MyAccountAction::ROUTE);
} }
}
return $this->render('pages/register/step2.html.twig', compact('form', 'user')); return $this->render('pages/register/step2.html.twig', compact('form', 'user'));
} }

View file

@ -13,7 +13,9 @@ use App\Form\Type\User\EditProfileFormType;
use App\Repository\UserRepository; use App\Repository\UserRepository;
use App\Tests\Functional\Controller\User\Account\EditProfileActionTest; use App\Tests\Functional\Controller\User\Account\EditProfileActionTest;
use Doctrine\ORM\EntityManagerInterface; use Doctrine\ORM\EntityManagerInterface;
use libphonenumber\PhoneNumber;
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
use Symfony\Component\Form\FormError;
use Symfony\Component\HttpFoundation\File\UploadedFile; use Symfony\Component\HttpFoundation\File\UploadedFile;
use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpFoundation\Response;
@ -21,6 +23,7 @@ use Symfony\Component\Routing\Annotation\Route;
use Symfony\Component\Security\Core\Authorization\Voter\AuthenticatedVoter; use Symfony\Component\Security\Core\Authorization\Voter\AuthenticatedVoter;
use Symfony\Component\Security\Http\Attribute\CurrentUser; use Symfony\Component\Security\Http\Attribute\CurrentUser;
use Symfony\Component\Security\Http\Attribute\IsGranted; use Symfony\Component\Security\Http\Attribute\IsGranted;
use Symfony\Contracts\Translation\TranslatorInterface;
/** /**
* @see EditProfileActionTest * @see EditProfileActionTest
@ -34,6 +37,7 @@ final class EditProfileAction extends AbstractController
private readonly UserRepository $userRepository, private readonly UserRepository $userRepository,
private readonly UserManager $userManager, private readonly UserManager $userManager,
private readonly EntityManagerInterface $entityManager, private readonly EntityManagerInterface $entityManager,
private readonly TranslatorInterface $translator,
) { ) {
} }
@ -45,7 +49,22 @@ final class EditProfileAction extends AbstractController
public function __invoke(Request $request, #[CurrentUser] User $user): Response public function __invoke(Request $request, #[CurrentUser] User $user): Response
{ {
$form = $this->createForm(EditProfileFormType::class, $user)->handleRequest($request); $form = $this->createForm(EditProfileFormType::class, $user)->handleRequest($request);
if ($form->isSubmitted() && $form->isValid()) { if ($form->isSubmitted()) {
$phone = $form->get('phone')->getData() ?? '';
if (!$phone instanceof PhoneNumber || $phone->getNationalNumber() === '') {
$form->get('phone')->addError(
new FormError($this->translator->trans('account_create.phone.empty.error', [], 'validators'))
);
}
if ($phone instanceof PhoneNumber && \strlen($phone->getNationalNumber() ?? '') < 8) {
$form->get('phone')->addError(
new FormError($this->translator->trans('account_create.phone.short.error', [], 'validators'))
);
}
if ($form->isValid()) {
/** @var UploadedFile|null $avatar */ /** @var UploadedFile|null $avatar */
$avatar = $form->get('avatar')->getData(); $avatar = $form->get('avatar')->getData();
$this->userManager->upload($avatar, $user); $this->userManager->upload($avatar, $user);
@ -54,6 +73,7 @@ final class EditProfileAction extends AbstractController
return $this->redirectToRoute(MyAccountAction::ROUTE); return $this->redirectToRoute(MyAccountAction::ROUTE);
} }
}
// In case of error, we must reload the original firstname (to display it in navbar) // In case of error, we must reload the original firstname (to display it in navbar)
$this->entityManager->refresh($user); $this->entityManager->refresh($user);

View file

@ -44,7 +44,7 @@ final class EditProfileActionTest extends WebTestCase
$form->getName().'[category]' => TestReference::CATEGORY_OBJECT_1, $form->getName().'[category]' => TestReference::CATEGORY_OBJECT_1,
$form->getName().'[description]' => 'description test', $form->getName().'[description]' => 'description test',
$form->getName().'[phone][country]' => 'FR', $form->getName().'[phone][country]' => 'FR',
$form->getName().'[phone][number]' => '', $form->getName().'[phone][number]' => '634563424',
$form->getName().'[smsNotifications]' => false, $form->getName().'[smsNotifications]' => false,
]); ]);
@ -53,7 +53,7 @@ final class EditProfileActionTest extends WebTestCase
/** @var User $editedUser */ /** @var User $editedUser */
$editedUser = $repo->find(TestReference::USER_16); $editedUser = $repo->find(TestReference::USER_16);
self::assertNull($editedUser->getPhoneNumber()); self::assertNotNull($editedUser->getPhoneNumber());
self::assertResponseRedirects(); self::assertResponseRedirects();
$client->followRedirect(); $client->followRedirect();

View file

@ -26,6 +26,16 @@
<target>Le nom du lieu est obligatoire</target> <target>Le nom du lieu est obligatoire</target>
</trans-unit> </trans-unit>
<trans-unit id="tY0WDru" resname="account_create.phone.empty.error">
<source>account_create.phone.empty.error</source>
<target>Le numéro de téléphone est obligatoire</target>
</trans-unit>
<trans-unit id="tY0WDzu" resname="account_create.phone.short.error">
<source>account_create.phone.short.error</source>
<target>Veuillez indiquer un numéro de téléphone valide</target>
</trans-unit>
</body> </body>
</file> </file>
</xliff> </xliff>