From 7c8dbeef485871a6ff2131d9208d2974fd2abca2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Fromme?= Date: Wed, 28 Oct 2026 13:48:51 +0100 Subject: [PATCH] fix: validate newly entered phone numbers --- src/BusProNet/Model/Communication.php | 31 +++++ .../XmlParser/PersonalDataParser.php | 1 + src/Form/BookingApplicantType.php | 1 - src/Form/BookingParticipantType.php | 1 - src/Form/Model/ParticipantDto.php | 15 +++ src/Form/Model/VoucherPurchaseDto.php | 27 +++++ src/Form/PersonalDataType.php | 2 - src/Form/VoucherPurchaseStep2Type.php | 2 - src/Service/ParticipantDataPrefiller.php | 1 + src/Validator/Constraints/PhoneNumber.php | 44 +++++++ .../Validator/Constraints/PhoneNumberTest.php | 108 ++++++++++++++++++ 11 files changed, 227 insertions(+), 6 deletions(-) create mode 100644 src/Validator/Constraints/PhoneNumber.php create mode 100644 tests/Validator/Constraints/PhoneNumberTest.php diff --git a/src/BusProNet/Model/Communication.php b/src/BusProNet/Model/Communication.php index a08b1584..2532500b 100644 --- a/src/BusProNet/Model/Communication.php +++ b/src/BusProNet/Model/Communication.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace App\BusProNet\Model; +use App\Validator\Constraints as AppAssert; use Symfony\Component\Validator\Constraints as Assert; /** @@ -15,17 +16,47 @@ use Symfony\Component\Validator\Constraints as Assert; */ class Communication { + #[Assert\When( + expression: 'this.isPhoneNewlyEntered()', + constraints: [new AppAssert\PhoneNumber()], + groups: ['personal_data'], + )] public ?string $phone = null; #[Assert\NotBlank(message: 'Bitte angeben', groups: ['personal_data'])] + #[Assert\When( + expression: 'this.isMobileNewlyEntered()', + constraints: [new AppAssert\PhoneNumber()], + groups: ['personal_data'], + )] public ?string $mobile = null; + // The numbers as loaded from BusPro, which go back unchanged and are exempt from PhoneNumber. + public ?string $loadedPhone = null; + public ?string $loadedMobile = null; + #[Assert\NotBlank(message: 'Bitte angeben', groups: ['personal_identity'])] #[Assert\Email(message: 'Bitte eine gültige Adresse angeben', mode: 'strict', groups: ['personal_identity'])] public ?string $email = null; public bool $newsletter = false; + public function rememberLoadedNumbers(): void + { + $this->loadedPhone = $this->phone; + $this->loadedMobile = $this->mobile; + } + + public function isPhoneNewlyEntered(): bool + { + return AppAssert\PhoneNumber::isNewlyEntered($this->phone, $this->loadedPhone); + } + + public function isMobileNewlyEntered(): bool + { + return AppAssert\PhoneNumber::isNewlyEntered($this->mobile, $this->loadedMobile); + } + /** * Converts the communication data to API payload format. * diff --git a/src/BusProNet/XmlParser/PersonalDataParser.php b/src/BusProNet/XmlParser/PersonalDataParser.php index e9dcb0c6..8a8343f5 100644 --- a/src/BusProNet/XmlParser/PersonalDataParser.php +++ b/src/BusProNet/XmlParser/PersonalDataParser.php @@ -50,6 +50,7 @@ class PersonalDataParser extends AbstractParser $communication->mobile = $this->getStringOrNullValue($contactDataNode->filterXPath('//telefonmobil')); $communication->email = $this->getStringOrNullValue($contactDataNode->filterXPath('//email')); $communication->newsletter = $this->getBoolValue($contactDataNode->filterXPath('//newsletter')); + $communication->rememberLoadedNumbers(); $personalData->communication = $communication; } diff --git a/src/Form/BookingApplicantType.php b/src/Form/BookingApplicantType.php index 240fe510..27436a0c 100644 --- a/src/Form/BookingApplicantType.php +++ b/src/Form/BookingApplicantType.php @@ -74,7 +74,6 @@ class BookingApplicantType extends AbstractType ->add('mobile', TextType::class, [ 'label' => 'Telefon (mobil)', 'required' => true, - 'sanitize_html' => true, 'attr' => [ 'autocomplete' => 'leave-me-alone-chrome', ], diff --git a/src/Form/BookingParticipantType.php b/src/Form/BookingParticipantType.php index 994953e4..a1e9dc0c 100644 --- a/src/Form/BookingParticipantType.php +++ b/src/Form/BookingParticipantType.php @@ -222,7 +222,6 @@ class BookingParticipantType extends AbstractType $form->add('mobile', TextType::class, $this->mergeFieldState([ 'label' => 'Telefon (mobil)', 'required' => false, - 'sanitize_html' => true, 'property_path' => 'participant.mobile', 'attr' => [ 'autocomplete' => 'leave-me-alone-chrome', diff --git a/src/Form/Model/ParticipantDto.php b/src/Form/Model/ParticipantDto.php index 0ac178c6..0a1cd4bc 100644 --- a/src/Form/Model/ParticipantDto.php +++ b/src/Form/Model/ParticipantDto.php @@ -93,8 +93,17 @@ class ParticipantDto )] public ?string $email = null; + #[Assert\When( + expression: 'this.isMobileNewlyEntered()', + constraints: [new AppAssert\PhoneNumber()], + groups: ['booking_edit', 'booking_create'], + )] public ?string $mobile = null; + // The number as loaded from BusPro, which goes back unchanged and is exempt from PhoneNumber. + // Public so it survives the session round trip through __unserialize(). + public ?string $loadedMobile = null; + #[Assert\Valid(groups: ['booking_edit', 'booking_create'])] #[Assert\NotNull(message: 'Bitte Adresse angeben', groups: ['applicant_address'])] public ?Address $address = null; @@ -240,6 +249,7 @@ class ParticipantDto $instance->nationality = $personalData->nationality ?: 'D'; $instance->email = $personalData->communication->email; $instance->mobile = $personalData->communication->mobile; + $instance->loadedMobile = $personalData->communication->mobile; $instance->dateOfBirth = $personalData->dateOfBirth; $instance->height = $personalData->height; $instance->weight = $personalData->weight; @@ -254,6 +264,11 @@ class ParticipantDto return $instance; } + public function isMobileNewlyEntered(): bool + { + return AppAssert\PhoneNumber::isNewlyEntered($this->mobile, $this->loadedMobile); + } + public function isApplicant(): bool { return 0 === $this->index; diff --git a/src/Form/Model/VoucherPurchaseDto.php b/src/Form/Model/VoucherPurchaseDto.php index afe2d216..31688f7b 100644 --- a/src/Form/Model/VoucherPurchaseDto.php +++ b/src/Form/Model/VoucherPurchaseDto.php @@ -8,6 +8,7 @@ use App\BusProNet\Model\Address; use App\BusProNet\Model\PersonalData; use App\BusProNet\Model\VoucherPaymentMethod; use App\BusProNet\Model\VoucherTemplate; +use App\Validator\Constraints as AppAssert; use Symfony\Component\Validator\Constraints as Assert; /** @@ -69,10 +70,24 @@ class VoucherPurchaseDto #[Assert\Valid] public Address $address; + #[Assert\When( + expression: 'this.isPhoneNewlyEntered()', + constraints: [new AppAssert\PhoneNumber()], + groups: ['voucher_step_2'], + )] public ?string $phone = null; + #[Assert\When( + expression: 'this.isMobileNewlyEntered()', + constraints: [new AppAssert\PhoneNumber()], + groups: ['voucher_step_2'], + )] public ?string $mobile = null; + // The numbers prefilled from BusPro, which are exempt from PhoneNumber. + public ?string $loadedPhone = null; + public ?string $loadedMobile = null; + #[Assert\NotBlank(message: 'Bitte angeben', groups: ['voucher_step_2'])] #[Assert\Email(message: 'Bitte eine gültige Adresse angeben', mode: 'strict', groups: ['voucher_step_2'])] public ?string $email = null; @@ -90,6 +105,16 @@ class VoucherPurchaseDto $this->address->country = 'D'; } + public function isPhoneNewlyEntered(): bool + { + return AppAssert\PhoneNumber::isNewlyEntered($this->phone, $this->loadedPhone); + } + + public function isMobileNewlyEntered(): bool + { + return AppAssert\PhoneNumber::isNewlyEntered($this->mobile, $this->loadedMobile); + } + public function getBuyerFullName(): string { return trim(sprintf('%s %s', $this->firstName ?? '', $this->lastName ?? '')); @@ -119,6 +144,8 @@ class VoucherPurchaseDto $this->address->country ??= 'D'; $this->phone = $personalData->communication->phone; $this->mobile = $personalData->communication->mobile; + $this->loadedPhone = $this->phone; + $this->loadedMobile = $this->mobile; $this->email = $loginEmail; } } diff --git a/src/Form/PersonalDataType.php b/src/Form/PersonalDataType.php index 4b949434..a32f1230 100644 --- a/src/Form/PersonalDataType.php +++ b/src/Form/PersonalDataType.php @@ -81,12 +81,10 @@ class PersonalDataType extends AbstractType 'label' => 'Telefon', 'required' => false, 'property_path' => 'communication.phone', - 'sanitize_html' => true, ]) ->add('mobile', TextType::class, [ 'label' => 'Mobil', 'property_path' => 'communication.mobile', - 'sanitize_html' => true, ]) ; } diff --git a/src/Form/VoucherPurchaseStep2Type.php b/src/Form/VoucherPurchaseStep2Type.php index 5960a7a5..a6b4db07 100644 --- a/src/Form/VoucherPurchaseStep2Type.php +++ b/src/Form/VoucherPurchaseStep2Type.php @@ -55,12 +55,10 @@ class VoucherPurchaseStep2Type extends AbstractType ->add('phone', TextType::class, [ 'label' => 'Telefon privat', 'required' => false, - 'sanitize_html' => true, ]) ->add('mobile', TextType::class, [ 'label' => 'Mobil', 'required' => false, - 'sanitize_html' => true, ]) ; } diff --git a/src/Service/ParticipantDataPrefiller.php b/src/Service/ParticipantDataPrefiller.php index 5c70d14c..09a9e37b 100644 --- a/src/Service/ParticipantDataPrefiller.php +++ b/src/Service/ParticipantDataPrefiller.php @@ -67,6 +67,7 @@ class ParticipantDataPrefiller $applicant->dateOfBirth = $prepopulated->dateOfBirth; $applicant->email = $prepopulated->email; $applicant->mobile = $prepopulated->mobile; + $applicant->loadedMobile = $prepopulated->loadedMobile; $applicant->address = $prepopulated->address; $applicant->height = $prepopulated->height; $applicant->weight = $prepopulated->weight; diff --git a/src/Validator/Constraints/PhoneNumber.php b/src/Validator/Constraints/PhoneNumber.php new file mode 100644 index 00000000..e7d879f7 --- /dev/null +++ b/src/Validator/Constraints/PhoneNumber.php @@ -0,0 +1,44 @@ +trim()->toString() !== u($loaded ?? '')->trim()->toString(); + } + + public function validatedBy(): string + { + return RegexValidator::class; + } +} diff --git a/tests/Validator/Constraints/PhoneNumberTest.php b/tests/Validator/Constraints/PhoneNumberTest.php new file mode 100644 index 00000000..f9256bac --- /dev/null +++ b/tests/Validator/Constraints/PhoneNumberTest.php @@ -0,0 +1,108 @@ +validator = Validation::createValidatorBuilder()->enableAttributeMapping()->getValidator(); + } + + /** @return iterable */ + public static function numbers(): iterable + { + yield 'digits' => ['01711234567', true]; + yield 'international' => ['+49 171 1234567', true]; + yield 'hyphen' => ['0171-1234567', true]; + yield 'slash' => ['0171/1234567', true]; + yield 'brackets' => ['+49 (0) 171 1234567', false]; + yield 'sanitized plus' => ['+49 171 1234567', false]; + yield 'plus inside' => ['0171+1234567', false]; + yield 'letters' => ['0171 privat', false]; + } + + #[DataProvider('numbers')] + public function testOnlyDigitsSpacesHyphensAndALeadingPlusPass(string $number, bool $valid): void + { + self::assertSame($valid, 0 === $this->validator->validate($number, new PhoneNumber())->count()); + } + + public function testAnEnteredParticipantNumberIsValidated(): void + { + $participant = new ParticipantDto(); + $participant->mobile = '0171 (1234567)'; + + self::assertCount(1, $this->validator->validateProperty($participant, 'mobile', ['booking_create'])); + self::assertCount(1, $this->validator->validateProperty($participant, 'mobile', ['booking_edit'])); + } + + /** + * A number loaded from BusPro goes back unchanged, so it must not block the form, even when it + * is one the rule would reject. + */ + public function testAParticipantNumberLoadedFromBusProIsExemptUntilChanged(): void + { + $personalData = new PersonalData(); + $personalData->communication->mobile = '+49 171 1234567'; + $participant = ParticipantDto::fromPersonalData($personalData); + + self::assertCount(0, $this->validator->validateProperty($participant, 'mobile', ['booking_edit'])); + + $participant->mobile = '0171 (1234567)'; + self::assertCount(1, $this->validator->validateProperty($participant, 'mobile', ['booking_edit'])); + } + + public function testTheExemptionSurvivesTheSessionRoundTrip(): void + { + $personalData = new PersonalData(); + $personalData->communication->mobile = '+49 (171) 1234567'; + $participant = unserialize(serialize(ParticipantDto::fromPersonalData($personalData))); + + self::assertCount(0, $this->validator->validateProperty($participant, 'mobile', ['booking_edit'])); + } + + public function testCommunicationExemptsOnlyTheNumbersItWasLoadedWith(): void + { + $communication = new Communication(); + $communication->phone = '030 (123456)'; + $communication->mobile = '+49 (171) 1234567'; + $communication->rememberLoadedNumbers(); + + self::assertCount(0, $this->validator->validate($communication, null, ['personal_data'])); + + $communication->phone = '030 (654321)'; + $violations = $this->validator->validate($communication, null, ['personal_data']); + self::assertCount(1, $violations); + self::assertSame('phone', $violations->get(0)->getPropertyPath()); + } + + public function testVoucherBuyerNumbersAreExemptOnlyAsPrefilled(): void + { + $personalData = new PersonalData(); + $personalData->communication->phone = '030 (123456)'; + $personalData->communication->mobile = '+49 (171) 1234567'; + $dto = new VoucherPurchaseDto(); + $dto->prefillBuyer($personalData, null); + + self::assertCount(0, $this->validator->validateProperty($dto, 'phone', ['voucher_step_2'])); + self::assertCount(0, $this->validator->validateProperty($dto, 'mobile', ['voucher_step_2'])); + + $dto->mobile = '0171 (1234567)'; + self::assertCount(1, $this->validator->validateProperty($dto, 'mobile', ['voucher_step_2'])); + } +}