From 58112fb1913f3bdea652b7aca9b23fff207b1233 Mon Sep 17 00:00:00 2001 From: Lukas Date: Fri, 17 Apr 2026 15:33:09 +0200 Subject: [PATCH] Move auth session signing into auth_service.login --- backend/app/routers/auth.py | 10 +- backend/app/services/auth_service.py | 13 ++- backend/app/services/protocols.py | 70 ++++++------- .../tests/test_services/test_auth_service.py | 97 ++++++++++++++----- 4 files changed, 119 insertions(+), 71 deletions(-) diff --git a/backend/app/routers/auth.py b/backend/app/routers/auth.py index d2383f1..187a539 100644 --- a/backend/app/routers/auth.py +++ b/backend/app/routers/auth.py @@ -20,7 +20,6 @@ from app.dependencies import ( SettingsDep, ) from app.models.auth import LoginRequest, LoginResponse, LogoutResponse -from app.services.auth_service import sign_session_token from app.utils.constants import SESSION_COOKIE_NAME log: structlog.stdlib.BoundLogger = structlog.get_logger() @@ -59,10 +58,11 @@ async def login( HTTPException: 401 if the password is incorrect. """ try: - session = await auth_service.login( + signed_token, expires_at = await auth_service.login( db, password=body.password, session_duration_minutes=settings.session_duration_minutes, + session_secret=settings.session_secret, session_repo=session_repo, ) except ValueError as exc: @@ -71,10 +71,6 @@ async def login( detail=str(exc), ) from exc - signed_token = sign_session_token( - session.token, - settings.session_secret, - ) response.set_cookie( key=SESSION_COOKIE_NAME, value=signed_token, @@ -83,7 +79,7 @@ async def login( secure=settings.session_cookie_secure, max_age=settings.session_duration_minutes * 60, ) - return LoginResponse(token=signed_token, expires_at=session.expires_at) + return LoginResponse(token=signed_token, expires_at=expires_at) @router.post( diff --git a/backend/app/services/auth_service.py b/backend/app/services/auth_service.py index ce1e4c1..81c8ee5 100644 --- a/backend/app/services/auth_service.py +++ b/backend/app/services/auth_service.py @@ -79,17 +79,19 @@ async def login( db: aiosqlite.Connection, password: str, session_duration_minutes: int, + session_secret: str, session_repo: SessionRepository = default_session_repo, -) -> Session: - """Verify *password* and create a new session on success. +) -> tuple[str, str]: + """Verify *password*, create a new session, and sign the token. Args: db: Active aiosqlite connection. password: Plain-text password supplied by the user. session_duration_minutes: How long the new session is valid for. + session_secret: Secret used to sign the session token. Returns: - A :class:`~app.models.auth.Session` domain model for the new session. + A tuple of the signed session token and its expiry timestamp. Raises: ValueError: If the password is incorrect or no password hash is stored. @@ -111,8 +113,9 @@ async def login( session = await session_repo.create_session( db, token=token, created_at=created_iso, expires_at=expires_iso ) - log.info("bangui_login_success", token_prefix=token[:8]) - return session + signed_token = sign_session_token(session.token, session_secret) + log.info("bangui_login_success", token_prefix=session.token[:8]) + return signed_token, session.expires_at async def validate_session( diff --git a/backend/app/services/protocols.py b/backend/app/services/protocols.py index dd65795..019647f 100644 --- a/backend/app/services/protocols.py +++ b/backend/app/services/protocols.py @@ -6,41 +6,42 @@ layers depend on, without binding them to concrete module implementations. from __future__ import annotations -from collections.abc import Awaitable, Callable -from typing import Protocol, runtime_checkable +from typing import TYPE_CHECKING, Protocol, runtime_checkable -import aiosqlite -import aiohttp +if TYPE_CHECKING: + from collections.abc import Awaitable, Callable -from app.models.auth import Session -from app.models.ban import BanOrigin, JailBannedIpsResponse, TimeRange -from app.models.blocklist import ( - BlocklistSource, - ImportLogListResponse, - ImportRunResult, - ImportSourceResult, - PreviewResponse, - ScheduleConfig, - ScheduleInfo, -) -from app.models.config import ( - AddLogPathRequest, - GlobalConfigResponse, - GlobalConfigUpdate, - JailConfigListResponse, - JailConfigResponse, - JailConfigUpdate, - LogPreviewRequest, - LogPreviewResponse, - MapColorThresholdsResponse, - MapColorThresholdsUpdate, - RegexTestResponse, - Fail2BanLogResponse, - ServiceStatusResponse, -) -from app.models.geo import GeoBatchLookup, GeoEnricher, GeoInfo -from app.models.history import HistoryListResponse, IpDetailResponse -from app.models.server import ServerSettingsResponse, ServerSettingsUpdate, ServerStatus + import aiohttp + import aiosqlite + + from app.models.auth import Session + from app.models.ban import BanOrigin, JailBannedIpsResponse, TimeRange + from app.models.blocklist import ( + BlocklistSource, + ImportLogListResponse, + ImportRunResult, + ImportSourceResult, + PreviewResponse, + ScheduleConfig, + ScheduleInfo, + ) + from app.models.config import ( + AddLogPathRequest, + GlobalConfigResponse, + GlobalConfigUpdate, + JailConfigListResponse, + JailConfigResponse, + JailConfigUpdate, + LogPreviewRequest, + LogPreviewResponse, + MapColorThresholdsResponse, + MapColorThresholdsUpdate, + RegexTestResponse, + ) + from app.models.geo import GeoBatchLookup, GeoEnricher, GeoInfo + from app.models.history import HistoryListResponse, IpDetailResponse + from app.models.jail import JailDetailResponse, JailListResponse + from app.models.server import ServerSettingsResponse, ServerSettingsUpdate, ServerStatus class AuthService(Protocol): @@ -51,8 +52,9 @@ class AuthService(Protocol): db: aiosqlite.Connection, password: str, session_duration_minutes: int, + session_secret: str, session_repo: object | None = None, - ) -> Session: + ) -> tuple[str, str]: ... async def validate_session( diff --git a/backend/tests/test_services/test_auth_service.py b/backend/tests/test_services/test_auth_service.py index 8c47595..4beee78 100644 --- a/backend/tests/test_services/test_auth_service.py +++ b/backend/tests/test_services/test_auth_service.py @@ -77,37 +77,58 @@ class TestCheckPasswordAsync: class TestLogin: - async def test_login_returns_session_on_correct_password( + async def test_login_returns_signed_token_on_correct_password( self, db: aiosqlite.Connection ) -> None: - """login() returns a Session on the correct password.""" - session = await auth_service.login(db, password="correctpassword1", session_duration_minutes=60) - assert session.token - assert len(session.token) == 64 # 32 bytes → 64 hex chars - assert session.expires_at > session.created_at + """login() returns a signed token and expiry on the correct password.""" + signed_token, expires_at = await auth_service.login( + db, + password="correctpassword1", + session_duration_minutes=60, + session_secret="test-secret", + ) + assert signed_token + assert "." in signed_token + assert expires_at async def test_login_raises_on_wrong_password( self, db: aiosqlite.Connection ) -> None: """login() raises ValueError for an incorrect password.""" with pytest.raises(ValueError, match="Incorrect password"): - await auth_service.login(db, password="wrongpassword", session_duration_minutes=60) + await auth_service.login( + db, + password="wrongpassword", + session_duration_minutes=60, + session_secret="test-secret", + ) async def test_login_raises_when_no_hash_configured( self, db_no_setup: aiosqlite.Connection ) -> None: """login() raises ValueError when setup has not been run.""" with pytest.raises(ValueError, match="No password is configured"): - await auth_service.login(db_no_setup, password="any", session_duration_minutes=60) + await auth_service.login( + db_no_setup, + password="any", + session_duration_minutes=60, + session_secret="test-secret", + ) async def test_login_persists_session(self, db: aiosqlite.Connection) -> None: """login() stores the session in the database.""" from app.repositories import session_repo - session = await auth_service.login(db, password="correctpassword1", session_duration_minutes=60) - stored = await session_repo.get_session(db, session.token) + signed_token, _ = await auth_service.login( + db, + password="correctpassword1", + session_duration_minutes=60, + session_secret="test-secret", + ) + raw_token = auth_service.unwrap_session_token(signed_token, "test-secret") + stored = await session_repo.get_session(db, raw_token) assert stored is not None - assert stored.token == session.token + assert stored.token == raw_token class TestValidateSession: @@ -115,27 +136,42 @@ class TestValidateSession: self, db: aiosqlite.Connection ) -> None: """validate_session() returns the session for a valid token.""" - session = await auth_service.login(db, password="correctpassword1", session_duration_minutes=60) - validated = await auth_service.validate_session(db, session.token) - assert validated.token == session.token + signed_token, _ = await auth_service.login( + db, + password="correctpassword1", + session_duration_minutes=60, + session_secret="test-secret", + ) + raw_token = auth_service.unwrap_session_token(signed_token, "test-secret") + validated = await auth_service.validate_session(db, raw_token) + assert validated.token == raw_token async def test_validate_accepts_signed_token( self, db: aiosqlite.Connection ) -> None: """validate_session() accepts a token signed with the configured secret.""" - session = await auth_service.login(db, password="correctpassword1", session_duration_minutes=60) - signed_token = auth_service.sign_session_token(session.token, "test-secret") + signed_token, _ = await auth_service.login( + db, + password="correctpassword1", + session_duration_minutes=60, + session_secret="test-secret", + ) validated = await auth_service.validate_session( db, signed_token, session_secret="test-secret" ) - assert validated.token == session.token + raw_token = auth_service.unwrap_session_token(signed_token, "test-secret") + assert validated.token == raw_token async def test_validate_rejects_tampered_signed_token( self, db: aiosqlite.Connection ) -> None: """validate_session() rejects signed tokens with an invalid signature.""" - session = await auth_service.login(db, password="correctpassword1", session_duration_minutes=60) - signed_token = auth_service.sign_session_token(session.token, "test-secret") + signed_token, _ = await auth_service.login( + db, + password="correctpassword1", + session_duration_minutes=60, + session_secret="test-secret", + ) tampered_token = signed_token[:-1] + ("0" if signed_token[-1] != "0" else "1") with pytest.raises(ValueError, match="invalid"): @@ -177,18 +213,29 @@ class TestLogout: """logout() deletes the session so it can no longer be validated.""" from app.repositories import session_repo - session = await auth_service.login(db, password="correctpassword1", session_duration_minutes=60) - await auth_service.logout(db, session.token) - stored = await session_repo.get_session(db, session.token) + signed_token, _ = await auth_service.login( + db, + password="correctpassword1", + session_duration_minutes=60, + session_secret="test-secret", + ) + raw_token = auth_service.unwrap_session_token(signed_token, "test-secret") + await auth_service.logout(db, raw_token) + stored = await session_repo.get_session(db, raw_token) assert stored is None async def test_logout_accepts_signed_token(self, db: aiosqlite.Connection) -> None: """logout() accepts a signed token and revokes the underlying raw session.""" from app.repositories import session_repo - session = await auth_service.login(db, password="correctpassword1", session_duration_minutes=60) - signed_token = auth_service.sign_session_token(session.token, "test-secret") + signed_token, _ = await auth_service.login( + db, + password="correctpassword1", + session_duration_minutes=60, + session_secret="test-secret", + ) + raw_token = auth_service.unwrap_session_token(signed_token, "test-secret") await auth_service.logout(db, signed_token, session_secret="test-secret") - stored = await session_repo.get_session(db, session.token) + stored = await session_repo.get_session(db, raw_token) assert stored is None