From 021d827048b20a400e473ae4ce3b4d95d213e019 Mon Sep 17 00:00:00 2001
From: David Beniamine <david.beniamine@tetras-libre.fr>
Date: Fri, 28 Jan 2022 17:16:30 +0100
Subject: [PATCH] FIX password and force dt_maj not null
---
migrations/Version20211230115034.php | 2 +-
migrations/Version20220128162555.php | 31 +++++++
src/Builder/UserBuilder.php | 82 +++++++++----------
src/Controller/RegistrationController.php | 20 +++--
src/DataFixtures/UserFixtures.php | 12 ++-
src/Entity/User.php | 47 ++++++++++-
src/Form/RegistrationFormType.php | 5 +-
src/Helper/StringHelper.php | 5 ++
templates/registration/register.html.twig | 2 +-
.../functional/RegistrationControllerTest.php | 23 +++++-
translations/messages.fr.yaml | 4 +-
11 files changed, 164 insertions(+), 69 deletions(-)
create mode 100644 migrations/Version20220128162555.php
diff --git a/migrations/Version20211230115034.php b/migrations/Version20211230115034.php
index 0901b3b..ffcecec 100644
--- a/migrations/Version20211230115034.php
+++ b/migrations/Version20211230115034.php
@@ -20,7 +20,7 @@ final class Version20211230115034 extends AbstractMigration
public function up(Schema $schema): void
{
// this up() migration is auto-generated, please modify it to your needs
- $this->addSql('CREATE TABLE capsule (id INT AUTO_INCREMENT NOT NULL, aut_crea INT DEFAULT NULL, aut_maj INT DEFAULT NULL, nom VARCHAR(255) NOT NULL, dt_crea DATETIME NOT NULL, dt_maj DATETIME NOT NULL, link VARCHAR(255) NOT NULL, edition_link VARCHAR(255) NOT NULL, INDEX IDX_C268A183B11ABDF2 (aut_crea), INDEX IDX_C268A183E5F0D775 (aut_maj), UNIQUE INDEX index_capsule_nom (nom), PRIMARY KEY(id)) DEFAULT CHARACTER SET utf8 COLLATE `utf8_unicode_ci` ENGINE = InnoDB');
+ $this->addSql('CREATE TABLE capsule (id INT AUTO_INCREMENT NOT NULL, aut_crea INT NOT NULL, aut_maj INT DEFAULT NULL, nom VARCHAR(255) NOT NULL, dt_crea DATETIME NOT NULL, dt_maj DATETIME NOT NULL, link VARCHAR(255) NOT NULL, edition_link VARCHAR(255) NOT NULL, INDEX IDX_C268A183B11ABDF2 (aut_crea), INDEX IDX_C268A183E5F0D775 (aut_maj), UNIQUE INDEX index_capsule_nom (nom), PRIMARY KEY(id)) DEFAULT CHARACTER SET utf8 COLLATE `utf8_unicode_ci` ENGINE = InnoDB');
$this->addSql('CREATE TABLE editeur_capsule (capsule_id INT NOT NULL, user_id INT NOT NULL, INDEX IDX_A18592E2714704E9 (capsule_id), INDEX IDX_A18592E2A76ED395 (user_id), PRIMARY KEY(capsule_id, user_id)) DEFAULT CHARACTER SET utf8 COLLATE `utf8_unicode_ci` ENGINE = InnoDB');
$this->addSql('ALTER TABLE capsule ADD CONSTRAINT FK_C268A183B11ABDF2 FOREIGN KEY (aut_crea) REFERENCES `user` (id)');
$this->addSql('ALTER TABLE capsule ADD CONSTRAINT FK_C268A183E5F0D775 FOREIGN KEY (aut_maj) REFERENCES `user` (id)');
diff --git a/migrations/Version20220128162555.php b/migrations/Version20220128162555.php
new file mode 100644
index 0000000..32ee062
--- /dev/null
+++ b/migrations/Version20220128162555.php
@@ -0,0 +1,31 @@
+<?php
+
+declare(strict_types=1);
+
+namespace DoctrineMigrations;
+
+use Doctrine\DBAL\Schema\Schema;
+use Doctrine\Migrations\AbstractMigration;
+
+/**
+ * Auto-generated Migration: Please modify to your needs!
+ */
+final class Version20220128162555 extends AbstractMigration
+{
+ public function getDescription(): string
+ {
+ return '';
+ }
+
+ public function up(Schema $schema): void
+ {
+ // this up() migration is auto-generated, please modify it to your needs
+ $this->addSql('ALTER TABLE user ADD accept_gnl_conditions TINYINT(1) NOT NULL DEFAULT FALSE, ADD inscription_newsletter TINYINT(1) NOT NULL DEFAULT FALSE');
+ }
+
+ public function down(Schema $schema): void
+ {
+ // this down() migration is auto-generated, please modify it to your needs
+ $this->addSql('ALTER TABLE `user` DROP accept_gnl_conditions, DROP inscription_newsletter');
+ }
+}
diff --git a/src/Builder/UserBuilder.php b/src/Builder/UserBuilder.php
index 12b29b7..e314af4 100644
--- a/src/Builder/UserBuilder.php
+++ b/src/Builder/UserBuilder.php
@@ -4,62 +4,52 @@ namespace App\Builder;
use App\Entity\User;
use App\Helper\ContractHelper;
+use App\Helper\StringHelper;
use Symfony\Component\PasswordHasher\Hasher\UserPasswordHasherInterface;
class UserBuilder
{
private UserPasswordHasherInterface $password_hasher;
public User $user;
- private bool $hasRequiredEmail = false;
- private bool $hasRequiredFirstName = false;
- private bool $hasRequiredLastName = false;
- private bool $hasRequiredPassword = false;
- private bool $hasRequiredSalt = false;
- private bool $hasRequiredRoles = false;
- private bool $hasRequiredIsVerified = false;
-
- public function __construct(UserPasswordHasherInterface $password_hasher)
+
+ public function __construct(UserPasswordHasherInterface $password_hasher, ?User $user = null)
{
- $this->user = new User();
+ if (!$user instanceof User) {
+ $this->user = new User();
+ } else {
+ $this->user = $user;
+ }
$this->password_hasher = $password_hasher;
+
+ $this->user->setIsVerified(false);
}
public function withEmail(string $email): UserBuilder
{
$this->user->setEmail($email);
- $this->hasRequiredEmail = true;
return $this;
}
public function withFirstName(string $firstName): UserBuilder
{
$this->user->setFirstName($firstName);
- $this->hasRequiredFirstName = true;
return $this;
}
public function withLastName(string $lastName): UserBuilder
{
$this->user->setLastName($lastName);
- $this->hasRequiredLastName = true;
return $this;
}
- public function withSalt(string $salt): UserBuilder
- {
- $this->user->setSalt($salt);
- $this->hasRequiredSalt = true;
- return $this;
- }
-
- public function withPassword(string $password): UserBuilder
+ public function withPassword(string $salt, string $plainPassword): UserBuilder
{
ContractHelper::requires(
- $this->hasRequiredSalt,
- "The call of UserBuilder::withSalt should be called before UserBuilder::withPassword"
+ !StringHelper::isNullOrWhitespace($plainPassword),
+ 'A user should have none empty password'
);
- $this->user->setPassword($this->password_hasher->hashPassword($this->user, $password));
- $this->hasRequiredPassword = true;
+ $this->user->setSalt($salt);
+ $this->user->setPassword($this->password_hasher->hashPassword($this->user, $plainPassword));
return $this;
}
@@ -73,48 +63,50 @@ class UserBuilder
}
$this->user->setRoles($roles);
- $this->hasRequiredRoles = true;
return $this;
}
public function withIsVerified(bool $is_verified): UserBuilder
{
$this->user->setIsVerified($is_verified);
- $this->hasRequiredIsVerified = true;
return $this;
}
public function createUser(): User
{
ContractHelper::requires(
- $this->hasRequiredEmail,
- "The call of UserBuilder::withEmail should be called before UserBuilder::create"
- );
- ContractHelper::requires(
- $this->hasRequiredLastName,
- "The call of UserBuilder::withLastName should be called before UserBuilder::create"
+ $this->user->getEmail() !== null && filter_var($this->user->getEmail(), FILTER_VALIDATE_EMAIL),
+ "A user should have a valid email (current:'" . $this->user->getEmail() . "')"
);
ContractHelper::requires(
- $this->hasRequiredFirstName,
- "The call of UserBuilder::withFirstName should be called before UserBuilder::create"
+ !StringHelper::isNullOrWhitespace($this->user->getLastName()),
+ "A user must have a last name (current:'" . $this->user->getLastName() . "')"
);
ContractHelper::requires(
- $this->hasRequiredPassword,
- "The call of UserBuilder::withPassword should be called before UserBuilder::create"
+ !StringHelper::isNullOrWhitespace($this->user->getFirstName()),
+ "A user must have a first name (current:'" . $this->user->getFirstName() . "')"
);
ContractHelper::requires(
- $this->hasRequiredSalt,
- "The call of UserBuilder::withSalt should be called before UserBuilder::create"
+ !StringHelper::isNullOrWhitespace($this->user->getPassword()),
+ "A user must have a have a none empty or whitespace password"
);
ContractHelper::requires(
- $this->hasRequiredRoles,
- "The call of UserBuilder::withRoles should be called before UserBuilder::create"
- );
- ContractHelper::requires(
- $this->hasRequiredIsVerified,
- "The call of UserBuilder::withIsVerified should be called before UserBuilder::create"
+ !empty($this->user->getRoles()),
+ "A user must have a have roles"
);
return $this->user;
}
+
+ public function withAcceptGeneralConditions(bool $value): UserBuilder
+ {
+ $this->user->setAcceptGeneralConditions($value);
+
+ return $this;
+ }
+
+ public function withNewsLetterSubscription(bool $newsLetterSubscription)
+ {
+ $this->user->setSubscribedToNewsLetter($newsLetterSubscription);
+ }
}
diff --git a/src/Controller/RegistrationController.php b/src/Controller/RegistrationController.php
index faf3031..7c98102 100644
--- a/src/Controller/RegistrationController.php
+++ b/src/Controller/RegistrationController.php
@@ -2,6 +2,7 @@
namespace App\Controller;
+use App\Builder\UserBuilder;
use App\Entity\Capsule;
use App\Entity\PendingEditorInvitation;
use App\Entity\User;
@@ -53,17 +54,20 @@ class RegistrationController extends AbstractController
$form->handleRequest($request);
if ($form->isSubmitted() && $form->isValid()) {
- $user->setSalt(random_bytes(100));
+ $user = $form->getData();
+ $userBuilder = new UserBuilder($userPasswordHasher, $user);
- // encode the plain password
- $user->setPassword(
- $userPasswordHasher->hashPassword(
- $user,
+ // Ugly fix because I don't understand why those values aren't set correctly
+ $userBuilder->withAcceptGeneralConditions($form->get('acceptGeneralConditions')->getData());
+ $userBuilder->withNewsLetterSubscription($form->get('subscribedToNewsLetter')->getData() ?? false);
+
+ $userBuilder
+ ->withPassword(
+ random_bytes(100),
$form->get('plainPassword')->getData()
- )
- );
+ );
- $this->entity_manager->persist($user);
+ $this->entity_manager->persist($userBuilder->createUser());
$this->entity_manager->flush();
// generate a signed url and email it to the user
diff --git a/src/DataFixtures/UserFixtures.php b/src/DataFixtures/UserFixtures.php
index e04a4c2..769fe86 100644
--- a/src/DataFixtures/UserFixtures.php
+++ b/src/DataFixtures/UserFixtures.php
@@ -26,8 +26,7 @@ class UserFixtures extends Fixture
return $builder->withEmail("notVerified@localhost.com")
->withFirstName("Bob")
->withLastName("Smith")
- ->withSalt("")
- ->withPassword('password')
+ ->withPassword('', 'password')
->withRoles([])
->withIsVerified(false);
}
@@ -38,10 +37,10 @@ class UserFixtures extends Fixture
return $builder->withEmail("defaultUser@localhost.com")
->withFirstName("Alice")
->withLastName("Rango")
- ->withSalt("")
- ->withPassword('password')
+ ->withPassword('', 'password')
->withRoles([])
- ->withIsVerified(true);
+ ->withIsVerified(true)
+ ->withAcceptGeneralConditions(true);
}
);
@@ -50,8 +49,7 @@ class UserFixtures extends Fixture
return $builder->withEmail("defaultUser2@localhost.com")
->withFirstName("John")
->withLastName("Doe")
- ->withSalt("")
- ->withPassword('password')
+ ->withPassword("", 'password')
->withRoles([])
->withIsVerified(true);
}
diff --git a/src/Entity/User.php b/src/Entity/User.php
index 5e2fa78..e707f67 100644
--- a/src/Entity/User.php
+++ b/src/Entity/User.php
@@ -26,6 +26,16 @@ class User implements UserInterface, LegacyPasswordAuthenticatedUserInterface
*/
private int $id;
+ /**
+ * @ORM\Column(type="boolean", name="accept_gnl_conditions")
+ */
+ private bool $acceptGeneralConditions;
+
+ /**
+ * @ORM\Column(type="boolean", name="inscription_newsletter")
+ */
+ private bool $is_subscribed_news_letter;
+
/**
* @ORM\Column(type="string", length=255)
* @Assert\Email(message = "The email {{ value }} is not a valid email.")
@@ -76,7 +86,7 @@ class User implements UserInterface, LegacyPasswordAuthenticatedUserInterface
/**
* @ORM\Column(type="boolean", name="enabled")
*/
- private bool $isVerified = false;
+ private bool $isVerified;
/**
* @ORM\Column(type="string", length=255, name="salt")
@@ -92,6 +102,9 @@ class User implements UserInterface, LegacyPasswordAuthenticatedUserInterface
public function __construct()
{
$this->capsules = new ArrayCollection();
+ $this->acceptGeneralConditions = false;
+ $this->isVerified = false;
+ $this->credentialExpired = false;
}
public function getId(): int
@@ -248,4 +261,36 @@ class User implements UserInterface, LegacyPasswordAuthenticatedUserInterface
$this->capsules->removeElement($capsule);
return $this;
}
+
+ /**
+ * @return bool
+ */
+ public function hasAcceptGeneralConditions(): bool
+ {
+ return $this->acceptGeneralConditions;
+ }
+
+ /**
+ * @param bool $acceptGeneralConditions
+ */
+ public function setAcceptGeneralConditions(bool $acceptGeneralConditions): void
+ {
+ $this->acceptGeneralConditions = $acceptGeneralConditions;
+ }
+
+ /**
+ * @return bool
+ */
+ public function isSubscribedToNewsLetter(): bool
+ {
+ return $this->is_subscribed_news_letter;
+ }
+
+ /**
+ * @param bool $is_subscribed_news_letter
+ */
+ public function setSubscribedToNewsLetter(bool $is_subscribed_news_letter): void
+ {
+ $this->is_subscribed_news_letter = $is_subscribed_news_letter;
+ }
}
diff --git a/src/Form/RegistrationFormType.php b/src/Form/RegistrationFormType.php
index 6985979..9129195 100644
--- a/src/Form/RegistrationFormType.php
+++ b/src/Form/RegistrationFormType.php
@@ -5,8 +5,10 @@ namespace App\Form;
use App\Entity\User;
use Gregwar\CaptchaBundle\Type\CaptchaType;
use Symfony\Component\Form\AbstractType;
+use Symfony\Component\Form\CallbackTransformer;
use Symfony\Component\Form\Extension\Core\Type\CheckboxType;
use Symfony\Component\Form\Extension\Core\Type\EmailType;
+use Symfony\Component\Form\Extension\Core\Type\HiddenType;
use Symfony\Component\Form\Extension\Core\Type\PasswordType;
use Symfony\Component\Form\Extension\Core\Type\RepeatedType;
use Symfony\Component\Form\Extension\Core\Type\SubmitType;
@@ -75,7 +77,7 @@ class RegistrationFormType extends AbstractType
]
)
->add(
- 'agreeTerms',
+ 'acceptGeneralConditions',
CheckboxType::class,
[
'mapped' => false,
@@ -86,6 +88,7 @@ class RegistrationFormType extends AbstractType
'label' => 'registration.agreeTerms'
]
)
+ ->add('subscribedToNewsLetter', HiddenType::class)
->add(
'submit',
SubmitType::class,
diff --git a/src/Helper/StringHelper.php b/src/Helper/StringHelper.php
index 741c4b8..83e078e 100644
--- a/src/Helper/StringHelper.php
+++ b/src/Helper/StringHelper.php
@@ -8,4 +8,9 @@ class StringHelper
{
return strtoupper(sha1(random_bytes(100)));
}
+
+ public static function isNullOrWhitespace(?string $str): bool
+ {
+ return null === $str || '' === trim($str);
+ }
}
diff --git a/templates/registration/register.html.twig b/templates/registration/register.html.twig
index c7096b6..3f9b4c0 100644
--- a/templates/registration/register.html.twig
+++ b/templates/registration/register.html.twig
@@ -17,7 +17,7 @@
{{ form_row(registrationForm.plainPassword.first, {'row_attr': {'class' : 'form-group d-flex flex-column m-auto mb-4 col-12 col-sm-10 col-md-9 col-lg-8 col-xl-7 col-xxl-5'}}) }}
{{ form_row(registrationForm.plainPassword.second, {'row_attr': {'class' : 'form-group d-flex flex-column m-auto mb-4 col-12 col-sm-10 col-md-9 col-lg-8 col-xl-7 col-xxl-5'}}) }}
{{ form_row(registrationForm.captcha, {'row_attr': {'class' : 'form-group d-flex flex-column m-auto mb-5 col-12 col-sm-10 col-md-9 col-lg-8 col-xl-7 col-xxl-5'}}) }}
- {{ form_row(registrationForm.agreeTerms, {'row_attr': {'class' : 'form-group d-flex flex-column m-auto mb-4 col-auto justify-content-center'}, 'label_attr': { 'class' : 'ms-3'}}) }}
+ {{ form_row(registrationForm.acceptGeneralConditions, {'row_attr': {'class' : 'form-group d-flex flex-column m-auto mb-4 col-auto justify-content-center'}, 'label_attr': { 'class' : 'ms-3'}}) }}
{{ form_row(registrationForm.submit, {'row_attr': {'class' : 'form-group d-flex flex-column m-auto mb-5'}}) }}
{{ form_end(registrationForm) }}
</div>
diff --git a/tests/functional/RegistrationControllerTest.php b/tests/functional/RegistrationControllerTest.php
index 3095ea1..222376a 100644
--- a/tests/functional/RegistrationControllerTest.php
+++ b/tests/functional/RegistrationControllerTest.php
@@ -5,6 +5,7 @@ namespace App\Tests\functional;
use App\Entity\Capsule;
use App\Entity\User;
use App\Repository\UserRepository;
+use Exception;
use Symfony\Bundle\FrameworkBundle\KernelBrowser;
use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;
use Symfony\Component\DomCrawler\Crawler;
@@ -21,6 +22,21 @@ class RegistrationControllerTest extends WebTestCase
$this->client = static::createClient();
}
+ public function testNewUserRegistrationShouldAcceptGeneralConditions(): void
+ {
+ $userEmail = 'newUser@localhost.com';
+
+ $this->registerUser($userEmail, $this->client);
+
+ $user = $this->getUserByEmail($userEmail);
+
+ $this->assertEquals(
+ true,
+ $user->hasAcceptGeneralConditions(),
+ 'The user should accept the general conditions before registration'
+ );
+ }
+
public function testNewUserRegistrationShouldBeNotifiedOfAccountValidationByMail(): void
{
$userEmail = 'newUser@localhost.com';
@@ -115,6 +131,7 @@ class RegistrationControllerTest extends WebTestCase
/**
* @param string $userEmail The registered user email
* @return RawMessage The email message sent to the user
+ * @throws Exception
*/
private function checkEmailHasBeenSentAndGetEmailMessage(string $userEmail): RawMessage
{
@@ -126,7 +143,7 @@ class RegistrationControllerTest extends WebTestCase
$emailMessage = $this->getMailerMessage(0);
if (null === $emailMessage) {
- throw new \Exception("Email message could not be found");
+ throw new Exception("Email message could not be found");
}
$this->assertEmailAddressContains(
@@ -168,7 +185,7 @@ class RegistrationControllerTest extends WebTestCase
$form['registration_form[email]'] = $userEmail;
$form['registration_form[plainPassword][first]'] = 'password';
$form['registration_form[plainPassword][second]'] = 'password';
- $form['registration_form[agreeTerms]'] = "1";
+ $form['registration_form[acceptGeneralConditions]'] = "1";
return $client->submit($form);
}
@@ -187,7 +204,7 @@ class RegistrationControllerTest extends WebTestCase
$user = $userRepository->findOneByEmail($userEmail);
if (! $user instanceof User) {
- throw new \Exception("User does not exist.");
+ throw new Exception("User does not exist.");
}
return $user;
diff --git a/translations/messages.fr.yaml b/translations/messages.fr.yaml
index f9bee5f..010c565 100644
--- a/translations/messages.fr.yaml
+++ b/translations/messages.fr.yaml
@@ -3,10 +3,10 @@ general:
password: Mot de passe
sign_in: Se connecter
log_out: Se déconnecter
- link_expire: Le lien expirera dans
- greeting: Salutation !
go_back_to_home_page: Page d'accueil
cancel_button: Annuler
+ link_expire: Le lien expirera dans
+ greeting: Salutation !
validate: Valider
save: Enregistrer
--
GitLab