diff --git a/script/odoo/migration/check_hidden_models.py b/script/odoo/migration/check_hidden_models.py new file mode 100755 index 0000000..dc567ad --- /dev/null +++ b/script/odoo/migration/check_hidden_models.py @@ -0,0 +1,206 @@ +#!/usr/bin/env python3 +# © 2021-2026 TechnoLibre (http://www.technolibre.ca) +# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl) + +"""Les modèles qui ont des données que PLUS PERSONNE ne peut voir. + +D'où vient ce test +------------------ +Après une migration 12 → 18, les documents DMS avaient « disparu ». Ils +n'avaient pas disparu : 69 fichiers et 23 Mo étaient intacts en base. Ce +qui avait changé, c'est le modèle de sécurité — OCA DMS pose une règle +GLOBALE sur une permission accordée par un `dms.access.group`, et la +conversion depuis MuK n'en avait créé aucun, faute d'équivalent à +convertir. + +Rien ne pouvait le voir venir. Les comptages de lignes disaient « tout est +là », et c'était vrai. Le test de fumée ouvrait des pages publiques, et +elles répondaient. Le trou est exactement entre les deux : des données +présentes, et une règle qui les masque intégralement. + +Ce qu'on cherche +---------------- +Un modèle tel que : il a des lignes, il porte au moins une règle GLOBALE, +et AUCUN utilisateur interne actif n'en voit une seule. « Aucun » est le +mot important — un modèle que seul un comptable voit est normal ; un +modèle que personne ne voit est soit une refonte de sécurité ratée, soit +des données devenues inatteignables. + +Pourquoi pas en SQL +------------------- +Une règle est un domaine Odoo, parfois avec `user.`, `company_ids`, ou un +champ calculé cherchable — `permission_read` de DMS en est un, et il +déclenche une sous-requête que rien dans `ir_rule` ne laisse deviner. Il +faut l'ORM pour l'évaluer, donc le shell. + +Le super-utilisateur est exclu : les règles ne s'appliquent pas à lui, et +un comptage fait en son nom déclarerait saine une base muette. C'est +précisément l'erreur qui aurait laissé passer le cas DMS. + +Codes de sortie : 0 rien à signaler, 1 des trouvailles, 2 l'outil a échoué. +""" + +import argparse +import os +import sys + +sys.path.append( + os.path.normpath(os.path.join(os.path.dirname(__file__), "..", "..", "..")) +) + +from script.odoo.migration import database_cleanup # noqa: E402 + +try: + from script.todo.todo_i18n import t +except Exception: # pragma: no cover - repli si i18n indisponible + + def t(key: str) -> str: + return key + + +DEBUT = database_cleanup.START +FIN = database_cleanup.END + +# Ces modèles sont invisibles PAR DESSEIN : ils portent des règles globales +# et ne s'adressent à personne en particulier. Les signaler à chaque +# migration noierait la vraie trouvaille sous le bruit, et un rapport +# qu'on apprend à ignorer ne sert plus à rien. +ATTENDUS = ( + "ir.rule", + "ir.model.access", + "res.users.log", + "bus.bus", + "bus.presence", + "mail.notification", + "ir.logging", + "ir.autovacuum", +) + +SCRIPT = """ +import json +LIMITE = {limite} +ATTENDUS = set({attendus!r}) +rapport = {{"models": [], "checked": 0, "users": []}} +try: + # Les utilisateurs INTERNES actifs, sans le super-utilisateur : les + # règles ne s'appliquent pas à lui, et compter en son nom + # déclarerait saine une base que personne ne peut lire. + membres = env["res.users"].sudo().search([ + ("active", "=", True), + ("id", "!=", 1), + ("share", "=", False), + ], limit=LIMITE) + rapport["users"] = membres.mapped("login") + if not membres: + rapport["no_user"] = True + else: + vus = set( + env["ir.rule"].sudo() + .search([("global", "=", True), ("active", "=", True)]) + .mapped("model_id.model") + ) + for nom in sorted(vus - ATTENDUS): + modele = env.get(nom) + if modele is None or modele._abstract or modele._transient: + continue + try: + total = modele.sudo().search_count([]) + except Exception: + continue + if not total: + continue + rapport["checked"] += 1 + # Court-circuit : dès qu'UN utilisateur voit une ligne, le + # modèle n'est pas muet. Inutile d'interroger les autres. + visible = False + for membre in membres: + try: + if modele.with_user(membre).search([], limit=1): + visible = True + break + except Exception: + # Un refus d'accès n'est pas une ligne visible. + continue + if not visible: + rapport["models"].append({{"model": nom, "rows": total}}) +except Exception as exc: + rapport["error"] = "%s: %s" % (type(exc).__name__, exc) +print({debut!r}) +print(json.dumps(rapport)) +print({fin!r}) +""" + + +def build_script(limite=25): + return SCRIPT.format( + limite=limite, attendus=list(ATTENDUS), debut=DEBUT, fin=FIN + ) + + +def render(rapport): + if rapport.get("no_user"): + return [f"⚠ {t('No internal user to test visibility with.')}"] + muets = rapport.get("models") or [] + lignes = [ + f"🔍 {rapport.get('checked', 0)}" + f" {t('model(s) with a global rule and some data,')}" + f" {t('checked against')} {len(rapport.get('users') or [])}" + f" {t('internal user(s)')}" + ] + if not muets: + lignes.append(f" ✅ {t('Every one of them is visible to someone.')}") + return lignes + lignes.append( + f" ❌ {len(muets)} {t('model(s) nobody can see a single row of')} :" + ) + for entree in sorted(muets, key=lambda item: -item["rows"]): + lignes.append( + f" {entree['model']:<38} {entree['rows']:>8}" + f" {t('row(s)')}" + ) + lignes.append( + f" {t('The data is there; a global rule hides all of it.')}" + ) + return lignes + + +def main(argv=None): + parser = argparse.ArgumentParser( + description=( + "Report models that hold data no internal user can see," + " because a global record rule filters every row out." + ) + ) + parser.add_argument("-d", "--database", required=True) + parser.add_argument("-c", "--config", default="config.conf") + parser.add_argument( + "--users", + type=int, + default=25, + help="how many internal users to test with (default: 25)", + ) + config = parser.parse_args(argv) + + souci = database_cleanup.require_matching_version(config.database) + if souci: + print(f"❌ {souci}") + return 2 + try: + rapport = database_cleanup.run_shell( + config.database, + config.config, + build_script(config.users), + echo=lambda texte: print(f"⧖ {texte}", flush=True), + ) + except RuntimeError as exc: + print(f"❌ {exc}") + return 2 + if rapport.get("error"): + print(f"❌ {rapport['error']}") + return 2 + print("\n".join(render(rapport))) + return 1 if rapport.get("models") else 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/script/odoo/migration/dms_access_repair.py b/script/odoo/migration/dms_access_repair.py index 8982198..1407328 100755 --- a/script/odoo/migration/dms_access_repair.py +++ b/script/odoo/migration/dms_access_repair.py @@ -83,6 +83,15 @@ DRY = {dry} NOM = {nom!r} rapport = {{"dry_run": DRY}} try: + # Sans DMS, il n'y a rien à réparer et ce n'est pas une erreur : cet + # outil tourne à chaque migration vers la 13, et la plupart des bases + # n'ont jamais eu de DMS. Lever ici les ferait toutes échouer. + if "dms.file" not in env: + rapport["absent"] = True + print({debut!r}) + print(json.dumps(rapport)) + print({fin!r}) + raise SystemExit(0) Dossier = env["dms.directory"].sudo() Fichier = env["dms.file"].sudo() Groupe = env["dms.access.group"].sudo() @@ -144,6 +153,8 @@ def build_script(dry_run): def render(rapport, dry_run): + if rapport.get("absent"): + return [f"ℹ️ {t('No DMS in this database, nothing to repair.')}"] lignes = [ f"📁 {t('DMS documents in the database')} :" f" {rapport.get('files', '?')} {t('file(s)')}," @@ -220,7 +231,17 @@ def main(argv=None): print(f"❌ {rapport['error']}") return 2 print("\n".join(render(rapport, not config.apply))) + if rapport.get("absent"): + return 0 avant = rapport.get("before") or {} + if config.apply: + # Une réparation RÉUSSIE doit rendre 0. Rendre « il y a des + # trouvailles » après avoir tout remis en place ferait échouer + # n'importe quelle chaîne make qui appelle l'outil. + apres = rapport.get("after") or {} + if rapport.get("already_repaired"): + return 0 + return 0 if apres.get("files") else 2 return 1 if not avant.get("files") else 0 diff --git a/script/todo/todo_i18n.py b/script/todo/todo_i18n.py index 44f4597..4998b01 100644 --- a/script/todo/todo_i18n.py +++ b/script/todo/todo_i18n.py @@ -5855,6 +5855,42 @@ TRANSLATIONS = { "fr": "dossier(s) racine", "en": "root folder(s)", }, + "No DMS in this database, nothing to repair.": { + "fr": "Pas de DMS dans cette base, rien à réparer.", + "en": "No DMS in this database, nothing to repair.", + }, + "Every one of them is visible to someone.": { + "fr": "Chacun est visible par quelqu'un.", + "en": "Every one of them is visible to someone.", + }, + "No internal user to test visibility with.": { + "fr": "Aucun utilisateur interne pour éprouver la visibilité.", + "en": "No internal user to test visibility with.", + }, + "The data is there; a global rule hides all of it.": { + "fr": "Les données sont là ; une règle globale les masque entièrement.", + "en": "The data is there; a global rule hides all of it.", + }, + "checked against": { + "fr": "éprouvés avec", + "en": "checked against", + }, + "internal user(s)": { + "fr": "utilisateur(s) interne(s)", + "en": "internal user(s)", + }, + "model(s) nobody can see a single row of": { + "fr": "modèle(s) dont personne ne voit une seule ligne", + "en": "model(s) nobody can see a single row of", + }, + "model(s) with a global rule and some data,": { + "fr": "modèle(s) avec une règle globale et des données,", + "en": "model(s) with a global rule and some data,", + }, + "row(s)": { + "fr": "ligne(s)", + "en": "row(s)", + }, "Census": { "fr": "Recensement", "en": "Census", diff --git a/script/todo/todo_upgrade.py b/script/todo/todo_upgrade.py index 72c9836..6f68877 100755 --- a/script/todo/todo_upgrade.py +++ b/script/todo/todo_upgrade.py @@ -2727,6 +2727,23 @@ class TodoUpgrade: f"./script/addons/update_addons_all.sh {database_name_upgrade}", ) + # Au palier 13 SEULEMENT, et après la mise à jour des + # modules : c'est là que MuK DMS devient OCA DMS, et donc + # là que le modèle de sécurité change. OCA pose des règles + # GLOBALES sur `permission_read`, accordé par une + # `dms.access.group` — que la conversion ne crée pas, + # puisque MuK n'en avait aucune. Les documents restent en + # base, intacts, et plus personne ne les voit. + # + # L'outil ne fait rien s'il n'y a pas de DMS, et rien non + # plus s'il a déjà réparé : le rejouer est sans effet. + if next_version == 13: + self.run_on_terminal( + f"{PYTHON_BIN}" + " ./script/odoo/migration/dms_access_repair.py" + f" -d {database_name_upgrade} --apply" + ) + print( f"✅ -> {t('Database upgrade done for Odoo')}" f"{next_version}" @@ -2743,6 +2760,19 @@ class TodoUpgrade: self.prompt_database_cleanup(database_name_upgrade) self.prompt_smoke_public_url(database_name_upgrade) + # Le trou que ni les comptages ni le test de fumée ne + # voient : des données PRÉSENTES qu'une règle globale + # masque intégralement. Les comptages disent « tout est + # là » — et c'est vrai. Les pages publiques répondent — et + # c'est vrai aussi. Pourtant plus personne n'atteint les + # données. Mesuré sur DMS au palier 13 : 69 fichiers et + # 23 Mo intacts, zéro visible, pour tous les utilisateurs. + self.run_on_terminal( + f"{PYTHON_BIN}" + " ./script/odoo/migration/check_hidden_models.py" + f" -d {database_name_upgrade}" + ) + print(f"[y] {t('Open the server with Selenium')}") print(f"[a] {t('Open it at EVERY version bump, stop asking')}") # Une migration traverse jusqu'à six paliers. Répondre « y » diff --git a/test/test_check_hidden_models.py b/test/test_check_hidden_models.py new file mode 100644 index 0000000..75fe51f --- /dev/null +++ b/test/test_check_hidden_models.py @@ -0,0 +1,241 @@ +#!/usr/bin/env python3 +# © 2021-2026 TechnoLibre (http://www.technolibre.ca) +# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl) + +"""Le test qui aurait vu venir la disparition des documents DMS. + +Une propriété porte tout : le comptage se fait avec de VRAIS utilisateurs +internes, jamais avec le super-utilisateur. Les règles ne s'appliquent pas +à lui ; un comptage fait en son nom déclarerait saine une base que +personne ne peut lire — exactement l'erreur qui a laissé passer le cas +DMS pendant six paliers. +""" + +import ast +import io +import os +import sys +import unittest +from contextlib import redirect_stdout + +sys.path.append( + os.path.normpath(os.path.join(os.path.dirname(__file__), "..")) +) + +from script.odoo.migration import check_hidden_models as check # noqa: E402 +from script.todo import todo_i18n # noqa: E402 + + +class TestTheGeneratedScript(unittest.TestCase): + def test_it_is_valid_python(self): + ast.parse(check.build_script()) + + def test_the_superuser_is_excluded(self): + # Sans cette exclusion, tout paraît visible et le test ne sert à + # rien : c'est le cœur du sujet. + code = check.build_script() + self.assertIn('("id", "!=", 1)', code) + + def test_only_internal_users_count(self): + # Un portail ne voit presque rien par construction : l'inclure + # ferait crier au loup sur des modèles parfaitement sains. + self.assertIn('("share", "=", False)', check.build_script()) + + def test_only_active_users_count(self): + self.assertIn('("active", "=", True)', check.build_script()) + + def test_it_looks_at_global_rules_only(self): + # Une règle non globale ne s'applique qu'à certains groupes : + # qu'elle masque tout pour eux est normal. + code = check.build_script() + self.assertIn('("global", "=", True)', code) + self.assertIn('("active", "=", True)', code) + + def test_empty_models_are_skipped(self): + # Un modèle sans ligne n'a rien à cacher ; le signaler noierait la + # vraie trouvaille. + self.assertIn("if not total:", check.build_script()) + + def test_it_stops_at_the_first_user_who_sees_something(self): + # Sans court-circuit, le coût est modèles × utilisateurs sur une + # base où presque tout est visible. + self.assertIn("break", check.build_script()) + + def test_the_user_limit_is_honoured(self): + code = check.build_script(limite=7) + self.assertIn("LIMITE = 7", code) + self.assertIn("limit=LIMITE", code) + + def test_technical_models_are_excluded_by_name(self): + code = check.build_script() + for nom in ("ir.rule", "ir.model.access"): + self.assertIn(nom, code) + + def test_the_exclusion_list_never_swallows_business_models(self): + # Une exclusion trop large rendrait le test muet sans le dire. + for nom in check.ATTENDUS: + self.assertTrue( + nom.startswith(("ir.", "bus.", "res.users.log", "mail.")), + f"exclusion suspecte : {nom}", + ) + + def test_it_reuses_the_shared_sentinels(self): + from script.odoo.migration import database_cleanup + + self.assertEqual(check.DEBUT, database_cleanup.START) + self.assertEqual(check.FIN, database_cleanup.END) + + +class TestTheReport(unittest.TestCase): + def test_a_clean_database_says_so(self): + texte = "\n".join( + check.render({"models": [], "checked": 85, "users": ["a", "b"]}) + ) + self.assertIn( + todo_i18n.t("Every one of them is visible to someone."), texte + ) + + def test_a_finding_names_the_model_and_the_volume(self): + rapport = { + "models": [ + {"model": "dms.file", "rows": 69}, + {"model": "dms.directory", "rows": 16}, + ], + "checked": 85, + "users": ["a"], + } + texte = "\n".join(check.render(rapport)) + self.assertIn("dms.file", texte) + self.assertIn("69", texte) + self.assertIn( + todo_i18n.t("The data is there; a global rule hides all of it."), + texte, + ) + + def test_the_biggest_loss_comes_first(self): + rapport = { + "models": [ + {"model": "aaa.petit", "rows": 3}, + {"model": "zzz.gros", "rows": 900}, + ], + "checked": 2, + "users": ["a"], + } + texte = "\n".join(check.render(rapport)) + self.assertLess(texte.index("zzz.gros"), texte.index("aaa.petit")) + + def test_no_internal_user_is_flagged_not_called_clean(self): + # Sans utilisateur, on ne SAIT pas. Dire « tout va bien » serait + # un mensonge tranquille. + texte = "\n".join(check.render({"no_user": True})) + self.assertIn( + todo_i18n.t("No internal user to test visibility with."), texte + ) + self.assertNotIn( + todo_i18n.t("Every one of them is visible to someone."), texte + ) + + def test_every_translation_key_exists(self): + with io.open(check.__file__, encoding="utf-8") as handle: + src = handle.read() + for node in ast.walk(ast.parse(src)): + if ( + isinstance(node, ast.Call) + and isinstance(node.func, ast.Name) + and node.func.id == "t" + and node.args + and isinstance(node.args[0], ast.Constant) + ): + cle = node.args[0].value + self.assertTrue( + cle in todo_i18n.TRANSLATIONS, + f"clé sans traduction : {cle!r}", + ) + + +class TestTheExitCodes(unittest.TestCase): + def setUp(self): + from script.odoo.migration import database_cleanup + + self.cleanup = database_cleanup + self.vraie = database_cleanup.require_matching_version + self.vrai_shell = database_cleanup.run_shell + database_cleanup.require_matching_version = lambda base: None + + def tearDown(self): + self.cleanup.require_matching_version = self.vraie + self.cleanup.run_shell = self.vrai_shell + + def lance(self, rapport): + self.cleanup.run_shell = lambda *a, **k: rapport + tampon = io.StringIO() + with redirect_stdout(tampon): + code = check.main(["-d", "db"]) + return code, tampon.getvalue() + + def test_nothing_hidden_exits_zero(self): + code, _ = self.lance({"models": [], "checked": 3, "users": ["a"]}) + self.assertEqual(code, 0) + + def test_something_hidden_exits_one(self): + code, _ = self.lance( + { + "models": [{"model": "dms.file", "rows": 69}], + "checked": 3, + "users": ["a"], + } + ) + self.assertEqual(code, 1) + + def test_a_shell_error_exits_two(self): + # 2 dit « l'outil a échoué », pas « rien trouvé » : les confondre + # ferait conclure qu'une migration est saine sans l'avoir vérifiée. + code, _ = self.lance({"error": "boom"}) + self.assertEqual(code, 2) + + def test_a_version_mismatch_stops_before_opening_the_database(self): + self.cleanup.require_matching_version = lambda base: "18.0 vs 12.0" + appels = [] + self.cleanup.run_shell = lambda *a, **k: appels.append(a) or {} + with redirect_stdout(io.StringIO()): + code = check.main(["-d", "db"]) + self.assertEqual(code, 2) + self.assertEqual(appels, []) + + +class TestTheWiring(unittest.TestCase): + RACINE = os.path.normpath(os.path.join(os.path.dirname(__file__), "..")) + + def source(self): + with io.open( + os.path.join(self.RACINE, "script", "todo", "todo_upgrade.py"), + encoding="utf-8", + ) as handle: + return handle.read() + + def test_the_detector_runs_at_every_bump(self): + src = self.source() + self.assertIn("check_hidden_models.py", src) + + def test_the_dms_repair_runs_at_the_13_bump_only(self): + # MuK devient OCA DMS à ce palier-là et à aucun autre. Le lancer + # partout coûterait un démarrage d'Odoo par palier pour rien. + src = self.source() + self.assertIn( + "if next_version == 13:", src, "le garde de palier a disparu" + ) + debut = src.index("if next_version == 13:") + fin = src.index("dms_access_repair.py") + self.assertLess(debut, fin) + self.assertLess(fin - debut, 700, "le garde de palier s'est éloigné") + + def test_the_repair_is_applied_not_only_reported(self): + # Sans --apply il n'écrit rien : câblé sans, il ne réparerait + # jamais et la migration resterait cassée en silence. + src = self.source() + debut = src.index("dms_access_repair.py") + self.assertIn("--apply", src[debut : debut + 200]) + + +if __name__ == "__main__": + unittest.main() diff --git a/test/test_dms_access_repair.py b/test/test_dms_access_repair.py index bfa9f96..e5742e9 100644 --- a/test/test_dms_access_repair.py +++ b/test/test_dms_access_repair.py @@ -143,7 +143,11 @@ class TestTheReport(unittest.TestCase): and node.args and isinstance(node.args[0], ast.Constant) ): - self.assertIn(node.args[0].value, todo_i18n.TRANSLATIONS) + cle = node.args[0].value + self.assertTrue( + cle in todo_i18n.TRANSLATIONS, + f"clé sans traduction : {cle!r}", + ) class TestTheVersionGuard(unittest.TestCase):