From 098f2301d386880e7eb0ecb76505d022f554bfb1 Mon Sep 17 00:00:00 2001
From: Cristian Magherusan-Stanciu <cristi.magherusan@gmail.com>
Date: Wed, 3 Jun 2026 23:44:48 +0200
Subject: [PATCH] sec(api/auth): return opaque 401 in login catch-all instead
 of err.Error() (closes #937)

The login() catch-all error path was forwarding raw service errors verbatim
via NewClientError(401, err.Error()), potentially exposing internal account
state (e.g. lock status, user IDs, timestamps) to unauthenticated callers.

Replace it with the opaque hardcoded string "invalid credentials" so all
non-sentinel auth failures (wrong password, account not found, locked, etc.)
collapse to the same response, regardless of what the service returns.

The typed-sentinel arms above (ErrMFARequired -> "mfa_required",
ErrInvalidMFACode -> "invalid_mfa_code") are unchanged.

Adds TestHandler_login_OpaqueError_HidesInternalMessage as a regression test
that pins the service returning a distinctive internal message and asserts
the 401 body is "invalid credentials" with no internal detail leaking through.

Salvaged from closed #886.
---
 internal/api/handler_auth.go      |  5 ++++-
 internal/api/handler_auth_test.go | 37 +++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/internal/api/handler_auth.go b/internal/api/handler_auth.go
index bf924201a..048165eb5 100644
--- a/internal/api/handler_auth.go
+++ b/internal/api/handler_auth.go
@@ -50,7 +50,10 @@ func (h *Handler) login(ctx context.Context, req *events.LambdaFunctionURLReques
 		if errors.Is(err, auth.ErrInvalidMFACode) {
 			return nil, NewClientError(401, "invalid_mfa_code")
 		}
-		return nil, NewClientError(401, err.Error())
+		// All other auth failures (wrong password, account not found, locked, etc.)
+		// collapse to a single opaque 401. Never forward err.Error() verbatim - it
+		// may reveal internal account state.
+		return nil, NewClientError(401, "invalid credentials")
 	}
 
 	return response, nil
diff --git a/internal/api/handler_auth_test.go b/internal/api/handler_auth_test.go
index 1ebea1c26..1fd3d93bf 100644
--- a/internal/api/handler_auth_test.go
+++ b/internal/api/handler_auth_test.go
@@ -101,6 +101,43 @@ func TestHandler_login_AuthError(t *testing.T) {
 	assert.Equal(t, 401, ce.code)
 }
 
+// TestHandler_login_OpaqueError_HidesInternalMessage is a regression test for
+// issue #937: the login catch-all 401 must return the opaque "invalid
+// credentials" string regardless of what the auth service returns. Internal
+// error messages (e.g. "internal: user 42 locked since 2025-01-01") must never
+// be forwarded to the client because they reveal account state.
+func TestHandler_login_OpaqueError_HidesInternalMessage(t *testing.T) {
+	ctx := context.Background()
+	mockAuth := new(MockAuthService)
+	t.Cleanup(func() { mockAuth.AssertExpectations(t) })
+
+	// A service error containing distinctive internal detail that must never
+	// reach the client.
+	internalMsg := "internal: user 42 locked since 2025-01-01T00:00:00Z"
+	mockAuth.On("Login", ctx, mock.Anything).Return((*LoginResponse)(nil), errors.New(internalMsg)).Once()
+
+	handler := &Handler{auth: mockAuth}
+
+	encodedPassword := base64.StdEncoding.EncodeToString([]byte("anypassword"))
+	req := &events.LambdaFunctionURLRequest{
+		Body: `{"email": "victim@example.com", "password": "` + encodedPassword + `"}`,
+	}
+
+	result, err := handler.login(ctx, req)
+	assert.Nil(t, result)
+	require.Error(t, err)
+
+	ce, ok := IsClientError(err)
+	require.True(t, ok, "login failure must be a ClientError")
+	assert.Equal(t, 401, ce.code)
+	// The opaque message must be returned.
+	assert.Equal(t, "invalid credentials", ce.message)
+	// The internal message must not leak to the client.
+	assert.NotContains(t, ce.message, "internal:", "internal error detail must not be forwarded to the client")
+	assert.NotContains(t, ce.message, "user 42", "user identifier must not appear in the 401 response")
+	assert.NotContains(t, ce.message, "locked", "account state must not appear in the 401 response")
+}
+
 func TestHandler_logout_Success(t *testing.T) {
 	ctx := context.Background()
 	mockAuth := new(MockAuthService)