From 7c1c1b61ccf5fd1815de8486932734564adfecab Mon Sep 17 00:00:00 2001 From: Jared Vititoe Date: Tue, 8 Sep 2026 11:41:30 -0400 Subject: [PATCH] Fix race in first-time login user creation (#96) syncUserFromAuthelia() did a plain check-then-insert with no transaction, so two simultaneous first-visit requests for the same brand-new user (e.g. two tabs opened right after SSO login) could race: the second INSERT hits users.username's UNIQUE KEY, which mysqli throws on (uncaught, PHP 8.1+ default report mode) rather than returning false. Switched to INSERT ... ON DUPLICATE KEY UPDATE followed by a re-fetch by username, so the losing request updates the winner's row instead of throwing. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X --- models/UserModel.php | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/models/UserModel.php b/models/UserModel.php index 0664d94..1524349 100644 --- a/models/UserModel.php +++ b/models/UserModel.php @@ -98,19 +98,30 @@ class UserModel $user['groups'] = $groups; $user['is_admin'] = $isAdmin; } else { - // Create new user + // Create new user. Uses INSERT ... ON DUPLICATE KEY UPDATE (rather than + // a plain INSERT) so two concurrent first-visit requests for the same + // brand-new username can't race: the losing request updates the row the + // winner just created instead of throwing an uncaught duplicate-key + // exception (users.username has a UNIQUE KEY, and mysqli throws on + // constraint violation under PHP 8.1+'s default report mode). $insertStmt = $this->conn->prepare( - "INSERT INTO users (username, display_name, email, `groups`, is_admin, last_login) VALUES (?, ?, ?, ?, ?, NOW())" + "INSERT INTO users (username, display_name, email, `groups`, is_admin, last_login) + VALUES (?, ?, ?, ?, ?, NOW()) + ON DUPLICATE KEY UPDATE + display_name = VALUES(display_name), + email = VALUES(email), + `groups` = VALUES(groups), + is_admin = VALUES(is_admin), + last_login = NOW()" ); $insertStmt->bind_param("ssssi", $username, $displayName, $email, $groups, $isAdmin); $insertStmt->execute(); - - $userId = $this->conn->insert_id; $insertStmt->close(); - // Get the newly created user - $stmt = $this->conn->prepare("SELECT * FROM users WHERE user_id = ?"); - $stmt->bind_param("i", $userId); + // Re-fetch by username — works whether this request won the insert or + // lost the race and only updated the winner's row. + $stmt = $this->conn->prepare("SELECT * FROM users WHERE username = ?"); + $stmt->bind_param("s", $username); $stmt->execute(); $result = $stmt->get_result(); $user = $result->fetch_assoc();