diff --git a/CHANGELOG.md b/CHANGELOG.md index 280f20505..2b5b53863 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ Write the date in place of the "Unreleased" in the case a new version is released. --> # Changelog +### Fixed + +- Ensured that stale sessions that can no longer be refreshed are purged from the auth database prior to the expiration time (default 1 year) to avoid bloat. Updated tests accordingly and fixed related SQLite bugs for the purge_expired function. ## 0.1.0-b20 (2025-03-07) diff --git a/tiled/_tests/test_authentication.py b/tiled/_tests/test_authentication.py index 8c31c9b4a..37e2c05cd 100644 --- a/tiled/_tests/test_authentication.py +++ b/tiled/_tests/test_authentication.py @@ -512,6 +512,57 @@ def test_session_limit(enter_username_password, config): authentication.SESSION_LIMIT = original_limit +def test_expired_refresh(enter_username_password, config): + # Pathological configuration: refresh tokens do not last + # Case where the refresh token is never used. (time_last_refreshed < refresh_max_age) + import tiled.server.app + import tiled.server.authentication + + try: + _purge_interval = tiled.server.app.PURGE_INTERVAL + _session_limit = tiled.server.authentication.SESSION_LIMIT + tiled.server.app.PURGE_INTERVAL = 0.1 + tiled.server.authentication.SESSION_LIMIT = 3 + config["authentication"]["refresh_token_max_age"] = 1 + with Context.from_app(build_app_from_config(config)) as context: + with enter_username_password("alice", "secret1"): + for _ in range(authentication.SESSION_LIMIT): + context.authenticate() + context.force_auth_refresh() + context.logout() + time.sleep(1.2) + # Stale refresh tokens should have been purged by now + context.authenticate() + finally: + tiled.server.app.PURGE_INTERVAL = _purge_interval + tiled.server.authentication.SESSION_LIMIT = _session_limit + + +def test_expired_refresh_unused(enter_username_password, config): + # Pathological configuration: refresh tokens do not last + # Case where the refresh token is never used. (time_last_refreshed is None) + import tiled.server.app + import tiled.server.authentication + + try: + _purge_interval = tiled.server.app.PURGE_INTERVAL + _session_limit = tiled.server.authentication.SESSION_LIMIT + tiled.server.app.PURGE_INTERVAL = 0.1 + tiled.server.authentication.SESSION_LIMIT = 3 + config["authentication"]["refresh_token_max_age"] = 1 + with Context.from_app(build_app_from_config(config)) as context: + with enter_username_password("alice", "secret1"): + for _ in range(authentication.SESSION_LIMIT): + context.authenticate() + context.logout() + time.sleep(1.2) + # Stale refresh tokens should have been purged by now + context.authenticate() + finally: + tiled.server.app.PURGE_INTERVAL = _purge_interval + tiled.server.authentication.SESSION_LIMIT = _session_limit + + @pytest.fixture def principals_context(enter_username_password, config): """ diff --git a/tiled/authn_database/core.py b/tiled/authn_database/core.py index b20aa49a6..787e2611a 100644 --- a/tiled/authn_database/core.py +++ b/tiled/authn_database/core.py @@ -1,7 +1,8 @@ import hashlib import uuid as uuid_module -from datetime import datetime, timezone +from datetime import datetime, timedelta, timezone +from sqlalchemy import and_, or_ from sqlalchemy.ext.asyncio import AsyncSession from sqlalchemy.future import select from sqlalchemy.orm import selectinload @@ -75,19 +76,56 @@ async def initialize_database(engine): await create_default_roles(db) -async def purge_expired(db, cls): +def lt_expiration_filter(dialect_name, exp_time, ref_time): + """Dialect dependent timestamp comparison (less than) for expiration filter.""" + if dialect_name == "postgresql": + # Use func.timezone() for PostgreSQL, but remove for SQLite + return func.timezone("UTC", exp_time) < ref_time + else: + # SQLite handles timestamps differently + return exp_time < ref_time + + +async def purge_expired(db, cls, refresh_token_max_age: timedelta = None): """ Remove expired entries. """ now = datetime.now(timezone.utc) num_expired = 0 - statement = ( - select(cls) - .filter(cls.expiration_time.is_not(None)) - .filter(cls.expiration_time.replace(tzinfo=timezone.utc) < now) - ) + # Check the database dialect (SQLite vs PostgreSQL) + dialect_name = db.bind.dialect.name + if cls.__name__ == "Session": + statement = select(cls).filter( + or_( + and_( + cls.expiration_time.is_not(None), + lt_expiration_filter(dialect_name, cls.expiration_time, now), + ), + and_( + cls.time_last_refreshed.is_not(None), + lt_expiration_filter( + dialect_name, + cls.time_last_refreshed, + now - refresh_token_max_age, + ), + ), + and_( + cls.time_last_refreshed.is_(None), + lt_expiration_filter( + dialect_name, cls.time_created, now - refresh_token_max_age + ), + ), + ) + ) + else: + statement = ( + select(cls) + .filter(cls.expiration_time.is_not(None)) + .filter(lt_expiration_filter(dialect_name, cls.expiration_time, now)) + ) result = await db.execute(statement) - for obj in result.scalars(): + rows_to_delete = result.unique().scalars() + for obj in rows_to_delete: num_expired += 1 await db.delete(obj) if num_expired: diff --git a/tiled/server/app.py b/tiled/server/app.py index 54429b2c5..2befc174c 100644 --- a/tiled/server/app.py +++ b/tiled/server/app.py @@ -70,6 +70,8 @@ MINIMUM_SUPPORTED_PYTHON_CLIENT_VERSION = packaging.version.parse("0.1.0a104") +PURGE_INTERVAL = 600 # seconds + logger = logging.getLogger(__name__) logger.setLevel("INFO") handler = logging.StreamHandler() @@ -638,13 +640,12 @@ async def startup_event(): ) async def purge_expired_sessions_and_api_keys(): - PURGE_INTERVAL = 600 # seconds while True: async with AsyncSession( engine, autoflush=False, expire_on_commit=False ) as db_session: num_expired_sessions = await purge_expired( - db_session, orm.Session + db_session, orm.Session, settings.refresh_token_max_age ) if num_expired_sessions: logger.info(