From 8a2b95ce68d9d5a67cd0b596db0b813611fb6bb1 Mon Sep 17 00:00:00 2001 From: Jage9 Date: Sat, 28 Feb 2026 04:53:11 -0500 Subject: [PATCH] Reapply "Return friendly generic auth messages for login and resume failures" This reverts commit 9c5011a8fddd51d9f7c81f166a0eb9cd93983693. --- server/app/server.py | 9 +++- server/tests/test_server_message_handling.py | 55 +++++++++++++++++++- 2 files changed, 62 insertions(+), 2 deletions(-) diff --git a/server/app/server.py b/server/app/server.py index 4ecf9fc..50ffebe 100644 --- a/server/app/server.py +++ b/server/app/server.py @@ -125,6 +125,8 @@ AUTH_SESSION_COOKIE_MAX_AGE_SECONDS = 14 * 24 * 60 * 60 AUTH_SESSION_COOKIE_SET_PATH = "/auth/session/set" AUTH_SESSION_COOKIE_CLEAR_PATH = "/auth/session/clear" AUTH_SESSION_COOKIE_CLIENT_HEADER = "X-Chgrid-Auth-Client" +AUTH_LOGIN_FAILURE_MESSAGE = "We couldn't log you in. Check your details and try again." +AUTH_RESUME_FAILURE_MESSAGE = "We couldn't restore your session. Please log in again." ADMIN_MENU_ACTION_DEFINITIONS: tuple[dict[str, str], ...] = ( {"id": "manage_roles", "label": "Role management", "permission": "role.manage"}, {"id": "change_user_role", "label": "Change user role", "permission": "user.change_role"}, @@ -1540,6 +1542,11 @@ class SignalingServer: if isinstance(packet, (AuthLoginPacket, AuthRegisterPacket, AuthResumePacket)): self._record_auth_failure(client, packet) await self._sleep_auth_failure_jitter() + response_message = str(exc) + if isinstance(packet, AuthLoginPacket): + response_message = AUTH_LOGIN_FAILURE_MESSAGE + elif isinstance(packet, AuthResumePacket): + response_message = AUTH_RESUME_FAILURE_MESSAGE LOGGER.warning( "auth failure id=%s ip=%s packet=%s reason=%s", client.id, @@ -1552,7 +1559,7 @@ class SignalingServer: AuthResultPacket( type="auth_result", ok=False, - message=str(exc), + message=response_message, authPolicy=self._auth_policy(), ), ) diff --git a/server/tests/test_server_message_handling.py b/server/tests/test_server_message_handling.py index fd5ea03..d7fe2bd 100644 --- a/server/tests/test_server_message_handling.py +++ b/server/tests/test_server_message_handling.py @@ -10,7 +10,8 @@ import pytest from websockets.asyncio.server import ServerConnection from app.client import ClientConnection -from app.server import SignalingServer +from app.auth_service import AuthError +from app.server import AUTH_LOGIN_FAILURE_MESSAGE, AUTH_RESUME_FAILURE_MESSAGE, SignalingServer def _fake_ws() -> ServerConnection: @@ -248,6 +249,58 @@ async def test_auth_rate_limit_blocks_before_hash(monkeypatch: pytest.MonkeyPatc assert "too many" in send_payloads[-1].message.lower() +@pytest.mark.asyncio +async def test_auth_login_failure_message_is_generic(monkeypatch: pytest.MonkeyPatch) -> None: + server = SignalingServer("127.0.0.1", 8765, None, None) + ws = _fake_ws() + client = ClientConnection(websocket=ws, id="u1", nickname="tester") + send_payloads: list[object] = [] + + async def fake_send(websocket: ServerConnection, packet: object) -> None: + send_payloads.append(packet) + + monkeypatch.setattr(server, "_send", fake_send) + monkeypatch.setattr(server, "_sleep_auth_failure_jitter", lambda: asyncio.sleep(0)) + + def fake_login(_username: str, _password: str): + raise AuthError("Account is disabled.") + + monkeypatch.setattr(server.auth_service, "login", fake_login) + + await server._handle_message(client, json.dumps({"type": "auth_login", "username": "alpha", "password": "wrongpass"})) + + auth_results = [packet for packet in send_payloads if getattr(packet, "type", "") == "auth_result"] + assert auth_results + assert auth_results[-1].ok is False + assert auth_results[-1].message == AUTH_LOGIN_FAILURE_MESSAGE + + +@pytest.mark.asyncio +async def test_auth_resume_failure_message_is_generic(monkeypatch: pytest.MonkeyPatch) -> None: + server = SignalingServer("127.0.0.1", 8765, None, None) + ws = _fake_ws() + client = ClientConnection(websocket=ws, id="u1", nickname="tester") + send_payloads: list[object] = [] + + async def fake_send(websocket: ServerConnection, packet: object) -> None: + send_payloads.append(packet) + + monkeypatch.setattr(server, "_send", fake_send) + monkeypatch.setattr(server, "_sleep_auth_failure_jitter", lambda: asyncio.sleep(0)) + + def fake_resume(_token: str): + raise AuthError("Session has expired.") + + monkeypatch.setattr(server.auth_service, "resume", fake_resume) + + await server._handle_message(client, json.dumps({"type": "auth_resume", "sessionToken": "expired-token"})) + + auth_results = [packet for packet in send_payloads if getattr(packet, "type", "") == "auth_result"] + assert auth_results + assert auth_results[-1].ok is False + assert auth_results[-1].message == AUTH_RESUME_FAILURE_MESSAGE + + @pytest.mark.asyncio async def test_item_drop_rejects_out_of_bounds(monkeypatch: pytest.MonkeyPatch) -> None: server = SignalingServer("127.0.0.1", 8765, None, None, grid_size=41)