Commit 6d20edbc authored by Dimitrios Gogos's avatar Dimitrios Gogos
Browse files

feat: extend update_tokens method to include session expiration handling and update related tests

parent c55b38f4
Loading
Loading
Loading
Loading
+10 −2
Changes for src/portal_service/adapters/database/repos/login_sessions.py: 10 added lines, 2 removed lines.
Original line number Diff line number Diff line
@@ -38,14 +38,22 @@ class SqlLoginSessionRepository(LoginSessionRepository):
        return saved

    async def update_tokens(
        self, session_id_hash: str, encrypted_tokens: bytes, access_token_expires_at: datetime
        self,
        session_id_hash: str,
        encrypted_tokens: bytes,
        access_token_expires_at: datetime,
        expires_at: datetime,
    ) -> bool:
        # A plain UPDATE, not merge: merge would INSERT the row again if a logout
        # deleted it while the refresh was talking to Keycloak.
        stmt = (
            update(LoginSessionRow)
            .where(LoginSessionRow.id == session_id_hash)
            .values(tokens=encrypted_tokens, access_token_expires_at=access_token_expires_at)
            .values(
                tokens=encrypted_tokens,
                access_token_expires_at=access_token_expires_at,
                expires_at=expires_at,
            )
        )
        result = cast(CursorResult[Any], await self._session.execute(stmt))
        return result.rowcount == 1
+17 −3
Changes for src/portal_service/application/session_service.py: 17 added lines, 3 removed lines.
Original line number Diff line number Diff line
@@ -57,10 +57,20 @@ class SessionService:
        self._session_ttl = session_ttl
        self._post_logout_redirect_uri = post_logout_redirect_uri

    def _expires_at(self, now: datetime, issued: IssuedTokens) -> datetime:
        """
        The session lives as long as Keycloak will accept its refresh token, which
        covers both the realm's idle and max limits. The configured TTL is only a
        fallback for when Keycloak does not say (0 means an offline token).
        """
        if issued.refresh_expires_in:
            return now + timedelta(seconds=issued.refresh_expires_in)
        return now + self._session_ttl

    async def create(self, subject: str, issued: IssuedTokens) -> CreatedSession:
        credentials = SessionCredentials.generate()
        now = datetime.now(UTC)
        expires_at = now + self._session_ttl
        expires_at = self._expires_at(now, issued)

        await self._sessions.save(
            LoginSession(
@@ -135,17 +145,21 @@ class SessionService:
            id_token=issued.tokens.id_token or tokens.id_token,
        )
        access_token_expires_at = now + timedelta(seconds=issued.expires_in)
        # A refresh resets Keycloak's idle timer, so the session's deadline moves too.
        expires_at = self._expires_at(now, issued)
        # Same key as before: it lives only in the cookie the browser still holds.
        encrypted = encrypt_tokens(tokens, active.credentials)
        if not await self._sessions.update_tokens(current.id, encrypted, access_token_expires_at):
        if not await self._sessions.update_tokens(
            current.id, encrypted, access_token_expires_at, expires_at
        ):
            # Logged out while we were refreshing: the session stays ended.
            raise InvalidSessionError("Session ended during refresh")

        # The session's own deadline is unchanged by a refresh.
        row = current.model_copy(
            update={
                "encrypted_tokens": encrypted,
                "access_token_expires_at": access_token_expires_at,
                "expires_at": expires_at,
            }
        )
        return ActiveSession(session=row, tokens=tokens, credentials=active.credentials)
+4 −1
Changes for src/portal_service/config.py: 4 added lines, 1 removed line.
Original line number Diff line number Diff line
@@ -55,7 +55,10 @@ class OIDCSettings(BaseModel):

class SessionSettings(BaseModel):
    cookie_name: str = "oop_session"
    max_age: int = 3600
    # Keycloak's refresh token decides when a session ends. This only caps the cookie,
    # so keep it at least the realm's SSO Session Max, and is the fallback deadline
    # when Keycloak does not send refresh_expires_in.
    max_age: int = 36000
    # The cookie naming a login in flight: only needs to outlive the trip to Keycloak.
    attempt_cookie_name: str = "oop_login"
    attempt_max_age: int = 300
+6 −2
Changes for src/portal_service/domain/ports/database/login_sessions.py: 6 added lines, 2 removed lines.
Original line number Diff line number Diff line
@@ -29,10 +29,14 @@ class LoginSessionRepository(ABC):

    @abstractmethod
    async def update_tokens(
        self, session_id_hash: str, encrypted_tokens: bytes, access_token_expires_at: datetime
        self,
        session_id_hash: str,
        encrypted_tokens: bytes,
        access_token_expires_at: datetime,
        expires_at: datetime,
    ) -> bool:
        """
        Replaces the tokens of an existing session. Returns False, and changes nothing,
        Replaces the tokens and deadline of an existing session. Returns False, and changes nothing,
        if the session is gone: a refresh must never bring back a logged-out session.
        """

+6 −1
Changes for tests/fakes.py: 6 added lines, 1 removed line.
Original line number Diff line number Diff line
@@ -42,7 +42,11 @@ class FakeLoginSessionRepository(LoginSessionRepository):
        return login_session

    async def update_tokens(
        self, session_id_hash: str, encrypted_tokens: bytes, access_token_expires_at: datetime
        self,
        session_id_hash: str,
        encrypted_tokens: bytes,
        access_token_expires_at: datetime,
        expires_at: datetime,
    ) -> bool:
        row = self.rows.get(session_id_hash)
        if row is None:
@@ -51,6 +55,7 @@ class FakeLoginSessionRepository(LoginSessionRepository):
            update={
                "encrypted_tokens": encrypted_tokens,
                "access_token_expires_at": access_token_expires_at,
                "expires_at": expires_at,
            }
        )
        return True
Loading