Merge SEC-02: normalize lockout response

# Conflicts:
#	internal/grpcserver/grpcserver_test.go
#	internal/server/server_test.go
This commit is contained in:
2026-03-13 01:05:56 -07:00
6 changed files with 228 additions and 4 deletions

View File

@@ -60,7 +60,9 @@ func (a *authServiceServer) Login(ctx context.Context, req *mciasv1.LoginRequest
if locked {
_, _ = auth.VerifyPassword("dummy", auth.DummyHash())
a.s.db.WriteAuditEvent(model.EventLoginFail, &acct.ID, nil, ip, `{"reason":"account_locked"}`) //nolint:errcheck
return nil, status.Error(codes.ResourceExhausted, "account temporarily locked")
// Security: return the same Unauthenticated / "invalid credentials" as wrong-password
// to prevent user-enumeration via lockout differentiation (SEC-02).
return nil, status.Error(codes.Unauthenticated, "invalid credentials")
}
ok, err := auth.VerifyPassword(req.Password, acct.PasswordHash)

View File

@@ -776,3 +776,71 @@ func TestGRPCClientIP_NoPeer(t *testing.T) {
t.Errorf("grpcClientIP(no peer) = %q, want %q", got, "")
}
}
// TestLoginLockedAccountReturnsUnauthenticated verifies that a locked-out
// account gets the same gRPC Unauthenticated / "invalid credentials" as a
// wrong-password attempt, preventing user-enumeration via lockout
// differentiation (SEC-02).
func TestLoginLockedAccountReturnsUnauthenticated(t *testing.T) {
e := newTestEnv(t)
acct := e.createHumanAccount(t, "lockgrpc")
// Lower the lockout threshold so we don't need 10 failures.
origThreshold := db.LockoutThreshold
db.LockoutThreshold = 3
t.Cleanup(func() { db.LockoutThreshold = origThreshold })
for range db.LockoutThreshold {
if err := e.db.RecordLoginFailure(acct.ID); err != nil {
t.Fatalf("RecordLoginFailure: %v", err)
}
}
locked, err := e.db.IsLockedOut(acct.ID)
if err != nil {
t.Fatalf("IsLockedOut: %v", err)
}
if !locked {
t.Fatal("expected account to be locked out after threshold failures")
}
cl := mciasv1.NewAuthServiceClient(e.conn)
// Attempt login on the locked account.
_, lockedErr := cl.Login(context.Background(), &mciasv1.LoginRequest{
Username: "lockgrpc",
Password: "testpass123",
})
if lockedErr == nil {
t.Fatal("Login on locked account: expected error, got nil")
}
// Attempt login with wrong password for comparison.
_, wrongErr := cl.Login(context.Background(), &mciasv1.LoginRequest{
Username: "lockgrpc",
Password: "wrongpassword",
})
if wrongErr == nil {
t.Fatal("Login with wrong password: expected error, got nil")
}
lockedSt, _ := status.FromError(lockedErr)
wrongSt, _ := status.FromError(wrongErr)
// Both must return Unauthenticated, not ResourceExhausted.
if lockedSt.Code() != codes.Unauthenticated {
t.Errorf("locked: got code %v, want Unauthenticated", lockedSt.Code())
}
if wrongSt.Code() != codes.Unauthenticated {
t.Errorf("wrong password: got code %v, want Unauthenticated", wrongSt.Code())
}
// Messages must be identical.
if lockedSt.Message() != wrongSt.Message() {
t.Errorf("locked message %q differs from wrong-password message %q",
lockedSt.Message(), wrongSt.Message())
}
if lockedSt.Message() != "invalid credentials" {
t.Errorf("locked message = %q, want %q", lockedSt.Message(), "invalid credentials")
}
}

View File

