feat: add server-side session store with expiry and invalidation to AuthService - #3305
Open
deacon-mp wants to merge 3 commits into
Open
feat: add server-side session store with expiry and invalidation to AuthService#3305deacon-mp wants to merge 3 commits into
deacon-mp wants to merge 3 commits into
Conversation
Add in-memory session registry with register, validate, invalidate lifecycle.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds an in-memory, server-side session store to AuthService with registration, validation (expiry), and invalidation primitives, plus unit tests to establish baseline behavior for future login/logout integration.
Changes:
- Introduces
register_session(),is_session_valid(),invalidate_session(), andinvalidate_all_sessions_for_user()onAuthService - Adds in-memory
_active_sessionsstore with per-session expiry timestamps - Adds unit tests covering session creation, validation, invalidation, per-user invalidation, and expiry behavior
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| app/service/auth_svc.py | Adds session store state and new session lifecycle methods with expiry handling |
| tests/security/test_session_store.py | Adds unit tests exercising the new session store API |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+91
to
+100
| def register_session(self, username): | ||
| """Create a server-side session and return the token.""" | ||
| token = str(uuid.uuid4()) | ||
| lifetime = (self.get_config('session_lifetime_hours') or self.SESSION_LIFETIME_HOURS) * 3600 | ||
| now = time.time() | ||
| self._active_sessions[token] = { | ||
| 'username': username, | ||
| 'created_at': now, | ||
| 'expires_at': now + lifetime, | ||
| } |
| self.log = self.add_service('auth_svc', self) | ||
| self._login_handler = None | ||
| self._default_login_handler = None | ||
| self._active_sessions = {} # {session_token: {username, created_at, expires_at}} |
Comment on lines
+107
to
+115
| def is_session_valid(self, token): | ||
| """Check if a session exists and is not expired.""" | ||
| session = self._active_sessions.get(token) | ||
| if not session: | ||
| return False | ||
| if time.time() > session['expires_at']: | ||
| self._active_sessions.pop(token, None) | ||
| return False | ||
| return True |
Comment on lines
+7
to
+19
| sys.modules.setdefault('app.service.interfaces.i_auth_svc', MagicMock()) | ||
|
|
||
|
|
||
| class TestSessionStore(unittest.TestCase): | ||
| def _make_auth_svc(self): | ||
| from app.service.auth_svc import AuthService | ||
| with patch.object(AuthService, 'add_service', return_value=MagicMock()): | ||
| svc = AuthService.__new__(AuthService) | ||
| svc._active_sessions = {} | ||
| svc.log = MagicMock() | ||
| svc.user_map = {} | ||
| svc._login_handler = None | ||
| svc._default_login_handler = None |
Comment on lines
+11
to
+20
| def _make_auth_svc(self): | ||
| from app.service.auth_svc import AuthService | ||
| with patch.object(AuthService, 'add_service', return_value=MagicMock()): | ||
| svc = AuthService.__new__(AuthService) | ||
| svc._active_sessions = {} | ||
| svc.log = MagicMock() | ||
| svc.user_map = {} | ||
| svc._login_handler = None | ||
| svc._default_login_handler = None | ||
| return svc |
| def register_session(self, username): | ||
| """Create a server-side session and return the token.""" | ||
| token = str(uuid.uuid4()) | ||
| lifetime = (self.get_config('session_lifetime_hours') or self.SESSION_LIFETIME_HOURS) * 3600 |
- Fix register_session to cast get_config() result to float before multiplying, handling string config values correctly; use is None check so 0 hours is respected rather than falling back to default - Add purge_expired_sessions() to prevent unbounded _active_sessions growth when expired tokens are never revalidated - Fix test isolation: use patch.dict(sys.modules) instead of bare sys.modules assignment to prevent cross-test contamination - Add tests: string config, zero config, None config default fallback, and purge_expired_sessions behaviour
Contributor
There was a problem hiding this comment.
Pull request overview
Adds an in-memory, server-side session store to AuthService with expiry, validation, and invalidation APIs, plus unit tests to cover expected behavior and config edge cases.
Changes:
- Introduces session lifecycle methods on
AuthService(register/validate/invalidate/purge/invalidate-by-user) with configurable lifetime. - Adds an in-memory
_active_sessionsstore and default lifetime constant. - Adds a new test module covering session creation, expiry, invalidation, and config parsing behaviors.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| app/service/auth_svc.py | Adds in-memory session store + helper methods for expiry and invalidation. |
| tests/security/test_session_store.py | Adds unit tests to validate session store behavior and lifetime configuration handling. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+7
to
+11
| """Create an AuthService instance with dependencies mocked. | ||
|
|
||
| Uses patch.dict to scope sys.modules injection to the import call, | ||
| avoiding global test-process contamination. Calls __init__ so that | ||
| _active_sessions and other attributes are properly initialised. |
Comment on lines
+21
to
+26
| svc = AuthService.__new__(AuthService) | ||
| svc._active_sessions = {} | ||
| svc.log = MagicMock() | ||
| svc.user_map = {} | ||
| svc._login_handler = None | ||
| svc._default_login_handler = None |
Comment on lines
+95
to
+100
| from app.service.auth_svc import AuthService | ||
| svc = _make_auth_svc() | ||
| with patch.object(type(svc), 'get_config', return_value=None): | ||
| token = svc.register_session('admin') | ||
| session = svc._active_sessions[token] | ||
| expected_lifetime = AuthService.SESSION_LIFETIME_HOURS * 3600 |
Comment on lines
+91
to
+97
| def register_session(self, username): | ||
| """Create a server-side session and return the token.""" | ||
| token = str(uuid.uuid4()) | ||
| _cfg = self.get_config('session_lifetime_hours') | ||
| lifetime_hours = float(_cfg) if _cfg is not None else self.SESSION_LIFETIME_HOURS | ||
| lifetime = lifetime_hours * 3600 | ||
| now = time.time() |
| self.log = self.add_service('auth_svc', self) | ||
| self._login_handler = None | ||
| self._default_login_handler = None | ||
| self._active_sessions = {} # {session_token: {username, created_at, expires_at}} |
…ests
- Document single-process limitation of in-memory session store
- Add try/except for non-numeric config values in register_session
- Call purge_expired_sessions on each registration to bound memory
- Fix test helper docstring to accurately describe __new__ usage
- Add test for invalid string config ('8h') falling back to default
- Fix test_none_config to access class constant via type(svc)
- Remove __main__ block from test file
Contributor
There was a problem hiding this comment.
Pull request overview
Adds an in-memory, server-side session store API to AuthService with configurable expiry and invalidation, along with unit tests to validate lifetime parsing, expiry behavior, and purge logic.
Changes:
- Introduces server-side session registration/validation/invalidation methods and an in-memory backing store.
- Adds expiry purging to keep the session store bounded.
- Adds unit tests covering session lifecycle, expiry, purge, and config parsing edge cases.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| app/service/auth_svc.py | Adds in-memory session store + session lifecycle APIs with configurable expiry. |
| tests/security/test_session_store.py | Adds unit tests for session registration, validation, invalidation, expiry, purge, and config parsing edge cases. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+17
to
+22
| with patch.dict('sys.modules', { | ||
| 'app.service.interfaces.i_auth_svc': mock_interface_module, | ||
| }): | ||
| with patch('app.service.auth_svc.BaseService.add_service', return_value=MagicMock()): | ||
| from app.service.auth_svc import AuthService | ||
| svc = AuthService.__new__(AuthService) |
Comment on lines
+102
to
+103
| self.purge_expired_sessions() | ||
| token = str(uuid.uuid4()) |
Comment on lines
+111
to
+116
| now = time.time() | ||
| self._active_sessions[token] = { | ||
| 'username': username, | ||
| 'created_at': now, | ||
| 'expires_at': now + lifetime, | ||
| } |
| } | ||
| return token | ||
|
|
||
| def purge_expired_sessions(self): |
Comment on lines
+126
to
+129
| now = time.time() | ||
| expired = [t for t, s in self._active_sessions.items() if now > s['expires_at']] | ||
| for token in expired: | ||
| del self._active_sessions[token] |
| lifetime_hours = float(_cfg) if _cfg is not None else self.SESSION_LIFETIME_HOURS | ||
| except (ValueError, TypeError): | ||
| self.log.warning('Invalid session_lifetime_hours config value %r; using default', _cfg) | ||
| lifetime_hours = self.SESSION_LIFETIME_HOURS |
|
deacon-mp
commented
May 18, 2026
deacon-mp
left a comment
deacon-mp
left a comment
Contributor
Author
There was a problem hiding this comment.
The session-store design (token + expiry + invalidation map) is fine in isolation, but the new methods aren't called from anywhere — this is dead code as-merged.
To make it functional:
- On login —
auth_svc.handle_successful_loginshould callregister_session(username)and stash the returned token into the aiohttp_session cookie (or wherever you want to round-trip it). - On every request —
is_request_authenticatedshould pull the token from the session cookie and callis_session_validbefore returning True. Today it only checksidentity in self.user_map, which means a stale aiohttp_session cookie keeps working afterinvalidate_sessionis called. - On logout —
logout_usershould callinvalidate_session(token)so the token can't be replayed.
Without those three hookups, register_session / is_session_valid / invalidate_session exist but never run.
Operational note (worth a docstring): the in-memory dict won't survive restart and won't share across multiple aiohttp workers if anyone ever runs >1. That's acceptable for the single-process default but should be flagged as a limitation now, with a Redis-backed migration path on the roadmap.
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Added register_session(), invalidate_session(), is_session_valid(), and invalidate_all_sessions_for_user() to AuthService in auth_svc.py. Sessions have configurable lifetime (session_lifetime_hours, default 8h). Foundation for forced logout and session expiry. NOTE: this is additive — does not change existing cookie-based auth flow. A follow-up PR can wire these methods into the login/logout handlers. Includes tests (5 tests across 5 test methods). NOTE: complementary to PR #3282 (persists cookie key) — different concern.