From 0ca412cf50767857425a86476d8fc509a8f46d5e Mon Sep 17 00:00:00 2001 From: Paul Date: Sun, 29 Mar 2026 22:27:56 +0100 Subject: [PATCH 1/4] fix: release DB connection before Argon2 verification in authUser Split authUser into three phases so the connection is only held during actual DB work, not during CPU-bound Argon2 hashing: 1. Fetch user record into local variables (~1-5ms DB hold) 2. Verify password with Argon2 (no connection held, ~100-200ms) 3. Re-acquire connection briefly for userBadLoginReset if needed Previously the entire method ran inside a single try-with-resources block, holding a pool connection for ~200ms per login attempt. Under 20 concurrent logins this exhausted the pool and blocked unrelated endpoints (challenge lookups, scoreboards, module status). Auth decision flow and side effects are identical to previous implementation; only connection scope changes. No schema changes or stored procedure modifications required. Closes #819 Made-with: Cursor --- src/main/java/dbProcs/Getter.java | 215 ++++++++++++------------------ 1 file changed, 82 insertions(+), 133 deletions(-) diff --git a/src/main/java/dbProcs/Getter.java b/src/main/java/dbProcs/Getter.java index efec4e032..2bb00dc70 100644 --- a/src/main/java/dbProcs/Getter.java +++ b/src/main/java/dbProcs/Getter.java @@ -71,162 +71,111 @@ public class Getter { * authentication process. */ public static String[] authUser(String ApplicationRoot, String userName, String password) { - String[] result = null; log.debug("$$$ Getter.authUser $$$"); - log.debug("userName = " + userName); + // Phase 1: Fetch user record (short DB hold, ~1-5ms) + String userId; + String dbUserName; + String dbHash; + String userRole; + int badLoginCount; + boolean tempPassword; + String classId; + Timestamp suspendedUntil; + String loginType; + boolean tempUsername; boolean userFound = false; - boolean userVerified = false; - - try (Connection conn = Database.getCoreConnection(ApplicationRoot)) { - // See if user Exists - PreparedStatement prestmt; - CallableStatement callstmt; - try { - prestmt = + try (Connection conn = Database.getCoreConnection(ApplicationRoot); + PreparedStatement prestmt = conn.prepareStatement( "SELECT userId, userName, userPass, userRole, badLoginCount, tempPassword, classId," - + " suspendedUntil, loginType, tempUsername FROM `users` WHERE userName = ?"); - } catch (SQLException e) { - log.fatal("Could create call statement: " + e.toString()); - throw new RuntimeException(e); - } - - log.debug("Gathering results from query"); - ResultSet userResult; - try { - prestmt.setString(1, userName); - userResult = prestmt.executeQuery(); - } catch (SQLException e) { - log.fatal("Could not execute db query: " + e.toString()); - throw new RuntimeException(e); - } - - log.debug("Opening Result Set from query"); - - try { + + " suspendedUntil, loginType, tempUsername FROM `users` WHERE userName = ?")) { + prestmt.setString(1, userName); + try (ResultSet userResult = prestmt.executeQuery()) { if (userResult.next()) { - log.debug( - "User Found"); // User found if a row is in the database, this line will not work if - // the - // result - // set is empty + log.debug("User Found"); + userId = userResult.getString(1); + dbUserName = userResult.getString(2); + dbHash = userResult.getString(3); + userRole = userResult.getString(4); + badLoginCount = userResult.getInt(5); + tempPassword = userResult.getBoolean(6); + classId = userResult.getString(7); + suspendedUntil = userResult.getTimestamp(8); + loginType = userResult.getString(9); + tempUsername = userResult.getBoolean(10); userFound = true; } else { log.debug("User did not exist"); - userFound = false; + log.debug("$$$ End authUser $$$"); + return null; } - } catch (SQLException e) { - log.debug("User did not exist"); - userFound = false; } + } catch (SQLException e) { + log.fatal("authUser failed: " + e.toString()); + throw new RuntimeException(e); + } + // Connection released — all user data extracted into local variables - if (userFound) { - // Authenticate User - Argon2 argon2 = Argon2Factory.create(); - - log.debug("Getting password hash"); - String dbHash; - try { - dbHash = userResult.getString(3); - log.debug("Verifying hash"); - - userVerified = argon2.verify(dbHash, password.toCharArray()); - - } catch (SQLException e) { - log.fatal("Could not retrieve password hash from db: " + e.toString()); - result = null; - userVerified = false; - throw new RuntimeException(e); - // TODO: We should throw a checked exception here instead - } - - if (userVerified) { - // Hash matches - log.debug("Hash matches"); - - result = new String[6]; - - int badLoginCount; - String loginType = new String(); - - Timestamp suspendedUntil; + // Phase 2: Verify password (CPU-bound Argon2, no DB connection held) + log.debug("Verifying hash"); + Argon2 argon2 = Argon2Factory.create(); + boolean userVerified = argon2.verify(dbHash, password.toCharArray()); - try { - result[0] = userResult.getString(1); - result[1] = userResult.getString(2); // userName - result[2] = userResult.getString(4); // role - badLoginCount = userResult.getInt(5); - result[3] = Boolean.toString(userResult.getBoolean(6)); - result[4] = userResult.getString(7); // classId - suspendedUntil = userResult.getTimestamp(8); - loginType = userResult.getString(9); - result[5] = Boolean.toString(userResult.getBoolean(10)); - } catch (SQLException e) { + if (!userVerified) { + log.debug("Hash did not match, authentication failed"); + log.debug("$$$ End authUser $$$"); + return null; + } - log.fatal("Could not retrieve auth data from db: " + e.toString()); - throw new RuntimeException(e); - } + // Phase 3: Post-verification checks and DB updates (short DB hold if needed) + log.debug("Hash matches"); - if (!loginType.equals("login")) { - // Login type must be "login" and not "saml" if password login is to be allowed - log.debug("User is SSO user, can't login with password!"); - result = null; - return result; - } + if (!loginType.equals("login")) { + log.debug("User is SSO user, can't login with password!"); + return null; + } - // Get current system time - Timestamp currentTime = new Timestamp(System.currentTimeMillis()); + Timestamp currentTime = new Timestamp(System.currentTimeMillis()); + if (suspendedUntil.after(currentTime)) { + return null; + } - if (suspendedUntil.after(currentTime)) { - // User is suspended - result = null; - return result; - } + if (!dbUserName.equalsIgnoreCase(userName)) { + log.fatal( + "User Name used (" + + userName + + ") and User Name retrieved (" + + dbUserName + + ") were not the Same. Nulling Result"); + return null; + } - if (!result[1].equalsIgnoreCase( - userName)) // If somehow this functionality has been compromised to sign - // in as - // other users, this will limit the expoitability. But the method is - // sql injection safe, so it should be ok - { - log.fatal( - "User Name used (" - + userName - + ") and User Name retrieved (" - + result[1] - + ") were not the Same. Nulling Result"); - result = null; - } else { - log.debug("User '" + userName + "' has logged in"); - // Before finishing, check if user had a badlogin history, if so, Clear it - if (badLoginCount > 0) { - log.debug("Clearing Bad Login History"); - try { - callstmt = conn.prepareCall("call userBadLoginReset(?)"); - callstmt.setString(1, result[0]); - callstmt.execute(); - } catch (SQLException e) { - log.fatal("Could not reset bad login count: " + e.toString()); - throw new RuntimeException(e); - } + log.debug("User '" + userName + "' has logged in"); - log.debug("userBadLoginReset executed!"); - } - } - return result; - } else { - // Hash did not match - log.debug("Hash did not match, authentication failed"); - } + if (badLoginCount > 0) { + log.debug("Clearing Bad Login History"); + try (Connection conn = Database.getCoreConnection(ApplicationRoot); + CallableStatement callstmt = conn.prepareCall("call userBadLoginReset(?)")) { + callstmt.setString(1, userId); + callstmt.execute(); + log.debug("userBadLoginReset executed!"); + } catch (SQLException e) { + log.fatal("Could not reset bad login count: " + e.toString()); + throw new RuntimeException(e); } - - } catch (SQLException e) { - log.fatal("authUser failed: " + e.toString()); - throw new RuntimeException(e); } + + String[] result = new String[6]; + result[0] = userId; + result[1] = dbUserName; + result[2] = userRole; + result[3] = Boolean.toString(tempPassword); + result[4] = classId; + result[5] = Boolean.toString(tempUsername); + log.debug("$$$ End authUser $$$"); return result; } From a2c2e08ec0564a6af46fce25f72862e76e067d85 Mon Sep 17 00:00:00 2001 From: Paul Date: Mon, 30 Mar 2026 09:51:28 +0100 Subject: [PATCH 2/4] fix: null-safe loginType check, remove unused variable - Use "login".equals(loginType) to prevent NPE if loginType is null - Remove unused userFound variable left over from refactor Made-with: Cursor --- src/main/java/dbProcs/Getter.java | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/main/java/dbProcs/Getter.java b/src/main/java/dbProcs/Getter.java index 2bb00dc70..5c0096827 100644 --- a/src/main/java/dbProcs/Getter.java +++ b/src/main/java/dbProcs/Getter.java @@ -85,8 +85,6 @@ public static String[] authUser(String ApplicationRoot, String userName, String Timestamp suspendedUntil; String loginType; boolean tempUsername; - boolean userFound = false; - try (Connection conn = Database.getCoreConnection(ApplicationRoot); PreparedStatement prestmt = conn.prepareStatement( @@ -106,7 +104,6 @@ public static String[] authUser(String ApplicationRoot, String userName, String suspendedUntil = userResult.getTimestamp(8); loginType = userResult.getString(9); tempUsername = userResult.getBoolean(10); - userFound = true; } else { log.debug("User did not exist"); log.debug("$$$ End authUser $$$"); @@ -133,7 +130,7 @@ public static String[] authUser(String ApplicationRoot, String userName, String // Phase 3: Post-verification checks and DB updates (short DB hold if needed) log.debug("Hash matches"); - if (!loginType.equals("login")) { + if (!"login".equals(loginType)) { log.debug("User is SSO user, can't login with password!"); return null; } From 14d571e4d1368ec905878724d23947fa956318e3 Mon Sep 17 00:00:00 2001 From: Paul Date: Tue, 31 Mar 2026 22:07:41 +0100 Subject: [PATCH 3/4] fix: check suspension and login type before Argon2 verification Move loginType and suspendedUntil checks before the expensive Argon2 hash so suspended and SSO users are rejected cheaply. Also adds a null guard on suspendedUntil to prevent NPE when the column is NULL. --- src/main/java/dbProcs/Getter.java | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/src/main/java/dbProcs/Getter.java b/src/main/java/dbProcs/Getter.java index 5c0096827..86f8799cf 100644 --- a/src/main/java/dbProcs/Getter.java +++ b/src/main/java/dbProcs/Getter.java @@ -116,6 +116,17 @@ public static String[] authUser(String ApplicationRoot, String userName, String } // Connection released — all user data extracted into local variables + // Fail-fast: reject suspended and SSO users before expensive Argon2 work + if (!"login".equals(loginType)) { + log.debug("User is SSO user, can't login with password!"); + return null; + } + + Timestamp currentTime = new Timestamp(System.currentTimeMillis()); + if (suspendedUntil != null && suspendedUntil.after(currentTime)) { + return null; + } + // Phase 2: Verify password (CPU-bound Argon2, no DB connection held) log.debug("Verifying hash"); Argon2 argon2 = Argon2Factory.create(); @@ -127,19 +138,9 @@ public static String[] authUser(String ApplicationRoot, String userName, String return null; } - // Phase 3: Post-verification checks and DB updates (short DB hold if needed) + // Phase 3: Post-verification DB updates (short DB hold if needed) log.debug("Hash matches"); - if (!"login".equals(loginType)) { - log.debug("User is SSO user, can't login with password!"); - return null; - } - - Timestamp currentTime = new Timestamp(System.currentTimeMillis()); - if (suspendedUntil.after(currentTime)) { - return null; - } - if (!dbUserName.equalsIgnoreCase(userName)) { log.fatal( "User Name used (" From e8b45d960eb2b1a5583060b4a050a90030381fbb Mon Sep 17 00:00:00 2001 From: Paul Date: Tue, 31 Mar 2026 22:20:55 +0100 Subject: [PATCH 4/4] test: add integration tests for authUser fail-fast checks Add GetterAuthIT with JUnit 5 tests verifying that: - suspended users are rejected before Argon2 verification - SSO users cannot authenticate via password login These cover the fail-fast checks moved before Argon2 in the previous commit. --- src/it/java/dbProcs/GetterAuthIT.java | 92 +++++++++++++++++++++++++++ 1 file changed, 92 insertions(+) create mode 100644 src/it/java/dbProcs/GetterAuthIT.java diff --git a/src/it/java/dbProcs/GetterAuthIT.java b/src/it/java/dbProcs/GetterAuthIT.java new file mode 100644 index 000000000..cae74dac0 --- /dev/null +++ b/src/it/java/dbProcs/GetterAuthIT.java @@ -0,0 +1,92 @@ +package dbProcs; + +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assumptions.assumeTrue; + +import java.io.IOException; +import java.sql.SQLException; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import testUtils.TestProperties; + +/** + * Integration tests for Getter.authUser() fail-fast checks: suspended users and SSO users should be + * rejected before Argon2 verification. + */ +public class GetterAuthIT { + + private static final Logger log = LogManager.getLogger(GetterAuthIT.class); + private static boolean databaseAvailable = false; + private static final String applicationRoot = ""; + + @BeforeAll + public static void setup() throws IOException, SQLException { + TestProperties.setTestPropertiesFileDirectory(log); + TestProperties.createMysqlResource(); + TestProperties.executeSql(log); + try { + ConnectionPool.initialize(); + Getter.getClassCount(applicationRoot); + databaseAvailable = ConnectionPool.isInitialized(); + } catch (Exception e) { + log.warn("Database not available for GetterAuthIT: {}", e.getMessage()); + databaseAvailable = false; + } + } + + @AfterAll + public static void cleanup() { + ConnectionPool.shutdown(); + } + + private void requireDatabase() { + assumeTrue(databaseAvailable, "Database not available"); + } + + @Test + public void suspendedUserIsRejected() throws SQLException { + requireDatabase(); + String userName = "authSuspendedUser"; + + // Create user and verify they can authenticate + Setter.userCreate( + applicationRoot, null, userName, userName, "player", userName + "@test.com", false); + String[] user = Getter.authUser(applicationRoot, userName, userName); + assertNotNull(user, "User should be able to authenticate before suspension"); + + String userId = user[0]; + + // Suspend and verify rejection + assertTrue(Setter.suspendUser(applicationRoot, userId, 10), "Could not suspend user"); + assertNull( + Getter.authUser(applicationRoot, userName, userName), + "Suspended user should not be able to authenticate"); + + // Unsuspend and verify access restored + assertTrue(Setter.unSuspendUser(applicationRoot, userId), "Could not unsuspend user"); + assertNotNull( + Getter.authUser(applicationRoot, userName, userName), + "User should be able to authenticate after unsuspension"); + } + + @Test + public void ssoUserIsRejectedFromPasswordLogin() { + requireDatabase(); + String userName = "authRejectSSOUser Lastname"; + String ssoName = "authrejectssouser@example.com"; + + // Create an SSO user + String[] user = Getter.authUserSSO(applicationRoot, null, userName, ssoName, "player"); + assertNotNull(user, "Could not create SSO user"); + + // SSO user should not be able to authenticate with password login + assertNull( + Getter.authUser(applicationRoot, userName, "anyPassword"), + "SSO user should not be able to authenticate via password login"); + } +}