@@ -248,7 +248,9 @@ func (s *Server) handleLogin(w http.ResponseWriter, r *http.Request) {
if locked {
_, _ = auth.VerifyPassword("dummy", auth.DummyHash())
s.writeAudit(r, model.EventLoginFail, &acct.ID, nil, `{"reason":"account_locked"}`)
middleware.WriteError(w, http.StatusTooManyRequests, "account temporarily locked", "account_locked")
// Security: return the same 401 "invalid credentials" as wrong-password
// to prevent user-enumeration via lockout differentiation (SEC-02).
middleware.WriteError(w, http.StatusUnauthorized, "invalid credentials", "unauthorized")
return
}
@@ -1034,7 +1036,9 @@ func (s *Server) handleChangePassword(w http.ResponseWriter, r *http.Request) {
}
if locked {
s.writeAudit(r, model.EventPasswordChanged, &acct.ID, &acct.ID, `{"result":"locked"}`)
middleware.WriteError(w, http.StatusTooManyRequests, "account temporarily locked", "account_locked")
// Security: return the same 401 as wrong-password to prevent
// user-enumeration via lockout differentiation (SEC-02).
middleware.WriteError(w, http.StatusUnauthorized, "invalid credentials", "unauthorized")
return
}

View File

@@ -656,3 +656,76 @@ func TestSecurityHeadersOnAPIResponses(t *testing.T) {
}
})
}
// TestLoginLockedAccountReturns401 verifies that a locked-out account gets the
// same HTTP 401 / "invalid credentials" response as a wrong-password attempt,
// preventing user-enumeration via lockout differentiation (SEC-02).
func TestLoginLockedAccountReturns401(t *testing.T) {
srv, _, _, database := newTestServer(t)
acct := createTestHumanAccount(t, srv, "lockuser")
handler := srv.Handler()
// Lower the lockout threshold so we don't need 10 failures.
origThreshold := db.LockoutThreshold
db.LockoutThreshold = 3
t.Cleanup(func() { db.LockoutThreshold = origThreshold })
// Record enough failures to trigger lockout.
for range db.LockoutThreshold {
if err := database.RecordLoginFailure(acct.ID); err != nil {
t.Fatalf("RecordLoginFailure: %v", err)
}
}
// Confirm the account is locked.
locked, err := database.IsLockedOut(acct.ID)
if err != nil {
t.Fatalf("IsLockedOut: %v", err)
}
if !locked {
t.Fatal("expected account to be locked out after threshold failures")
}
// Attempt login on the locked account.
lockedRR := doRequest(t, handler, "POST", "/v1/auth/login", map[string]string{
"username": "lockuser",
"password": "testpass123",
}, "")
// Also attempt login with a wrong password (not locked) for comparison.
wrongRR := doRequest(t, handler, "POST", "/v1/auth/login", map[string]string{
"username": "lockuser",
"password": "wrongpassword",
}, "")
// Both must return 401, not 429.
if lockedRR.Code != http.StatusUnauthorized {
t.Errorf("locked account: status = %d, want %d", lockedRR.Code, http.StatusUnauthorized)
}
if wrongRR.Code != http.StatusUnauthorized {
t.Errorf("wrong password: status = %d, want %d", wrongRR.Code, http.StatusUnauthorized)
}
// Parse the JSON bodies and compare — they must be identical.
type errResp struct {
Error string `json:"error"`
Code string `json:"code"`
}
var lockedBody, wrongBody errResp
if err := json.Unmarshal(lockedRR.Body.Bytes(), &lockedBody); err != nil {
t.Fatalf("unmarshal locked body: %v", err)
}
if err := json.Unmarshal(wrongRR.Body.Bytes(), &wrongBody); err != nil {
t.Fatalf("unmarshal wrong body: %v", err)
}
if lockedBody != wrongBody {
t.Errorf("locked response %+v differs from wrong-password response %+v", lockedBody, wrongBody)
}
if lockedBody.Code != "unauthorized" {
t.Errorf("locked response code = %q, want %q", lockedBody.Code, "unauthorized")
}
if lockedBody.Error != "invalid credentials" {
t.Errorf("locked response error = %q, want %q", lockedBody.Error, "invalid credentials")
}
}

