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.
29 KiB
Design Considerations — Preauth
Summary
Preauth is a well-architected TOTP-based authentication gateway that has evolved from a single-file script into a clean, event-listener-driven Symfony application with 100% test coverage. The codebase demonstrates strong security fundamentals (host-prefixed cookies, nonce-based replay protection, rate limiting, backup code system) and thoughtful operational design (dual-layer cache with change tracking, FrankenPHP worker mode).
This document was originally prepared as a design review. Items that have been addressed are marked with ✅ and include a reference to the commit or change that resolved them. Items still open are marked with ⬜ and remain as recommendations for future work.
0. Passkey implementation notes
Four decisions are worth recording because the reasoning is not obvious from the code, and each looks like an odd choice without it.
0.1 The signature counter is checked leniently, against the library's default
webauthn-lib's default checker requires a strictly increasing counter. That
is wrong for the passkeys this feature targets: a synchronised passkey reports a
constant 0 forever, so the default rejects a brand-new credential on its
first login. Measured against the installed version: stored 0, reported
0 → CounterException.
The failure mode is what makes this worth a note. It cannot happen in a unit
test that increments the counter — only on real hardware, and only for the most
common kind of passkey. PasskeyCounterChecker therefore accepts equal-or-greater
and rejects only a counter that moves backwards. Clone detection is explicitly
not claimed as a property of this feature.
0.2 The ceremony replies are the only browser-facing 2xx
SecurityHeadersListener applies no-store to non-2xx responses only, on the
assumption that a 2xx is consumed by the proxy's forward_auth check. That
assumption is false for a ceremony reply: the auth subdomain is reverse_proxy-ed
with no forward_auth in front of it, so the JSON goes straight to the browser.
Left alone it would be cacheable, and a browser could replay a stale challenge.
The producer marks the response (PasskeyListener or LoginManager) and the
caching policy lives in one place that consumes the marker, rather than being
duplicated at each site that happens to return 2xx.
0.3 Registration is a checkbox on the login form, not an endpoint
A separate register-begin endpoint was the first design and would have been a
vulnerability: it hands out a challenge without proving anything. The TOTP check
is what authorises registration, and that check happens inside LoginManager as
part of an ordinary login submission — so LoginManager is where the ceremony
starts.
There is no session cookie to check at that point either, which makes the point
neatly: the whole flow is what produces the session, so anything gated on one
cannot be part of it. The capability at register-finish is the single-use
ceremony id, issued server-side and bound to the identity that passed the check.
0.4 Both login paths share one session-issuing implementation
SessionIssuer was extracted from LoginManager when the passkey ceremony
needed the same behaviour. Two implementations would have drifted, and the most
likely place to drift is cookie attributes — where a difference is invisible
until it breaks in a browser, on one path only. A functional test compares the
cookies the two paths produce, field by field.
1. Security
1.1 Missing Security Response Headers [HIGH PRIORITY] ✅ Addressed
Current state: Fixed. A SecurityHeadersListener (response event, priority 0) now sets the following headers on all main-request responses:
X-Content-Type-Options: nosniff
X-Frame-Options: DENY
Content-Security-Policy: default-src 'none'; script-src 'unsafe-inline'; style-src 'unsafe-inline'
Referrer-Policy: strict-origin-when-cross-origin
Strict-Transport-Security: max-age=31536000
The inline <script> and <style> in the templates mean a CSP with 'unsafe-inline' for script-src and style-src is the strictest practical policy today. Moving scripts/styles to external files would allow a stricter CSP in the future.
1.2 Remote-User Header Value is User-Controlled [HIGH PRIORITY] ✅ Addressed
Current state: Fixed. The Remote-User header value is now configurable via the REMOTE_USER environment variable, which supports four modes:
session(default, backward-compatible): Sends the session id, as before. The value is still sanitized viamakeCacheKey().static: Sends a fixed string (configurable viaREMOTE_USER_STATIC, defaultauthenticated) for all authenticated requests. This eliminates the user-controlled header issue entirely.mapped: Looks up the session id in a configured map (REMOTE_USER_MAP, format:id1:user1,id2:user2) and sends the mapped value. Falls back to the session id if not found in the map. This is the path to multi-user support.none: Omits theRemote-Userheader entirely. Caddy'sforward_authstill accepts the request based on the 200 status code.
The RemoteUserMode enum (src/Enum/RemoteUserMode.php) encapsulates the modes. StringTrait::authSuccessResponse() resolves the header value based on the configured mode, and ConfigBag handles parsing the map string and validating the mode (invalid values fall back to session). AcceptListener now receives ConfigBag as a constructor dependency to support this.
1.3 No CSRF Protection on POST Form Login [MEDIUM PRIORITY] ✅ Addressed
Current state: Resolved through documentation and analysis. The nonce system provides CSRF protection for the POST form path: nonces are server-generated, single-use, and have a 120-second TTL. An attacker cannot forge a POST request without first loading the login page to obtain a valid nonce, which requires being on the auth subdomain. The LoginListener class docblock and login.html.twig template comment now explicitly document this CSRF protection model. The AJAX (header) path embeds the nonce in the base64url payload.
1.4 TOTP Verification Leeway May Be Too Generous [LOW PRIORITY] ✅ Addressed
Current state: Fixed. The TOTP verification window has been reduced from 10 periods (±5 minutes) to 1 period (±30 seconds). With the default 30-second TOTP period, a code is now valid for at most 90 seconds (the current window plus one window on each side), down from the previous 50 seconds per window with 10-period leeway. The ROADMAP has been updated to reflect this change.
1.5 Backup Code Logging Reveals Code Value [LOW PRIORITY] ✅ Addressed
Current state: Fixed. The debug log in BackupCodeManager::verifyAndConsume() no longer includes the backup key name. It now logs only the hit/miss and valid/invalid status: "checking backup code: HIT & VALID" or "checking backup code: miss & invalid".
1.6 TOTP Object Reconstructed on Every Verification [LOW PRIORITY] ⬜ Open
Current state: GetTotpTrait::getTotp() calls OTHP\Factory::loadFromProvisioningUri() on every invocation. This parses the OTP URI string and constructs a new TOTP object each time a token is verified.
Note: An attempt was made to memoize the TOTP object within the request cycle, but PHP 8.4's readonly class constraint prevents traits from defining mutable properties in readonly classes (LoginManager and BackupCodeManager are both final readonly). Resolving this would require either removing readonly from these classes, using a separate memoization service, or refactoring GetTotpTrait into a dedicated injectable service.
Why: This is a minor performance concern — URI parsing and TOTP object construction happen on every login attempt. In a FrankenPHP worker process that handles many requests, this adds unnecessary overhead. It's not a security issue, but it's an easy optimization if the readonly constraint is relaxed.
1.7 CSS Injection in Style Template [MEDIUM PRIORITY] ✅ Addressed
Current state: Fixed. Environment-configured color values (bg_color, fg_color, error_color) in _style.html.twig are now escaped with Twig's |e('css') filter to prevent CSS injection from malicious environment variable values.
1.8 $payload->json Access on Possibly-Null Payload [HIGH PRIORITY] ✅ Addressed
Current state: Fixed. In LoginListener::onKernelRequest(), the $payload->json access on a possibly-null $payload has been replaced with $payload?->json ?? true, and $payload->id with $payload?->id ?? ''. This prevents a crash when a login attempt is detected (e.g., via the X-Preauth header) but the payload is invalid (malformed base64, non-object JSON, etc.).
1.9 validReturn() Doesn't Check false from parse_url [MEDIUM PRIORITY] ✅ Addressed
Current state: Fixed. DomainManager::validReturn() now checks for false and empty string in addition to null when examining the return value of parse_url($url, PHP_URL_HOST). This prevents a TypeError on malformed URLs that filter_var(FILTER_VALIDATE_URL) accepts but parse_url cannot parse.
1.10 Incomplete TLD List in DomainManager [HIGH PRIORITY] ✅ Addressed
Current state: Fixed. The TLD lookup table in DomainManager has been significantly expanded with many previously missing multi-part TLDs, including .com.au, .co.jp, .com.br, .co.kr, .com.tw, .co.za, and dozens more. Without these entries, domains like evil.com.au would incorrectly match auth.example.com.au (both would resolve to base com.au), creating an open redirect vulnerability. The host is also now lowercased before TLD lookup to fix a case-sensitivity issue.
2. Architecture & Code Quality
2.1 Trait-Based Dependency Injection Pattern [MEDIUM PRIORITY] ⬜ Open
Current state: Several traits (HasLoggerTrait, GetTotpTrait, MakeNonceTrait) use #[Required] attribute for setter injection into readonly classes. For example, LoginManager receives $config, $logger, and $nonceCache via traits rather than through its constructor. The constructor only accepts three parameters; the rest are wired via setter methods called by the service container after construction.
Recommendation: Move these dependencies into the constructors of the classes that use them. If multiple classes share the same dependencies, that's fine — PHP constructors can accept many parameters, and it makes the dependency graph explicit. Alternatively, create a shared Dependencies value object that bundles logger, config, and nonce cache.
Why: The trait-based setter injection pattern makes it non-obvious what dependencies a class has — you have to look at both the constructor and all the traits it uses. It also creates a temporal coupling issue: the object exists in a partially-constructed state between construction and setter calls. With readonly classes, this works only because the trait properties are declared in the trait, not the class, which is a subtle language detail that could confuse future maintainers. Standard constructor injection is more explicit, testable, and conventional in Symfony.
2.2 Duplicated Cookie Logic Between LoginManager and InterceptListener [MEDIUM PRIORITY] ✅ Addressed
Current state: Resolved. The duplicated cookie name and domain selection logic has been extracted into two shared methods on CookieNameTrait:
sessionCookieName(DomainInterface $domainManager): string— Returns the appropriate cookie name (__Host-Http-Preauthor__Http-Domain-Preauth) based on whether central auth is active.sessionCookieDomain(DomainInterface $domainManager, string $host): ?string— Returns the cookie domain for central auth mode, or null for single-domain mode.
LoginManager::setCookie(), AcceptListener::onKernelRequest(), and InterceptListener::pruneInvalidCookie() all now use these shared methods. The fragile "changes here must be reflected in InterceptListener::pruneInvalidCookie()" comment has been removed.
2.3 MonitorCacheKeys Instantiated Multiple Times for Same Pool [MEDIUM PRIORITY] ⬜ Open
Current state: MonitorCacheKeys is a decorator that tracks cache key changes. It's instantiated independently in PersistCache, LoginManager, and BackupCodeManager, each wrapping the same underlying CacheItemPoolInterface. The key list (__key_list) and change list (__chg_list) are stored in the cache itself, so the instances share state — but each instance calls initialize() in its constructor if the lists don't exist yet, and each save()/saveDeferred() call triggers additional metadata writes.
Recommendation: Register MonitorCacheKeys as a decorated service in the DI container (using Symfony's decorates feature) so there's a single instance per cache pool. Or, make MonitorCacheKeys a stateless service that's injected once, rather than having each consumer create its own wrapper.
Why: Multiple instances wrapping the same pool is wasteful — each save() call triggers a cascade of metadata operations (update key list, log change, commit). With three instances, a single cache write could trigger nine additional cache operations. A single decorator service would be more efficient and would make the lifecycle clearer.
2.4 Payload Base64url Decoding Has Broken Padding [MEDIUM PRIORITY] ✅ Already Correct
Current state: Not an issue. The code correctly uses:
$base64 = strtr($base64url, '-_', '+/');
$base64 .= str_repeat('=', (4 - strlen($base64) % 4) % 4);
This was fixed in a prior commit ("Fix docs, add .dockerignore, fix base64url padding, fix typo"). The original review incorrectly reported the use of str_pad; the implementation now correctly uses str_repeat to add the proper number of = padding characters.
2.5 Symfony Sessions Enabled But Unused [LOW PRIORITY] ✅ Addressed
Current state: Fixed. config/packages/framework.yaml now has session: false with a comment explaining that preauth implements its own cookie/cache-based session management and does not use Symfony's session subsystem.
2.6 config/reference.php Committed to Repository [LOW PRIORITY] ✅ Already Handled
Current state: Not an issue. config/reference.php is already listed in .gitignore under the project-specific section and is not tracked in version control.
2.7 Public Properties on Payload DTO [LOW PRIORITY] ⬜ Open
Current state: Payload uses public properties ($id, $token, $nonce, $json, $scope) with no encapsulation. The object is mutable after construction.
Recommendation: Consider making Payload a readonly class (PHP 8.4+ supports readonly classes natively) with a constructor that takes all fields, or use Symfony's Stringable/value object patterns. Since LoginManager mutates $payload->scope (downgrading IP to Cookie), the current design requires mutability — but this could be handled by returning a new instance instead.
Why: Immutable DTOs are safer to pass around, especially in an event-driven system where the same object might be referenced by multiple listeners. The current mutation in LoginManager::checkToken() (changing $payload->scope) is a side effect that's not obvious from the method signature.
2.8 Duplicated "hi $id" Response Construction [LOW PRIORITY] ✅ Addressed
Current state: Fixed. The duplicated new Response("hi $id", headers: ['Content-Type' => 'text/plain', 'Remote-User' => $id]) pattern in AcceptListener, AllowListener, and LoginManager has been extracted into StringTrait::authSuccessResponse(string $id): Response, which all three classes now use.
2.9 Duplicated Constants [LOW PRIORITY] ✅ Addressed
Current state: Fixed. The duplicated '2999-12-31' far-future date string (previously in Utilities::makeTotp() and BackupCodeManager::verifyAndConsume()/saveCodes()) and the 128 max input length (previously in StringTrait::makeCacheKey() and Payload::create()) have been extracted into AppConstants::FAR_FUTURE_DATE and AppConstants::MAX_INPUT_LENGTH respectively.
3. Testing
3.1 No Tests for Concurrent Access / Race Conditions [LOW PRIORITY] ⬜ Open
Current state: The test suite is excellent — 222 tests, 100% coverage, good edge case coverage. However, there are no tests for concurrent access scenarios, such as two requests using the same nonce simultaneously, or cache initialization race conditions in MonitorCacheKeys.
Recommendation: Add a few integration tests that simulate concurrent access (e.g., using process forks or mock caches with delays). At minimum, document that concurrent access is expected to be handled by APCu's atomic operations.
Why: MonitorCacheKeys::initialize() checks if key lists exist and creates them if not — under concurrent startup, two instances could both see missing lists and both call initialize(). This is likely fine because APCu operations are atomic, but it's worth having a test or at least a documented assumption. The race condition between hasItem() and getItem() in AcceptListener and AllowListener is now handled with an isHit() check, but is not tested.
3.2 No Security-Focused Test Suite [LOW PRIORITY] ⬜ Open
Current state: Security behaviors (nonce replay, backup code reuse, rate limiting) are tested as part of the functional and unit tests, but there's no dedicated security test suite that systematically probes for common vulnerabilities.
Recommendation: Consider adding a tests/Security/ directory with tests for: XSS attempts in the username field, header injection via the return parameter, cookie attribute verification (Secure, HttpOnly, SameSite), and response header presence (now that security headers are added).
Why: For an authentication gateway, security testing deserves its own focused suite that's easy to find and extend. This also makes it easier for security reviewers to understand what's been tested.
4. Docker & Deployment
4.1 Healthcheck Depends on curl Which May Not Be Installed [MEDIUM PRIORITY] ✅ Addressed
Current state: Fixed. The Dockerfile now explicitly installs curl in the final image with apt-get install -y --no-install-recommends curl and cleans up the apt lists to keep the image small.
4.2 Typo in bin/franken.sh [LOW PRIORITY] ✅ Already Fixed / Addressed
Current state: Fixed. The typo (digtialadapt → digitaladapt) was corrected in a prior commit. The script has since been further improved: the hardcoded APP_SECRET has been removed (now uses the APP_SECRET environment variable or generates a random secret), and the docker container rm command now suppresses errors when the container doesn't exist.
4.3 No .dockerignore File [LOW PRIORITY] ✅ Already Handled
Current state: Not an issue. A .dockerignore file exists and excludes .git/, .gitignore, var/, vendor/, tests/, .phpunit.cache/, docs/, *.md, .env, .env.test, .env.local, and composer.phar from the Docker build context. This was added in a prior commit.
4.4 Dockerfile Uses PHP 8.5 Which Is Bleeding Edge [LOW PRIORITY] ⬜ Open (Deliberate)
Current state: The Dockerfile uses php:8.5-trixie for the build stage and dunglas/frankenphp:php8.5-trixie for the final image. composer.json requires php >= 8.4. The CI workflow in tests.yaml also uses PHP 8.5.
Recommendation: This is a deliberate choice and likely fine for a personal project. If broader compatibility is desired, consider testing against both PHP 8.4 and 8.5 in CI. The composer.json already allows 8.4+.
5. Error Handling
5.1 Cache Exceptions Propagate as 500 Errors [MEDIUM PRIORITY] ✅ Addressed
Current state: Fixed. Cache operations in AcceptListener::onKernelRequest() and AllowListener::onKernelRequest() are now wrapped in try/catch blocks that catch Psr\Cache\InvalidArgumentException. On a cache error, the listener logs the error at error level and returns without setting a response — causing the request to fall through to the next listener, which will eventually present the login page. This is a "fail closed" approach: if the cache is unavailable, the user is not authenticated.
Note: LoginManager::checkToken() and BackupCodeManager::verifyAndConsume() still declare @throws InvalidArgumentException. These are called from LoginListener, which does not catch the exception. A cache failure during login verification would still result in a 500 error. This is a lower-priority concern since login failures already result in a 401 response path.
5.2 No Global Exception Handling for Auth Flow [LOW PRIORITY] ⬜ Open
Current state: There is no ExceptionListener or ErrorController configured. Symfony's default error handling will produce a generic error page for uncaught exceptions. In dev mode (APP_DEBUG=1), this shows a full stack trace.
Recommendation: Add a simple exception listener that catches exceptions from the auth flow and returns a clean 401 or 503 response with the login page or error template. Alternatively, configure framework.error_controller to use a custom controller that renders the error template.
Why: For an auth gateway, every response should be intentional. A raw Symfony error page (even in production mode) doesn't match the styled login/error pages and could leak information about the internal architecture.
5.3 Kernel::terminate() Not Using try/finally [MEDIUM PRIORITY] ✅ Addressed
Current state: Fixed. Kernel::terminate() now wraps $this->persistCache->persist() in a try block with a finally block that calls parent::terminate(). This ensures that the Symfony kernel termination always runs, even if the cache persistence throws an exception.
6. Frontend
6.1 document.write() in Login Script [MEDIUM PRIORITY] ✅ Addressed
Current state: Fixed. The document.open(); document.write(html); document.close(); pattern in _script.html.twig has been replaced with document.documentElement.innerHTML = html;. This avoids the deprecated document.write() call and is compatible with the Content-Security-Policy now set by SecurityHeadersListener.
6.2 No Input Sanitization in Username Echo [LOW PRIORITY] ⬜ Open
Current state: In login.html.twig, the username is echoed back into the input value: value="{{ username }}". The username comes from the sanitized makeCacheKey() output, which restricts to [A-Za-z0-9_.], so HTML injection is not possible with the current sanitization. Twig's auto-escaping is also on by default.
Recommendation: Add Twig's escape filter explicitly for defense-in-depth: value="{{ username|e('html_attr') }}". Also consider whether the message variable in <p id="preauth-message">{{ message|default }}</p> could ever contain user input.
Why: While the current sanitization prevents XSS, relying on makeCacheKey() for HTML safety is an implicit coupling between cache key logic and output safety. If makeCacheKey() were ever relaxed to allow more characters, the template would become vulnerable. Twig auto-escaping handles HTML body context, but html_attr escaping is more appropriate for attribute contexts.
7. Configuration
7.1 No Validation of Environment Variables [LOW PRIORITY] ✅ Addressed
Current state: Fixed. Environment variables in config/services.yaml now use Symfony's env var processors for type casting:
app.cookie_ttl: '%env(int:COOKIE_TTL)%'app.subdomain_redirect: '%env(bool:SUBDOMAIN_REDIRECT)%'app.ip_ttl: '%env(int:IP_TTL)%'app.teapot: '%env(bool:TEAPOT)%'
This ensures invalid values fail fast at container compilation rather than at runtime with a confusing type error. The rate_limiter.yaml already used %env(int:...)% — this pattern is now applied consistently.
7.2 APP_SECRET Not Used Meaningfully [LOW PRIORITY] ✅ Addressed (Documented)
Current state: APP_SECRET is configured in framework.yaml and is required by Symfony. Preauth doesn't use Symfony sessions (now explicitly disabled), CSRF tokens, or signed cookies — the main uses of APP_SECRET. The README now documents that APP_SECRET is a Symfony requirement and that session cookies are random ULIDs looked up in cache, not signed tokens. The hardcoded APP_SECRET in bin/franken.sh has also been removed.
8. Documentation
8.1 Missing Security Model Documentation [MEDIUM PRIORITY] ✅ Addressed
Current state: Addressed. The README now includes a comprehensive "Security Model" section under "Architecture" that covers:
- Cookie security attributes (
__Host-prefix,SameSite=Strict,Secure,HttpOnly) - Nonce system (15-byte random, single-use, 120s TTL)
- TOTP verification window (±1 period / ±30 seconds)
- Backup codes (case-insensitive, single-use, alphanumeric)
- Rate limiting (per-IP, compound sliding window, cannot be disabled)
- Security headers (CSP, X-Frame-Options, X-Content-Type-Options, Referrer-Policy, HSTS)
A dedicated docs/SECURITY.md with the full threat model and Remote-User guidance (see item 1.2) could still be valuable as a standalone document.
8.2 Missing CHANGELOG.md [MEDIUM PRIORITY] ✅ Addressed
Current state: Fixed. A CHANGELOG.md has been created following the Keep a Changelog format, with full version history from v0.0.1 through the unreleased v1.0 changes. The version history was previously inline in the README.
8.3 Missing CONTRIBUTING.md [LOW PRIORITY] ✅ Addressed
Current state: Fixed. A CONTRIBUTING.md has been created with development setup instructions, code style guidelines, testing requirements, PR process, commit message conventions, and architecture overview.
8.4 Stale Branch References in ROADMAP [LOW PRIORITY] ✅ Addressed
Current state: Fixed. The ROADMAP's branch status table has been updated to reflect that all feature branches have been pruned and development uses a feature-branch + PR workflow into main. Completed security review items are now checked off, and the TOTP leeway description has been updated from "10-second leeway" to "±1 period leeway (±30 seconds)".
9. CI & Workflows
9.1 Inconsistent Tag Format [MEDIUM PRIORITY] ✅ Addressed
Current state: Fixed. Git tags are now standardized on the v prefix (e.g., v1.0.0 instead of 1.0.0). The Docker workflow (docker.yaml) now triggers on v*.*.* tag patterns and includes a step to extract the version number without the v prefix for the Docker image tag. The existing un-prefixed tags (0.7.0 through 0.10.0) remain in the repository but all future releases will use the v prefix.
9.2 Stale develop Branch in CI Triggers [LOW PRIORITY] ✅ Addressed
Current state: Fixed. The tests.yaml and develop.yaml workflows no longer reference the develop branch, which has been pruned. CI now triggers on main only (for push) and main only (for pull requests).
9.3 publish.yaml Fails on Re-run [LOW PRIORITY] ✅ Addressed
Current state: Fixed. The GitHub sync workflow (publish.yaml) now uses git remote add ... 2>/dev/null || git remote set-url ... instead of bare git remote add, which would fail if the remote already existed from a previous run.
What's Done Well
- Listener-based architecture is a good fit for this use case — each listener has a single responsibility, and the priority chain creates a clear request processing pipeline.
- Cookie security is excellent:
__Host-prefix,Secure,HttpOnly,SameSite=Strict, and a separate non-prefixed cookie for domain-scoped central auth. Cookie name and domain selection logic is now shared viaCookieNameTrait::sessionCookieName()andsessionCookieDomain(). - Nonce-based replay protection with single-use, TTL-limited nonces and collision retry is well-designed. The nonce also serves as CSRF protection for the POST form path.
- Rate limiting with compound sliding windows (burst + sustained) and the humorous teapot option is practical and well-implemented.
- Test suite is exemplary: 100% coverage, good use of test helpers, functional tests that exercise the full kernel, and edge cases like ULID collisions and nonce reuse.
- Dual-layer cache (APCu + filesystem with change tracking) is a clever solution for persistence without a database.
- Backup code system with single-use enforcement, case-insensitivity, and audit trail (keeping consumed codes with
falsevalue) is well thought out. Backup code values are no longer logged. - Interfaces (
LoginInterface,DomainInterface,BackupCodeInterface) enable clean mocking in tests. All now havedeclare(strict_types=1). - FrankenPHP worker mode via the Caddyfile and Dockerfile is a modern, performant serving strategy.
- Security headers are now set on all responses via
SecurityHeadersListener. - Error handling in cache-dependent listeners now fails closed (denies access on cache errors) rather than propagating 500 errors.
Originally prepared as a design review. Updated to reflect the state of the fix/v1.0-must-fix branch.