huntingclubroster/MULTI-TENANT-READINESS-2026-10-20.md

# Multi-tenant production readiness review — October 20, 2026

Follow-up: the two MFA defects below have been repaired. See
[MFA lifecycle repairs](MFA-REPAIRS-2026-10-20.md) for the implementation and
regression coverage. The findings below preserve the original review evidence.

Reviewed the current authentication/MFA lifecycle, tenant permissions and routes,
invitations, roster/ledger paging and exports, shared budgets, deployment tooling
and dependency state. No application code, server configuration or development
database records were changed by this review. Reproductions used disposable
local databases and generated accounts, with external email disabled.

## Confirmed defects, recommended repair order

### 1. P1 — MFA replacement temporarily permits password-only factor takeover

Location: `classes/MfaService.php:182`–`200`, particularly the deletion at
lines 188–191; enrollment checks at lines 107–141.

After verifying the current password and an existing authenticator/recovery code,
replacement immediately deletes the current factor and all recovery codes. The
replacement authenticator has not yet been configured or proved. The account
therefore returns to the initial-enrollment path for any new password login.

Reproduced through the real HTTP handlers with two independent cookie jars:

1. A temporary Super System Administrator enrolls MFA normally.
2. Its legitimate session starts authenticator replacement with valid credentials.
3. Before replacement is confirmed, a separate session supplies only the account
   password. It has neither the old authenticator nor an unused recovery code.
4. That session receives a new setup secret, enrolls its own authenticator and
   successfully opens the Super System Administrator account-management page.

This requires knowledge of the password and overlap with an authorized
replacement window. It does not bypass a stable enrolled factor outside that
window. Its impact is nevertheless platform-wide for the Super account.

Fix: retain the existing factor while a replacement is pending. Bind replacement
authorization and the new secret to the authorized session and account version,
then atomically swap the factor, regenerate recovery codes and revoke other
sessions only after confirmation of the new authenticator. New password-only
sessions must continue to face the existing MFA challenge. Abandonment, expiry,
concurrent replacements and password resets must not reopen initial enrollment.

### 2. P2 — Wrong password consumes a valid recovery code or authenticator step

Location: `classes/MfaService.php:177`–`181`; consumption in `verify()` at
lines 80–105.

The page computes `$passwordOk`, but invokes the mutating factor verifier before
testing that boolean. The verifier commits recovery-code deletion/counter
advancement and its audit before the page rejects the incorrect password.

Reproduced: an account has ten recovery codes; submitting one with an incorrect
password returns HTTP 403 and leaves nine codes. Submitting that same recovery
code with the correct password also returns HTTP 403. The valid recovery code
has been lost despite the failed authentication.

Fix: validate both credentials before committing factor consumption. Preserve
rate limiting for incorrect passwords as well as incorrect codes, and keep
consumption/auditing atomic. Verify rollback and concurrent use explicitly.

## Security recommendation before public launch

MFA enrollment, recovery use and replacement write audit records, but do not
notify the account owner through an independent channel. Add security notices
for factor changes/recovery, with visible or monitored delivery failures and
without putting secrets or recovery codes in email. This is a recommendation,
not another reproduced authorization bypass. It follows
[OWASP's factor-change notification guidance](https://cheatsheetseries.owasp.org/cheatsheets/Multifactor_Authentication_Cheat_Sheet.html).

## Production acceptance and operations still required

The previous development fixes are present: administrator MFA/QR setup, absolute
and idle timeouts, bounded lists/exports, and shared club budgets. The recent
server follow-up confirmed Apache key access and private-path denial on
ositsdevels. Development HTTP and its approved database connection are not new
production defects.

Before public onboarding:

- Preserve all required new/untracked classes, migrations, tests, assets and
  dependency changes in a reviewed, versioned release; run CI on that exact
  release. A current workspace build does not prove a clean checkout is complete.
- Run acceptance on the actual production HTTPS host: proxy trust, cookies and
  headers, private paths, session/log permissions, and injected production secrets.
  The development key-file override is deliberately unavailable in Production;
  the production MFA key and a protected recovery copy must be provisioned. If
  migrating existing enrolled accounts, preserve their key or perform an explicit
  factor migration rather than silently changing it.
- Exercise complete tenant, MFA, invitation, reset and financial workflows with
  the actual restricted production database account. Verify remote production
  database TLS and separate migration/backup credentials.
- Verify real invitation, verification and reset inbox delivery, trusted links,
  sender authentication and monitored delivery failures.
- Verify scheduled backups, cleanup and health checks, alert routing, log
  rotation and storage monitoring. Test off-host restore of both SQL and MFA
  key material. Document and exercise recovery of a single club without rolling
  back unrelated clubs.
- Establish expected club sizes/concurrency and measure sustained workloads.
  Bounded rows and request budgets improve resource control but do not establish
  production throughput or response-time targets.

These are requirements to verify on the selected production infrastructure,
not claims that externally configured jobs or controls are absent.

## Evidence and limits

- Both MFA findings reproduced against actual service/HTTP behavior using
  independent sessions and disposable data.
- No additional cross-club authorization bypass was demonstrated in this review;
  this does not replace a full penetration test or independent security review.
- `composer audit --locked` reported no security vulnerability advisories.
- Did not rerun the entire regression suite, load-test production, send real
  email, or modify real accounts. Existing happy-path replacement tests do not
  cover the competing password-only session reproduced here.
- Private reproduction helper: `.tools/review-mfa-replacement.py`.
- Removed the disposable databases and stopped their loopback server after review.