6.6 KiB
Multi-tenant production readiness review — October 20, 2026
Follow-up: the two MFA defects below have been repaired. See MFA lifecycle repairs 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:
- A temporary Super System Administrator enrolls MFA normally.
- Its legitimate session starts authenticator replacement with valid credentials.
- Before replacement is confirmed, a separate session supplies only the account password. It has neither the old authenticator nor an unused recovery code.
- 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.
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 --lockedreported 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.