From ad6c775aee1e832e8cd7b436ce7b75229b7cf4fd Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Wed, 12 Nov 2025 16:29:21 +0100 Subject: [PATCH] Allow editing emails after registration This will hopefully let people who made a typo realize it and fix it themselves --- src/Controller/RegistrationController.php | 128 +++++++++- src/Form/UpdateEmailFormType.php | 42 ++++ templates/registration/check_email.html.twig | 57 +++++ .../Controller/RegistrationControllerTest.php | 225 ++++++++++++++++++ 4 files changed, 449 insertions(+), 3 deletions(-) create mode 100644 src/Form/UpdateEmailFormType.php create mode 100644 templates/registration/check_email.html.twig diff --git a/src/Controller/RegistrationController.php b/src/Controller/RegistrationController.php index e4677c1151..f4c41ef340 100644 --- a/src/Controller/RegistrationController.php +++ b/src/Controller/RegistrationController.php @@ -15,9 +15,11 @@ use App\Entity\User; use App\Entity\UserRepository; use App\Form\RegistrationFormType; +use App\Form\UpdateEmailFormType; use App\Security\BruteForceLoginFormAuthenticator; use App\Security\EmailVerifier; use App\Security\UserChecker; +use Psr\Clock\ClockInterface; use Symfony\Bridge\Twig\Mime\TemplatedEmail; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; @@ -30,7 +32,7 @@ class RegistrationController extends Controller { - public function __construct(private EmailVerifier $emailVerifier) + public function __construct(private EmailVerifier $emailVerifier, private string $internalSecret, private ClockInterface $clock) { } @@ -68,9 +70,11 @@ public function register(Request $request, UserPasswordHasherInterface $password ->htmlTemplate('registration/confirmation_email.html.twig') ->textTemplate('registration/confirmation_email.txt.twig') ); - $this->addFlash('success', 'Your account has been created. Please check your email inbox to confirm the account.'); - return $this->redirectToRoute('home'); + // Redirect to confirmation page with signed token + $token = $this->generateRegistrationToken($user); + + return $this->redirectToRoute('register_check_email', ['token' => $token]); } return $this->render('registration/register.html.twig', [ @@ -78,6 +82,71 @@ public function register(Request $request, UserPasswordHasherInterface $password ]); } + #[Route(path: '/register/check-email/{token}', name: 'register_check_email')] + public function checkEmailConfirmation(string $token, UserRepository $userRepository): Response + { + $result = $this->validateRegistrationToken($token, $userRepository); + + if ($result === null) { + $this->addFlash('error', 'This link is invalid or has expired. Please register again.'); + return $this->redirectToRoute('register'); + } + + $form = $this->createForm(UpdateEmailFormType::class, $result['user']); + + return $this->render('registration/check_email.html.twig', [ + 'email' => $result['email'], + 'token' => $token, + 'form' => $form, + ]); + } + + #[Route(path: '/register/resend/{token}', name: 'register_resend', methods: ['POST'])] + public function resendConfirmation(string $token, Request $request, UserRepository $userRepository, string $mailFromEmail, string $mailFromName): Response + { + $result = $this->validateRegistrationToken($token, $userRepository); + + if ($result === null) { + $this->addFlash('error', 'This link is invalid or has expired. Please register again.'); + return $this->redirectToRoute('register'); + } + + $user = $result['user']; + $form = $this->createForm(UpdateEmailFormType::class, $user); + $form->handleRequest($request); + + if ($form->isSubmitted() && $form->isValid()) { + // Persist email change if it was modified + $this->getEM()->flush(); + + // Resend confirmation email + $this->emailVerifier->sendEmailConfirmation( + 'register_confirm_email', + $user, + new TemplatedEmail() + ->from(new Address($mailFromEmail, $mailFromName)) + ->to($user->getEmail()) + ->subject('Please confirm your email') + ->htmlTemplate('registration/confirmation_email.html.twig') + ->textTemplate('registration/confirmation_email.txt.twig') + ); + + // Generate new token with updated email + $newToken = $this->generateRegistrationToken($user); + + $this->addFlash('success', 'Confirmation email has been sent to ' . $user->getEmail()); + + return $this->redirectToRoute('register_check_email', ['token' => $newToken]); + } + + // If form is invalid, redisplay the page with errors + return $this->render('registration/check_email.html.twig', [ + 'email' => $user->getEmail(), + 'token' => $token, + 'form' => $form, + ]); + } + /** * @param BruteForceLoginFormAuthenticator $authenticator */ @@ -119,4 +188,57 @@ public function confirmEmail(Request $request, UserRepository $userRepository, U return $this->redirectToRoute('home'); } + + private function generateRegistrationToken(User $user): string + { + $timestamp = $this->clock->now()->getTimestamp(); + $data = $user->getId() . '|' . $user->getEmail() . '|' . $timestamp; + $signature = hash_hmac('sha256', $data, $this->internalSecret); + + return base64_encode($data . '|' . $signature); + } + + /** + * @return array{user: User, email: string}|null + */ + private function validateRegistrationToken(string $token, UserRepository $userRepository): ?array + { + $decoded = base64_decode($token, true); + if ($decoded === false) { + return null; + } + + $parts = explode('|', $decoded); + if (count($parts) !== 4) { + return null; + } + + [$userId, $email, $timestamp, $providedSignature] = $parts; + + // Check expiration (10 minutes = 600 seconds) + $now = $this->clock->now()->getTimestamp(); + if ($now - (int) $timestamp > 600) { + return null; + } + + // Verify signature + $data = $userId . '|' . $email . '|' . $timestamp; + $expectedSignature = hash_hmac('sha256', $data, $this->internalSecret); + if (!hash_equals($expectedSignature, $providedSignature)) { + return null; + } + + // Load user + $user = $userRepository->find((int) $userId); + if ($user === null) { + return null; + } + + // Check if user is already enabled + if ($user->isEnabled()) { + return null; + } + + return ['user' => $user, 'email' => $email]; + } } diff --git a/src/Form/UpdateEmailFormType.php b/src/Form/UpdateEmailFormType.php new file mode 100644 index 0000000000..d855402ded --- /dev/null +++ b/src/Form/UpdateEmailFormType.php @@ -0,0 +1,42 @@ + + * Nils Adermann + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace App\Form; + +use App\Entity\User; +use Symfony\Component\Form\AbstractType; +use Symfony\Component\Form\Extension\Core\Type\EmailType; +use Symfony\Component\Form\FormBuilderInterface; +use Symfony\Component\OptionsResolver\OptionsResolver; + +/** + * Used to update the email post-registration and before email has been confirmed, in case of typos + * + * @extends AbstractType + */ +class UpdateEmailFormType extends AbstractType +{ + public function buildForm(FormBuilderInterface $builder, array $options): void + { + $builder + ->add('email', EmailType::class, ['empty_data' => '']) + ; + } + + public function configureOptions(OptionsResolver $resolver): void + { + $resolver->setDefaults([ + 'data_class' => User::class, + 'validation_groups' => ['Default', 'Registration'], + ]); + } +} diff --git a/templates/registration/check_email.html.twig b/templates/registration/check_email.html.twig new file mode 100644 index 0000000000..a488369344 --- /dev/null +++ b/templates/registration/check_email.html.twig @@ -0,0 +1,57 @@ +{% extends 'user/layout.html.twig' %} + +{% block title %}Check your email - {{ parent() }}{% endblock %} + +{% block user_content %} +
+

+ Check your email +

+
+ +
+ + +

+ We've sent a confirmation email to {{ email }}. + Please check your inbox and click the link in the email to activate your account. +

+ +
+ +

Made a typo?

+

If you notice an error in your email address, you can correct it below and resend the confirmation email.

+ + {{ form_start(form, { + action: path('register_resend', {token: token}), + attr: {class: "form-horizontal"} + }) }} +
+ +
+ {{ form_errors(form.email) }} + {{ form_widget(form.email, {attr: {class: 'form-control'}}) }} +
+
+ +
+
+ +
+ Note: This link is valid for 10 minutes. +
+
+ {{ form_end(form) }} + +

+ + Didn't receive the email? Check your spam folder. If you still can't find it, + use the form above to verify your email address and resend. + +

+
+{% endblock %} diff --git a/tests/Controller/RegistrationControllerTest.php b/tests/Controller/RegistrationControllerTest.php index b849c8c203..c6e944552b 100644 --- a/tests/Controller/RegistrationControllerTest.php +++ b/tests/Controller/RegistrationControllerTest.php @@ -16,6 +16,9 @@ use App\Tests\IntegrationTestCase; use App\Validator\NotProhibitedPassword; use PHPUnit\Framework\Attributes\TestWith; +use Symfony\Component\Clock\Clock; +use Symfony\Component\Clock\MockClock; +use Symfony\Component\Clock\NativeClock; class RegistrationControllerTest extends IntegrationTestCase { @@ -34,10 +37,16 @@ public function testRegisterWithoutOAuth(): void $this->client->submit($form); $this->assertResponseStatusCodeSame(302); + // Should redirect to check-email page + $this->assertResponseRedirects(); + $redirectUrl = $this->client->getResponse()->headers->get('Location'); + $this->assertStringStartsWith('/register/check-email/', $redirectUrl); + $em = self::getEM(); $user = $em->getRepository(User::class)->findOneBy(['username' => 'max.example']); $this->assertInstanceOf(User::class, $user); $this->assertSame('max@example.com', $user->getEmailCanonical(), 'user email should have been canonicalized'); + $this->assertFalse($user->isEnabled(), 'user should not be enabled yet'); } #[TestWith(['max.example'])] @@ -60,4 +69,220 @@ public function testRegisterWithTooSimplePasswords(string $password): void $this->assertFormError(new NotProhibitedPassword()->message, 'registration_form', $crawler); } + + public function testCheckEmailPageDisplaysCorrectly(): void + { + $token = $this->registerUserAndGetToken('test@example.com', 'test.user'); + + $crawler = $this->client->request('GET', '/register/check-email/' . $token); + $this->assertResponseStatusCodeSame(200); + + $this->assertStringContainsString('test@example.com', $crawler->filter('body')->text()); + $this->assertCount(1, $crawler->filter('form[action*="/register/resend/"]')); + $this->assertCount(1, $crawler->filter('input[name="update_email_form[email]"]')); + } + + public function testEmailUpdateAndResend(): void + { + $token = $this->registerUserAndGetToken('old@example.com', 'test.user2'); + + $crawler = $this->client->request('GET', '/register/check-email/' . $token); + $this->assertResponseStatusCodeSame(200); + + $form = $crawler->selectButton('Update & Resend Confirmation Email')->form(); + $form->setValues([ + 'update_email_form[email]' => 'new@example.com', + ]); + + $this->client->submit($form); + $this->assertResponseStatusCodeSame(302); + + // Verify email was updated + $em = self::getEM(); + $em->clear(); + $user = $em->getRepository(User::class)->findOneBy(['username' => 'test.user2']); + $this->assertNotNull($user); + $this->assertSame('new@example.com', $user->getEmail()); + + // Should redirect to check-email with new token + $this->assertResponseRedirects(); + $redirectUrl = $this->client->getResponse()->headers->get('Location'); + $this->assertStringStartsWith('/register/check-email/', $redirectUrl); + } + + public function testInvalidEmailRejected(): void + { + $token = $this->registerUserAndGetToken('test@example.com', 'test.user3'); + + $crawler = $this->client->request('GET', '/register/check-email/' . $token); + $form = $crawler->selectButton('Update & Resend Confirmation Email')->form(); + $form->setValues([ + 'update_email_form[email]' => 'invalid-email', + ]); + + $crawler = $this->client->submit($form); + $this->assertResponseStatusCodeSame(422); + + // Should display validation error + $this->assertCount(1, $crawler->filter('.alert-danger')); + } + + public function testExpiredTokenRejected(): void + { + Clock::set($mockClock = new MockClock()); + + $token = $this->registerUserAndGetToken('test@example.com', 'test.user4'); + + $mockClock->sleep(11*60); + + $this->client->request('GET', '/register/check-email/' . $token); + $this->assertResponseStatusCodeSame(302); + $this->assertResponseRedirects('/register/'); + + Clock::set(new NativeClock()); + } + + public function testInvalidTokenRejected(): void + { + $this->client->request('GET', '/register/check-email/invalid-token'); + $this->assertResponseStatusCodeSame(302); + $this->assertResponseRedirects('/register/'); + } + + public function testEnabledUserCannotUseToken(): void + { + $token = $this->registerUserAndGetToken('test@example.com', 'test.user5'); + + // Enable the user + $em = self::getEM(); + $user = $em->getRepository(User::class)->findOneBy(['username' => 'test.user5']); + $user->setEnabled(true); + $em->flush(); + + $this->client->request('GET', '/register/check-email/' . $token); + $this->assertResponseStatusCodeSame(302); + $this->assertResponseRedirects('/register/'); + } + + public function testTamperedTokenRejected(): void + { + $token = $this->registerUserAndGetToken('test@example.com', 'test.user6'); + + // Tamper with the token by changing a character + $tamperedToken = substr($token, 0, -5) . 'XXXXX'; + + $this->client->request('GET', '/register/check-email/' . $tamperedToken); + $this->assertResponseStatusCodeSame(302); + $this->assertResponseRedirects('/register/'); + } + + public function testEmailConfirmationLinkInvalidatedAfterEmailUpdate(): void + { + $this->client->enableProfiler(); + + // Register with email A + $crawler = $this->client->request('GET', '/register/'); + $form = $crawler->filter('[name="registration_form"]')->form(); + $form->setValues([ + 'registration_form[email]' => 'emailA@example.com', + 'registration_form[username]' => 'test.user7', + 'registration_form[plainPassword]' => 'SuperSecret123', + ]); + + $this->client->submit($form); + $this->assertResponseStatusCodeSame(302); + + // Capture the confirmation email sent to email A + $this->assertEmailCount(1); + $emailA = $this->getMailerMessage(); + $this->assertNotNull($emailA); + $emailABody = $emailA->getTextBody(); + + // Extract the verification URL from email A + preg_match('/http[s]?:\/\/[^\s]+\/register\/verify[^\s]+/', $emailABody, $matches); + $this->assertNotEmpty($matches, 'Should find verification URL in email A'); + $originalVerificationUrl = $matches[0]; + + // Get the check-email token + $redirectUrl = $this->client->getResponse()->headers->get('Location'); + $token = substr($redirectUrl, strlen('/register/check-email/')); + + // Re-enable profiler for next request + $this->client->enableProfiler(); + + // Update email to B and resend + $crawler = $this->client->request('GET', '/register/check-email/' . $token); + $form = $crawler->selectButton('Update & Resend Confirmation Email')->form(); + $form->setValues([ + 'update_email_form[email]' => 'emailB@example.com', + ]); + + $this->client->submit($form); + $this->assertResponseStatusCodeSame(302); + + // Capture the new confirmation email sent to email B + $this->assertEmailCount(1); // One email sent in this request + $emailB = $this->getMailerMessage(); + $this->assertNotNull($emailB); + $emailBBody = $emailB->getTextBody(); + + // Extract the verification URL from email B + preg_match('/http[s]?:\/\/[^\s]+\/register\/verify[^\s]+/', $emailBBody, $matches); + $this->assertNotEmpty($matches, 'Should find verification URL in email B'); + $newVerificationUrl = $matches[0]; + + // Verify email was updated and user is still disabled + $em = self::getEM(); + $em->clear(); + $user = $em->getRepository(User::class)->findOneBy(['username' => 'test.user7']); + $this->assertSame('emailB@example.com', $user->getEmail()); + $this->assertFalse($user->isEnabled(), 'User should still be disabled after email update'); + + // Try to activate with the ORIGINAL link for email A (should fail) + $urlParts = parse_url($originalVerificationUrl); + $verificationPathA = $urlParts['path'] . '?' . $urlParts['query']; + + $this->client->request('GET', $verificationPathA); + + // Should redirect with an error (link is no longer valid because email changed) + $this->assertResponseRedirects('/register/'); + + // Verify user is STILL disabled (proving link A didn't work) + $em->clear(); + $user = $em->getRepository(User::class)->findOneBy(['username' => 'test.user7']); + $this->assertFalse($user->isEnabled(), 'User should still be disabled after trying email A link'); + + // Now activate with the NEW email B's verification link (should succeed) + $urlParts = parse_url($newVerificationUrl); + $verificationPathB = $urlParts['path'] . '?' . $urlParts['query']; + + $this->client->request('GET', $verificationPathB); + $this->assertResponseStatusCodeSame(302); + $this->assertResponseRedirects('/'); + + // Verify user is now enabled + $em->clear(); + $user = $em->getRepository(User::class)->findOneBy(['username' => 'test.user7']); + $this->assertTrue($user->isEnabled(), 'User should be enabled after confirming email B'); + } + + private function registerUserAndGetToken(string $email, string $username): string + { + $crawler = $this->client->request('GET', '/register/'); + $form = $crawler->filter('[name="registration_form"]')->form(); + $form->setValues([ + 'registration_form[email]' => $email, + 'registration_form[username]' => $username, + 'registration_form[plainPassword]' => 'SuperSecret123', + ]); + + $this->client->submit($form); + $this->assertResponseStatusCodeSame(302); + + $redirectUrl = $this->client->getResponse()->headers->get('Location'); + $this->assertStringStartsWith('/register/check-email/', $redirectUrl); + + // Extract token from redirect URL + return substr($redirectUrl, strlen('/register/check-email/')); + } }