fix(g2): Session-Revocation bei Passwortaenderung auf beiden Pfaden
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.
This commit is contained in:
@@ -85,6 +85,38 @@ def generate_csrf_token() -> str:
|
|||||||
return secrets.token_urlsafe(32)
|
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:
|
def hash_token(token: str) -> str:
|
||||||
"""SHA-256 hash a token for storage."""
|
"""SHA-256 hash a token for storage."""
|
||||||
return hashlib.sha256(token.encode()).hexdigest()
|
return hashlib.sha256(token.encode()).hexdigest()
|
||||||
|
|||||||
@@ -15,7 +15,6 @@ from app.config import get_settings
|
|||||||
from app.core.audit import log_audit
|
from app.core.audit import log_audit
|
||||||
from app.core.auth import (
|
from app.core.auth import (
|
||||||
create_session,
|
create_session,
|
||||||
get_redis,
|
|
||||||
get_session_data,
|
get_session_data,
|
||||||
hash_password,
|
hash_password,
|
||||||
hash_token,
|
hash_token,
|
||||||
@@ -328,24 +327,10 @@ class AuthService:
|
|||||||
await db.flush()
|
await db.flush()
|
||||||
|
|
||||||
# Invalidate all active Redis sessions for this user
|
# Invalidate all active Redis sessions for this user
|
||||||
try:
|
# (shared helper — same mechanism as the profile/admin change path)
|
||||||
redis = get_redis()
|
from app.core.auth import revoke_user_redis_sessions
|
||||||
# Scan for session keys and check which belong to this user
|
|
||||||
import json
|
|
||||||
|
|
||||||
async for key in redis.scan_iter(match="session:*", count=100):
|
await revoke_user_redis_sessions(user.id)
|
||||||
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)
|
|
||||||
|
|
||||||
# Audit log entry for password reset — use separate API session (crm_api)
|
# Audit log entry for password reset — use separate API session (crm_api)
|
||||||
# to avoid requiring audit_log INSERT grants on crm_auth
|
# to avoid requiring audit_log INSERT grants on crm_auth
|
||||||
|
|||||||
@@ -201,6 +201,14 @@ class UserService:
|
|||||||
user.password_hash = hash_password(new_password)
|
user.password_hash = hash_password(new_password)
|
||||||
|
|
||||||
await db.flush()
|
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
|
return user, user_tenant
|
||||||
|
|
||||||
async def delete_user(
|
async def delete_user(
|
||||||
|
|||||||
Reference in New Issue
Block a user