From 27986609f419e3886359420365bb4925a1c0a4e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9lo=C3=AFse=20Joffe?= Date: Tue, 24 Feb 2026 09:14:30 +0100 Subject: [PATCH 1/6] feat: add functions to cleanup the AuthDB tables --- diracx-core/src/diracx/core/settings.py | 14 +++++++ diracx-db/src/diracx/db/sql/auth/db.py | 46 ++++++++++++++++++++- diracx-logic/src/diracx/logic/auth/token.py | 34 +++++++++++++++ docs/admin/reference/env-variables.md | 18 ++++++++ 4 files changed, 111 insertions(+), 1 deletion(-) diff --git a/diracx-core/src/diracx/core/settings.py b/diracx-core/src/diracx/core/settings.py index 6e97dcfbe..39439e640 100644 --- a/diracx-core/src/diracx/core/settings.py +++ b/diracx-core/src/diracx/core/settings.py @@ -183,6 +183,13 @@ class AuthSettings(ServiceSettingsBase): before it must be exchanged for tokens. Default: 5 minutes. """ + completed_flow_retention_minutes: int = 60 + """Retention time in minutes for completed flow. + + The maximum retention time of flow after being completed + and before they are deleted. Default: 60 minutes. + """ + state_key: FernetKey """Encryption key used to encrypt/decrypt the state parameter passed to the IAM. @@ -226,6 +233,13 @@ class AuthSettings(ServiceSettingsBase): through a new authentication flow. Default: 60 minutes. """ + revoked_refresh_token_retention_days: int = 30 + """Retention time in days for revoked refresh tokens. + + The maximum retention time of refresh tokens after being + revoked and before they are deleted. Default: 30 days. + """ + available_properties: set[SecurityProperty] = Field( default_factory=SecurityProperty.available_properties ) diff --git a/diracx-db/src/diracx/db/sql/auth/db.py b/diracx-db/src/diracx/db/sql/auth/db.py index bcbe814dc..b7874f5de 100644 --- a/diracx-db/src/diracx/db/sql/auth/db.py +++ b/diracx-db/src/diracx/db/sql/auth/db.py @@ -6,7 +6,7 @@ from itertools import pairwise from dateutil.rrule import MONTHLY, rrule -from sqlalchemy import insert, select, text, update +from sqlalchemy import delete, insert, select, text, update from sqlalchemy.exc import IntegrityError, NoResultFound from sqlalchemy.ext.asyncio import AsyncConnection from uuid_utils import UUID, uuid7 @@ -339,3 +339,47 @@ async def revoke_user_refresh_tokens(self, subject): .where(RefreshTokens.sub == subject) .values(status=RefreshTokenStatus.REVOKED) ) + + async def clean_expired_refresh_token( + self, max_validity: int, max_retention: int + ) -> tuple[int, int]: + """Delete expired and old revoked refresh tokens.""" + expired_date = str( + uuid7_from_datetime(substract_date(minutes=max_validity), randomize=False) + ) + stmt_expired = delete(RefreshTokens).where( + RefreshTokens.status == RefreshTokenStatus.CREATED, + RefreshTokens.jti < expired_date, + ) + res_expired = await self.conn.execute(stmt_expired) + + revoked_date = str( + uuid7_from_datetime(substract_date(days=max_retention), randomize=False) + ) + stmt_revoked = delete(RefreshTokens).where( + RefreshTokens.status == RefreshTokenStatus.REVOKED, + RefreshTokens.jti < revoked_date, + ) + res_revoked = await self.conn.execute(stmt_revoked) + + return res_expired.rowcount, res_revoked.rowcount + + async def clean_expired_authorization_flows(self, max_retention: int) -> int: + """Delete old DONE/ERROR authorization flows.""" + stmt_auth = delete(AuthorizationFlows).where( + AuthorizationFlows.status.in_([FlowStatus.DONE, FlowStatus.ERROR]), + AuthorizationFlows.creation_time < substract_date(minutes=max_retention), + ) + res_auth = await self.conn.execute(stmt_auth) + + return res_auth.rowcount + + async def clean_expired_device_flows(self, max_retention: int) -> int: + """Delete old DONE/ERROR device flows.""" + stmt_device = delete(DeviceFlows).where( + DeviceFlows.status.in_([FlowStatus.DONE, FlowStatus.ERROR]), + DeviceFlows.creation_time < substract_date(minutes=max_retention), + ) + res_device = await self.conn.execute(stmt_device) + + return res_device.rowcount diff --git a/diracx-logic/src/diracx/logic/auth/token.py b/diracx-logic/src/diracx/logic/auth/token.py index 0caecbb71..e5dc386c5 100644 --- a/diracx-logic/src/diracx/logic/auth/token.py +++ b/diracx-logic/src/diracx/logic/auth/token.py @@ -4,6 +4,7 @@ import base64 import hashlib +import logging import re from datetime import datetime, timedelta, timezone from typing import cast @@ -37,6 +38,8 @@ verify_dirac_refresh_token, ) +logger = logging.getLogger(__name__) + async def get_oidc_token( grant_type: GrantType, @@ -429,6 +432,8 @@ async def get_authorization_flow(auth_db: AuthDB, code: str, max_validity: int): """Get the authorization flow from the DB and check few parameters before returning it.""" res = await auth_db.get_authorization_flow(code, max_validity) + print(f"Flow status : {res['Status']}") + if res["Status"] == FlowStatus.READY: await auth_db.update_authorization_flow_status(code, FlowStatus.DONE) return res @@ -437,3 +442,32 @@ async def get_authorization_flow(auth_db: AuthDB, code: str, max_validity: int): raise AuthorizationError("Code was already used") raise AuthorizationError("Bad state in authorization flow") + + +async def cleanup_expired_data(auth_db: AuthDB, settings: AuthSettings) -> None: + """Remove expired data from the auth database.""" + expired_tokens, revoked_tokens = await auth_db.clean_expired_refresh_token( + max_validity=settings.refresh_token_expire_minutes, + max_retention=settings.revoked_refresh_token_retention_days, + ) + logger.info( + "Deleted %d expired and %d revoked refresh tokens", + expired_tokens, + revoked_tokens, + ) + + auth = await auth_db.clean_expired_authorization_flows( + max_retention=settings.completed_flow_retention_minutes, + ) + logger.info( + "Deleted %d expired authorization flows", + auth, + ) + + device = await auth_db.clean_expired_device_flows( + max_retention=settings.completed_flow_retention_minutes, + ) + logger.info( + "Deleted %d expired device flows", + device, + ) diff --git a/docs/admin/reference/env-variables.md b/docs/admin/reference/env-variables.md index 1770414e0..86054db9a 100644 --- a/docs/admin/reference/env-variables.md +++ b/docs/admin/reference/env-variables.md @@ -42,6 +42,15 @@ Expiration time in seconds for authorization code flow. The time window during which the authorization code remains valid before it must be exchanged for tokens. Default: 5 minutes. +### `DIRACX_SERVICE_AUTH_COMPLETED_FLOW_RETENTION_MINUTES` + +*Optional*, default value: `60` + +Retention time in minutes for completed flow. + +The maximum retention time of flow after being completed +and before they are deleted. Default: 60 minutes. + ### `DIRACX_SERVICE_AUTH_STATE_KEY` **Required** @@ -96,6 +105,15 @@ Expiration time in minutes for refresh tokens. The maximum lifetime of refresh tokens before they must be re-issued through a new authentication flow. Default: 60 minutes. +### `DIRACX_SERVICE_AUTH_REVOKED_REFRESH_TOKEN_RETENTION_DAYS` + +*Optional*, default value: `30` + +Retention time in days for revoked refresh tokens. + +The maximum retention time of refresh tokens after being +revoked and before they are deleted. Default: 30 days. + ### `DIRACX_SERVICE_AUTH_AVAILABLE_PROPERTIES` *Optional* From 8a67a9439727bd986be3f299c3d232cf41d3e1f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9lo=C3=AFse=20Joffe?= Date: Tue, 24 Feb 2026 09:16:14 +0100 Subject: [PATCH 2/6] feat: add tests for the AuthDB cleanup function --- .../tests/auth/test_authorization_flow.py | 37 +++++++++++++++++ diracx-db/tests/auth/test_device_flow.py | 32 ++++++++++++++- diracx-db/tests/auth/test_refresh_token.py | 41 +++++++++++++++++++ 3 files changed, 109 insertions(+), 1 deletion(-) diff --git a/diracx-db/tests/auth/test_authorization_flow.py b/diracx-db/tests/auth/test_authorization_flow.py index 56a3d332c..9664dbbe3 100644 --- a/diracx-db/tests/auth/test_authorization_flow.py +++ b/diracx-db/tests/auth/test_authorization_flow.py @@ -5,6 +5,7 @@ from diracx.core.exceptions import AuthorizationError from diracx.db.sql.auth.db import AuthDB +from diracx.db.sql.auth.schema import FlowStatus MAX_VALIDITY = 2 EXPIRED = 0 @@ -74,3 +75,39 @@ async def test_insert(auth_db: AuthDB): ) assert uuid1 != uuid2 + + +async def test_clean_authorization_flows(auth_db: AuthDB): + # Insert two authorization flows + async with auth_db as auth_db: + uuid1 = await auth_db.insert_authorization_flow( + "client_id", "scope", "code_challenge", "S256", "redirect_uri" + ) + uuid2 = await auth_db.insert_authorization_flow( + "client_id2", "scope2", "code_challenge2", "S256", "redirect_uri2" + ) + + id_token = {"sub": "myIdToken"} + + async with auth_db as auth_db: + code1, _ = await auth_db.authorization_flow_insert_id_token(uuid1, id_token, 1) + code2, _ = await auth_db.authorization_flow_insert_id_token(uuid2, id_token, 1) + + async with auth_db as auth_db: + await auth_db.update_authorization_flow_status(code1, FlowStatus.DONE) + await auth_db.update_authorization_flow_status(code2, FlowStatus.ERROR) + + # Check the number of deleted authorization flow (should be 0) + async with auth_db as auth_db: + deleted_auth = await auth_db.clean_expired_authorization_flows(max_retention=30) + assert deleted_auth == 0 + + # Check the number of deleted authorization flow (should be 2) + async with auth_db as auth_db: + deleted_auth = await auth_db.clean_expired_authorization_flows(max_retention=0) + assert deleted_auth == 2 + + # Check the number of deleted authorization flow (should be 0 because there is nothing left to delete) + async with auth_db as auth_db: + deleted_auth = await auth_db.clean_expired_authorization_flows(max_retention=0) + assert deleted_auth == 0 diff --git a/diracx-db/tests/auth/test_device_flow.py b/diracx-db/tests/auth/test_device_flow.py index 112e77898..a3e645c35 100644 --- a/diracx-db/tests/auth/test_device_flow.py +++ b/diracx-db/tests/auth/test_device_flow.py @@ -8,7 +8,7 @@ from diracx.core.exceptions import AuthorizationError from diracx.db.sql.auth.db import AuthDB -from diracx.db.sql.auth.schema import USER_CODE_LENGTH +from diracx.db.sql.auth.schema import USER_CODE_LENGTH, FlowStatus from diracx.db.sql.utils.functions import substract_date MAX_VALIDITY = 2 @@ -139,3 +139,33 @@ async def test_device_flow_insert_id_token(auth_db: AuthDB): async with auth_db as auth_db: res = await auth_db.get_device_flow(device_code) assert res["IDToken"] == id_token + + +async def test_clean_device_flows(auth_db: AuthDB): + # Insert two device flows + async with auth_db as auth_db: + user_code1, device_code1 = await auth_db.insert_device_flow( + "client_id", "scope" + ) + user_code2, device_code2 = await auth_db.insert_device_flow( + "client_id", "scope" + ) + + async with auth_db as auth_db: + await auth_db.update_device_flow_status(device_code1, FlowStatus.DONE) + await auth_db.update_device_flow_status(device_code2, FlowStatus.ERROR) + + # Check the number of deleted device flows (should be 0) + async with auth_db as auth_db: + deleted_device = await auth_db.clean_expired_device_flows(max_retention=30) + assert deleted_device == 0 + + # Check the number of deleted device flows (should be 2) + async with auth_db as auth_db: + deleted_device = await auth_db.clean_expired_device_flows(max_retention=0) + assert deleted_device == 2 + + # Check the number of deleted device flow (should be 0 because there is nothing left to delete) + async with auth_db as auth_db: + deleted_device = await auth_db.clean_expired_device_flows(max_retention=0) + assert deleted_device == 0 diff --git a/diracx-db/tests/auth/test_refresh_token.py b/diracx-db/tests/auth/test_refresh_token.py index 28d6dfe9d..619dd8eaa 100644 --- a/diracx-db/tests/auth/test_refresh_token.py +++ b/diracx-db/tests/auth/test_refresh_token.py @@ -257,3 +257,44 @@ async def test_get_refresh_tokens(auth_db: AuthDB): # Check the number of retrieved refresh tokens (should be 3 refresh tokens) assert len(refresh_tokens) == 2 + + +async def test_clean_refresh_tokens(auth_db: AuthDB): + # Insert two refresh tokens + jtis = [] + async with auth_db as auth_db: + for _ in range(2): + jti = uuid7() + await auth_db.insert_refresh_token( + jti, + "subject", + "scope", + ) + jtis.append(jti) + + # Revoke one of the refresh token + async with auth_db as auth_db: + await auth_db.revoke_refresh_token(jtis[0]) + + # Check the number of deleted refresh tokens (should be 0) + async with auth_db as auth_db: + deleted_expired, deleted_revoked = await auth_db.clean_expired_refresh_token( + max_validity=10, max_retention=30 + ) + assert deleted_expired == 0 + assert deleted_revoked == 0 + + # Check the number of deleted refresh tokens (should be 1 of each) + async with auth_db as auth_db: + deleted_expired, deleted_revoked = await auth_db.clean_expired_refresh_token( + max_validity=0, max_retention=0 + ) + assert deleted_expired == 1 + assert deleted_revoked == 1 + + # Get all refresh tokens (Admin) + async with auth_db as auth_db: + refresh_tokens = await auth_db.get_user_refresh_tokens() + + # Check the number of retrieved refresh tokens (should be 0) + assert len(refresh_tokens) == 0 From 8c8b889fb7cc3ac5b3f9a2254a2b1f60365213b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9lo=C3=AFse=20Joffe?= Date: Thu, 5 Mar 2026 09:02:13 +0100 Subject: [PATCH 3/6] fix: move cleanup function and minor log fixes --- .../src/diracx/logic/auth/management.py | 25 ++++++++++++++ diracx-logic/src/diracx/logic/auth/token.py | 34 ------------------- 2 files changed, 25 insertions(+), 34 deletions(-) diff --git a/diracx-logic/src/diracx/logic/auth/management.py b/diracx-logic/src/diracx/logic/auth/management.py index fb6a1963f..de756f687 100644 --- a/diracx-logic/src/diracx/logic/auth/management.py +++ b/diracx-logic/src/diracx/logic/auth/management.py @@ -2,6 +2,8 @@ from __future__ import annotations +import logging + from uuid_utils import UUID from diracx.core.exceptions import InvalidCredentialsError @@ -10,6 +12,8 @@ from diracx.db.sql import AuthDB from diracx.logic.auth.utils import verify_dirac_refresh_token +logger = logging.getLogger(__name__) + async def get_refresh_tokens( auth_db: AuthDB, @@ -57,3 +61,24 @@ async def revoke_refresh_token_by_refresh_token( # Decode and verify the refresh token jti, _, _ = await verify_dirac_refresh_token(token, settings) return await revoke_refresh_token_by_jti(auth_db=auth_db, subject=subject, jti=jti) + + +async def cleanup_expired_data(auth_db: AuthDB, settings: AuthSettings) -> None: + """Remove expired data from the auth database.""" + expired_tokens, revoked_tokens = await auth_db.clean_expired_refresh_token( + max_validity=settings.refresh_token_expire_minutes, + max_retention=settings.revoked_refresh_token_retention_days, + ) + logger.info( + f"Deleted {expired_tokens} expired and {revoked_tokens} revoked refresh tokens" + ) + + auth = await auth_db.clean_expired_authorization_flows( + max_retention=settings.completed_flow_retention_minutes, + ) + logger.info(f"Deleted {auth} expired authorization flows") + + device = await auth_db.clean_expired_device_flows( + max_retention=settings.completed_flow_retention_minutes, + ) + logger.info(f"Deleted {device} expired device flows") diff --git a/diracx-logic/src/diracx/logic/auth/token.py b/diracx-logic/src/diracx/logic/auth/token.py index e5dc386c5..0caecbb71 100644 --- a/diracx-logic/src/diracx/logic/auth/token.py +++ b/diracx-logic/src/diracx/logic/auth/token.py @@ -4,7 +4,6 @@ import base64 import hashlib -import logging import re from datetime import datetime, timedelta, timezone from typing import cast @@ -38,8 +37,6 @@ verify_dirac_refresh_token, ) -logger = logging.getLogger(__name__) - async def get_oidc_token( grant_type: GrantType, @@ -432,8 +429,6 @@ async def get_authorization_flow(auth_db: AuthDB, code: str, max_validity: int): """Get the authorization flow from the DB and check few parameters before returning it.""" res = await auth_db.get_authorization_flow(code, max_validity) - print(f"Flow status : {res['Status']}") - if res["Status"] == FlowStatus.READY: await auth_db.update_authorization_flow_status(code, FlowStatus.DONE) return res @@ -442,32 +437,3 @@ async def get_authorization_flow(auth_db: AuthDB, code: str, max_validity: int): raise AuthorizationError("Code was already used") raise AuthorizationError("Bad state in authorization flow") - - -async def cleanup_expired_data(auth_db: AuthDB, settings: AuthSettings) -> None: - """Remove expired data from the auth database.""" - expired_tokens, revoked_tokens = await auth_db.clean_expired_refresh_token( - max_validity=settings.refresh_token_expire_minutes, - max_retention=settings.revoked_refresh_token_retention_days, - ) - logger.info( - "Deleted %d expired and %d revoked refresh tokens", - expired_tokens, - revoked_tokens, - ) - - auth = await auth_db.clean_expired_authorization_flows( - max_retention=settings.completed_flow_retention_minutes, - ) - logger.info( - "Deleted %d expired authorization flows", - auth, - ) - - device = await auth_db.clean_expired_device_flows( - max_retention=settings.completed_flow_retention_minutes, - ) - logger.info( - "Deleted %d expired device flows", - device, - ) From 48ecec8e5cabda918a6537f76f43757d1430c641 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9lo=C3=AFse=20Joffe?= Date: Tue, 10 Mar 2026 09:51:25 +0100 Subject: [PATCH 4/6] feat: add cleanup authdb script --- diracx-logic/src/diracx/logic/__main__.py | 24 ++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/diracx-logic/src/diracx/logic/__main__.py b/diracx-logic/src/diracx/logic/__main__.py index 1fe642f4d..870abc89e 100644 --- a/diracx-logic/src/diracx/logic/__main__.py +++ b/diracx-logic/src/diracx/logic/__main__.py @@ -90,6 +90,20 @@ async def delete_jwk(args): save_jwks(path, jwks) +async def cleanup_authdb(args): + """Delete expired tokens and flows from the AuthDB.""" + logger.info("Deleting expired tokens and flows") + from diracx.core.settings import AuthSettings + from diracx.db.sql import AuthDB + from diracx.logic.auth.management import cleanup_expired_data + + settings = AuthSettings() + db = AuthDB(args.db_url) + async with db.engine_context(): + async with db: + await cleanup_expired_data(db, settings) + + def parse_args(): parser = argparse.ArgumentParser() subparsers = parser.add_subparsers(dest="command", required=True) @@ -120,8 +134,16 @@ def parse_args(): ) delete_jwk_parser.set_defaults(func=delete_jwk) + cleanup_authdb_parser = subparsers.add_parser( + "cleanup-authdb", help="Delete expired tokens and flows from the AuthDB" + ) + cleanup_authdb_parser.add_argument( + "--db-url", required=True, help="URL to the AuthDB" + ) + cleanup_authdb_parser.set_defaults(func=cleanup_authdb) + args = parser.parse_args() - logger.setLevel(logging.INFO) + logging.basicConfig(level=logging.INFO) asyncio.run(args.func(args)) From 0d150ebd832028cea760e22d531d63276e429753 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9lo=C3=AFse=20Joffe?= Date: Wed, 18 Mar 2026 14:22:45 +0100 Subject: [PATCH 5/6] refactor: split refresh tokens cleanup function, harmonize units and handle pending/ready flows --- diracx-core/src/diracx/core/settings.py | 22 ++++++-- diracx-db/src/diracx/db/sql/auth/db.py | 50 +++++++++++++++---- .../tests/auth/test_authorization_flow.py | 21 +++++--- diracx-db/tests/auth/test_device_flow.py | 22 ++++---- diracx-db/tests/auth/test_refresh_token.py | 14 +++--- diracx-logic/src/diracx/logic/__main__.py | 8 +-- .../src/diracx/logic/auth/management.py | 14 +++--- docs/admin/reference/env-variables.md | 8 +-- 8 files changed, 109 insertions(+), 50 deletions(-) diff --git a/diracx-core/src/diracx/core/settings.py b/diracx-core/src/diracx/core/settings.py index 39439e640..50fe61b72 100644 --- a/diracx-core/src/diracx/core/settings.py +++ b/diracx-core/src/diracx/core/settings.py @@ -32,6 +32,7 @@ SecretStr, TypeAdapter, UrlConstraints, + model_validator, ) from pydantic_settings import BaseSettings, SettingsConfigDict @@ -151,6 +152,21 @@ def create(cls) -> Self: class AuthSettings(ServiceSettingsBase): """Settings for the authentication service.""" + @model_validator(mode="after") + def check_retention_greater_than_expiration(self) -> Self: + """Ensure retention times are bigger than expiration times to avoid deleting valid flows.""" + if self.completed_flow_retention_minutes <= ( + self.device_flow_expiration_seconds / 60 + ) or self.completed_flow_retention_minutes <= ( + self.authorization_flow_expiration_seconds / 60 + ): + raise ValueError( + f"completed_flow_retention_minutes ({self.completed_flow_retention_minutes} minutes) must be bigger" + f" than device_flow_expiration_seconds ({self.device_flow_expiration_seconds / 60} minutes) and" + f" authorization_flow_expiration_seconds: ({self.authorization_flow_expiration_seconds / 60} minutes)" + ) + return self + model_config = SettingsConfigDict( env_prefix="DIRACX_SERVICE_AUTH_", use_attribute_docstrings=True ) @@ -233,11 +249,11 @@ class AuthSettings(ServiceSettingsBase): through a new authentication flow. Default: 60 minutes. """ - revoked_refresh_token_retention_days: int = 30 - """Retention time in days for revoked refresh tokens. + revoked_refresh_token_retention_minutes: int = 43200 + """Retention time in minutes for revoked refresh tokens. The maximum retention time of refresh tokens after being - revoked and before they are deleted. Default: 30 days. + revoked and before they are deleted. Default: 43200 minutes (30 days). """ available_properties: set[SecurityProperty] = Field( diff --git a/diracx-db/src/diracx/db/sql/auth/db.py b/diracx-db/src/diracx/db/sql/auth/db.py index b7874f5de..46233d568 100644 --- a/diracx-db/src/diracx/db/sql/auth/db.py +++ b/diracx-db/src/diracx/db/sql/auth/db.py @@ -340,10 +340,11 @@ async def revoke_user_refresh_tokens(self, subject): .values(status=RefreshTokenStatus.REVOKED) ) - async def clean_expired_refresh_token( - self, max_validity: int, max_retention: int - ) -> tuple[int, int]: - """Delete expired and old revoked refresh tokens.""" + async def clean_expired_refresh_tokens(self, max_validity: int) -> int: + """Delete expired refresh tokens. + + max_validity: Maximum validity time in minutes for refresh tokens. + """ expired_date = str( uuid7_from_datetime(substract_date(minutes=max_validity), randomize=False) ) @@ -353,8 +354,15 @@ async def clean_expired_refresh_token( ) res_expired = await self.conn.execute(stmt_expired) + return res_expired.rowcount + + async def clean_revoked_refresh_tokens(self, max_retention: int) -> int: + """Delete old revoked refresh tokens. + + max_retention: Maximum retention time in minutes for revoked refresh tokens. + """ revoked_date = str( - uuid7_from_datetime(substract_date(days=max_retention), randomize=False) + uuid7_from_datetime(substract_date(minutes=max_retention), randomize=False) ) stmt_revoked = delete(RefreshTokens).where( RefreshTokens.status == RefreshTokenStatus.REVOKED, @@ -362,12 +370,23 @@ async def clean_expired_refresh_token( ) res_revoked = await self.conn.execute(stmt_revoked) - return res_expired.rowcount, res_revoked.rowcount + return res_revoked.rowcount async def clean_expired_authorization_flows(self, max_retention: int) -> int: - """Delete old DONE/ERROR authorization flows.""" + """Delete old authorization flows. + + max_retention: Maximum retention time in minutes for expired authorization flows. + Must be bigger than authorization_flow_expiration_seconds. + """ stmt_auth = delete(AuthorizationFlows).where( - AuthorizationFlows.status.in_([FlowStatus.DONE, FlowStatus.ERROR]), + AuthorizationFlows.status.in_( + [ + FlowStatus.PENDING, + FlowStatus.READY, + FlowStatus.DONE, + FlowStatus.ERROR, + ] + ), AuthorizationFlows.creation_time < substract_date(minutes=max_retention), ) res_auth = await self.conn.execute(stmt_auth) @@ -375,9 +394,20 @@ async def clean_expired_authorization_flows(self, max_retention: int) -> int: return res_auth.rowcount async def clean_expired_device_flows(self, max_retention: int) -> int: - """Delete old DONE/ERROR device flows.""" + """Delete old device flows. + + max_retention: Maximum retention time in minutes for expired device flows. + Must be bigger than device_flow_expiration_seconds. + """ stmt_device = delete(DeviceFlows).where( - DeviceFlows.status.in_([FlowStatus.DONE, FlowStatus.ERROR]), + DeviceFlows.status.in_( + [ + FlowStatus.PENDING, + FlowStatus.READY, + FlowStatus.DONE, + FlowStatus.ERROR, + ] + ), DeviceFlows.creation_time < substract_date(minutes=max_retention), ) res_device = await self.conn.execute(stmt_device) diff --git a/diracx-db/tests/auth/test_authorization_flow.py b/diracx-db/tests/auth/test_authorization_flow.py index 9664dbbe3..1fb41731e 100644 --- a/diracx-db/tests/auth/test_authorization_flow.py +++ b/diracx-db/tests/auth/test_authorization_flow.py @@ -80,32 +80,39 @@ async def test_insert(auth_db: AuthDB): async def test_clean_authorization_flows(auth_db: AuthDB): # Insert two authorization flows async with auth_db as auth_db: - uuid1 = await auth_db.insert_authorization_flow( + _ = await auth_db.insert_authorization_flow( "client_id", "scope", "code_challenge", "S256", "redirect_uri" ) uuid2 = await auth_db.insert_authorization_flow( "client_id2", "scope2", "code_challenge2", "S256", "redirect_uri2" ) + uuid3 = await auth_db.insert_authorization_flow( + "client_id3", "scope3", "code_challenge3", "S256", "redirect_uri3" + ) + uuid4 = await auth_db.insert_authorization_flow( + "client_id4", "scope4", "code_challenge4", "S256", "redirect_uri4" + ) id_token = {"sub": "myIdToken"} async with auth_db as auth_db: - code1, _ = await auth_db.authorization_flow_insert_id_token(uuid1, id_token, 1) - code2, _ = await auth_db.authorization_flow_insert_id_token(uuid2, id_token, 1) + _, _ = await auth_db.authorization_flow_insert_id_token(uuid2, id_token, 1) + code3, _ = await auth_db.authorization_flow_insert_id_token(uuid3, id_token, 1) + code4, _ = await auth_db.authorization_flow_insert_id_token(uuid4, id_token, 1) async with auth_db as auth_db: - await auth_db.update_authorization_flow_status(code1, FlowStatus.DONE) - await auth_db.update_authorization_flow_status(code2, FlowStatus.ERROR) + await auth_db.update_authorization_flow_status(code3, FlowStatus.DONE) + await auth_db.update_authorization_flow_status(code4, FlowStatus.ERROR) # Check the number of deleted authorization flow (should be 0) async with auth_db as auth_db: deleted_auth = await auth_db.clean_expired_authorization_flows(max_retention=30) assert deleted_auth == 0 - # Check the number of deleted authorization flow (should be 2) + # Check the number of deleted authorization flow (should be 4: 1 PENDING, 1 READY, 1 DONE, 1 ERROR) async with auth_db as auth_db: deleted_auth = await auth_db.clean_expired_authorization_flows(max_retention=0) - assert deleted_auth == 2 + assert deleted_auth == 4 # Check the number of deleted authorization flow (should be 0 because there is nothing left to delete) async with auth_db as auth_db: diff --git a/diracx-db/tests/auth/test_device_flow.py b/diracx-db/tests/auth/test_device_flow.py index a3e645c35..168888f7e 100644 --- a/diracx-db/tests/auth/test_device_flow.py +++ b/diracx-db/tests/auth/test_device_flow.py @@ -144,26 +144,28 @@ async def test_device_flow_insert_id_token(auth_db: AuthDB): async def test_clean_device_flows(auth_db: AuthDB): # Insert two device flows async with auth_db as auth_db: - user_code1, device_code1 = await auth_db.insert_device_flow( - "client_id", "scope" - ) - user_code2, device_code2 = await auth_db.insert_device_flow( - "client_id", "scope" - ) + _, _ = await auth_db.insert_device_flow("client_id1", "scope1") + user_code2, _ = await auth_db.insert_device_flow("client_id2", "scope2") + _, device_code3 = await auth_db.insert_device_flow("client_id3", "scope3") + _, device_code4 = await auth_db.insert_device_flow("client_id4", "scope4") async with auth_db as auth_db: - await auth_db.update_device_flow_status(device_code1, FlowStatus.DONE) - await auth_db.update_device_flow_status(device_code2, FlowStatus.ERROR) + await auth_db.device_flow_validate_user_code(user_code2, MAX_VALIDITY) + await auth_db.device_flow_insert_id_token( + user_code2, {"sub": "myIdToken"}, MAX_VALIDITY + ) + await auth_db.update_device_flow_status(device_code3, FlowStatus.DONE) + await auth_db.update_device_flow_status(device_code4, FlowStatus.ERROR) # Check the number of deleted device flows (should be 0) async with auth_db as auth_db: deleted_device = await auth_db.clean_expired_device_flows(max_retention=30) assert deleted_device == 0 - # Check the number of deleted device flows (should be 2) + # Check the number of deleted device flows (should be 4: 1 PENDING, 1 READY, 1 DONE, 1 ERROR) async with auth_db as auth_db: deleted_device = await auth_db.clean_expired_device_flows(max_retention=0) - assert deleted_device == 2 + assert deleted_device == 4 # Check the number of deleted device flow (should be 0 because there is nothing left to delete) async with auth_db as auth_db: diff --git a/diracx-db/tests/auth/test_refresh_token.py b/diracx-db/tests/auth/test_refresh_token.py index 619dd8eaa..39fc69b37 100644 --- a/diracx-db/tests/auth/test_refresh_token.py +++ b/diracx-db/tests/auth/test_refresh_token.py @@ -278,18 +278,20 @@ async def test_clean_refresh_tokens(auth_db: AuthDB): # Check the number of deleted refresh tokens (should be 0) async with auth_db as auth_db: - deleted_expired, deleted_revoked = await auth_db.clean_expired_refresh_token( - max_validity=10, max_retention=30 - ) + deleted_expired = await auth_db.clean_expired_refresh_tokens(max_validity=10) assert deleted_expired == 0 + + async with auth_db as auth_db: + deleted_revoked = await auth_db.clean_revoked_refresh_tokens(max_retention=30) assert deleted_revoked == 0 # Check the number of deleted refresh tokens (should be 1 of each) async with auth_db as auth_db: - deleted_expired, deleted_revoked = await auth_db.clean_expired_refresh_token( - max_validity=0, max_retention=0 - ) + deleted_expired = await auth_db.clean_expired_refresh_tokens(max_validity=0) assert deleted_expired == 1 + + async with auth_db as auth_db: + deleted_revoked = await auth_db.clean_revoked_refresh_tokens(max_retention=0) assert deleted_revoked == 1 # Get all refresh tokens (Admin) diff --git a/diracx-logic/src/diracx/logic/__main__.py b/diracx-logic/src/diracx/logic/__main__.py index 870abc89e..d7b7674a5 100644 --- a/diracx-logic/src/diracx/logic/__main__.py +++ b/diracx-logic/src/diracx/logic/__main__.py @@ -93,12 +93,15 @@ async def delete_jwk(args): async def cleanup_authdb(args): """Delete expired tokens and flows from the AuthDB.""" logger.info("Deleting expired tokens and flows") + import os + from diracx.core.settings import AuthSettings from diracx.db.sql import AuthDB from diracx.logic.auth.management import cleanup_expired_data settings = AuthSettings() - db = AuthDB(args.db_url) + db_url = os.environ["DIRACX_DB_URL_AUTHDB"] + db = AuthDB(db_url) async with db.engine_context(): async with db: await cleanup_expired_data(db, settings) @@ -137,9 +140,6 @@ def parse_args(): cleanup_authdb_parser = subparsers.add_parser( "cleanup-authdb", help="Delete expired tokens and flows from the AuthDB" ) - cleanup_authdb_parser.add_argument( - "--db-url", required=True, help="URL to the AuthDB" - ) cleanup_authdb_parser.set_defaults(func=cleanup_authdb) args = parser.parse_args() diff --git a/diracx-logic/src/diracx/logic/auth/management.py b/diracx-logic/src/diracx/logic/auth/management.py index de756f687..57027ad4f 100644 --- a/diracx-logic/src/diracx/logic/auth/management.py +++ b/diracx-logic/src/diracx/logic/auth/management.py @@ -65,20 +65,22 @@ async def revoke_refresh_token_by_refresh_token( async def cleanup_expired_data(auth_db: AuthDB, settings: AuthSettings) -> None: """Remove expired data from the auth database.""" - expired_tokens, revoked_tokens = await auth_db.clean_expired_refresh_token( + expired_tokens = await auth_db.clean_expired_refresh_tokens( max_validity=settings.refresh_token_expire_minutes, - max_retention=settings.revoked_refresh_token_retention_days, ) - logger.info( - f"Deleted {expired_tokens} expired and {revoked_tokens} revoked refresh tokens" + logger.info("Deleted %d expired refresh tokens", expired_tokens) + + revoked_tokens = await auth_db.clean_revoked_refresh_tokens( + max_retention=settings.revoked_refresh_token_retention_minutes, ) + logger.info("Deleted %d revoked refresh tokens", revoked_tokens) auth = await auth_db.clean_expired_authorization_flows( max_retention=settings.completed_flow_retention_minutes, ) - logger.info(f"Deleted {auth} expired authorization flows") + logger.info("Deleted %d expired authorization flows", auth) device = await auth_db.clean_expired_device_flows( max_retention=settings.completed_flow_retention_minutes, ) - logger.info(f"Deleted {device} expired device flows") + logger.info("Deleted %d expired device flows", device) diff --git a/docs/admin/reference/env-variables.md b/docs/admin/reference/env-variables.md index 86054db9a..64c5625db 100644 --- a/docs/admin/reference/env-variables.md +++ b/docs/admin/reference/env-variables.md @@ -105,14 +105,14 @@ Expiration time in minutes for refresh tokens. The maximum lifetime of refresh tokens before they must be re-issued through a new authentication flow. Default: 60 minutes. -### `DIRACX_SERVICE_AUTH_REVOKED_REFRESH_TOKEN_RETENTION_DAYS` +### `DIRACX_SERVICE_AUTH_REVOKED_REFRESH_TOKEN_RETENTION_MINUTES` -*Optional*, default value: `30` +*Optional*, default value: `43200` -Retention time in days for revoked refresh tokens. +Retention time in minutes for revoked refresh tokens. The maximum retention time of refresh tokens after being -revoked and before they are deleted. Default: 30 days. +revoked and before they are deleted. Default: 43200 minutes (30 days). ### `DIRACX_SERVICE_AUTH_AVAILABLE_PROPERTIES` From a23e7a54c0bbcb741bd84140f786bbd94695e6ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9lo=C3=AFse=20Joffe?= Date: Wed, 18 Mar 2026 15:39:00 +0100 Subject: [PATCH 6/6] fix: remove unnecessary filter --- diracx-db/src/diracx/db/sql/auth/db.py | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/diracx-db/src/diracx/db/sql/auth/db.py b/diracx-db/src/diracx/db/sql/auth/db.py index 46233d568..caf1b0226 100644 --- a/diracx-db/src/diracx/db/sql/auth/db.py +++ b/diracx-db/src/diracx/db/sql/auth/db.py @@ -379,14 +379,6 @@ async def clean_expired_authorization_flows(self, max_retention: int) -> int: Must be bigger than authorization_flow_expiration_seconds. """ stmt_auth = delete(AuthorizationFlows).where( - AuthorizationFlows.status.in_( - [ - FlowStatus.PENDING, - FlowStatus.READY, - FlowStatus.DONE, - FlowStatus.ERROR, - ] - ), AuthorizationFlows.creation_time < substract_date(minutes=max_retention), ) res_auth = await self.conn.execute(stmt_auth) @@ -400,14 +392,6 @@ async def clean_expired_device_flows(self, max_retention: int) -> int: Must be bigger than device_flow_expiration_seconds. """ stmt_device = delete(DeviceFlows).where( - DeviceFlows.status.in_( - [ - FlowStatus.PENDING, - FlowStatus.READY, - FlowStatus.DONE, - FlowStatus.ERROR, - ] - ), DeviceFlows.creation_time < substract_date(minutes=max_retention), ) res_device = await self.conn.execute(stmt_device)