From 3f445d153290d64398eaf11ea9566354b602da96 Mon Sep 17 00:00:00 2001 From: Janardan Singh Kavia Date: Fri, 29 May 2026 15:42:25 -0400 Subject: [PATCH] fix: add failed login logging with IP tracking and other relevant details (#13409) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(security): add failed login logging with IP tracking and comprehensive tests * fix: remove PII from login logs and fix structlog format * docs: agent improvements (#13269) * docs: add structured output for agents * docs: add structured output note * docs: clarify both agent outputs and add release notes * docs: when langflow sends events to the playground and chat history * fix(playground): expand session checkbox click target to full row height (#13349) * fix(playground): expand session checkbox click target to full row height The selectable-row checkbox wrapper was ``w-4 h-4`` (16x16 px) but the row is ``h-8`` (32 px). The 8 px band above and the 8 px band below the visible icon were dead zones that bubbled to the row's ``toggleVisibility`` and navigated to the session instead of toggling selection — exactly the accidental-navigation UX the bug ticket describes. Grow the wrapper to ``w-4 h-8`` so the click target captures the full row height. Width stays narrow so the text alignment is unchanged; the icon itself stays ``h-4 w-4``, centered via the existing ``flex items-center justify-center``. Visual position is identical. Adds ``__tests__/session-selector-checkbox-click-target.test.tsx`` (3 cases): wrapper className guard (h-8 + w-4, not h-4), click on the wrapper outside the icon triggers selection without bubbling to toggleVisibility, and click on the inner icon still toggles selection. * test(playground): drop checkbox-click-target test file (not needed per review) * [autofix.ci] apply automated fixes * test: align session-selector checkbox assertions with full-height click target --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> * fix: improve trace search functionality (#13383) * improve trace search functionality * [autofix.ci] apply automated fixes --------- Co-authored-by: Olayinka Adelakun Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> * test: fix test_login_logs_real_output_format to use mocking - Updated test to use patch() instead of capsys for consistency - Verifies structlog kwargs format (not nested extra dict) - All 7 login logging tests now passing --------- Co-authored-by: Janardan S Kavia Co-authored-by: Mendon Kissling <59585235+mendonk@users.noreply.github.com> Co-authored-by: keval shah Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: olayinkaadelakun Co-authored-by: Olayinka Adelakun --- src/backend/base/langflow/api/v1/login.py | 3 +- .../base/langflow/services/auth/service.py | 44 +++- src/backend/tests/unit/test_login_logging.py | 205 ++++++++++++++++++ src/lfx/src/lfx/services/auth/base.py | 13 +- src/lfx/src/lfx/services/auth/service.py | 1 + 5 files changed, 262 insertions(+), 4 deletions(-) create mode 100644 src/backend/tests/unit/test_login_logging.py diff --git a/src/backend/base/langflow/api/v1/login.py b/src/backend/base/langflow/api/v1/login.py index ee8737fec5..17f2105a0f 100644 --- a/src/backend/base/langflow/api/v1/login.py +++ b/src/backend/base/langflow/api/v1/login.py @@ -26,6 +26,7 @@ class SessionResponse(BaseModel): @router.post("/login", response_model=Token, include_in_schema=False) async def login_to_get_access_token( + request: Request, response: Response, form_data: Annotated[OAuth2PasswordRequestForm, Depends()], db: DbSession, @@ -33,7 +34,7 @@ async def login_to_get_access_token( auth_settings = get_settings_service().auth_settings try: auth = get_auth_service() - user = await auth.authenticate_user(form_data.username, form_data.password, db) + user = await auth.authenticate_user(form_data.username, form_data.password, db, request) except Exception as exc: if isinstance(exc, HTTPException): raise diff --git a/src/backend/base/langflow/services/auth/service.py b/src/backend/base/langflow/services/auth/service.py index 3097e746c2..df32a81a40 100644 --- a/src/backend/base/langflow/services/auth/service.py +++ b/src/backend/base/langflow/services/auth/service.py @@ -1,5 +1,6 @@ from __future__ import annotations +import hashlib import warnings from collections.abc import Coroutine from datetime import datetime, timedelta, timezone @@ -662,18 +663,57 @@ class AuthService(BaseAuthService): detail="Invalid refresh token", ) from e - async def authenticate_user(self, username: str, password: str, db: AsyncSession) -> User | None: + async def authenticate_user( + self, username: str, password: str, db: AsyncSession, request: Request | None = None + ) -> User | None: user = await get_user_by_username(db, username) if not user: + if request and request.client: + # Hash username for correlation without exposing PII + username_hash = hashlib.sha256(username.lower().encode()).hexdigest()[:16] + logger.warning( + "Login failed: user not found", + auth_event="login_failed", + reason="user_not_found", + username_hash=username_hash, + client_ip=request.client.host, + ) return None if not user.is_active: + if request and request.client: + logger.warning( + "Login failed: inactive user", + auth_event="login_failed", + reason="user_inactive", + auth_id=str(user.id), + client_ip=request.client.host, + ) if not user.last_login_at: raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail="Waiting for approval") raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="Inactive user") - return user if self.verify_password(password, user.password) else None + if not self.verify_password(password, user.password): + if request and request.client: + logger.warning( + "Login failed: incorrect password", + auth_event="login_failed", + reason="incorrect_password", + auth_id=str(user.id), + client_ip=request.client.host, + ) + return None + + # Successful login + if request and request.client: + logger.info( + "Login successful", + auth_event="login_success", + auth_id=str(user.id), + client_ip=request.client.host, + ) + return user def _get_fernet(self) -> Fernet: from langflow.services.auth.utils import ensure_fernet_key diff --git a/src/backend/tests/unit/test_login_logging.py b/src/backend/tests/unit/test_login_logging.py new file mode 100644 index 0000000000..3b0bc7d813 --- /dev/null +++ b/src/backend/tests/unit/test_login_logging.py @@ -0,0 +1,205 @@ +"""Tests for login logging functionality.""" + +import hashlib +from unittest.mock import patch + +import pytest +from httpx import AsyncClient +from langflow.services.database.models.user.model import User +from langflow.services.deps import session_scope +from sqlmodel import select + + +@pytest.mark.asyncio +async def test_failed_login_nonexistent_user_logs(client: AsyncClient): + """Test that failed login for non-existent user is logged with hashed username.""" + with patch("langflow.services.auth.service.logger") as mock_logger: + response = await client.post( + "/api/v1/login", + data={"username": "nonexistent_user", "password": "wrong_password"}, # pragma: allowlist secret + ) + + assert response.status_code == 401 + assert response.json()["detail"] == "Incorrect username or password" + + # Verify warning was logged with structlog kwargs + mock_logger.warning.assert_called_once() + call_args = mock_logger.warning.call_args + assert call_args[0][0] == "Login failed: user not found" + # Structlog uses kwargs, not extra dict + assert call_args[1]["auth_event"] == "login_failed" + assert call_args[1]["reason"] == "user_not_found" + # Username should be hashed, not plain text + assert "username_hash" in call_args[1] + assert call_args[1]["username_hash"] == hashlib.sha256(b"nonexistent_user").hexdigest()[:16] + assert "client_ip" in call_args[1] + # Should NOT contain plain username + assert "username" not in call_args[1] + + +@pytest.mark.asyncio +async def test_failed_login_wrong_password_logs(client: AsyncClient, active_user): + """Test that failed login with wrong password is logged.""" + with patch("langflow.services.auth.service.logger") as mock_logger: + response = await client.post( + "/api/v1/login", + data={"username": active_user.username, "password": "wrong_password"}, # pragma: allowlist secret + ) + + assert response.status_code == 401 + assert response.json()["detail"] == "Incorrect username or password" + + # Verify warning was logged with structlog kwargs + mock_logger.warning.assert_called_once() + call_args = mock_logger.warning.call_args + assert call_args[0][0] == "Login failed: incorrect password" + assert call_args[1]["auth_event"] == "login_failed" + assert call_args[1]["reason"] == "incorrect_password" + assert "auth_id" in call_args[1] + assert "client_ip" in call_args[1] + + +@pytest.mark.asyncio +async def test_failed_login_inactive_user_logs(client: AsyncClient): + """Test that failed login for inactive user is logged.""" + # Create an inactive user with last_login_at set (so it returns 401, not 400) + async with session_scope() as session: + from datetime import datetime, timezone + + from langflow.services.deps import get_auth_service + + inactive_user = User( + username="inactiveuser", + password=get_auth_service().get_password_hash("testpassword"), # pragma: allowlist secret + is_active=False, + is_superuser=False, + last_login_at=datetime.now(timezone.utc), # Set to avoid "Waiting for approval" + ) + session.add(inactive_user) + await session.commit() + await session.refresh(inactive_user) + user_id = str(inactive_user.id) + + with patch("langflow.services.auth.service.logger") as mock_logger: + response = await client.post( + "/api/v1/login", + data={"username": "inactiveuser", "password": "testpassword"}, # pragma: allowlist secret + ) + + assert response.status_code == 401 + assert response.json()["detail"] == "Inactive user" + + # Verify warning was logged with structlog kwargs + mock_logger.warning.assert_called_once() + call_args = mock_logger.warning.call_args + assert call_args[0][0] == "Login failed: inactive user" + assert call_args[1]["auth_event"] == "login_failed" + assert call_args[1]["reason"] == "user_inactive" + assert call_args[1]["auth_id"] == user_id + assert "client_ip" in call_args[1] + + # Cleanup + async with session_scope() as session: + stmt = select(User).where(User.username == "inactiveuser") + user = (await session.exec(stmt)).first() + if user: + await session.delete(user) + await session.commit() + + +@pytest.mark.asyncio +async def test_successful_login_logs(client: AsyncClient, active_user): + """Test that successful login is logged.""" + with patch("langflow.services.auth.service.logger") as mock_logger: + response = await client.post( + "/api/v1/login", + data={"username": active_user.username, "password": "testpassword"}, # pragma: allowlist secret + ) + + assert response.status_code == 200 + assert "access_token" in response.json() + + # Verify info log was called with structlog kwargs + mock_logger.info.assert_called_once() + call_args = mock_logger.info.call_args + assert call_args[0][0] == "Login successful" + assert call_args[1]["auth_event"] == "login_success" + assert "auth_id" in call_args[1] + assert "client_ip" in call_args[1] + + +@pytest.mark.asyncio +async def test_login_logs_contain_ip_address(client: AsyncClient): + """Test that login logs contain IP address.""" + with patch("langflow.services.auth.service.logger") as mock_logger: + response = await client.post( + "/api/v1/login", + data={"username": "nonexistent", "password": "wrong"}, # pragma: allowlist secret + ) + + assert response.status_code == 401 + + # Verify IP was captured in structlog kwargs + call_args = mock_logger.warning.call_args + assert "client_ip" in call_args[1] + # IP should be a string (testclient uses "testclient" as default) + assert isinstance(call_args[1]["client_ip"], str) + + +@pytest.mark.asyncio +async def test_login_logs_no_pii(client: AsyncClient): + """Test that login logs do not contain PII (email, full name, plain username).""" + with patch("langflow.services.auth.service.logger") as mock_logger: + response = await client.post( + "/api/v1/login", + data={"username": "test_user", "password": "wrong"}, # pragma: allowlist secret + ) + + assert response.status_code == 401 + + # Verify no PII in logs (structlog kwargs) + call_args = mock_logger.warning.call_args + kwargs = call_args[1] + + # Should NOT contain these PII fields + assert "email" not in kwargs + assert "first_name" not in kwargs + assert "last_name" not in kwargs + assert "full_name" not in kwargs + assert "phone" not in kwargs + assert "address" not in kwargs + assert "username" not in kwargs # Plain username is PII + + # Should contain safe identifiers + assert "username_hash" in kwargs or "auth_id" in kwargs + assert "auth_event" in kwargs + assert "reason" in kwargs + + +@pytest.mark.asyncio +async def test_login_logs_real_output_format(client: AsyncClient): + """Test that login logs use structlog kwargs format (not nested extra dict).""" + with patch("langflow.services.auth.service.logger") as mock_logger: + response = await client.post( + "/api/v1/login", + data={"username": "nonexistent_user", "password": "wrong"}, # pragma: allowlist secret + ) + + assert response.status_code == 401 + + # Verify logger was called with kwargs (flat format), not extra dict + mock_logger.warning.assert_called_once() + call_args = mock_logger.warning.call_args + + # Verify it's using kwargs format (call_args[1] contains kwargs) + assert "auth_event" in call_args[1] + assert "reason" in call_args[1] + assert "username_hash" in call_args[1] + assert "client_ip" in call_args[1] + + # Verify it's NOT using the old extra dict format + assert "extra" not in call_args[1] + + # Verify the actual values + assert call_args[1]["auth_event"] == "login_failed" + assert call_args[1]["reason"] == "user_not_found" diff --git a/src/lfx/src/lfx/services/auth/base.py b/src/lfx/src/lfx/services/auth/base.py index 7af62f70d0..1c234e1baa 100644 --- a/src/lfx/src/lfx/services/auth/base.py +++ b/src/lfx/src/lfx/services/auth/base.py @@ -94,8 +94,19 @@ class BaseAuthService(Service, abc.ABC): username: str, password: str, db: Any, + request: Any | None = None, ) -> Any | None: - """Authenticate with username and password. Returns user or None.""" + """Authenticate with username and password. Returns user or None. + + Args: + username: Username to authenticate + password: Password to verify + db: Database session + request: Optional HTTP request for logging IP address + + Returns: + User object if authentication succeeds, None otherwise + """ # ------------------------------------------------------------------------- # User validation diff --git a/src/lfx/src/lfx/services/auth/service.py b/src/lfx/src/lfx/services/auth/service.py index f1481a4ff0..065cf2a470 100644 --- a/src/lfx/src/lfx/services/auth/service.py +++ b/src/lfx/src/lfx/services/auth/service.py @@ -72,6 +72,7 @@ class AuthService(BaseAuthService): username: str, password: str, db: Any, + request: Any | None = None, ) -> Any | None: logger.debug("Auth: authenticate_user (no-op)") return None