From 0baec2792c925324243ccb5d192303c838c85d82 Mon Sep 17 00:00:00 2001 From: Agent Zero Date: Wed, 26 Aug 2026 00:00:19 +0200 Subject: [PATCH] fix(g2): Session-Revocation bei Passwortaenderung auf beiden Pfaden MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Befund differenzierter als Plan annahm: Reset-via-Token revocierte Sessions bereits korrekt, aber Profil-/Admin-Pfad (users.py PATCH -> update_user mit new_password) liess alle anderen Sessions aktiv — ein Angreifer mit gestohlener Session blieb aktiv. Fix nach DRY: revoke_user_redis_sessions(user_id)-Helper in app/core/auth.py extrahiert (scan_iter session:* + user_id-Match + delete, never-raises), von beiden Pfaden genutzt: confirm_password_reset ersetzt den Inline-Duplikat-Block, update_user ruft den Helper wenn new_password gesetzt wurde. Postgres sessions-Tabelle bleibt unberuehrt (Audit-Trail by Design, Redis ist Runtime-Store). Beweis: auth+user_service+rbac_comprehensive 120/120 gruen in 144s; ruff clean. --- app/core/auth.py | 32 ++++++++++++++++++++++++++++++++ app/services/auth_service.py | 21 +++------------------ app/services/user_service.py | 8 ++++++++ 3 files changed, 43 insertions(+), 18 deletions(-) diff --git a/app/core/auth.py b/app/core/auth.py index 5f8dea2..f163c85 100644 --- a/app/core/auth.py +++ b/app/core/auth.py @@ -85,6 +85,38 @@ def generate_csrf_token() -> str: return secrets.token_urlsafe(32) +async def revoke_user_redis_sessions(user_id: str | uuid.UUID) -> int: + """Delete every active Redis session belonging to the user (G2). + + Shared by both password-change paths (token reset + profile/admin change): + after a password change, stolen or lingering sessions must die. + + Returns the number of deleted session keys. Never raises — a Redis outage + must not break the password change itself. + """ + try: + redis = get_redis() + deleted = 0 + async for key in redis.scan_iter(match="session:*", count=100): + raw = await redis.get(key) + if raw is None: + continue + try: + import json + + session_data = json.loads(raw) + except (json.JSONDecodeError, TypeError): + continue + if session_data.get("user_id") == str(user_id): + await redis.delete(key) + deleted += 1 + logger.info("Deleted session %s for user %s", key, user_id) + return deleted + except Exception: + logger.warning("Failed to invalidate Redis sessions for user %s", user_id, exc_info=True) + return 0 + + def hash_token(token: str) -> str: """SHA-256 hash a token for storage.""" return hashlib.sha256(token.encode()).hexdigest() diff --git a/app/services/auth_service.py b/app/services/auth_service.py index 523736f..d9621e1 100644 --- a/app/services/auth_service.py +++ b/app/services/auth_service.py @@ -15,7 +15,6 @@ from app.config import get_settings from app.core.audit import log_audit from app.core.auth import ( create_session, - get_redis, get_session_data, hash_password, hash_token, @@ -328,24 +327,10 @@ class AuthService: await db.flush() # Invalidate all active Redis sessions for this user - try: - redis = get_redis() - # Scan for session keys and check which belong to this user - import json + # (shared helper — same mechanism as the profile/admin change path) + from app.core.auth import revoke_user_redis_sessions - async for key in redis.scan_iter(match="session:*", count=100): - raw = await redis.get(key) - if raw is None: - continue - try: - session_data = json.loads(raw) - except (json.JSONDecodeError, TypeError): - continue - if session_data.get("user_id") == str(user.id): - await redis.delete(key) - logger.info("Deleted session %s for user %s after password reset", key, user.id) - except Exception: - logger.warning("Failed to invalidate Redis sessions for user %s", user.id, exc_info=True) + await revoke_user_redis_sessions(user.id) # Audit log entry for password reset — use separate API session (crm_api) # to avoid requiring audit_log INSERT grants on crm_auth diff --git a/app/services/user_service.py b/app/services/user_service.py index 7a45391..62036a9 100644 --- a/app/services/user_service.py +++ b/app/services/user_service.py @@ -201,6 +201,14 @@ class UserService: user.password_hash = hash_password(new_password) await db.flush() + + # G2: after a profile/admin password change, kill all other sessions — + # a stolen or lingering session must not survive the change. + if new_password is not None: + from app.core.auth import revoke_user_redis_sessions + + await revoke_user_redis_sessions(user.id) + return user, user_tenant async def delete_user(