From e4430ac5c5bdf82b8c19c6a8c48ea85785e0eecc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Fromme?= Date: Thu, 28 May 2026 15:02:22 +0200 Subject: [PATCH] feat: backup previous password, change password via personal data form --- migrations/Version20260528120000.php | 26 +++++ .../Account/PersonalDataController.php | 12 +- src/Entity/User.php | 15 +++ src/Form/AccountPersonalDataType.php | 108 ++++++++++++++++++ src/Security/BpnAuthenticator.php | 3 + templates/account/personal_data.html.twig | 28 +++-- .../Account/PersonalDataControllerTest.php | 103 +++++++++++++++++ tests/Security/BpnAuthenticatorTest.php | 1 + tests/Service/PasswordResetManagerTest.php | 2 + 9 files changed, 285 insertions(+), 13 deletions(-) create mode 100644 migrations/Version20260528120000.php create mode 100644 src/Form/AccountPersonalDataType.php diff --git a/migrations/Version20260528120000.php b/migrations/Version20260528120000.php new file mode 100644 index 00000000..46e0d883 --- /dev/null +++ b/migrations/Version20260528120000.php @@ -0,0 +1,26 @@ +addSql('ALTER TABLE user ADD previous_password LONGTEXT DEFAULT NULL'); + } + + public function down(Schema $schema): void + { + $this->addSql('ALTER TABLE user DROP previous_password'); + } +} diff --git a/src/Controller/Account/PersonalDataController.php b/src/Controller/Account/PersonalDataController.php index b0297b53..3237854e 100644 --- a/src/Controller/Account/PersonalDataController.php +++ b/src/Controller/Account/PersonalDataController.php @@ -10,7 +10,7 @@ use App\BusProNet\Model\Notification; use App\BusProNet\Model\PersonalData; use App\Entity\User; use App\Exception\NewsletterProviderException; -use App\Form\PersonalDataType; +use App\Form\AccountPersonalDataType; use App\Htmx\HxTrait; use App\Model\NewsletterSubscriptionRequestResult; use App\Service\BookingEditDataLoader; @@ -20,6 +20,7 @@ use Psr\Log\LoggerInterface; use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\PasswordHasher\Hasher\UserPasswordHasherInterface; use Symfony\Component\Routing\Attribute\Route; use Symfony\Component\Security\Http\Attribute\IsGranted; use Symfony\Component\Validator\Validator\ValidatorInterface; @@ -43,6 +44,7 @@ class PersonalDataController extends AbstractController private readonly ValidatorInterface $validator, private readonly EntityManagerInterface $entityManager, private readonly NewsletterManager $newsletterManager, + private readonly UserPasswordHasherInterface $passwordHasher, private readonly LoggerInterface $logger, ) { } @@ -71,9 +73,8 @@ class PersonalDataController extends AbstractController $newsletterSubscribed = $this->newsletterManager->hasConfirmedOptIn($email); $newsletterPendingConfirmation = $this->newsletterManager->hasPendingConfirmation($email); - $personalDataForm = $this->createForm(PersonalDataType::class, $personalData, [ + $personalDataForm = $this->createForm(AccountPersonalDataType::class, $personalData, [ 'attr' => ['novalidate' => 'novalidate'], - 'validation_groups' => ['personal_data'], ]); $personalDataForm->handleRequest($request); @@ -81,6 +82,11 @@ class PersonalDataController extends AbstractController try { $this->apiClient->updatePersonalData($email, $personalData); + $newPassword = (string) $personalDataForm->get('newPassword')->getData(); + if ('' !== $newPassword) { + $user->setPassword($this->passwordHasher->hashPassword($user, $newPassword)); + } + // Invalidate cached bookings to ensure edit mode shows updated applicant data $this->dataLoader->invalidateUserBookingCaches($user); diff --git a/src/Entity/User.php b/src/Entity/User.php index 5b179011..9d1a8039 100644 --- a/src/Entity/User.php +++ b/src/Entity/User.php @@ -21,6 +21,9 @@ class User implements UserInterface, PasswordAuthenticatedUserInterface #[ORM\Column(type: 'text')] private ?string $password = null; + #[ORM\Column(type: 'text', nullable: true)] + private ?string $previousPassword = null; + #[ORM\Column(type: 'integer', nullable: true)] private ?int $personId = null; @@ -106,6 +109,18 @@ class User implements UserInterface, PasswordAuthenticatedUserInterface return $this; } + public function getPreviousPassword(): ?string + { + return $this->previousPassword; + } + + public function setPreviousPassword(?string $previousPassword): static + { + $this->previousPassword = $previousPassword; + + return $this; + } + public function hasLocalPasswordHash(): bool { if (null === $this->password || '' === $this->password) { diff --git a/src/Form/AccountPersonalDataType.php b/src/Form/AccountPersonalDataType.php new file mode 100644 index 00000000..42eaad65 --- /dev/null +++ b/src/Form/AccountPersonalDataType.php @@ -0,0 +1,108 @@ + */ +class AccountPersonalDataType extends AbstractType +{ + public function buildForm(FormBuilderInterface $builder, array $options): void + { + $builder + ->add('personalData', PersonalDataType::class, [ + 'label' => false, + 'inherit_data' => true, + ]) + ->add('currentPassword', PasswordType::class, [ + 'label' => 'Aktuelles Passwort', + 'mapped' => false, + 'required' => false, + 'attr' => [ + 'autocomplete' => 'current-password', + ], + 'constraints' => [ + new Sequentially([ + new NotBlank([ + 'message' => 'Bitte angeben', + ]), + new UserPassword([ + 'message' => 'Das aktuelle Passwort ist nicht korrekt.', + ]), + ], groups: ['password_change']), + ], + ]) + ->add('newPassword', RepeatedType::class, [ + 'type' => PasswordType::class, + 'mapped' => false, + 'required' => false, + 'invalid_message' => 'Die beiden Passwortangaben müssen identisch sein.', + 'first_options' => [ + 'label' => 'Neues Passwort', + 'required' => false, + 'attr' => [ + 'autocomplete' => 'new-password', + ], + 'constraints' => [ + new Sequentially([ + new NotBlank([ + 'message' => 'Bitte angeben', + ]), + new Length( + min: 8, + minMessage: 'Das Passwort muss mindestens {{ limit }} Zeichen lang sein.', + ), + ], groups: ['password_change']), + ], + ], + 'second_options' => [ + 'label' => 'Neues Passwort wiederholen', + 'required' => false, + 'attr' => [ + 'autocomplete' => 'new-password', + ], + 'constraints' => [ + new NotBlank([ + 'message' => 'Bitte angeben', + 'groups' => ['password_change'], + ]), + ], + ], + ]) + ; + } + + public function configureOptions(OptionsResolver $resolver): void + { + $resolver->setDefaults([ + 'data_class' => PersonalData::class, + 'validation_groups' => function (FormInterface $form): array { + $groups = ['Default', 'personal_data']; + + if ($this->isPasswordSectionUsed($form)) { + $groups[] = 'password_change'; + } + + return $groups; + }, + ]); + } + + private function isPasswordSectionUsed(FormInterface $form): bool + { + return '' !== (string) $form->get('currentPassword')->getData() + || '' !== (string) $form->get('newPassword')->getData(); + } +} diff --git a/src/Security/BpnAuthenticator.php b/src/Security/BpnAuthenticator.php index 859758c4..16751182 100644 --- a/src/Security/BpnAuthenticator.php +++ b/src/Security/BpnAuthenticator.php @@ -88,6 +88,9 @@ class BpnAuthenticator extends AbstractLoginFormAuthenticator implements Authent throw new CustomUserMessageAuthenticationException('Benutzername oder Passwort ist nicht korrekt.'); } + if (null === $user->getPreviousPassword()) { + $user->setPreviousPassword($user->getPassword()); + } $user->setPassword($this->passwordHasher->hashPassword($user, $passwordPlain)); } diff --git a/templates/account/personal_data.html.twig b/templates/account/personal_data.html.twig index 2282a52f..3dad7b24 100644 --- a/templates/account/personal_data.html.twig +++ b/templates/account/personal_data.html.twig @@ -44,29 +44,37 @@
{{ form_start(personalDataForm) }} + {% from '_partials/_validation_errors.html.twig' import validation_alert %} + {{ validation_alert(personalDataForm, 'Bitte prüfe deine Angaben und versuche es erneut', false) }}

Persönliches

- {{ form_row(personalDataForm.gender, { 'label_attr': { 'class': 'text-white' } }) }} - {{ form_row(personalDataForm.dateOfBirth, { 'label_attr': { 'class': 'text-white' } }) }} - {{ form_row(personalDataForm.nationality, { 'label_attr': { 'class': 'text-white' } }) }} + {{ form_row(personalDataForm.personalData.gender, { 'label_attr': { 'class': 'text-white' } }) }} + {{ form_row(personalDataForm.personalData.dateOfBirth, { 'label_attr': { 'class': 'text-white' } }) }} + {{ form_row(personalDataForm.personalData.nationality, { 'label_attr': { 'class': 'text-white' } }) }}

Kontakt

- {{ form_row(personalDataForm.email, { 'label_attr': { 'class': 'text-white' } }) }} - {{ form_row(personalDataForm.mobile, { 'label_attr': { 'class': 'text-white' } }) }} - {{ form_row(personalDataForm.phone, { 'label_attr': { 'class': 'text-white' } }) }} + {{ form_row(personalDataForm.personalData.email, { 'label_attr': { 'class': 'text-white' } }) }} + {{ form_row(personalDataForm.personalData.mobile, { 'label_attr': { 'class': 'text-white' } }) }} + {{ form_row(personalDataForm.personalData.phone, { 'label_attr': { 'class': 'text-white' } }) }}

Anschrift

- {{ form_row(personalDataForm.street, { 'label_attr': { 'class': 'text-white' } }) }} - {{ form_row(personalDataForm.postCode, { 'label_attr': { 'class': 'text-white' } }) }} - {{ form_row(personalDataForm.city, { 'label_attr': { 'class': 'text-white' } }) }} - {{ form_row(personalDataForm.country, { 'label_attr': { 'class': 'text-white' } }) }} + {{ form_row(personalDataForm.personalData.street, { 'label_attr': { 'class': 'text-white' } }) }} + {{ form_row(personalDataForm.personalData.postCode, { 'label_attr': { 'class': 'text-white' } }) }} + {{ form_row(personalDataForm.personalData.city, { 'label_attr': { 'class': 'text-white' } }) }} + {{ form_row(personalDataForm.personalData.country, { 'label_attr': { 'class': 'text-white' } }) }} +

+ Passwort ändern +

+ {{ form_row(personalDataForm.currentPassword, { 'label_attr': { 'class': 'text-white' } }) }} + {{ form_row(personalDataForm.newPassword.first, { 'label_attr': { 'class': 'text-white' } }) }} + {{ form_row(personalDataForm.newPassword.second, { 'label_attr': { 'class': 'text-white' } }) }}
diff --git a/tests/Controller/Account/PersonalDataControllerTest.php b/tests/Controller/Account/PersonalDataControllerTest.php index 5d087b7d..eb921ccc 100644 --- a/tests/Controller/Account/PersonalDataControllerTest.php +++ b/tests/Controller/Account/PersonalDataControllerTest.php @@ -21,6 +21,7 @@ use Symfony\Component\HttpFoundation\Session\Session; use Symfony\Component\HttpFoundation\Session\Storage\MockArraySessionStorage; use Symfony\Component\Routing\Generator\UrlGeneratorInterface; use Symfony\Component\Security\Core\User\UserInterface; +use Symfony\Component\PasswordHasher\Hasher\UserPasswordHasherInterface; use Symfony\Component\Validator\Constraint; use Symfony\Component\Validator\ConstraintValidatorFactory; use Symfony\Component\Validator\ConstraintValidatorInterface; @@ -189,6 +190,7 @@ class PersonalDataControllerTest extends TestCase $controller->entityManager ->expects(self::once()) ->method('flush'); + $controller->passwordHasher->expects(self::never())->method('hashPassword'); $request = Request::create('/personal-data', 'POST'); $request->setSession(new Session(new MockArraySessionStorage())); @@ -196,18 +198,108 @@ class PersonalDataControllerTest extends TestCase self::assertSame(302, $response->getStatusCode()); self::assertTrue($controller->user->isProfileComplete()); + self::assertSame('secret', $controller->user->getPassword()); + self::assertSame('legacy-backup', $controller->user->getPreviousPassword()); self::assertSame([ ['success', 'Deine persönlichen Daten wurden aktualisiert'], ], $controller->flashes); } + public function testIndexHashesPasswordWhenNewPasswordWasProvided(): void + { + $personalData = $this->createCompletePersonalData(); + $controller = $this->createController(true, true, $this->validator(), 'NewPassw0rd!'); + + $controller->apiClient + ->expects(self::once()) + ->method('getPersonalData') + ->with('customer@example.com', null, null) + ->willReturn($personalData); + $controller->apiClient + ->expects(self::once()) + ->method('updatePersonalData') + ->with('customer@example.com', $personalData) + ->willReturn($personalData); + $controller->newsletterManager + ->expects(self::once()) + ->method('hasConfirmedOptIn') + ->with('customer@example.com') + ->willReturn(false); + $controller->newsletterManager + ->expects(self::once()) + ->method('hasPendingConfirmation') + ->with('customer@example.com') + ->willReturn(false); + $controller->dataLoader + ->expects(self::once()) + ->method('invalidateUserBookingCaches') + ->with($controller->user); + $controller->entityManager + ->expects(self::once()) + ->method('flush'); + $controller->passwordHasher + ->expects(self::once()) + ->method('hashPassword') + ->with($controller->user, 'NewPassw0rd!') + ->willReturn('new-local-hash'); + + $request = Request::create('/personal-data', 'POST'); + $request->setSession(new Session(new MockArraySessionStorage())); + $response = $controller->index($request); + + self::assertSame(302, $response->getStatusCode()); + self::assertTrue($controller->user->isProfileComplete()); + self::assertSame('new-local-hash', $controller->user->getPassword()); + self::assertSame('legacy-backup', $controller->user->getPreviousPassword()); + self::assertSame([ + ['success', 'Deine persönlichen Daten wurden aktualisiert'], + ], $controller->flashes); + } + + public function testIndexDoesNotPersistInvalidFormSubmission(): void + { + $controller = $this->createController(true, false, $this->validator(), 'NewPassw0rd!'); + + $controller->apiClient + ->expects(self::once()) + ->method('getPersonalData') + ->with('customer@example.com', null, null) + ->willReturn($this->createCompletePersonalData()); + $controller->apiClient->expects(self::never())->method('updatePersonalData'); + $controller->newsletterManager + ->expects(self::once()) + ->method('hasConfirmedOptIn') + ->with('customer@example.com') + ->willReturn(false); + $controller->newsletterManager + ->expects(self::once()) + ->method('hasPendingConfirmation') + ->with('customer@example.com') + ->willReturn(false); + $controller->dataLoader->expects(self::never())->method('invalidateUserBookingCaches'); + $controller->entityManager->expects(self::never())->method('flush'); + $controller->passwordHasher->expects(self::never())->method('hashPassword'); + + $request = Request::create('/personal-data', 'POST'); + $request->setSession(new Session(new MockArraySessionStorage())); + $response = $controller->index($request); + + self::assertSame(200, $response->getStatusCode()); + self::assertSame('account/personal_data.html.twig', $controller->renderedView); + self::assertSame([], $controller->flashes); + self::assertSame('secret', $controller->user->getPassword()); + self::assertSame('legacy-backup', $controller->user->getPreviousPassword()); + } + private function createController( bool $submitted = false, bool $valid = false, ?ValidatorInterface $validator = null, + string $newPassword = '', ): TestablePersonalDataController { $user = new User('customer@example.com'); $user->setPassword('secret'); + $user->setPreviousPassword('legacy-backup'); $apiClient = $this->createMock(ApiClient::class); @@ -221,6 +313,14 @@ class PersonalDataControllerTest extends TestCase $form ->method('isValid') ->willReturn($valid); + $newPasswordField = $this->createMock(FormInterface::class); + $newPasswordField + ->method('getData') + ->willReturn($newPassword); + $form + ->method('get') + ->with('newPassword') + ->willReturn($newPasswordField); $validator ??= $this->createMock(ValidatorInterface::class); @@ -230,6 +330,7 @@ class PersonalDataControllerTest extends TestCase $validator, $this->createMock(EntityManagerInterface::class), $this->createMock(NewsletterManager::class), + $this->createMock(UserPasswordHasherInterface::class), $this->createMock(LoggerInterface::class), $user, $form, @@ -301,6 +402,7 @@ final class TestablePersonalDataController extends PersonalDataController public readonly ValidatorInterface $validator, public readonly EntityManagerInterface $entityManager, public readonly NewsletterManager $newsletterManager, + public readonly UserPasswordHasherInterface $passwordHasher, LoggerInterface $logger, public readonly User $user, private readonly FormInterface $form, @@ -311,6 +413,7 @@ final class TestablePersonalDataController extends PersonalDataController $validator, $entityManager, $newsletterManager, + $passwordHasher, $logger, ); } diff --git a/tests/Security/BpnAuthenticatorTest.php b/tests/Security/BpnAuthenticatorTest.php index 3c425967..a9e09a47 100644 --- a/tests/Security/BpnAuthenticatorTest.php +++ b/tests/Security/BpnAuthenticatorTest.php @@ -155,6 +155,7 @@ class BpnAuthenticatorTest extends TestCase $passport = $authenticator->authenticate($this->createRequest('customer@example.com', 'secret')); self::assertInstanceOf(SelfValidatingPassport::class, $passport); + self::assertSame(base64_encode('legacy-ciphertext'), $user->getPreviousPassword()); self::assertSame('local-hash', $user->getPassword()); self::assertSame(123, $user->getAddressId()); self::assertSame(456, $user->getPersonId()); diff --git a/tests/Service/PasswordResetManagerTest.php b/tests/Service/PasswordResetManagerTest.php index 985cc42f..60b3215c 100644 --- a/tests/Service/PasswordResetManagerTest.php +++ b/tests/Service/PasswordResetManagerTest.php @@ -145,6 +145,7 @@ class PasswordResetManagerTest extends TestCase { $user = new User('customer@example.com'); $user->setPasswordResetNonce('current-nonce'); + $user->setPreviousPassword('legacy-backup'); $userRepository = $this->createMock(UserRepository::class); $entityManager = $this->createMock(EntityManagerInterface::class); @@ -167,6 +168,7 @@ class PasswordResetManagerTest extends TestCase $manager->resetPassword($user, 'new-password'); self::assertSame('hashed-password', $user->getPassword()); + self::assertSame('legacy-backup', $user->getPreviousPassword()); self::assertNotSame('current-nonce', $user->getPasswordResetNonce()); self::assertMatchesRegularExpression('/^[a-f0-9]{64}$/', (string) $user->getPasswordResetNonce()); }