From a5c4307671d7913e3844503fe5d33824abf3cfd2 Mon Sep 17 00:00:00 2001 From: martinydeAI Date: Fri, 19 Jun 2026 10:25:21 +0200 Subject: [PATCH 1/7] feat(registration): anonymous self-signup with domain allow-list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the public `/register` route per ADR 006: - `App\Controller\RegistrationController` — thin: validates CSRF, delegates to a service, renders the form / pending page. GET + POST on `/register`; GET on `/register/pending`. Both routes are added to `access_control` as `PUBLIC_ACCESS`. Already-authenticated visitors are bounced to `/`. - `App\Security\Registration` — orchestrates email validation, the domain allow-list check, password-match check, name + password non-emptiness, and the persistence step via the existing `UserManager`. Persists with `status = Pending`. Each failure raises a `RegistrationException` carrying a translation key. - `App\Security\AllowedEmailDomains` — value object that wraps the comma-separated env var `REGISTRATION_ALLOWED_EMAIL_DOMAINS`, normalising on construction (lowercase, trim, dedup, drop empties). Bound to services via `%env(default:…:REGISTRATION_ALLOWED_EMAIL_DOMAINS)%` with `example.test` as the default so dev / tests boot without needing the var set. - Twig templates re-use the existing `Form/*`, `Eyebrow`, and `Layout/*` components; no new component families. - Translations live in the existing `messages` domain under `register.*`. A pending user created by this route cannot sign in (covered by the checker from PR #88) — the integration test exercises that hand-off end-to-end. Sequenced as PR 4 of the User management milestone plan. Stacked on PR 3 (#88) since `UserManager::createUser(..., status: Pending)` is only useful when the checker rejects pending logins; PR 3 is itself stacked on PR 1 (#86) for the `name` + `status` fields. Labelled `do-not-merge` until both bases land. The domain-extraction helper from PR #87 (`App\Security\EmailDomain`) isn't pulled in here to keep the stack chain clean — the same one-line extraction is inlined in `Registration` with a TODO to fold back into the helper once PR #87 lands on `develop`. Closes #62. Co-Authored-By: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 12 ++ config/packages/security.yaml | 1 + config/services.yaml | 7 + src/Controller/RegistrationController.php | 77 ++++++++ src/Security/AllowedEmailDomains.php | 58 ++++++ src/Security/Registration.php | 105 +++++++++++ src/Security/RegistrationException.php | 18 ++ templates/registration/pending.html.twig | 20 ++ templates/registration/register.html.twig | 76 ++++++++ .../Controller/RegistrationControllerTest.php | 176 ++++++++++++++++++ .../Unit/Security/AllowedEmailDomainsTest.php | 57 ++++++ tests/Unit/Security/RegistrationTest.php | 137 ++++++++++++++ translations/messages.da.yaml | 26 +++ 13 files changed, 770 insertions(+) create mode 100644 src/Controller/RegistrationController.php create mode 100644 src/Security/AllowedEmailDomains.php create mode 100644 src/Security/Registration.php create mode 100644 src/Security/RegistrationException.php create mode 100644 templates/registration/pending.html.twig create mode 100644 templates/registration/register.html.twig create mode 100644 tests/Integration/Controller/RegistrationControllerTest.php create mode 100644 tests/Unit/Security/AllowedEmailDomainsTest.php create mode 100644 tests/Unit/Security/RegistrationTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index bf952a7..9cef186 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- Anonymous self-signup at `/register` per ADR 006. The route is + open to unauthenticated visitors; submissions go through + `App\Security\Registration` which validates the email format, + checks the right-hand-side domain against an env-backed allow-list + (`REGISTRATION_ALLOWED_EMAIL_DOMAINS`, comma-separated; default + `example.test` for dev / tests), requires matching password + confirmation, and creates the `User` with `status = Pending`. The + user is redirected to `/register/pending` ("thanks, awaiting + approval") and cannot sign in until a domain manager approves + them. CSRF-protected via Symfony's `csrf_token('register')` + helper. Localised in the existing `messages` domain + ([#62](https://github.com/itk-dev/ai-lib/issues/62)). - `App\Security\AccountStatusChecker` implementing `UserCheckerInterface` — gates the login flow so a `User` with `status = Pending` or `status = Blocked` is rejected before the diff --git a/config/packages/security.yaml b/config/packages/security.yaml index ec318ec..9a5132c 100644 --- a/config/packages/security.yaml +++ b/config/packages/security.yaml @@ -33,6 +33,7 @@ security: access_control: - { path: ^/login, roles: PUBLIC_ACCESS } - { path: ^/logout, roles: PUBLIC_ACCESS } + - { path: ^/register, roles: PUBLIC_ACCESS } when@test: security: diff --git a/config/services.yaml b/config/services.yaml index 7adddb9..fcb7a01 100644 --- a/config/services.yaml +++ b/config/services.yaml @@ -7,12 +7,19 @@ # Put parameters here that don't need to change on each machine where the app is deployed # https://symfony.com/doc/current/best_practices.html#use-parameters-for-application-configuration parameters: + registration_allowed_email_domains_default: 'example.test' services: # default configuration for services in *this* file _defaults: autowire: true # Automatically injects dependencies in your services. autoconfigure: true # Automatically registers your services as commands, event subscribers, etc. + bind: + # ADR 006: registration self-signup allow-list. The env var is the + # production source of truth; the default below keeps dev / test + # bootable when the var is unset (only `example.test` lets through, + # matching the baseline fixtures and the integration test domain). + $allowedEmailDomainsRaw: '%env(default:registration_allowed_email_domains_default:REGISTRATION_ALLOWED_EMAIL_DOMAINS)%' # makes classes in src/ available to be used as services # this creates a service per class whose id is the fully-qualified class name diff --git a/src/Controller/RegistrationController.php b/src/Controller/RegistrationController.php new file mode 100644 index 0000000..7161df9 --- /dev/null +++ b/src/Controller/RegistrationController.php @@ -0,0 +1,77 @@ +getUser()) { + return $this->redirectToRoute('app_frontpage'); + } + + $submitted = [ + 'email' => '', + 'name' => '', + ]; + $error = null; + $status = Response::HTTP_OK; + + if ('POST' === $request->getMethod()) { + $submitted['email'] = (string) $request->request->get('email', ''); + $submitted['name'] = (string) $request->request->get('name', ''); + $plainPassword = (string) $request->request->get('password', ''); + $plainPasswordConfirm = (string) $request->request->get('password_confirm', ''); + + if (!$this->isCsrfTokenValid('register', (string) $request->request->get('_token'))) { + return $this->render('registration/register.html.twig', [ + 'submitted' => $submitted, + 'error' => 'register.error.invalid_token', + ], new Response('', Response::HTTP_FORBIDDEN)); + } + + try { + $this->registration->register( + $submitted['email'], + $submitted['name'], + $plainPassword, + $plainPasswordConfirm, + ); + + return $this->redirectToRoute('app_register_pending'); + } catch (RegistrationException $e) { + $error = $e->getMessage(); + $status = Response::HTTP_UNPROCESSABLE_ENTITY; + } + } + + return $this->render('registration/register.html.twig', [ + 'submitted' => $submitted, + 'error' => $error, + ], new Response('', $status)); + } + + #[Route(path: '/register/pending', name: 'app_register_pending', methods: ['GET'])] + public function pending(): Response + { + if ($this->getUser()) { + return $this->redirectToRoute('app_frontpage'); + } + + return $this->render('registration/pending.html.twig'); + } +} diff --git a/src/Security/AllowedEmailDomains.php b/src/Security/AllowedEmailDomains.php new file mode 100644 index 0000000..4d69db8 --- /dev/null +++ b/src/Security/AllowedEmailDomains.php @@ -0,0 +1,58 @@ + lowercased + trimmed domain entries + */ + private readonly array $domains; + + /** + * @param string $allowedEmailDomainsRaw the comma-separated env-var payload + */ + public function __construct(string $allowedEmailDomainsRaw) + { + $entries = []; + foreach (explode(',', $allowedEmailDomainsRaw) as $entry) { + $normalised = strtolower(trim($entry)); + if ('' !== $normalised) { + $entries[] = $normalised; + } + } + + $this->domains = array_values(array_unique($entries)); + } + + /** + * @return bool whether `$domain` (case-insensitive, with surrounding whitespace tolerated) is on the allow-list + */ + public function contains(string $domain): bool + { + return \in_array(strtolower(trim($domain)), $this->domains, true); + } + + /** + * @return list the normalised allow-list, for diagnostics / templating + */ + public function all(): array + { + return $this->domains; + } +} diff --git a/src/Security/Registration.php b/src/Security/Registration.php new file mode 100644 index 0000000..9f4dbde --- /dev/null +++ b/src/Security/Registration.php @@ -0,0 +1,105 @@ +allowedEmailDomains->contains($domain)) { + throw new RegistrationException('register.error.domain_not_allowed'); + } + + if ($plainPassword !== $plainPasswordConfirm) { + throw new RegistrationException('register.error.password_mismatch'); + } + + if ('' === trim($name)) { + throw new RegistrationException('register.error.empty_name'); + } + + if ('' === $plainPassword) { + throw new RegistrationException('register.error.empty_password'); + } + + try { + return $this->userManager->createUser( + $email, + trim($name), + $plainPassword, + status: UserStatus::Pending, + ); + } catch (\DomainException) { + throw new RegistrationException('register.error.email_in_use'); + } + } +} diff --git a/src/Security/RegistrationException.php b/src/Security/RegistrationException.php new file mode 100644 index 0000000..b20ed55 --- /dev/null +++ b/src/Security/RegistrationException.php @@ -0,0 +1,18 @@ + + {{ 'register.pending.eyebrow'|trans }} +

+ {{ 'register.pending.heading'|trans }} +

+ +

{{ 'register.pending.body'|trans }}

+ +

+ + {{ 'register.pending.login_link'|trans }} + +

+ +{% endblock %} diff --git a/templates/registration/register.html.twig b/templates/registration/register.html.twig new file mode 100644 index 0000000..cf46b93 --- /dev/null +++ b/templates/registration/register.html.twig @@ -0,0 +1,76 @@ +{% extends 'base.html.twig' %} + +{% block title %}{{ 'register.title'|trans({'%brand%': brand_name}) }}{% endblock %} + +{% block body %} +
+ {{ 'register.eyebrow'|trans }} +

+ {{ 'register.heading'|trans }} +

+ +

{{ 'register.lead'|trans }}

+ + {% if error %} + + {% endif %} + +
+ + + + + + + + + + + + + + + + + + +
+ + {{ 'register.submit'|trans }} + + + {{ 'register.login_link'|trans }} + +
+
+
+{% endblock %} diff --git a/tests/Integration/Controller/RegistrationControllerTest.php b/tests/Integration/Controller/RegistrationControllerTest.php new file mode 100644 index 0000000..77322f8 --- /dev/null +++ b/tests/Integration/Controller/RegistrationControllerTest.php @@ -0,0 +1,176 @@ +client = self::createClient(); + } + + public function testRegisterPageRenders(): void + { + $this->client->request('GET', '/register'); + + self::assertResponseIsSuccessful(); + self::assertSelectorExists('input[name="email"]'); + self::assertSelectorExists('input[name="name"]'); + self::assertSelectorExists('input[name="password"]'); + self::assertSelectorExists('input[name="password_confirm"]'); + self::assertSelectorExists('input[name="_token"]'); + } + + public function testSuccessfulRegistrationCreatesPendingUserAndRedirects(): void + { + $crawler = $this->client->request('GET', '/register'); + $form = $crawler->filter('form')->form(); + $form['email'] = 'eve@example.test'; + $form['name'] = 'Eve'; + $form['password'] = 'secret'; + $form['password_confirm'] = 'secret'; + $this->client->submit($form); + + self::assertResponseRedirects('/register/pending'); + $crawler = $this->client->followRedirect(); + self::assertResponseIsSuccessful(); + self::assertStringContainsString('venter på godkendelse', $crawler->filter('body')->text()); + + $user = self::getContainer()->get(UserRepository::class)->findOneBy(['email' => 'eve@example.test']); + self::assertNotNull($user); + self::assertSame('Eve', $user->getName()); + self::assertSame(UserStatus::Pending, $user->getStatus()); + } + + public function testPendingUserCreatedByRegistrationCannotLogIn(): void + { + $crawler = $this->client->request('GET', '/register'); + $form = $crawler->filter('form')->form(); + $form['email'] = 'frank@example.test'; + $form['name'] = 'Frank'; + $form['password'] = 'secret'; + $form['password_confirm'] = 'secret'; + $this->client->submit($form); + $this->client->followRedirect(); + + $crawler = $this->client->request('GET', '/login'); + $form = $crawler->filter('form')->form(); + $form['_username'] = 'frank@example.test'; + $form['_password'] = 'secret'; + $this->client->submit($form); + + self::assertResponseRedirects('/login'); + $crawler = $this->client->followRedirect(); + self::assertStringContainsString('venter på godkendelse', $crawler->filter('body')->text()); + self::assertNull( + $this->client->getContainer()->get('security.token_storage')->getToken(), + ); + } + + public function testRejectsNonAllowListedDomain(): void + { + $crawler = $this->client->request('GET', '/register'); + $form = $crawler->filter('form')->form(); + $form['email'] = 'mallory@other.invalid'; + $form['name'] = 'Mallory'; + $form['password'] = 'secret'; + $form['password_confirm'] = 'secret'; + $crawler = $this->client->submit($form); + + self::assertResponseStatusCodeSame(422); + self::assertStringContainsString('domæne er ikke godkendt', $crawler->filter('body')->text()); + self::assertNull( + self::getContainer()->get(UserRepository::class)->findOneBy(['email' => 'mallory@other.invalid']), + ); + } + + public function testRejectsPasswordMismatch(): void + { + $crawler = $this->client->request('GET', '/register'); + $form = $crawler->filter('form')->form(); + $form['email'] = 'grace@example.test'; + $form['name'] = 'Grace'; + $form['password'] = 'secret'; + $form['password_confirm'] = 'different'; + $crawler = $this->client->submit($form); + + self::assertResponseStatusCodeSame(422); + self::assertStringContainsString('ikke ens', $crawler->filter('body')->text()); + } + + public function testRejectsDuplicateEmail(): void + { + $crawler = $this->client->request('GET', '/register'); + $form = $crawler->filter('form')->form(); + // alice@example.test is in the baseline fixtures. + $form['email'] = 'alice@example.test'; + $form['name'] = 'Alice'; + $form['password'] = 'secret'; + $form['password_confirm'] = 'secret'; + $crawler = $this->client->submit($form); + + self::assertResponseStatusCodeSame(422); + self::assertStringContainsString('allerede en konto', $crawler->filter('body')->text()); + } + + public function testRejectsInvalidCsrfToken(): void + { + $this->client->request('POST', '/register', [ + 'email' => 'henry@example.test', + 'name' => 'Henry', + 'password' => 'secret', + 'password_confirm' => 'secret', + '_token' => 'nope', + ]); + + self::assertResponseStatusCodeSame(403); + self::assertNull( + self::getContainer()->get(UserRepository::class)->findOneBy(['email' => 'henry@example.test']), + ); + } + + public function testLoggedInUserIsRedirectedAwayFromRegister(): void + { + $this->loginAsAlice(); + + $this->client->request('GET', '/register'); + + self::assertResponseRedirects('/'); + } + + public function testLoggedInUserIsRedirectedAwayFromPendingPage(): void + { + $this->loginAsAlice(); + + $this->client->request('GET', '/register/pending'); + + self::assertResponseRedirects('/'); + } + + private function loginAsAlice(): void + { + $crawler = $this->client->request('GET', '/login'); + $form = $crawler->filter('form')->form(); + $form['_username'] = 'alice@example.test'; + $form['_password'] = 'password'; + $this->client->submit($form); + $this->client->followRedirect(); + } +} diff --git a/tests/Unit/Security/AllowedEmailDomainsTest.php b/tests/Unit/Security/AllowedEmailDomainsTest.php new file mode 100644 index 0000000..40641fd --- /dev/null +++ b/tests/Unit/Security/AllowedEmailDomainsTest.php @@ -0,0 +1,57 @@ +all()); + self::assertFalse($allow->contains('aarhus.dk')); + } + + public function testSingleEntryIsMatched(): void + { + $allow = new AllowedEmailDomains('aarhus.dk'); + + self::assertSame(['aarhus.dk'], $allow->all()); + self::assertTrue($allow->contains('aarhus.dk')); + } + + public function testMultipleEntriesArePreservedInOrderAndDeduplicated(): void + { + $allow = new AllowedEmailDomains('aarhus.dk,kk.dk,aarhus.dk'); + + self::assertSame(['aarhus.dk', 'kk.dk'], $allow->all()); + } + + public function testEntriesAreLowercasedAndTrimmed(): void + { + $allow = new AllowedEmailDomains(' Aarhus.DK , AARHUS.DK , kk.dk '); + + self::assertSame(['aarhus.dk', 'kk.dk'], $allow->all()); + } + + public function testBlankEntriesAreSilentlyDropped(): void + { + $allow = new AllowedEmailDomains(',,aarhus.dk,,'); + + self::assertSame(['aarhus.dk'], $allow->all()); + } + + public function testContainsIsCaseInsensitiveAndWhitespaceTolerant(): void + { + $allow = new AllowedEmailDomains('aarhus.dk'); + + self::assertTrue($allow->contains('AARHUS.DK')); + self::assertTrue($allow->contains(' aarhus.dk ')); + self::assertFalse($allow->contains('other.dk')); + } +} diff --git a/tests/Unit/Security/RegistrationTest.php b/tests/Unit/Security/RegistrationTest.php new file mode 100644 index 0000000..97ad6e0 --- /dev/null +++ b/tests/Unit/Security/RegistrationTest.php @@ -0,0 +1,137 @@ +registration(allowList: 'example.test'); + + $this->expectException(RegistrationException::class); + $this->expectExceptionMessage('register.error.invalid_email'); + + $reg->register('not-an-email', 'Carol', 'secret', 'secret'); + } + + public function testRejectsDomainNotOnAllowList(): void + { + $reg = $this->registration(allowList: 'aarhus.dk'); + + $this->expectException(RegistrationException::class); + $this->expectExceptionMessage('register.error.domain_not_allowed'); + + $reg->register('carol@example.test', 'Carol', 'secret', 'secret'); + } + + public function testRejectsPasswordMismatch(): void + { + $reg = $this->registration(allowList: 'example.test'); + + $this->expectException(RegistrationException::class); + $this->expectExceptionMessage('register.error.password_mismatch'); + + $reg->register('carol@example.test', 'Carol', 'secret', 'different'); + } + + public function testRejectsEmptyName(): void + { + $reg = $this->registration(allowList: 'example.test'); + + $this->expectException(RegistrationException::class); + $this->expectExceptionMessage('register.error.empty_name'); + + $reg->register('carol@example.test', ' ', 'secret', 'secret'); + } + + public function testRejectsEmptyPassword(): void + { + $reg = $this->registration(allowList: 'example.test'); + + $this->expectException(RegistrationException::class); + $this->expectExceptionMessage('register.error.empty_password'); + + $reg->register('carol@example.test', 'Carol', '', ''); + } + + public function testTranslatesDuplicateEmailIntoRegistrationException(): void + { + $em = $this->createMock(EntityManagerInterface::class); + $repo = $this->createMock(UserRepository::class); + $hasher = $this->createMock(UserPasswordHasherInterface::class); + + // First call (in `register`) finds an existing user; UserManager throws DomainException. + $repo->method('findOneBy')->willReturn(new User()); + + $reg = new Registration( + new UserManager($em, $repo, $hasher), + new AllowedEmailDomains('example.test'), + ); + + $this->expectException(RegistrationException::class); + $this->expectExceptionMessage('register.error.email_in_use'); + + $reg->register('carol@example.test', 'Carol', 'secret', 'secret'); + } + + public function testPersistsPendingUserOnHappyPath(): void + { + $em = $this->createMock(EntityManagerInterface::class); + $repo = $this->createMock(UserRepository::class); + $hasher = $this->createMock(UserPasswordHasherInterface::class); + + $repo->method('findOneBy')->willReturn(null); + $hasher->method('hashPassword')->willReturn('hashed-secret'); + + $captured = null; + $em->expects(self::once()) + ->method('persist') + ->willReturnCallback(function (object $entity) use (&$captured): void { + \assert($entity instanceof User); + $captured = $entity; + }); + $em->expects(self::once())->method('flush'); + + $reg = new Registration( + new UserManager($em, $repo, $hasher), + new AllowedEmailDomains('example.test'), + ); + + $user = $reg->register('Carol@Example.test', ' Carol ', 'secret', 'secret'); + + self::assertSame($user, $captured); + self::assertSame('Carol@Example.test', $user->getEmail()); + self::assertSame('Carol', $user->getName(), 'Name is trimmed before persistence.'); + self::assertSame(UserStatus::Pending, $user->getStatus()); + self::assertSame('hashed-secret', $user->getPassword()); + } + + /** + * Build a Registration with mock collaborators that never reach + * the persistence step. + */ + private function registration(string $allowList): Registration + { + $em = $this->createMock(EntityManagerInterface::class); + $repo = $this->createMock(UserRepository::class); + $hasher = $this->createMock(UserPasswordHasherInterface::class); + + return new Registration( + new UserManager($em, $repo, $hasher), + new AllowedEmailDomains($allowList), + ); + } +} diff --git a/translations/messages.da.yaml b/translations/messages.da.yaml index f71b193..527e494 100644 --- a/translations/messages.da.yaml +++ b/translations/messages.da.yaml @@ -42,6 +42,32 @@ security: password_label: "Adgangskode" submit: "Log ind" +register: + title: "Opret bruger – %brand%" + eyebrow: "Opret bruger" + heading: "Opret bruger" + lead: "Brug din arbejds-e-mail. Når du har oprettet kontoen, skal en administrator godkende den, før du kan logge ind." + email_label: "E-mail" + name_label: "Navn" + password_label: "Adgangskode" + password_confirm_label: "Bekræft adgangskode" + submit: "Opret bruger" + login_link: "Har du allerede en konto? Log ind" + error: + invalid_email: "Indtast en gyldig e-mailadresse." + domain_not_allowed: "E-mailens domæne er ikke godkendt til at oprette konti her." + password_mismatch: "De to adgangskoder er ikke ens." + empty_name: "Navnet må ikke være tomt." + empty_password: "Adgangskoden må ikke være tom." + email_in_use: "Der findes allerede en konto med den e-mail." + invalid_token: "Sessionen er udløbet. Indsend formularen igen." + pending: + title: "Konto oprettet – %brand%" + eyebrow: "Tak" + heading: "Tak — vi har modtaget din oprettelse" + body: "Din konto venter på godkendelse fra en administrator. Du modtager besked, når kontoen er klar, og kan så logge ind." + login_link: "← Tilbage til log ind" + assistant: detail: title: "%title% – %brand%" From 8b4dbb8db64b631c067f27f6eb94d7d798fdc731 Mon Sep 17 00:00:00 2001 From: Martin Yde Granath Date: Fri, 19 Jun 2026 10:31:50 +0200 Subject: [PATCH 2/7] Fix comment in services.yaml for clarity Correct comment formatting for allowed email domains. --- config/services.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/services.yaml b/config/services.yaml index fcb7a01..e888655 100644 --- a/config/services.yaml +++ b/config/services.yaml @@ -15,7 +15,7 @@ services: autowire: true # Automatically injects dependencies in your services. autoconfigure: true # Automatically registers your services as commands, event subscribers, etc. bind: - # ADR 006: registration self-signup allow-list. The env var is the + # The env var is the # production source of truth; the default below keeps dev / test # bootable when the var is unset (only `example.test` lets through, # matching the baseline fixtures and the integration test domain). From 5f1422c25e2eaa1bf4735f4a7340b20d031e3086 Mon Sep 17 00:00:00 2001 From: Martin Yde Granath Date: Fri, 19 Jun 2026 10:36:10 +0200 Subject: [PATCH 3/7] Update AllowedEmailDomains.php --- src/Security/AllowedEmailDomains.php | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/Security/AllowedEmailDomains.php b/src/Security/AllowedEmailDomains.php index 4d69db8..0bd51aa 100644 --- a/src/Security/AllowedEmailDomains.php +++ b/src/Security/AllowedEmailDomains.php @@ -8,11 +8,10 @@ * Allow-list of email domains accepted by anonymous self-signup. * * Sourced from a comma-separated env var - * (`REGISTRATION_ALLOWED_EMAIL_DOMAINS=aarhus.dk,kk.dk,…`) per ADR - * 006. Domains are normalised to lowercase + trimmed on construction + * (`REGISTRATION_ALLOWED_EMAIL_DOMAINS=aarhus.dk,kk.dk,…`). + * Domains are normalised to lowercase + trimmed on construction * so `aarhus.dk`, `Aarhus.DK`, and ` AARHUS.dk` all match the same - * way and the `Organization.emailDomains` follow-up (#76) doesn't - * have to re-normalise the same data on every read. + * way. * * Empty / blank entries are dropped silently — an env var like * `,,aarhus.dk,` is interpreted as a single-entry list. From 92e4b6ac6e05932705ec7cec69c211c8fd0f93e2 Mon Sep 17 00:00:00 2001 From: Martin Yde Granath Date: Fri, 19 Jun 2026 10:44:51 +0200 Subject: [PATCH 4/7] Refactor comments and clarify self-signup rules Removed inlined comments regarding EmailDomain dependency and clarified the self-signup rules. --- src/Security/Registration.php | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/src/Security/Registration.php b/src/Security/Registration.php index 9f4dbde..219c21a 100644 --- a/src/Security/Registration.php +++ b/src/Security/Registration.php @@ -7,26 +7,19 @@ use App\Entity\User; use App\Enum\UserStatus; -// `App\Security\EmailDomain::of()` lives in PR #87 (issue #84). Until -// that PR lands on develop, this branch can't depend on it without -// pulling in unrelated security-voter code, so the same one-line -// extraction is inlined below. When #87 + this PR are both on develop, -// fold this back into a call to `EmailDomain::of()` (no test churn -// expected — the contract is identical: lowercased post-`@`, or null). - /** * Public-signup orchestration. * * Sits between {@see \App\Controller\RegistrationController} and the * existing {@see UserManager}, owning the rules that distinguish a - * legitimate self-signup attempt from one that should be rejected - * (ADR 006): + * legitimate self-signup attempt from one that should be rejected. * * 1. The submitted email must syntactically parse as an email. * 2. The right-hand side of the email must be on the allow-list * {@see AllowedEmailDomains}. * 3. The two password fields must match. * 4. The name must be non-empty (rule shared with {@see UserManager}). + * * On success the new {@see User} is persisted with * `status = Pending`. The {@see \App\Security\AccountStatusChecker} From 0baae5081f8b98dc397f697b0c4c8bbc29187d07 Mon Sep 17 00:00:00 2001 From: Martin Yde Granath Date: Fri, 19 Jun 2026 10:53:59 +0200 Subject: [PATCH 5/7] Update comment in RegistrationControllerTest.php --- tests/Integration/Controller/RegistrationControllerTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Integration/Controller/RegistrationControllerTest.php b/tests/Integration/Controller/RegistrationControllerTest.php index 77322f8..4b86825 100644 --- a/tests/Integration/Controller/RegistrationControllerTest.php +++ b/tests/Integration/Controller/RegistrationControllerTest.php @@ -11,7 +11,7 @@ /** * End-to-end self-signup flow against the real `form_login` firewall - * and the new `AccountStatusChecker` (PR 3). + * and the `AccountStatusChecker` * * The test env's allow-list default is `example.test` (see * `config/services.yaml`), so `*.@example.test` emails are accepted From 539296e1c63dc64a0a5a58f0d60121c890b9b9b5 Mon Sep 17 00:00:00 2001 From: Martin Yde Granath Date: Fri, 19 Jun 2026 10:59:36 +0200 Subject: [PATCH 6/7] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9cef186..f7fe899 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- Anonymous self-signup at `/register` per ADR 006. The route is +- Anonymous self-signup at `/register`. The route is open to unauthenticated visitors; submissions go through `App\Security\Registration` which validates the email format, checks the right-hand-side domain against an env-backed allow-list From b0754e7b984236b07f3a1c637af03acbe4cf3213 Mon Sep 17 00:00:00 2001 From: martinydeAI Date: Fri, 19 Jun 2026 12:39:42 +0200 Subject: [PATCH 7/7] chore(tests): add one-line intent comments to registration tests Per review on PR #90 - apply the test-comment convention to the three new test files (RegistrationControllerTest, AllowedEmailDomainsTest, RegistrationTest). Each `test...` method now opens with a single-line "Tests ...", "Ensures ...", or "Verifies ..." comment naming what it asserts. Pure documentation change - no test logic touched. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Controller/RegistrationControllerTest.php | 9 +++++++++ tests/Unit/Security/AllowedEmailDomainsTest.php | 6 ++++++ tests/Unit/Security/RegistrationTest.php | 7 +++++++ 3 files changed, 22 insertions(+) diff --git a/tests/Integration/Controller/RegistrationControllerTest.php b/tests/Integration/Controller/RegistrationControllerTest.php index 4b86825..c263dcc 100644 --- a/tests/Integration/Controller/RegistrationControllerTest.php +++ b/tests/Integration/Controller/RegistrationControllerTest.php @@ -26,6 +26,7 @@ protected function setUp(): void $this->client = self::createClient(); } + // Tests that the /register form renders the expected fields and CSRF token. public function testRegisterPageRenders(): void { $this->client->request('GET', '/register'); @@ -38,6 +39,7 @@ public function testRegisterPageRenders(): void self::assertSelectorExists('input[name="_token"]'); } + // Verifies a valid submission creates a Pending user and redirects to the pending page. public function testSuccessfulRegistrationCreatesPendingUserAndRedirects(): void { $crawler = $this->client->request('GET', '/register'); @@ -59,6 +61,7 @@ public function testSuccessfulRegistrationCreatesPendingUserAndRedirects(): void self::assertSame(UserStatus::Pending, $user->getStatus()); } + // Verifies the hand-off through AccountStatusChecker: a freshly-registered user cannot log in. public function testPendingUserCreatedByRegistrationCannotLogIn(): void { $crawler = $this->client->request('GET', '/register'); @@ -84,6 +87,7 @@ public function testPendingUserCreatedByRegistrationCannotLogIn(): void ); } + // Ensures emails outside the allow-list are rejected with 422 and no user persisted. public function testRejectsNonAllowListedDomain(): void { $crawler = $this->client->request('GET', '/register'); @@ -101,6 +105,7 @@ public function testRejectsNonAllowListedDomain(): void ); } + // Ensures mismatched password + confirmation are rejected with 422. public function testRejectsPasswordMismatch(): void { $crawler = $this->client->request('GET', '/register'); @@ -115,6 +120,7 @@ public function testRejectsPasswordMismatch(): void self::assertStringContainsString('ikke ens', $crawler->filter('body')->text()); } + // Ensures registering with an existing email is rejected with 422. public function testRejectsDuplicateEmail(): void { $crawler = $this->client->request('GET', '/register'); @@ -130,6 +136,7 @@ public function testRejectsDuplicateEmail(): void self::assertStringContainsString('allerede en konto', $crawler->filter('body')->text()); } + // Ensures an invalid CSRF token yields 403 and no user is persisted. public function testRejectsInvalidCsrfToken(): void { $this->client->request('POST', '/register', [ @@ -146,6 +153,7 @@ public function testRejectsInvalidCsrfToken(): void ); } + // Tests that an authenticated visitor hitting /register is redirected to the frontpage. public function testLoggedInUserIsRedirectedAwayFromRegister(): void { $this->loginAsAlice(); @@ -155,6 +163,7 @@ public function testLoggedInUserIsRedirectedAwayFromRegister(): void self::assertResponseRedirects('/'); } + // Tests that an authenticated visitor hitting /register/pending is redirected to the frontpage. public function testLoggedInUserIsRedirectedAwayFromPendingPage(): void { $this->loginAsAlice(); diff --git a/tests/Unit/Security/AllowedEmailDomainsTest.php b/tests/Unit/Security/AllowedEmailDomainsTest.php index 40641fd..c2ccd25 100644 --- a/tests/Unit/Security/AllowedEmailDomainsTest.php +++ b/tests/Unit/Security/AllowedEmailDomainsTest.php @@ -9,6 +9,7 @@ final class AllowedEmailDomainsTest extends TestCase { + // Tests that an empty env string yields an empty allow-list with no matches. public function testEmptyEnvProducesEmptyList(): void { $allow = new AllowedEmailDomains(''); @@ -17,6 +18,7 @@ public function testEmptyEnvProducesEmptyList(): void self::assertFalse($allow->contains('aarhus.dk')); } + // Tests that a single allow-list entry matches the exact domain. public function testSingleEntryIsMatched(): void { $allow = new AllowedEmailDomains('aarhus.dk'); @@ -25,6 +27,7 @@ public function testSingleEntryIsMatched(): void self::assertTrue($allow->contains('aarhus.dk')); } + // Verifies multiple entries are kept in their original order and deduplicated. public function testMultipleEntriesArePreservedInOrderAndDeduplicated(): void { $allow = new AllowedEmailDomains('aarhus.dk,kk.dk,aarhus.dk'); @@ -32,6 +35,7 @@ public function testMultipleEntriesArePreservedInOrderAndDeduplicated(): void self::assertSame(['aarhus.dk', 'kk.dk'], $allow->all()); } + // Verifies entries are lowercased and trimmed during parsing. public function testEntriesAreLowercasedAndTrimmed(): void { $allow = new AllowedEmailDomains(' Aarhus.DK , AARHUS.DK , kk.dk '); @@ -39,6 +43,7 @@ public function testEntriesAreLowercasedAndTrimmed(): void self::assertSame(['aarhus.dk', 'kk.dk'], $allow->all()); } + // Ensures blank entries (e.g. leading/trailing commas) are silently dropped. public function testBlankEntriesAreSilentlyDropped(): void { $allow = new AllowedEmailDomains(',,aarhus.dk,,'); @@ -46,6 +51,7 @@ public function testBlankEntriesAreSilentlyDropped(): void self::assertSame(['aarhus.dk'], $allow->all()); } + // Verifies contains() is case-insensitive and tolerates surrounding whitespace. public function testContainsIsCaseInsensitiveAndWhitespaceTolerant(): void { $allow = new AllowedEmailDomains('aarhus.dk'); diff --git a/tests/Unit/Security/RegistrationTest.php b/tests/Unit/Security/RegistrationTest.php index 97ad6e0..a674b7c 100644 --- a/tests/Unit/Security/RegistrationTest.php +++ b/tests/Unit/Security/RegistrationTest.php @@ -17,6 +17,7 @@ final class RegistrationTest extends TestCase { + // Ensures malformed emails are rejected with the invalid_email translation key. public function testRejectsInvalidEmail(): void { $reg = $this->registration(allowList: 'example.test'); @@ -27,6 +28,7 @@ public function testRejectsInvalidEmail(): void $reg->register('not-an-email', 'Carol', 'secret', 'secret'); } + // Ensures emails outside the allow-list raise domain_not_allowed. public function testRejectsDomainNotOnAllowList(): void { $reg = $this->registration(allowList: 'aarhus.dk'); @@ -37,6 +39,7 @@ public function testRejectsDomainNotOnAllowList(): void $reg->register('carol@example.test', 'Carol', 'secret', 'secret'); } + // Ensures mismatched password + confirmation raise password_mismatch. public function testRejectsPasswordMismatch(): void { $reg = $this->registration(allowList: 'example.test'); @@ -47,6 +50,7 @@ public function testRejectsPasswordMismatch(): void $reg->register('carol@example.test', 'Carol', 'secret', 'different'); } + // Ensures whitespace-only names raise empty_name. public function testRejectsEmptyName(): void { $reg = $this->registration(allowList: 'example.test'); @@ -57,6 +61,7 @@ public function testRejectsEmptyName(): void $reg->register('carol@example.test', ' ', 'secret', 'secret'); } + // Ensures empty passwords raise empty_password. public function testRejectsEmptyPassword(): void { $reg = $this->registration(allowList: 'example.test'); @@ -67,6 +72,7 @@ public function testRejectsEmptyPassword(): void $reg->register('carol@example.test', 'Carol', '', ''); } + // Verifies UserManager's duplicate-email DomainException is translated into a localised RegistrationException. public function testTranslatesDuplicateEmailIntoRegistrationException(): void { $em = $this->createMock(EntityManagerInterface::class); @@ -87,6 +93,7 @@ public function testTranslatesDuplicateEmailIntoRegistrationException(): void $reg->register('carol@example.test', 'Carol', 'secret', 'secret'); } + // Tests the happy path: valid submission persists a Pending user with trimmed name and hashed password. public function testPersistsPendingUserOnHappyPath(): void { $em = $this->createMock(EntityManagerInterface::class);