sec(auth): collapse MFA-not-configured into generic auth failure (closes #391) (#830)

When MFA is enabled but the secret is missing (data-integrity anomaly),
returning a distinct "MFA is enabled but not configured" error reveals that
the supplied password was correct. Collapse the branch to the same generic
"Check your email address and password and try again" message used for wrong
passwords, and log the anomaly internally (by user ID, no PII) so operators
can detect it without relying on the API response.

Remove the dead substring match from mapMFAServiceError in handler_auth.go.
Add a sub-test that asserts both paths return an identical response body.

Author: cristim

internal/api/handler_auth.go

@@ -519,8 +519,7 @@ func mapMFAServiceError(err error) error {
 		strings.Contains(msg, "MFA code or recovery code required"),
 		strings.Contains(msg, "no MFA enrollment in progress"),
 		strings.Contains(msg, "MFA enrollment expired"),
-		strings.Contains(msg, "MFA is not enabled"),
-		strings.Contains(msg, "MFA is enabled but not configured"):
+		strings.Contains(msg, "MFA is not enabled"):
 		return NewClientError(400, msg)
 	case strings.Contains(msg, "authentication failed"):
 		return NewClientError(401, msg)

internal/auth/service.go

@@ -203,7 +203,11 @@ func (s *Service) verifyPasswordAndMFA(ctx context.Context, user *User, req Logi
 			return ErrMFARequired
 		}
 		if user.MFASecret == "" {
-			return fmt.Errorf("MFA is enabled but not configured")
+			// Data integrity anomaly: MFA is flagged enabled but has no secret.
+			// Log internally for operator visibility without leaking internal state
+			// to the caller -- a distinct message would confirm the password was correct.
+			logging.Errorf("MFA enabled but secret missing for user %s -- possible data integrity issue", user.ID)
+			return fmt.Errorf("Check your email address and password and try again")
 		}
 		// verifyTOTP is panic-safe: a malformed secret causes generateTOTP to return ""
 		// (base32 decode error), resulting in a comparison miss rather than a panic.

internal/auth/service_test.go

@@ -442,11 +442,12 @@ func TestLogin_WithMFA_MissingCode(t *testing.T) {
 	assert.ErrorIs(t, err, ErrMFARequired)
 }
 
+// TestLogin_WithMFA_NoSecret verifies that a correct password against an account
+// where MFA is enabled but the secret is missing returns the same generic error
+// as a wrong password -- the response must not reveal that the password was correct
+// (closes #391).
 func TestLogin_WithMFA_NoSecret(t *testing.T) {
 	ctx := context.Background()
-	mockStore := new(MockStore)
-	mockEmail := new(MockEmailSender)
-	service := createTestService(mockStore, mockEmail)
 
 	s := newTestService()
 	hash, _ := s.hashPassword("SecurePass@123")
@@ -455,24 +456,57 @@ func TestLogin_WithMFA_NoSecret(t *testing.T) {
 		ID:           "user-123",
 		Email:        "mfa@example.com",
 		PasswordHash: hash,
-		Salt:         "", // Not used anymore
+		Salt:         "",
 		Active:       true,
 		MFAEnabled:   true,
-		MFASecret:    "", // No secret configured
+		MFASecret:    "", // Data-integrity anomaly: MFA enabled but no secret stored
 	}
 
-	mockStore.On("GetUserByEmail", ctx, "mfa@example.com").Return(user, nil)
+	const genericMsg = "Check your email address and password and try again"
 
-	req := LoginRequest{
-		Email:    "mfa@example.com",
-		Password: "SecurePass@123",
-		MFACode:  "123456",
-	}
+	// Right password, no MFA secret -- must return the generic error.
+	t.Run("correct password no MFA secret returns generic error", func(t *testing.T) {
+		mockStore := new(MockStore)
+		mockEmail := new(MockEmailSender)
+		service := createTestService(mockStore, mockEmail)
+		t.Cleanup(func() { mockStore.AssertExpectations(t) })
 
-	resp, err := service.Login(ctx, req)
-	assert.Error(t, err)
-	assert.Nil(t, resp)
-	assert.Contains(t, err.Error(), "MFA is enabled but not configured")
+		mockStore.On("GetUserByEmail", ctx, "mfa@example.com").Return(user, nil).Once()
+
+		req := LoginRequest{
+			Email:    "mfa@example.com",
+			Password: "SecurePass@123",
+			MFACode:  "123456",
+		}
+
+		resp, err := service.Login(ctx, req)
+		assert.Error(t, err)
+		assert.Nil(t, resp)
+		assert.Contains(t, err.Error(), genericMsg)
+	})
+
+	// Wrong password -- must return the identical generic error so the two cases
+	// are indistinguishable to the caller.
+	t.Run("wrong password returns same generic error", func(t *testing.T) {
+		mockStore := new(MockStore)
+		mockEmail := new(MockEmailSender)
+		service := createTestService(mockStore, mockEmail)
+		t.Cleanup(func() { mockStore.AssertExpectations(t) })
+
+		mockStore.On("GetUserByEmail", ctx, "mfa@example.com").Return(user, nil).Once()
+		mockStore.On("UpdateUser", ctx, mock.AnythingOfType("*auth.User")).Return(nil).Maybe()
+
+		req := LoginRequest{
+			Email:    "mfa@example.com",
+			Password: "wrongpassword",
+			MFACode:  "123456",
+		}
+
+		resp, err := service.Login(ctx, req)
+		assert.Error(t, err)
+		assert.Nil(t, resp)
+		assert.Contains(t, err.Error(), genericMsg)
+	})
 }
 
 // Test UpdateUserProfile