View File

@@ -80,7 +80,9 @@ func (u *UIServer) handleLoginPost(w http.ResponseWriter, r *http.Request) {
if locked {
_, _ = auth.VerifyPassword("dummy", u.dummyHash())
u.writeAudit(r, model.EventLoginFail, &acct.ID, nil, `{"reason":"account_locked"}`)
u.render(w, "login", LoginData{Error: "account temporarily locked, please try again later"})
// Security: return the same "invalid credentials" as wrong-password
// to prevent user-enumeration via lockout differentiation (SEC-02).
u.render(w, "login", LoginData{Error: "invalid credentials"})
return
}

View File

@@ -13,6 +13,7 @@ import (
"testing"
"time"
"git.wntrmute.dev/kyle/mcias/internal/auth"
"git.wntrmute.dev/kyle/mcias/internal/config"
"git.wntrmute.dev/kyle/mcias/internal/db"
"git.wntrmute.dev/kyle/mcias/internal/model"
@@ -556,3 +557,77 @@ func TestAccountDetailShowsPGCredsSection(t *testing.T) {
t.Error("human account detail page must not include pgcreds-section")
}
}
// TestLoginLockedAccountShowsInvalidCredentials verifies that a locked-out
// account gets the same "invalid credentials" error as a wrong-password
// attempt in the UI login form, preventing user-enumeration via lockout
// differentiation (SEC-02).
func TestLoginLockedAccountShowsInvalidCredentials(t *testing.T) {
u := newTestUIServer(t)
// Create an account with a known password.
hash, err := auth.HashPassword("testpass123", auth.ArgonParams{Time: 3, Memory: 65536, Threads: 4})
if err != nil {
t.Fatalf("hash password: %v", err)
}
acct, err := u.db.CreateAccount("lockuiuser", model.AccountTypeHuman, hash)
if err != nil {
t.Fatalf("CreateAccount: %v", err)
}
// Lower the lockout threshold so we don't need 10 failures.
origThreshold := db.LockoutThreshold
db.LockoutThreshold = 3
t.Cleanup(func() { db.LockoutThreshold = origThreshold })
for range db.LockoutThreshold {
if err := u.db.RecordLoginFailure(acct.ID); err != nil {
t.Fatalf("RecordLoginFailure: %v", err)
}
}
locked, err := u.db.IsLockedOut(acct.ID)
if err != nil {
t.Fatalf("IsLockedOut: %v", err)
}
if !locked {
t.Fatal("expected account to be locked out after threshold failures")
}
mux := http.NewServeMux()
u.Register(mux)
// POST login for the locked account.
form := url.Values{}
form.Set("username", "lockuiuser")
form.Set("password", "testpass123")
req := httptest.NewRequest(http.MethodPost, "/login", strings.NewReader(form.Encode()))
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
lockedRR := httptest.NewRecorder()
mux.ServeHTTP(lockedRR, req)
// POST login with wrong password for comparison.
form2 := url.Values{}
form2.Set("username", "lockuiuser")
form2.Set("password", "wrongpassword")
req2 := httptest.NewRequest(http.MethodPost, "/login", strings.NewReader(form2.Encode()))
req2.Header.Set("Content-Type", "application/x-www-form-urlencoded")
wrongRR := httptest.NewRecorder()
mux.ServeHTTP(wrongRR, req2)
lockedBody := lockedRR.Body.String()
wrongBody := wrongRR.Body.String()
// Neither response should mention "locked" or "try again".
if strings.Contains(lockedBody, "locked") || strings.Contains(lockedBody, "try again") {
t.Error("locked account response leaks lockout state")
}
// Both must contain "invalid credentials".
if !strings.Contains(lockedBody, "invalid credentials") {
t.Error("locked account response does not contain 'invalid credentials'")
}
if !strings.Contains(wrongBody, "invalid credentials") {
t.Error("wrong password response does not contain 'invalid credentials'")
}
}