feat: implement commit method in login session repository and related services (cb8ac89c) · Commits · OOP / code / portal

src/portal_service/adapters/database/repos/login_sessions.py

+3 −0

Changes for src/portal_service/adapters/database/repos/login_sessions.py: 3 added lines, 0 removed lines.
Original line number	Diff line number	Diff line
		@@ -66,3 +66,6 @@ class SqlLoginSessionRepository(LoginSessionRepository):
		stmt = delete(LoginSessionRow).where(LoginSessionRow.expires_at <= now)
		result = cast(CursorResult[Any], await self._session.execute(stmt))
		return result.rowcount

		async def commit(self) -> None:
		await self._session.commit()

src/portal_service/application/session_service.py

+4 −0

Changes for src/portal_service/application/session_service.py: 4 added lines, 0 removed lines.
Original line number	Diff line number	Diff line
		@@ -136,6 +136,7 @@ class SessionService:
		issued = await self._identity_provider.refresh(tokens.refresh_token)
		except InvalidGrantError as exc:
		await self._sessions.delete_by_id(current.id)
		await self._sessions.commit()
		raise InvalidSessionError("Keycloak no longer accepts this session") from exc

		# Keycloak may return only some of the tokens; keep what it did not replace.
		@@ -154,6 +155,9 @@ class SessionService:
		):
		# Logged out while we were refreshing: the session stays ended.
		raise InvalidSessionError("Session ended during refresh")
		# Keycloak has already spent the old refresh token, so a failing route must
		# not roll the new one back.
		await self._sessions.commit()

		row = current.model_copy(
		update={

src/portal_service/domain/ports/database/login_sessions.py

+7 −0

Changes for src/portal_service/domain/ports/database/login_sessions.py: 7 added lines, 0 removed lines.
Original line number	Diff line number	Diff line
		@@ -48,3 +48,10 @@ class LoginSessionRepository(ABC):
		async def delete_expired(self, now: datetime) -> int:
		"""Deletes sessions whose expires_at has passed and returns how many."""
		pass

		@abstractmethod
		async def commit(self) -> None:
		"""
		Makes the writes so far permanent and releases the row lock, so a later
		failure in the same request cannot roll them back.
		"""

tests/fakes.py

+3 −0

Changes for tests/fakes.py: 3 added lines, 0 removed lines.
Original line number	Diff line number	Diff line
		@@ -69,6 +69,9 @@ class FakeLoginSessionRepository(LoginSessionRepository):
		del self.rows[key]
		return len(expired)

		async def commit(self) -> None:
		pass


		class FakeIdentityProvider(IdentityProvider):
		"""Keycloak's side of the flow, without Keycloak."""

tests/integration/test_concurrent_refresh.py

+38 −2

Changes for tests/integration/test_concurrent_refresh.py: 38 added lines, 2 removed lines.
Original line number	Diff line number	Diff line
		"""
		Concurrent refreshes of one session, against a real Postgres.
		Refreshing a session against a real Postgres.

		A page load sends several requests at once, and right after the access token
		expires they all want to refresh it. Only the row lock makes them take turns, and
		an in-memory fake cannot show that, so these run on separate database connections.
		Nor can a fake show what a rolled-back request keeps, which the last tests check.
		"""

		import asyncio
		@@ -18,7 +19,7 @@ from portal_service.adapters.database.repos import SqlLoginSessionRepository
		from portal_service.adapters.database.sql import LoginSessionRow, get_metadata
		from portal_service.application.session_crypto import SessionCredentials, encrypt_tokens
		from portal_service.application.session_service import ActiveSession, SessionService
		from portal_service.domain.errors import InvalidGrantError
		from portal_service.domain.errors import InvalidGrantError, InvalidSessionError
		from portal_service.domain.models import IssuedTokens, LoginSession, SessionTokens
		from tests.fakes import FakeIdentityProvider

		@@ -103,3 +104,38 @@ async def test_concurrent_requests_refresh_once_and_all_succeed(
		assert {r.tokens.access_token for r in results} == {"access-1"}
		# And the session survived for the next request.
		assert (await _request(session_maker, idp, cookie)).tokens.access_token == "access-1"


		async def test_a_revoked_session_stays_deleted_when_the_request_fails(
		session_maker: async_sessionmaker[AsyncSession],
		) -> None:
		cookie = await _expired_session(session_maker)
		idp = FakeIdentityProvider()
		idp.error = InvalidGrantError("session not active")

		# The refusal fails the request, which rolls its transaction back.
		with pytest.raises(InvalidSessionError):
		await _request(session_maker, idp, cookie)

		async with session_maker() as db:
		row = await SqlLoginSessionRepository(db).get_by_id(
		SessionCredentials.from_cookie(cookie).session_id_hash
		)
		assert row is None


		async def test_a_failing_route_keeps_the_refreshed_tokens(
		session_maker: async_sessionmaker[AsyncSession],
		) -> None:
		cookie = await _expired_session(session_maker)
		idp = RotatingIdentityProvider()

		with pytest.raises(RuntimeError):
		async with session_maker() as db:
		service = SessionService(SqlLoginSessionRepository(db), idp, timedelta(hours=1), "/")
		await service.ensure_fresh(await service.load_without_refreshing(cookie))
		raise RuntimeError("the route failed after the refresh")

		# Keycloak has spent refresh-0, so the user is still logged in only if we kept refresh-1.
		assert (await _request(session_maker, idp, cookie)).tokens.access_token == "access-1"
		assert idp.refreshed == ["refresh-0"]