Priority 70 sits after RejectListener (77) and before LoginListener (66), and
both bounds are load-bearing:
- After 77 so a rate-limited IP never reaches a ceremony. Passkeys cannot be
used to sidestep a lockout (D3), which is the point of the reviewer's third
clarification.
- Before 66 because LoginListener treats any POST to the auth subdomain as a
login attempt. A ceremony finish body has no username/totp, so Payload::load()
returns null and the request would be scored as a failed login, burning a
rate-limit token for every legitimate passkey login.
Verified in the live container rather than assumed: debug:event-dispatcher
confirms 77 -> 70 -> 66.
Other properties asserted by tests: every header-bearing request gets a JSON
response so fetch() callers never receive HTML; registration identity comes from
the live session, never the request body; a failed ceremony is indistinguishable
from a wrong TOTP code and spends the same shared budget; and begin is bounded
by a separate resource guard that deliberately does not consume failure budget.
The listener also marks its responses so SecurityHeadersListener can apply
no-store: these are the only browser-facing 2xx this application produces, since
the auth subdomain has no forward_auth in front of it. CSP gains
publickey-credentials-get/-create only when passkeys are available, so the
unavailable case stays byte-identical to before.
SessionIssuer now owns 'grant access after authenticating', which LoginManager
previously did internally. The passkey ceremony needs the same behaviour, and
two implementations would inevitably drift — most likely in cookie attributes,
where a difference stays invisible until it breaks in a browser.
LoginManager keeps what is specific to code-based login: verifying the TOTP or
backup code and enforcing the single-use nonce. Its 18 existing tests pass
unchanged, which is the evidence that this is behaviour-preserving rather than a
rewrite.
Also folds the redundant early-return into a single guard in checkToken so the
success path reads straight through.
Builds both WebAuthn ceremonies on top of the library, with real cryptography
proven in tests rather than stubbed:
- PasskeyCeremonyStore: server-authoritative, single-use challenge state in the
nonceCache pool. The client's challenge copy is never trusted, and consume()
deletes before verifying so a replay cannot retry the same challenge.
- PasskeyManager: registration and login ceremonies. Library types are confined
to this class and PasskeyCeremonyFactory. Failures return null rather than
distinguishing unknown-credential from bad-signature, so the endpoint is not
an enumeration oracle.
- PasskeyTestHelper: builds genuinely valid ceremonies (real P-256 keypair,
COSE key, signed authenticatorData, CBOR attestation object).
- PasskeyRealCryptoSpikeTest: proves registration and assertion verify, that
http:// origins are refused (D4), that challenges and rpIdHash are bound, and
that a synchronised passkey with a constant zero counter can log in repeatedly.
The library default (ThrowExceptionIfInvalid) requires the reported counter to
be strictly greater than the stored one. Synchronised passkeys report a
constant 0 forever, so the default rejects a brand-new credential on its first
login — and only on real hardware, never in a unit test that increments the
counter.
The replacement still rejects a counter that moves backwards, which is the only
signal the counter can carry. Clone detection remains explicitly not a property
this feature claims; see SECURITY.md.
Persistence for registered passkeys, backed by sessionCache so credentials
survive a container restart the way sessions do.
The pool is wrapped in MonitorCacheKeys, matching LoginManager and
BackupCodeManager. Without that wrapper the credentials would live only in the
APCu-side pool and vanish on the next restart, because PersistCache::persist()
only flushes keys a monitor recorded. A test asserts visibility to the
persistent pool rather than trusting the wrapper.
Two storage hazards found while building this and covered by tests:
- makeCacheKey() is not injective for base64url. It collapses the whole
punctuation alphabet to "_", so "abc-def" and "abc_def" would share one
cache slot and one credential would silently overwrite the other.
Credential ids are therefore hashed, and a test uses precisely that pair.
- A record's own credential id is authoritative. An index entry pointing at
a record that disagrees with its key is rejected rather than trusted.
Unreadable or wrong-shaped entries degrade to "credential unavailable" so a
corrupt value cannot 500 the login page.
PasskeyCeremonyFactory is the single seam onto webauthn-lib: it builds the
serializer and pins attestation to `none` only, so a future version that moves
or renames library types touches one file.
Suite: 353 tests / 831 assertions, 100% coverage on all new files.
phpstan level 6 clean, php-cs-fixer clean, conformance 35/35.
Groundwork for passkey authentication, with the feature switched off by
default and no behaviour change when it is off.
Decision D1: passkeys require central authentication. A passkey is scoped to
a relying party spanning the base domain, which only exists when
SUBDOMAIN_REDIRECT is on and AUTH_SUBDOMAIN resolves to a base domain. The
RP ID is therefore always that base domain, never the request host.
Decision D4: HTTPS is required and is not exemptible. The allowed origin is
built as https://{authSubdomain} from configuration and never from the
request, so an http:// origin cannot be accepted, and isAvailableFor()
additionally refuses to offer the UI on a non-secure connection. The
deprecated setSecuredRelyingPartyId() escape hatch is not used and there is
deliberately no override that could reintroduce one.
Enabling PASSKEY_ENABLED without a usable configuration is a hard error via
a non-optional cache warmer, because entrypoint.sh runs cache:warmup on every
production boot: a misconfigured deployment fails to start instead of
offering a button that cannot work.
Also drops 12 obsolete phpstan-baseline entries for TotpTestHelper: adding
#[\Override] to its anonymous clock removed the rule violation at its source
rather than suppressing it.
Suite: 333 tests / 770 assertions (was 313 / 738), 100% coverage on new
files. phpstan level 6 clean, php-cs-fixer clean, conformance 35/35.
Brings preauth from 18/34 to 30/34 conformance (auth-gateway profile). The
remaining four checks all depend on files this branch cannot change (see below).
PHP toolchain (§1)
- require.php >=8.4 -> ^8.5, and pin config.platform to 8.5.0. The old
constraint also permitted PHP 9, which is not a promise we can keep.
composer.lock regenerated with --lock: content-hash + platform-overrides
only, zero dependency version movement.
- friendsofphp/php-cs-fixer * -> ^3.95. A wildcard meant CI was not
reproducible.
PHPStan (§2.2)
- vendor the shared phpstan.neon.dist (level 6) + a generated baseline.
187 errors are captured rather than fixed; the baseline should only shrink
from here.
- add phpstan/phpstan:^2.1 to require-dev.
Code style (§8.2)
- vendor the shared .php-cs-fixer.dist.php (@Symfony + @Symfony:risky +
declare_strict_types) and apply it: 59 of 67 files reformatted.
- Verified this is a formatting change, not a behaviour change: all 313 tests
pass after the reformat, all in_array() calls already passed strict=true,
and the remaining edits are @Symfony:risky idiom (yoda conditions, \count(),
self:: over the class name).
Repository layout (§4.4)
- docs/{Caddyfile,compose.yaml,example.env} -> docs/examples/, with
example.env becoming the conventional .env.example. This is the layout
GUIDING-LIGHT already cites preauth as doing correctly — it just needed
renaming.
- update the four readme.md references and a stale compose.yaml comment.
- docs/v1.1-plan.md references are left alone deliberately: it is a historical
plan recording what was done at the time, not live documentation.
Licence and security policy (§7)
- add LICENSE (uniform MIT, matching composer.json).
- add SECURITY.md describing the actual threat model: per-request
allow/intercept, no caching of the login flow, app-set security headers,
TOTP, and the fact that REMOTE_USER is trusted input.
Mobile accessibility (§3.3a)
- templates/base.html.twig: drop maximum-scale=1 and add viewport-fit=cover.
preauth was the one app already past the font-size precondition (controls
render at 21.6px = 0.9em x 24px), so removing the lock is safe here and
restores pinch-zoom for Android users.
Conformance tooling (§8.2)
- vendor .ci/conformance.sh and .ci/css-control-size.py so the check runs
from a checkout rather than fetching from the LAN-only private/ci.
- .editorconfig synced from the version that keeps the Caddyfile tab rule.
Not included (blocked by the .gitea/workflows pre-receive hook):
- ci-composer-audit, ci-composer-validate, ci-reusable-workflows.
Workflow files may only change via a trusted ref, so the caller files are
staged but not committed.
Also not included: dockerfile-nonroot (§6.4). Adding USER to an image with
VOLUME [/config, /data] changes volume ownership and needs an actual container
build/run to verify, so it goes in its own change.
The login page, failed logins, redirects, and rate-limit/error pages could
be stored by the browser (Symfony's default 'no-cache, private' still
permits storage — it only requires revalidation). Older Safari builds may
then replay a stale pre-auth response on refresh, appearing to log the
user back out, or show a previous session after logging in again.
- SecurityHeadersListener: send strict anti-caching headers on non-2xx
responses only (no-store/no-cache/must-revalidate/proxy-revalidate,
max-age=0, s-maxage=0 + Pragma, Expires, Surrogate-Control, Vary: *).
2xx grants (already-authenticated / public access) are consumed by
Caddy's forward_auth check and never reach the browser, and protected
services' own cache headers must stay untouched.
- templates/_script.html.twig: fetch() with cache: 'no-store'; follow
redirects with location.replace() to keep the login page out of history
and the back-forward cache.
- docs/Caddyfile: reusable (preauth_no_store) snippet imported into every
forward_auth block, using header_down so the guarantee holds at the edge
(verified: replaces conflicting upstream values, leaves service
responses alone).
- tests: unit coverage for the listener and functional coverage for the
full HTTP kernel (login/failure/redirect/rate-limit not cacheable;
200 grants untouched); asserts the rendered page carries the JS changes.
- readme/CHANGELOG updates.
The host-prefix regex required at least one character after the slash
(/\+.+/), so a pattern like 'code.example.com/' was silently dropped
instead of matching the root path '/'. Changed \+.+ to \+.* so the
trailing slash alone is accepted as the path '/'.
Added tests covering the exact bug scenario from PUBLIC_PATHS
config: 'code.digitaladapt.com/,code.digitaladapt.com/public/**'
Add PublicAccessListener (priority 84) that allows rate-limited
unauthenticated access to configured public paths. Authenticated users
bypass this listener entirely via AcceptListener/AllowListener.
New components:
- PublicPathMatcher service with wildcard path matching (* and **)
and optional host-prefix scoping
- PublicAccessListener applying per-IP rate limiting to public paths
- Separate public_limiter compound rate limiter (burst + sustained)
- publicRateLimitCache pool (APCu in prod, array in tests)
New env vars:
- PUBLIC_PATHS (comma-separated path patterns, empty = disabled)
- PUBLIC_BURST_COUNT/PUBLIC_BURST_TIME (default 100/60s)
- PUBLIC_UPPER_COUNT/PUBLIC_UPPER_TIME (default 500/3600s)
Tests: 52 new tests (29 unit for PublicPathMatcher, 12 unit for
PublicAccessListener, 11 functional for PublicAccessFlowTest).
Total: 293 tests, 605 assertions, all passing.
PHP CS Fixer: 0 of 63 files need fixing.
Documentation: README, CHANGELOG, ROADMAP, Caddyfile, example.env
all updated with public access configuration and examples.
Add REMOTE_USER env var with four modes:
- session (default): sends session id, backward-compatible
- static: sends a fixed string (REMOTE_USER_STATIC)
- mapped: looks up session id in REMOTE_USER_MAP
- none: omits the header entirely
New RemoteUserMode enum, ConfigBag parsing/validation, and
StringTrait::authSuccessResponse resolves the header value based
on the configured mode. AcceptListener now receives ConfigBag as
a constructor dependency.
Addresses design consideration 1.2 (Remote-User header value is
user-controlled) from DESIGN_CONSIDERATIONS.md.
241 tests pass, 0 cs-fixer violations.
Security:
- Add SecurityHeadersListener (X-Content-Type-Options, X-Frame-Options,
CSP, Referrer-Policy, HSTS)
- Replace document.write() with document.documentElement.innerHTML
in login JS to avoid CSP violations
- Add CSS escaping (|e('css')) to env color values in _style.html.twig
- Document CSRF protection model: nonce serves as CSRF token for POST
form path (single-use, server-generated, 120s TTL)
- Reduce TOTP verification window from 10 periods (±5 min) to 1 (±30s)
- Remove hardcoded APP_SECRET from bin/franken.sh (now uses env or
generates random)
- Remove backup code values from debug log output
- Add .env to .gitignore
Bug fixes:
- Fix ->json access on possibly-null in LoginListener
(uses null-safe operator ?->)
- Fix validReturn() not checking false from parse_url (could cause
TypeError on malformed URLs)
- Add isHit() race condition check in AcceptListener and AllowListener
- Add try/finally in Kernel::terminate() so parent::terminate() always
runs even if persist() throws
- Add input validation to GenerateBackupCodesCommand (reject count < 1)
- Use Response::HTTP_INTERNAL_SERVER_ERROR constant in GetTotpTrait
instead of literal 500
Docker/CI:
- Explicitly install curl in Docker final image (needed for healthcheck)
- Update workflow tag pattern to v*.*.* (standardize on v-prefix)
- Extract version without v-prefix for Docker image tag
- Remove stale develop branch from CI triggers
- Fix publish.yaml git remote add to use set-url on re-runs
Code quality:
- Add declare(strict_types=1) to all interface files
- Add #[AsCommand] attribute to GenerateBackupCodesCommand
- Fix BackupCodeInterface default count to match implementation (10)
- Lowercase host before TLD lookup in DomainManager
- Expand TLD list with many missing multi-part TLDs (.com.au, .co.jp,
.com.br, .co.kr, .com.tw, .co.za, etc.) to prevent open redirect
vulnerabilities
- Disable unused Symfony sessions in framework.yaml
Tests:
- Update DomainManagerTest for corrected TLD parsing (.com.au, .co.jp,
.com.br now correctly recognized as multi-part)
- Update GetTotpTraitTest for corrected error message
- Update GenerateBackupCodesCommandTest: zero count now throws exception
- Add phpunit/phpunit ^13.2, symfony/browser-kit and symfony/css-selector
to require-dev, plus the autoload-dev mapping for App\Tests- Add phpunit.dist.xml (strict deprecation/notice/warning failures,
APP_ENV=test forced) and .env.test / bin/phpunit / tests/bootstrap.php
from the PHPUnit recipe
- Add tests/Support/TotpTestHelper providing a deterministic TOTP
fixture, frozen clock and ConfigBag/cache-pool helpers
- Add 121 unit tests covering Clock, ConfigBag, Data/Payload, Enum/Scope,
MonitorCacheKeys, PersistCache, Utilities, all five Traits and the
three Service managers (BackupCode, Domain, Login)
- Fix LoginManagerTest nonce lookups to use makeCacheKey() so the cache
key matches the one the manager actually reads/writes
- Gitignore bin/.phpunit.result.cache