fix(auth): canonicalize username for per-user login lockout keys

The per-username brute-force lockout keyed on the raw request username while GetUserByUsername matches COLLATE NOCASE, so case variants (admin/Admin/ADMIN) each got an independent 9-attempt bucket, multiplying allowed guesses per account. Lowercase the username before building the login_user_fail/login_user_lock keys so all casings share one bucket. (Security scan F1)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
J3vb
2026-07-23 11:59:13 +02:00
co-authored by Claude Opus 4.8
parent 801b7eb0db
commit 6bc5938ddf
2 changed files with 40 additions and 2 deletions
+6 -2
View File
@@ -287,7 +287,11 @@ func handleLogin(database *db.DB, limiter *auth.RateLimiter, partialStore *auth.
}
// BUG-110: Also check per-username lockout to prevent distributed brute force.
userLockKey := "login_user_lock:" + req.Username
// F1: canonicalize the username the same way GetUserByUsername does (COLLATE
// NOCASE) before keying the lockout, so case variants of one account
// (admin/Admin/ADMIN) share a single bucket instead of each getting its own.
unameKey := strings.ToLower(req.Username)
userLockKey := "login_user_lock:" + unameKey
if limiter.IsLockedOut(userLockKey) {
writeJSON(w, http.StatusTooManyRequests, errorResponse{
Error: "RATE_LIMITED",
@@ -316,7 +320,7 @@ func handleLogin(database *db.DB, limiter *auth.RateLimiter, partialStore *auth.
}
failKey := "login_fail:" + ip
userFailKey := "login_user_fail:" + req.Username
userFailKey := "login_user_fail:" + unameKey
// Always run the password check — with an empty hash when the user does
// not exist. auth.CheckPassword performs a dummy bcrypt comparison for an
// empty hash, so bcrypt executes on every path and response time stays
+34
View File
@@ -409,6 +409,40 @@ func TestLogin_UsernameLockoutBlocksCorrectPasswordFromFreshIP(t *testing.T) {
}
}
// TestLogin_UsernameLockoutIgnoresCasing locks F1: the per-username lockout key
// must be case-folded so it matches the DB's COLLATE NOCASE username lookup.
// Otherwise an attacker splits the 9-attempt lockout budget across case variants
// of one account (admin, Admin, ADMIN, …), all of which authenticate the same row.
func TestLogin_UsernameLockoutIgnoresCasing(t *testing.T) {
database := newAuthTestDB(t)
limiter := auth.NewRateLimiter()
router := buildAuthRouter(database, limiter)
hash, _ := auth.HashPassword("correctPass1")
_, _ = database.CreateUser("casehunt", hash, 4)
// Trip the per-username lockout using the lowercase spelling, from many IPs
// so the per-IP limiter is never the binding cap.
for i := 0; i < 10; i++ {
rr := postJSONFromIP(t, router, "/api/v1/auth/login", map[string]string{
"username": "casehunt",
"password": "wrongpassword",
}, fmt.Sprintf("198.51.100.%d", i+1))
if rr.Code != http.StatusUnauthorized {
t.Fatalf("setup attempt %d status = %d, want 401; body = %s", i+1, rr.Code, rr.Body.String())
}
}
// A different casing of the SAME account must land in the same lockout bucket.
rr := postJSONFromIP(t, router, "/api/v1/auth/login", map[string]string{
"username": "CASEHUNT",
"password": "wrongpassword",
}, "198.51.100.250")
if rr.Code != http.StatusTooManyRequests {
t.Fatalf("case-variant username bypassed the per-username lockout: status = %d, want 429; body = %s", rr.Code, rr.Body.String())
}
}
func TestLogin_SuccessResetsUsernameFailureCounter(t *testing.T) {
database := newAuthTestDB(t)
limiter := auth.NewRateLimiter()