Concurrent first-time logins for a brand-new user can throw an uncaught exception #96

New Issue

2026-09-01T00:13:37-04:00

jared commented

2026-09-01 00:13:37 -04:00

Severity: Low/Medium

models/UserModel.php::syncUserFromAuthelia() (~lines 78-117) does a plain check-then-insert (SELECT ... WHERE username = ? → if 0 rows, INSERT) with no transaction and no INSERT ... ON DUPLICATE KEY. users.username has a UNIQUE KEY constraint, and mysqli throws mysqli_sql_exception on constraint violation under PHP 8.1+'s default report mode (per an existing comment in helpers/Database.php).

Impact: Two simultaneous first-visit requests for the same brand-new user (e.g. two browser tabs opened right after completing SSO login) race: the second INSERT throws a duplicate-key exception, uncaught anywhere in syncUserFromAuthelia(), AuthMiddleware::authenticate(), or index.php (no try/catch around AuthMiddleware usage, and ErrorHandler::init() isn't wired in here either, consistent with the previously-filed finding that ErrorHandler is barely used app-wide). Result: an uncaught fatal error on the losing request, with stack-trace exposure depending on the server's global display_errors setting — on the authentication path itself. Self-heals on page reload since the winning row now exists.

Fix: Wrap the insert in INSERT ... ON DUPLICATE KEY UPDATE (updating the sync'd fields), or catch the duplicate-key exception and re-fetch the now-existing row.

**Severity:** Low/Medium `models/UserModel.php::syncUserFromAuthelia()` (~lines 78-117) does a plain check-then-insert (`SELECT ... WHERE username = ?` → if 0 rows, `INSERT`) with no transaction and no `INSERT ... ON DUPLICATE KEY`. `users.username` has a `UNIQUE KEY` constraint, and mysqli throws `mysqli_sql_exception` on constraint violation under PHP 8.1+'s default report mode (per an existing comment in `helpers/Database.php`). **Impact:** Two simultaneous first-visit requests for the same brand-new user (e.g. two browser tabs opened right after completing SSO login) race: the second `INSERT` throws a duplicate-key exception, uncaught anywhere in `syncUserFromAuthelia()`, `AuthMiddleware::authenticate()`, or `index.php` (no try/catch around `AuthMiddleware` usage, and `ErrorHandler::init()` isn't wired in here either, consistent with the previously-filed finding that `ErrorHandler` is barely used app-wide). Result: an uncaught fatal error on the losing request, with stack-trace exposure depending on the server's global `display_errors` setting — on the authentication path itself. Self-heals on page reload since the winning row now exists. **Fix:** Wrap the insert in `INSERT ... ON DUPLICATE KEY UPDATE` (updating the sync'd fields), or catch the duplicate-key exception and re-fetch the now-existing row.