The project's own bar is full coverage, and the new code had drifted from it —
notably every error path, which is exactly where a browser is least likely to
go on purpose and an attacker is most likely to.
Two real bugs surfaced, both of the same shape: a cache failure escaping as a
500 on the login page.
- `credentials->find()` was called outside the try block in `finishLogin()`, so
a store failure threw instead of reporting a failed ceremony.
- `credentials->save()` was likewise unguarded in `finishRegistration()`, and
there the consequence was worse: reporting success for a credential that was
never stored, so the user would believe their passkey was registered and
discover otherwise only at the next login.
Both now degrade to a failed ceremony, matching the rule the rest of the class
follows: a failure the user cannot act on must never look like a server fault.
Coverage is now at 98.7% of lines; the remainder is pre-existing defensive
catches in AcceptListener/AllowListener plus a couple of unreachable guards.
Covers the README (prerequisites, the two hard requirements, every new env var,
how registration and login work, and the counter caveat), SECURITY.md (the
ceremony model, single-use challenges, origin handling, the shared rate-limit
budget, and the attestation rationale with the conditions that would reverse
it), CHANGELOG (Added/Security/Changed), ROADMAP (Phase 2c complete, with the
deviations from the original sketch) and DESIGN_CONSIDERATIONS (the four
decisions whose reasoning is not visible in the code).
The docs lead with the two prerequisites because both are enforced rather than
advisory: enabling passkeys without central auth, or on a host that cannot
serve HTTPS, fails at container start. Neither is a runtime surprise, and a
reader needs to know that before they turn the feature on.
The plan document is updated to record that it is complete, and to note the two
places where implementation deviated from it — the listener/extraction order,
and register-begin not being a listener operation. The second was a design
error in the plan, not just an ordering change, so it is called out explicitly.
The whole flow through the real HTTP kernel, with nothing about the ceremony
stubbed: registration builds a genuine CBOR attestation object signed by a real
P-256 key, and login signs a real assertion. Only the browser's plumbing is
simulated — the fetch() calls become requests, which is the seam worth testing.
Covered: a real registration grants a session; a real passkey login grants a
session and reports the right Remote-User; a bad TOTP starts no ceremony; a
spent nonce starts no ceremony; a replayed ceremony fails; an assertion for
another challenge fails; an unknown credential fails with the same generic
message a wrong code gets; both login paths set an identical cookie; ceremony
responses are not cacheable and do not leak their marker; the ceremony is inert
when disabled; the page offers passkeys only when enabled; and the CSP permits
the two WebAuthn directives.
Writing these found a real bug. The registration checkbox originally submitted
a plain form POST, which returns HTML — and, more importantly, loses the fresh
nonce the failure response issues. The user's next attempt would then fail
against a nonce that had already been spent, with no visible reason why. It now
goes through the same X-Preauth AJAX path as an ordinary login, so failures come
back as JSON with a usable nonce; a non-JSON submission is treated as an
ordinary login, and LoginManager returns null for it rather than starting a
ceremony that nothing could finish.
Three test failures were also correct behaviour rather than bugs: once a session
cookie exists, AcceptListener (priority 99) answers before any ceremony listener
runs, so tests exercising a second ceremony need a visitor without that cookie.
That is the intended ordering, now documented in the tests.
The login page gains a sign-in button and, where the form actually POSTs, a
"register this device" checkbox. Both are rendered only when the policy says
passkeys are available for that request, so an unavailable configuration stays
byte-identical to before — a test asserts exactly that, rather than trusting the
conditional is in the right place.
The registration checkbox is omitted on hosts the form does not POST from,
because a registration ceremony is authorised by the TOTP code carried in that
submission. The button is still offered there; only the checkbox is not.
Writing the tests surfaced three real problems, all fixed here:
- ConfigBag had no passkeyButtonName()/passkeyRegisterName() accessors, so the
template referenced configuration that was never exposed.
- ListenerTestHelper's anonymous rate-limiter classes were missing #[Override],
and the baseline pinned them by line number — so adding two array keys broke
it. Fixed at the source instead: the attributes are now present, which also
let 36 line-pinned baseline entries be deleted.
- Those classes threw ReserveNotSupportedException with no arguments, which the
Symfony signature forbids. The baseline had been hiding this behind
path-specific ignores; phpstan reports it correctly now.
The net baseline change is 216 deletions and no additions: every entry removed
was one whose underlying issue is now genuinely fixed.
Corrects a design error in the previous commit. I had exposed register-begin as
a listener operation and gated it on a session cookie, but the approved flow has
no session at that point: the whole point is that a valid TOTP code is what
authorises registration, and the session is only issued once the new credential
has been verified.
Two consequences, both bad:
- There is no session cookie to check, so the gate could never have worked. It
would have been dead code that looked like a security control.
- More seriously, a listener-side register-begin would hand out a challenge
without proving anything. Anyone could obtain ceremony options and attempt
registration. The cookie check was not a weak control; the operation itself
was the hole.
Registration is now started by LoginManager, after it has verified both the code
and the nonce, and its options are returned with the login response. That is the
flow in the plan, and it keeps nonce validation in the one place that already
enforces it. The capability for register-finish is the single-use ceremonyId,
which is server-issued and bound to the identity that passed the check.
The listener now serves three operations, and a test pins that register-begin is
not one of them.
Also adds Payload::$register so the checkbox intent survives from the form to
LoginManager, and moves the ceremony marker constant to AppConstants since both
LoginManager and the listener now produce marked responses.
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.
private/ci gained the fix that makes ci-composer-audit / ci-composer-validate
follow a project's shared-workflow pin (`uses: private/ci/.../php-test.yaml@v1`)
instead of grepping for inline workflow text that an adopted repo no longer
contains.
That file is VENDORED into this repo at .ci/ (by design — §8.2, so CI never
fetches from the LAN-only private/ci at run time), so moving the v1 tag
updated the workflow but not this copy. Re-vendored with sync-configs.sh.
Before: 2 of 34 checks failed (the two false positives).
After: All 35 runnable checks passed.
Also brings .ci/validate-bake.py, which the new checker's bake-target-exists
check needs; without it that check correctly reports SKIPPED rather than
silently passing.
The build was described in two places kept in step by hand: the Dockerfile, and
the `with:` block of each docker workflow file. This moves the parts CI actually
decides — Dockerfile target, platforms, tags, build args — into docker-bake.hcl,
so the build is version-controlled with the code and reviewable in a diff.
Tags are byte-identical to the current workflows, verified with
`buildx bake --print`:
main push -> digitaladapt/preauth:develop
tag v1.2.0 -> digitaladapt/preauth:latest AND digitaladapt/preauth:1.2.0
Dockerfile: the final stage gains `AS app` so bake can target it. Naming the
last stage is a no-op for a plain `docker build` — it is still the default
target — so `docker build .` behaves exactly as before.
THREE THINGS FOUND WHILE DOING THIS
-----------------------------------
1. `buildx bake --print` does NOT read the Dockerfile, so it accepts a `target`
naming no stage. preauth's final stage was unnamed, so `target = "app"` would
have failed at build time in CI *after the push*, with --print reporting
success. Fixed by naming the stage.
2. The `bake` backend in docker-publish.yaml sets no layer cache, while the
`action` backend it replaces sets cache-from/cache-to: type=gha. Neither
existing bake user in the portfolio sets one either, so every bake build in
the portfolio runs cold. preauth compiles APCu from source (pecl) in both
stages, so this would have been a real regression. The bake file now sets the
gha cache, with the override for local builds documented.
3. `ARG MAX_REQUESTS` has been documented in the Dockerfile since the Symfony
8.1 upgrade, but no workflow ever passed it, so CI silently built with the
500 default. It is now an explicit named variable, overridable per build.
The workflow callers that select the bake backend are NOT in this commit:
.gitea/workflows/ is protected by a pre-receive hook and only changes via a
trusted ref. They are staged in the working tree.
Verified: Dockerfile stage/target contract holds; bake file parses and resolves
under buildx 0.37; 313 tests pass; conformance 31/34 with callers staged.
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 removed runtime/frankenphp-symfony package force-restarted the
worker after FRANKENPHP_LOOP_MAX requests (default 500) - memory-leak
paranoia the built-in 8.1 runner intentionally doesn't reimplement.
Restore the behavior natively so the upgrade is a no-op operationally:
- Caddyfile: global frankenphp block with max_requests {$MAX_REQUESTS}
- per-thread graceful restarts, other threads keep serving
- Dockerfile: ARG/ENV MAX_REQUESTS=500 bakes the same default the old
package had, overridable at docker build or runtime (-e, 0 disables)
- Stock FRANKENPHP_CONFIG env var remains the full-config escape hatch
- Documented in readme env table, docs/example.env, CHANGELOG, and the
upgrade plan (phase-4 staging note now checks thread recycling)
- Plan doc: mark Phases 0-3 + worker-mode state audit complete with
verified results; resolve open questions (all src/ services are final
readonly - no ResetInterface needed under kernel-reusing runner)
- CHANGELOG: Unreleased entries for the 8.1 bump and the
runtime/frankenphp-symfony removal (incl. FRANKENPHP_LOOP_MAX note)
- ROADMAP: current stack now Symfony 8.1; webauthn research note updated
- bin/dev.sh: correct stale PHP requirement comment
- All symfony/* constraints bumped 7.4.* -> 8.1.* (resolved to 8.1.2-8.1.6)
- Removed runtime/frankenphp-symfony: symfony/runtime 8.1 handles
FrankenPHP worker mode natively via FrankenPhpWorkerRunner; deleted
the extra.runtime class override so GenericRuntime auto-detects it
- phpunit 13.3.2, twig 3.28, otphp 11.5 ride along
- Deprecation sweep on 7.4 was clean (failOnDeprecation suite, zero hits)
- Verified: bin/console about boots on 8.1.6; 295 tests / 612 assertions
green after the bump
Covers deprecation sweep gate, composer.json constraint bumps to 8.1.*,
and removal of runtime/frankenphp-symfony (FrankenPHP worker mode is
handled natively by symfony/runtime in 8.x). Includes config recipe
re-sync, Docker/CI notes, phased rollout with rollback, and open
questions (kernel-reset audit for worker mode).
The test workflow was hitting GitHub's unauthenticated API rate
limit (60 req/hour) when downloading 95 packages via composer
install --prefer-dist, causing 429 Too Many Requests errors.
Two fixes applied:
1. Cache Composer's download cache (~/.composer/cache) keyed on
composer.lock hash, so repeated CI runs don't re-download
packages at all.
2. Configure GitHub OAuth token via SYNC_GITHUB_TOKEN secret to
raise the rate limit to 5,000 req/hour for cache misses.
When subdomain redirection is off, the login form is served inline on
the protected host and submission happens via a same-origin fetch() call
in _script.html.twig. The CSP default-src 'none' was blocking that
fetch (connect-src falls back to default-src).
Add connect-src 'self' to the CSP only when the request is not on the
auth subdomain (i.e. when the inline script is present). On the auth
subdomain the form POSTs normally with no inline script, so the stricter
policy still applies there.
This is the least-privilege relaxation: only same-origin connections,
only on pages that need them.
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/**'
The Twig |e('css') filter was escaping '#' (0x23) to '\23 ' in hex
color values (bg_color, fg_color, error_color), causing browsers to
not recognize them as valid CSS colors. These are admin-configured
environment variables, not user input, so CSS escaping is unnecessary.
Manages a local PHP dev server for end-to-end development and testing.
Binds to 0.0.0.0:8773, accessible via Caddy at
https://preauth.lyra-dev.devgnome.com.
Features:
- Self-bootstrapping: installs PHP 8.4 + extensions (including APCu,
which is critical for nonce cache, rate limiter, and session storage),
Composer, and project dependencies if missing. Survives terminal
resets/reboots.
- Enables apc.enable_cli=1 for console commands (matches Dockerfile)
- Subcommands: start, stop, status, restart
- Sets APP_SHARE_DIR to var/share for filesystem session persistence
- Clears dev cache on start
No database needed — preauth uses APCu + filesystem cache exclusively.
Port assignment: P-R-E = 7-7-3 → 8773
andrew
lyra