Merge fix/users-secret-leak: stop leaking password hashes + TOTP secrets (#870)

Co-Authored-By: Gerald KERMA <devel@cybermind.fr>

packages/secubox-users/api/main.py

@@ -39,6 +39,8 @@ config = get_config("users") if callable(get_config) else {}
 USERSCTL = "/usr/sbin/usersctl"
 USERS_FILE = os.environ.get("USERS_FILE", "/etc/secubox/users.json")
 ROLES_FILE = os.environ.get("ROLES_FILE", "/etc/secubox/roles.json")
+from .redact import redact_user  # expurgation des secrets (module dédié, testable seul)
+
 SERVICES = ["nextcloud", "gitea", "email", "matrix", "jellyfin", "peertube", "jabber"]
 # YaCy has a single admin account (no per-user accounts), so its password is
 # synced from exactly one SecuBox user: the master "admin". Changing that user's
@@ -502,12 +504,10 @@ async def get_access():
 
 @app.get("/users", dependencies=[Depends(require_jwt)])
 async def list_users():
-    """List all users."""
+    """List all users (redacted — never ships hashes or TOTP secrets)."""
     data = load_users()
-    return {
-        "users": data.get("users", []),
-        "total": len(data.get("users", []))
-    }
+    users = [redact_user(u) for u in data.get("users", [])]
+    return {"users": users, "total": len(users)}
 
 @app.get("/user/{username}", dependencies=[Depends(require_jwt)])
 async def get_user(username: str):
@@ -515,11 +515,12 @@ async def get_user(username: str):
     data = load_users()
     for user in data.get("users", []):
         if user.get("username") == username:
+            out = redact_user(user)
             # Add service status
-            user["service_status"] = {}
+            out["service_status"] = {}
             for svc in user.get("services", []):
-                user["service_status"][svc] = check_service(svc)
-            return user
+                out["service_status"][svc] = check_service(svc)
+            return out
     raise HTTPException(status_code=404, detail="User not found")
 
 @app.post("/user", dependencies=[Depends(require_jwt)])
@@ -1064,14 +1065,20 @@ async def validate_acl(entries: List[ACLEntry]):
 
 @app.get("/export", dependencies=[Depends(require_jwt)])
 async def export_users():
-    """Export all users."""
+    """Export all users (redacted).
+
+    This route previously claimed to "remove sensitive data" while dropping only
+    `provision_results` — it shipped every password hash and TOTP secret into a
+    downloadable file. A comment asserting a guarantee the code does not provide
+    is worse than no comment: it stops reviewers from looking. The guarantee now
+    lives in redact_user(), which is the single place that decides what a client
+    may see.
+    """
     data = load_users()
-    # Remove sensitive data
-    export_data = {"users": [], "groups": data.get("groups", [])}
-    for user in data.get("users", []):
-        export_user = {k: v for k, v in user.items() if k != "provision_results"}
-        export_data["users"].append(export_user)
-    return export_data
+    return {
+        "users": [redact_user(u) for u in data.get("users", [])],
+        "groups": data.get("groups", []),
+    }
 
 @app.post("/import", dependencies=[Depends(require_permission("system.import"))])
 async def import_users(file: UploadFile = File(...)):

packages/secubox-users/api/redact.py

@@ -0,0 +1,51 @@
+# SPDX-License-Identifier: LicenseRef-CMSD-1.0
+# Copyright (c) 2026 CyberMind — Gérald Kerma <devel@cybermind.fr>
+# Source-Disclosed License — All rights reserved except as expressly granted.
+# See LICENCE-CMSD-1.0.md for terms.
+
+"""
+SecuBox-Deb :: users — expurgation des secrets avant sortie du processus
+CyberMind — https://cybermind.fr
+
+Module séparé, sans dépendance : `api.main` lit la config au niveau module, donc
+l'importer exige de pouvoir lire /etc/secubox/secubox.conf. Une règle de sécurité
+doit rester testable sans privilèges — d'où ce fichier, importable seul.
+"""
+from __future__ import annotations
+
+# Jamais renvoyés à un client. `password_hash` est une primitive d'attaque
+# hors-ligne ; `provision_results` peut contenir des retours de service bruts.
+SECRET_KEYS = ("password_hash", "provision_results")
+
+# Dans le sous-dict `totp` : `secret` permet à lui seul de forger des codes à
+# 6 chiffres indéfiniment (contournement complet du MFA) ; `backup_codes` porte
+# les hashes argon2id des codes de secours ; `last_step` est un détail interne.
+TOTP_SECRET_KEYS = ("secret", "backup_codes", "last_step")
+
+
+def redact_user(user: dict) -> dict:
+    """Copie d'un enregistrement utilisateur sans aucun secret.
+
+    users.json contient le hash argon2id du mot de passe, le SECRET TOTP et les
+    hashes des codes de secours. Aucun n'a d'usage côté client.
+
+    Toute route renvoyant un utilisateur DOIT passer par ici. Ces routes sont
+    gardées par `require_jwt` — n'importe quel compte authentifié, pas seulement
+    un admin — donc un enregistrement non expurgé transforme le compte le moins
+    privilégié en moissonneuse d'identifiants de tous les autres.
+
+    `totp` est conservé mais réduit à ses métadonnées non secrètes, et sa
+    PRÉSENCE est préservée : la webui décide d'afficher le badge MFA par simple
+    test de véracité (`u.totp_enabled || u.totp || u.mfa_enabled`), donc
+    supprimer la clé éteindrait le badge en silence, et en émettre une toujours
+    l'allumerait pour des utilisateurs jamais enrôlés.
+
+    Ne mute jamais l'entrée : elle vient de users.json et serait réécrite telle
+    quelle au prochain enregistrement.
+    """
+    out = {k: v for k, v in user.items() if k not in SECRET_KEYS}
+    totp = user.get("totp")
+    if isinstance(totp, dict):
+        out["totp"] = {k: v for k, v in totp.items() if k not in TOTP_SECRET_KEYS}
+    out["totp_enabled"] = bool(isinstance(totp, dict) and totp.get("enabled"))
+    return out

packages/secubox-users/tests/test_redaction.py

@@ -0,0 +1,104 @@
+# SPDX-License-Identifier: LicenseRef-CMSD-1.0
+# Copyright (c) 2026 CyberMind — Gérald Kerma <devel@cybermind.fr>
+# Source-Disclosed License — All rights reserved except as expressly granted.
+# See LICENCE-CMSD-1.0.md for terms.
+
+"""
+SecuBox-Deb :: users — les secrets ne doivent jamais quitter le processus.
+
+users.json contient le hash argon2id du mot de passe, le SECRET TOTP et les
+hashes des codes de secours. Les routes qui renvoient un utilisateur sont
+gardées par `require_jwt` — donc n'importe quel compte authentifié, pas
+seulement un admin. Un enregistrement non expurgé transforme le compte le moins
+privilégié en moissonneuse d'identifiants.
+"""
+from api.redact import redact_user
+
+# La forme réelle observée dans /etc/secubox/users.json sur la board.
+REAL_SHAPE = {
+    "username": "admin",
+    "email": "gandalf@gk2.net",
+    "role": "admin",
+    "enabled": True,
+    "created": "2026-05-09T11:14:11.719355",
+    "last_login": "2026-07-17T09:01:36.952678+00:00",
+    "must_change_password": False,
+    "google": None,
+    "services": [],
+    "password_hash": "$argon2id$v=19$m=1024,t=1,p=1$c2FsdA$hash",
+    "totp": {
+        "enabled": True,
+        "enrolled_at": "2026-05-09T11:20:00",
+        "secret": "JBSWY3DPEHPK3PXP",
+        "last_step": 59475852,
+        "backup_codes": [{"hash": "$argon2id$v=19$m=65536,t=3,p=4$abc$def"}],
+    },
+}
+
+
+def _flat(obj) -> str:
+    """Sérialisation grossière : un secret imbriqué reste détectable."""
+    import json
+    return json.dumps(obj)
+
+
+def test_password_hash_never_leaves():
+    assert "password_hash" not in redact_user(REAL_SHAPE)
+
+
+def test_totp_secret_never_leaves():
+    # Le pire des trois : le secret TOTP seul permet de forger des codes à 6
+    # chiffres indéfiniment — un contournement complet du MFA.
+    out = redact_user(REAL_SHAPE)
+    assert "secret" not in out["totp"]
+    assert "JBSWY3DPEHPK3PXP" not in _flat(out)
+
+
+def test_backup_code_hashes_never_leave():
+    out = redact_user(REAL_SHAPE)
+    assert "backup_codes" not in out["totp"]
+    assert "argon2id" not in _flat(out)
+
+
+def test_no_argon2_hash_survives_anywhere():
+    # Filet global : attrape tout futur champ portant un hash.
+    assert "$argon2" not in _flat(redact_user(REAL_SHAPE))
+
+
+def test_fields_the_panel_needs_are_preserved():
+    out = redact_user(REAL_SHAPE)
+    for k in ("username", "email", "role", "enabled", "created",
+              "last_login", "must_change_password", "services"):
+        assert k in out, f"le panel a besoin de {k}"
+
+
+def test_totp_presence_is_preserved_for_the_mfa_badge():
+    # Le panel décide du badge MFA par simple véracité
+    # (`u.totp_enabled || u.totp || u.mfa_enabled`) : supprimer la clé
+    # éteindrait le badge en silence.
+    out = redact_user(REAL_SHAPE)
+    assert out["totp"]            # truthy
+    assert out["totp"]["enabled"] is True
+    assert out["totp_enabled"] is True
+
+
+def test_user_without_totp_does_not_gain_a_badge():
+    # Symétrique : ne jamais allumer le badge d'un utilisateur non enrôlé.
+    out = redact_user({"username": "gk2", "password_hash": "$argon2id$x"})
+    assert out.get("totp") is None
+    assert out["totp_enabled"] is False
+
+
+def test_totp_present_but_disabled_reports_not_enabled():
+    out = redact_user({"username": "u", "totp": {"enabled": False, "secret": "S"}})
+    assert out["totp_enabled"] is False
+    assert "secret" not in out["totp"]
+
+
+def test_redaction_does_not_mutate_the_stored_record():
+    # redact_user reçoit l'objet chargé depuis users.json ; le muter
+    # corromprait la base au prochain save.
+    import copy
+    original = copy.deepcopy(REAL_SHAPE)
+    redact_user(REAL_SHAPE)
+    assert REAL_SHAPE == original