feat(registration): anonymous self-signup with email-domain allow-list (#62) by martinydeAI · Pull Request #90 · itk-dev/ai-lib

martinydeAI · 2026-06-19T08:26:04Z

Links to issues

Closes #62.

do-not-merge — depends on PR #88 (#63) which depends on PR #86
(#45 + #83).

This branch is stacked on feature/issue-63-account-status-checker
because:

It creates users with status = Pending (needs the column from
PR feat(user): add name and status fields per ADR 006 (#45, #83) #86).

Without PR feat(security): reject pending and blocked users at login (#63) #88's UserCheckerInterface, a freshly-registered
user could sign in before approval — which defeats the point.

Rebase order once those PRs land:

PR feat(user): add name and status fields per ADR 006 (#45, #83) #86 lands → PR feat(security): reject pending and blocked users at login (#63) #88 rebases onto develop.

PR feat(security): reject pending and blocked users at login (#63) #88 lands → this PR rebases onto develop, do-not-merge
label removed.

Description

Anonymous self-signup at /register

GET /register — render the form (email, name, password, password
confirmation, CSRF token).
POST /register — validate input, create the user with
status = Pending, redirect to /register/pending. On validation
failure: 422 + re-render with a localised error.
GET /register/pending — "thanks, your account is awaiting
approval" page.

The validation logic lives in App\Security\Registration (service);
the controller stays thin per project conventions. Each failure
raises a RegistrationException carrying a translation key, which
the controller renders through |trans. CSRF-protected via Symfony's
csrf_token('register') helper.

Allow-list: comma-separated env var
REGISTRATION_ALLOWED_EMAIL_DOMAINS=aarhus.dk,kk.dk,…
Bound via %env(default:…:REGISTRATION_ALLOWED_EMAIL_DOMAINS)% with
example.test as the dev / test default so the app boots without
the var set. Parsing happens once in
App\Security\AllowedEmailDomains (lowercase + trim + dedup + drop
empties).

A pending user created by this route cannot sign in: the
integration test covers the hand-off through PR #88's
AccountStatusChecker.

Screenshot of the result

n/a — form re-uses the same Form/* components and base layout as
/login. Manual screenshot can be added on request.

Checklist

My code is covered by test cases.
My code passes our test (all our tests).
My code passes our static analysis suite.
My code passes our continuous integration process.

Additional comments or questions

The plan also lists a Domain entity follow-up: once PR #80
(Organization entity) lands, User.organization is added (separate
issue), and the registration allow-list switches from this env-var
to Organization.emailDomains. That swap is not in scope for
this PR.

UserManager::createUser() is reused as the persistence step — no
duplication of the password-hashing / unique-email logic.

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 comment pointing
at PR #87 so it can be folded back once both PRs land.

Details - AI specificities

ADR: docs/adr/006-user-approval-and-account-state.md
(Draft; lands via docs: add ADR 004 — user registration, approval, and account state #61). Specifically the "Registration" section.
No existing tests modified. The PR adds three new test files:
- tests/Integration/Controller/RegistrationControllerTest.php
  (functional end-to-end)
- tests/Unit/Security/RegistrationTest.php (service unit)
- tests/Unit/Security/AllowedEmailDomainsTest.php (value-object
  unit)
Out of scope:
- Email verification — separate decision (per ADR 006).
- CAPTCHA / rate limiting — defer until abuse becomes an issue
  (per ADR 006).
- Organization-backed allow-list — separate work after feat: admin CRUD for Organization #76 lands.
- Admin approval queue — PR 5 (feat: approval queue for domain managers #64 + feat: scoped user-management list view for admins and domain managers #85), not yet open.
Test design. Service-level unit tests exercise the validation
matrix with mocked collaborators (fast). The functional test
exercises the route end-to-end against the real firewall, the
real password hasher (cheap cost in test), and the
AccountStatusChecker from PR feat(security): reject pending and blocked users at login (#63) #88 so the "register → can't log in
yet" hand-off is genuinely covered.

martinyde · 2026-06-19T08:33:13Z

+    }
+
+    #[Route(path: '/register', name: 'app_register', methods: ['GET', 'POST'])]
+    public function register(Request $request): Response


Does this need a helper class to keep controller thin as per CLAUD.md directions

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) <noreply@anthropic.com>

tuj

We need an extra step where the user email is validated, to make sure the user actually owns the email it registers with.

martinyde · 2026-06-22T11:43:08Z

We need an extra step where the user email is validated, to make sure the user actually owns the email it registers with.

That is addressed in #103 , and future PRs holding email and email confirmation step form

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) <noreply@anthropic.com>

Correct comment formatting for allowed email domains.

Removed inlined comments regarding EmailDomain dependency and clarified the self-signup rules.

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) <noreply@anthropic.com>

PHP CS Fixer flagged a trailing-space-only line that survived the rebase. Pure formatting fix. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

martinydeAI · 2026-06-22T12:23:29Z

Superseded by #106 — rebased onto PR #105 (the replacement for PR #88), targeting develop directly. This PR's branch was based on PR #88's stacked branch which got orphaned when its content never reached develop. The registration code itself is preserved in #106.

martinydeAI added the do-not-merge Block merging until external dependency lands label Jun 19, 2026

martinyde requested changes Jun 19, 2026

View reviewed changes

martinydeAI mentioned this pull request Jun 19, 2026

feat(security): require authentication on every route except login/register #97

Open

5 tasks

martinyde requested a review from tuj June 19, 2026 11:47

This was referenced Jun 22, 2026

feat(components): shared Heading Twig component for H1-H6 #98

Merged

feat(components): shared Alert Twig component varying by type #99

Merged

feat(components): shared DescriptionList Twig component family #100

Merged

tuj requested changes Jun 22, 2026

View reviewed changes

martinydeAI force-pushed the feature/issue-63-account-status-checker branch from 71ced36 to 7497e88 Compare June 22, 2026 11:54

martinydeAI and others added 8 commits June 22, 2026 14:21

Fix comment in services.yaml for clarity

20afcfd

Correct comment formatting for allowed email domains.

Update AllowedEmailDomains.php

b047020

Refactor comments and clarify self-signup rules

839ae16

Removed inlined comments regarding EmailDomain dependency and clarified the self-signup rules.

Update comment in RegistrationControllerTest.php

0d5d1e3

Update CHANGELOG.md

327686f

chore(registration): trim trailing whitespace in Registration docblock

f557f60

PHP CS Fixer flagged a trailing-space-only line that survived the rebase. Pure formatting fix. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

martinydeAI force-pushed the feature/issue-62-self-signup branch from b0754e7 to f557f60 Compare June 22, 2026 12:22

martinydeAI mentioned this pull request Jun 22, 2026

feat(registration): anonymous self-signup with email-domain allow-list (#62) #106

Open

4 tasks

martinydeAI closed this Jun 22, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(registration): anonymous self-signup with email-domain allow-list (#62)#90

feat(registration): anonymous self-signup with email-domain allow-list (#62)#90
martinydeAI wants to merge 8 commits into
feature/issue-63-account-status-checkerfrom
feature/issue-62-self-signup

martinydeAI commented Jun 19, 2026 •

edited by martinyde

Loading

Uh oh!

martinyde Jun 19, 2026

Uh oh!

martinyde Jun 19, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tuj left a comment

Uh oh!

martinyde commented Jun 22, 2026

Uh oh!

martinydeAI commented Jun 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

martinydeAI commented Jun 19, 2026 • edited by martinyde Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Links to issues

Description

Screenshot of the result

Checklist

Additional comments or questions

Details - AI specificities

Uh oh!

martinyde Jun 19, 2026

Choose a reason for hiding this comment

Uh oh!

martinyde Jun 19, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tuj left a comment

Choose a reason for hiding this comment

Uh oh!

martinyde commented Jun 22, 2026

Uh oh!

martinydeAI commented Jun 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

martinydeAI commented Jun 19, 2026 •

edited by martinyde

Loading