From 0726e15c9722f7520fd8d2c26b81112786af82d6 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Thu, 23 Jul 2026 12:58:31 +0800 Subject: [PATCH 1/7] test(security): pin that key admin group must not grant system admin TDD red: group_key_admin currently implies base.group_system, escalating any Key Management Admin to full Settings/System admin. New tests assert the implication is gone (direct and transitive), that admin implies operator instead, that key admins keep field access to the KMS-wrapped key material (the AWS/Azure providers read it in user context), and that the 19.0.2.0.1 migration strips the link from existing databases. --- spp_key_management/tests/__init__.py | 1 + .../tests/test_key_admin_group.py | 116 ++++++++++++++++++ 2 files changed, 117 insertions(+) create mode 100644 spp_key_management/tests/test_key_admin_group.py diff --git a/spp_key_management/tests/__init__.py b/spp_key_management/tests/__init__.py index 9c906e8c9..bb85ec63b 100644 --- a/spp_key_management/tests/__init__.py +++ b/spp_key_management/tests/__init__.py @@ -3,3 +3,4 @@ from . import test_key_manager from . import test_asymmetric_key from . import test_security +from . import test_key_admin_group diff --git a/spp_key_management/tests/test_key_admin_group.py b/spp_key_management/tests/test_key_admin_group.py new file mode 100644 index 000000000..c7048bfe6 --- /dev/null +++ b/spp_key_management/tests/test_key_admin_group.py @@ -0,0 +1,116 @@ +# Part of OpenSPP. See LICENSE file for full copyright and licensing details. +"""Tests for the Key Management security groups.""" + +import importlib.util +from pathlib import Path + +from odoo import Command +from odoo.exceptions import AccessError +from odoo.tests import TransactionCase, tagged + + +@tagged("post_install", "-at_install") +class TestKeyAdminGroup(TransactionCase): + """Key Management Admin gates control of the platform's encryption + keys (providers, rotation, key records). + + The group must NEVER imply base.group_system: implied_ids grants the + implied groups to members, so such a link would turn the key + management role into full Settings/System administration. + """ + + def _create_user(self, login, groups): + return self.env["res.users"].create( + { + "name": login, + "login": login, + "group_ids": [Command.link(g.id) for g in groups], + } + ) + + def test_key_admin_group_exists(self): + group = self.env.ref("spp_key_management.group_key_admin", raise_if_not_found=False) + self.assertTrue(group, "spp_key_management.group_key_admin must exist") + self.assertEqual(group.name, "Key Management Admin") + + def test_key_admin_does_not_grant_system(self): + """Granting Key Management Admin must not escalate to system admin.""" + group = self.env.ref("spp_key_management.group_key_admin") + system = self.env.ref("base.group_system") + self.assertNotIn(system, group.implied_ids) + # Guard the transitive closure too, so the escalation cannot come + # back through an intermediate group. + self.assertNotIn(system, group.all_implied_ids) + + user = self._create_user( + "key_admin_only", + [self.env.ref("base.group_user"), group], + ) + self.assertTrue(user.has_group("spp_key_management.group_key_admin")) + self.assertFalse( + user.has_group("base.group_system"), + "Key Management Admin membership must not confer system administration", + ) + + def test_key_admin_implies_operator(self): + """Admin is a superset of operator (and stays an internal user).""" + group = self.env.ref("spp_key_management.group_key_admin") + operator = self.env.ref("spp_key_management.group_key_operator_officer") + internal = self.env.ref("base.group_user") + self.assertIn(operator, group.all_implied_ids) + self.assertIn(internal, group.all_implied_ids) + + def test_key_admin_reads_encrypted_key(self): + """Key admins keep field access to the KMS-wrapped key material + (the AWS/Azure providers read it in the calling user's context); + operators do not.""" + key = self.env["spp.encryption.key"].create( + { + "key_id": "test-field-gate", + "encrypted_key": "d3JhcHBlZA==", + } + ) + admin_user = self._create_user( + "key_admin_field", + [ + self.env.ref("base.group_user"), + self.env.ref("spp_key_management.group_key_admin"), + ], + ) + operator_user = self._create_user( + "key_operator_field", + [ + self.env.ref("base.group_user"), + self.env.ref("spp_key_management.group_key_operator_officer"), + ], + ) + self.assertEqual(key.with_user(admin_user).encrypted_key, "d3JhcHBlZA==") + with self.assertRaises(AccessError): + key.with_user(operator_user).encrypted_key # noqa: B018 + + def test_migration_removes_escalation(self): + """The 19.0.2.0.1 migration strips base.group_system from the + group on databases installed before the fix (removing the XML + line alone never unlinks an existing relation).""" + migration_path = ( + Path(__file__).parent.parent / "migrations" / "19.0.2.0.1" / "post-migration.py" + ) + spec = importlib.util.spec_from_file_location( + "spp_key_management_migration_19_0_2_0_1", migration_path + ) + migration = importlib.util.module_from_spec(spec) + spec.loader.exec_module(migration) + + group = self.env.ref("spp_key_management.group_key_admin") + system = self.env.ref("base.group_system") + # Recreate the released (vulnerable) state. + group.write({"implied_ids": [Command.link(system.id)]}) + self.assertIn(system, group.implied_ids) + + with self.assertLogs("spp_key_management_migration_19_0_2_0_1", level="WARNING") as capture: + migration.migrate(self.env.cr, "19.0.2.0.0") + self.assertIn("Removed the base.group_system implication", capture.output[0]) + + group.invalidate_recordset() + self.assertNotIn(system, group.implied_ids) + self.assertNotIn(system, group.all_implied_ids) From c8cad79135c8ae83b2932377230b8d24ff4d0d1e Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Thu, 23 Jul 2026 14:02:59 +0800 Subject: [PATCH 2/7] security(key_management): stop Key Management Admin from granting system admin spp_key_management.group_key_admin listed base.group_system in implied_ids. In Odoo, implied_ids grants the implied groups to members - it is not a membership precondition - so assigning the key management role silently made the user a full Settings/System administrator. - replace the base.group_system link with group_key_operator_officer (admin is a superset of operator and stays an internal user); system admins keep access through their own explicit ACL rows on every key model, so nothing is lost and nothing new is granted - extend the encrypted_key field gate to include group_key_admin: the AWS KMS / Azure Key Vault providers read the cached wrapped key in the calling user's context, which previously only worked because key admins were escalated to system; the field holds KMS-wrapped ciphertext, never plaintext key material - ship migrations/19.0.2.0.1/post-migration.py to strip the unsafe link from databases installed from the v19.0.2.0.0 / 2026.07 releases (removing the XML line alone never unlinks an existing relation); membership is a computed transitive closure, so removal revokes the escalation immediately - reorder the groups file so the operator group is defined before the admin group that now references it - bump to 19.0.2.0.1 with HISTORY fragment --- spp_key_management/__manifest__.py | 2 +- .../migrations/19.0.2.0.1/post-migration.py | 43 +++++++++++++++++++ spp_key_management/models/encryption_key.py | 6 ++- spp_key_management/readme/HISTORY.md | 10 +++++ .../security/security_groups.xml | 32 +++++++++----- .../tests/test_key_admin_group.py | 8 +--- 6 files changed, 82 insertions(+), 19 deletions(-) create mode 100644 spp_key_management/migrations/19.0.2.0.1/post-migration.py diff --git a/spp_key_management/__manifest__.py b/spp_key_management/__manifest__.py index 12b625afc..618f7e364 100644 --- a/spp_key_management/__manifest__.py +++ b/spp_key_management/__manifest__.py @@ -4,7 +4,7 @@ "name": "OpenSPP Key Management", "summary": "Centralized cryptographic key management with pluggable providers", "category": "OpenSPP/Identity", - "version": "19.0.2.0.0", + "version": "19.0.2.0.1", "sequence": 1, "author": "OpenSPP.org", "website": "https://github.com/OpenSPP/OpenSPP2", diff --git a/spp_key_management/migrations/19.0.2.0.1/post-migration.py b/spp_key_management/migrations/19.0.2.0.1/post-migration.py new file mode 100644 index 000000000..6f5fd68d5 --- /dev/null +++ b/spp_key_management/migrations/19.0.2.0.1/post-migration.py @@ -0,0 +1,43 @@ +# Part of OpenSPP. See LICENSE file for full copyright and licensing details. +"""Remove the base.group_system implication from the Key Management Admin group. + +Earlier versions shipped ``spp_key_management.group_key_admin`` with +``implied_ids`` linking ``base.group_system``. ``implied_ids`` GRANTS the +implied groups to members, so any user given the key management role +silently became a full Settings/System administrator. Removing the link +from the XML only stops adding it on fresh installs - it never removes an +existing relation - so strip it here. Odoo computes effective membership +as a transitive closure of ``implied_ids``, so removing the link +immediately revokes the escalated privileges from affected users. (The +replacement implication - admin implies operator - is applied by the +regular data load; the groups file is not noupdate.) +""" + +import logging + +from odoo import SUPERUSER_ID, Command, api + +_logger = logging.getLogger(__name__) + + +def migrate(cr, version): + if not version: + return + + env = api.Environment(cr, SUPERUSER_ID, {}) + group = env.ref("spp_key_management.group_key_admin", raise_if_not_found=False) + system = env.ref("base.group_system", raise_if_not_found=False) + if not group or not system or system not in group.implied_ids: + return + + affected = len(group.user_ids) + group.write({"implied_ids": [Command.unlink(system.id)]}) + _logger.warning( + "Removed the base.group_system implication from the Key Management " + "Admin group; %s user(s) held the group and lose the transitively " + "granted system administration rights. Audit changes made by these " + "users while escalated, grant base.group_system explicitly where it " + "is genuinely intended, and consider rotating data keys if key " + "custody policy separates platform admins from key admins.", + affected, + ) diff --git a/spp_key_management/models/encryption_key.py b/spp_key_management/models/encryption_key.py index 27ee771c0..478b75337 100644 --- a/spp_key_management/models/encryption_key.py +++ b/spp_key_management/models/encryption_key.py @@ -43,7 +43,11 @@ class EncryptionKey(models.Model): ) encrypted_key = fields.Char( required=True, - groups="base.group_system", + # Key admins need field access because the AWS KMS / Azure Key Vault + # providers read the cached wrapped key in the calling user's + # context. The value is ciphertext (wrapped by the KMS); plaintext + # key material is never stored here. + groups="base.group_system,spp_key_management.group_key_admin", help="Base64-encoded encrypted key material", ) algorithm = fields.Char( diff --git a/spp_key_management/readme/HISTORY.md b/spp_key_management/readme/HISTORY.md index 4aaf9afef..c04597d47 100644 --- a/spp_key_management/readme/HISTORY.md +++ b/spp_key_management/readme/HISTORY.md @@ -1,3 +1,13 @@ +### 19.0.2.0.1 + +- fix(security): stop the Key Management Admin group from granting full + system administration. `implied_ids` grants the implied groups to members, + so the previous `base.group_system` link escalated any holder of the key + management role to a Settings/System administrator. The admin group now + implies Key Operator instead, key admins keep field access to the + KMS-wrapped key material the cloud providers read in user context, and a + migration strips the unsafe link from existing databases. + ### 19.0.2.0.0 - Initial migration to OpenSPP2 diff --git a/spp_key_management/security/security_groups.xml b/spp_key_management/security/security_groups.xml index f5123ced8..329cb1e68 100644 --- a/spp_key_management/security/security_groups.xml +++ b/spp_key_management/security/security_groups.xml @@ -15,17 +15,9 @@ 20 - - - Key Management Admin - - Full access to key management. Can configure providers, rotate keys, and view all key operations. - - - - + Key Operator @@ -34,4 +26,22 @@ >Can use encryption keys for operations but cannot manage providers or rotate keys. + + + + Key Management Admin + + Full access to key management. Can configure providers, rotate keys, and view all key operations. + + diff --git a/spp_key_management/tests/test_key_admin_group.py b/spp_key_management/tests/test_key_admin_group.py index c7048bfe6..8b2b322c3 100644 --- a/spp_key_management/tests/test_key_admin_group.py +++ b/spp_key_management/tests/test_key_admin_group.py @@ -92,12 +92,8 @@ def test_migration_removes_escalation(self): """The 19.0.2.0.1 migration strips base.group_system from the group on databases installed before the fix (removing the XML line alone never unlinks an existing relation).""" - migration_path = ( - Path(__file__).parent.parent / "migrations" / "19.0.2.0.1" / "post-migration.py" - ) - spec = importlib.util.spec_from_file_location( - "spp_key_management_migration_19_0_2_0_1", migration_path - ) + migration_path = Path(__file__).parent.parent / "migrations" / "19.0.2.0.1" / "post-migration.py" + spec = importlib.util.spec_from_file_location("spp_key_management_migration_19_0_2_0_1", migration_path) migration = importlib.util.module_from_spec(spec) spec.loader.exec_module(migration) From c632c3ae0d08a4bbcee4f37a53590caf0536e54e Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Thu, 23 Jul 2026 14:18:07 +0800 Subject: [PATCH 3/7] docs(spp_key_management): regenerate README from fragments (CI oca-gen output) --- spp_key_management/README.rst | 12 ++++++++++++ spp_key_management/static/description/index.html | 13 +++++++++++++ 2 files changed, 25 insertions(+) diff --git a/spp_key_management/README.rst b/spp_key_management/README.rst index eb9a3a265..7f03dbe5a 100644 --- a/spp_key_management/README.rst +++ b/spp_key_management/README.rst @@ -165,6 +165,18 @@ External Python dependencies: ``cryptography`` Changelog ========= +19.0.2.0.1 +~~~~~~~~~~ + +- fix(security): stop the Key Management Admin group from granting full + system administration. ``implied_ids`` grants the implied groups to + members, so the previous ``base.group_system`` link escalated any + holder of the key management role to a Settings/System administrator. + The admin group now implies Key Operator instead, key admins keep + field access to the KMS-wrapped key material the cloud providers read + in user context, and a migration strips the unsafe link from existing + databases. + 19.0.2.0.0 ~~~~~~~~~~ diff --git a/spp_key_management/static/description/index.html b/spp_key_management/static/description/index.html index 10418817f..9e8c64fff 100644 --- a/spp_key_management/static/description/index.html +++ b/spp_key_management/static/description/index.html @@ -548,6 +548,19 @@

Changelog

+

19.0.2.0.1

+
    +
  • fix(security): stop the Key Management Admin group from granting full +system administration. implied_ids grants the implied groups to +members, so the previous base.group_system link escalated any +holder of the key management role to a Settings/System administrator. +The admin group now implies Key Operator instead, key admins keep +field access to the KMS-wrapped key material the cloud providers read +in user context, and a migration strips the unsafe link from existing +databases.
    • +
+
+

19.0.2.0.0

  • Initial migration to OpenSPP2
    • From 585ca36418d1b72411132d37ff090e37837b8e82 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Thu, 23 Jul 2026 14:34:54 +0800 Subject: [PATCH 4/7] test(security): pin that key admins can reach the Key Management menu TDD red: the menu root is parented under Settings, which Odoo only shows to base.group_erp_manager; load_menus prunes menus whose ancestors are invisible, so a user holding only the key admin role has no way to reach Key Management in the UI once the group no longer escalates to system admin. --- .../tests/test_key_admin_group.py | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/spp_key_management/tests/test_key_admin_group.py b/spp_key_management/tests/test_key_admin_group.py index 8b2b322c3..4ebee9473 100644 --- a/spp_key_management/tests/test_key_admin_group.py +++ b/spp_key_management/tests/test_key_admin_group.py @@ -88,6 +88,29 @@ def test_key_admin_reads_encrypted_key(self): with self.assertRaises(AccessError): key.with_user(operator_user).encrypted_key # noqa: B018 + def test_key_admin_sees_key_management_menu(self): + """A user holding only the key admin role must be able to reach the + Key Management menu. Odoo drops menus whose ancestors are invisible, + so parenting under Settings (base.group_erp_manager) would hide the + whole subtree from dedicated key-custody users.""" + user = self._create_user( + "key_admin_menu", + [ + self.env.ref("base.group_user"), + self.env.ref("spp_key_management.group_key_admin"), + ], + ) + root = self.env.ref("spp_key_management.menu_key_management_root") + # load_menus is what the web client uses; unlike _visible_menu_ids + # it also prunes menus whose ancestors are invisible ("not related + # to an app"), which is exactly the failure mode being pinned. + menus = self.env["ir.ui.menu"].with_user(user).load_menus(False) + self.assertIn( + root.id, + menus, + "Key Management menu must be reachable for key admins", + ) + def test_migration_removes_escalation(self): """The 19.0.2.0.1 migration strips base.group_system from the group on databases installed before the fix (removing the XML From d93c5a8bf9b7c7d0b93fe4a01e95433284df4410 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Thu, 23 Jul 2026 14:38:54 +0800 Subject: [PATCH 5/7] security(key_management): make Key Management menu reachable for key admins Review follow-ups: - move the Key Management root menu from Settings to top level: the Settings root is gated to base.group_erp_manager and Odoo prunes menus with invisible ancestors, so dedicated key admins (no longer system admins) had no way to reach the menu; disclosed in HISTORY - the encrypted_key field-gate comment also names GCP KMS, which reads the cached wrapped key in user context like AWS/Azure - the migration logs all_user_ids (includes members of any implying group), and its test now pins idempotence with assertNoLogs --- .../migrations/19.0.2.0.1/post-migration.py | 4 +++- spp_key_management/models/encryption_key.py | 8 ++++---- spp_key_management/readme/HISTORY.md | 5 ++++- spp_key_management/tests/test_key_admin_group.py | 4 ++++ spp_key_management/views/menu.xml | 8 ++++++-- 5 files changed, 21 insertions(+), 8 deletions(-) diff --git a/spp_key_management/migrations/19.0.2.0.1/post-migration.py b/spp_key_management/migrations/19.0.2.0.1/post-migration.py index 6f5fd68d5..92b2bbcd5 100644 --- a/spp_key_management/migrations/19.0.2.0.1/post-migration.py +++ b/spp_key_management/migrations/19.0.2.0.1/post-migration.py @@ -30,7 +30,9 @@ def migrate(cr, version): if not group or not system or system not in group.implied_ids: return - affected = len(group.user_ids) + # all_user_ids also counts members of any group implying key admin, so + # the log reflects everyone who actually held the escalation. + affected = len(group.all_user_ids) group.write({"implied_ids": [Command.unlink(system.id)]}) _logger.warning( "Removed the base.group_system implication from the Key Management " diff --git a/spp_key_management/models/encryption_key.py b/spp_key_management/models/encryption_key.py index 478b75337..f80f5a6ea 100644 --- a/spp_key_management/models/encryption_key.py +++ b/spp_key_management/models/encryption_key.py @@ -43,10 +43,10 @@ class EncryptionKey(models.Model): ) encrypted_key = fields.Char( required=True, - # Key admins need field access because the AWS KMS / Azure Key Vault - # providers read the cached wrapped key in the calling user's - # context. The value is ciphertext (wrapped by the KMS); plaintext - # key material is never stored here. + # Key admins need field access because the AWS KMS, Azure Key Vault + # and GCP KMS providers read the cached wrapped key in the calling + # user's context. The value is ciphertext (wrapped by the KMS); + # plaintext key material is never stored here. groups="base.group_system,spp_key_management.group_key_admin", help="Base64-encoded encrypted key material", ) diff --git a/spp_key_management/readme/HISTORY.md b/spp_key_management/readme/HISTORY.md index c04597d47..cbb5293f9 100644 --- a/spp_key_management/readme/HISTORY.md +++ b/spp_key_management/readme/HISTORY.md @@ -6,7 +6,10 @@ management role to a Settings/System administrator. The admin group now implies Key Operator instead, key admins keep field access to the KMS-wrapped key material the cloud providers read in user context, and a - migration strips the unsafe link from existing databases. + migration strips the unsafe link from existing databases. The Key + Management menu moved from Settings to a top-level menu: the Settings + root is only visible to ERP managers, so key admins who are no longer + system administrators could not reach it there. ### 19.0.2.0.0 diff --git a/spp_key_management/tests/test_key_admin_group.py b/spp_key_management/tests/test_key_admin_group.py index 4ebee9473..7cab34513 100644 --- a/spp_key_management/tests/test_key_admin_group.py +++ b/spp_key_management/tests/test_key_admin_group.py @@ -133,3 +133,7 @@ def test_migration_removes_escalation(self): group.invalidate_recordset() self.assertNotIn(system, group.implied_ids) self.assertNotIn(system, group.all_implied_ids) + + # Idempotent: a second run finds the link absent and stays silent. + with self.assertNoLogs("spp_key_management_migration_19_0_2_0_1", level="WARNING"): + migration.migrate(self.env.cr, "19.0.2.0.0") diff --git a/spp_key_management/views/menu.xml b/spp_key_management/views/menu.xml index b9d7d56a2..16bb508c7 100644 --- a/spp_key_management/views/menu.xml +++ b/spp_key_management/views/menu.xml @@ -1,10 +1,14 @@ - + From 349afbd78656c35c6b7029e52bce6292a1659902 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Thu, 23 Jul 2026 15:21:50 +0800 Subject: [PATCH 6/7] docs(spp_key_management): regenerate README from fragments (CI oca-gen output) --- spp_key_management/README.rst | 4 +++- spp_key_management/static/description/index.html | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/spp_key_management/README.rst b/spp_key_management/README.rst index 7f03dbe5a..a31bb296f 100644 --- a/spp_key_management/README.rst +++ b/spp_key_management/README.rst @@ -175,7 +175,9 @@ Changelog The admin group now implies Key Operator instead, key admins keep field access to the KMS-wrapped key material the cloud providers read in user context, and a migration strips the unsafe link from existing - databases. + databases. The Key Management menu moved from Settings to a top-level + menu: the Settings root is only visible to ERP managers, so key admins + who are no longer system administrators could not reach it there. 19.0.2.0.0 ~~~~~~~~~~ diff --git a/spp_key_management/static/description/index.html b/spp_key_management/static/description/index.html index 9e8c64fff..a16177040 100644 --- a/spp_key_management/static/description/index.html +++ b/spp_key_management/static/description/index.html @@ -557,7 +557,9 @@

    19.0.2.0.1

    The admin group now implies Key Operator instead, key admins keep field access to the KMS-wrapped key material the cloud providers read in user context, and a migration strips the unsafe link from existing -databases. +databases. The Key Management menu moved from Settings to a top-level +menu: the Settings root is only visible to ERP managers, so key admins +who are no longer system administrators could not reach it there.
From a464f75c7c7d7110bf636a66c8ee8a6b0bc5a326 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Thu, 23 Jul 2026 16:28:03 +0800 Subject: [PATCH 7/7] security(key_management): document deliberate group_spp_admin exclusion; pin admin retention Staff-review polish: - security_groups.xml now states that group_key_admin is deliberately NOT linked into spp_security.group_spp_admin (key custody separated from platform administration), so a future consistency sweep doesn't reintroduce the over-grant - add a test that a system admin (without the key group) retains field read access and menu visibility - pinning the other half of the design --- .../security/security_groups.xml | 8 ++++++- .../tests/test_key_admin_group.py | 23 +++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/spp_key_management/security/security_groups.xml b/spp_key_management/security/security_groups.xml index 329cb1e68..372efc0e3 100644 --- a/spp_key_management/security/security_groups.xml +++ b/spp_key_management/security/security_groups.xml @@ -32,7 +32,13 @@ groups to members, so that link would silently promote anyone holding this key management role to a full Settings/System administrator. System admins get their own explicit ACL rows on - the key models instead. --> + the key models instead. + + Deliberately NOT linked into spp_security.group_spp_admin (the + pattern groups_admin.xml invites domain modules to follow): key + custody is intentionally separated from platform administration, + so OpenSPP admins do not automatically gain key control. Do not + "fix" this to match the other domain modules. --> Key Management Admin diff --git a/spp_key_management/tests/test_key_admin_group.py b/spp_key_management/tests/test_key_admin_group.py index 7cab34513..82ba4d936 100644 --- a/spp_key_management/tests/test_key_admin_group.py +++ b/spp_key_management/tests/test_key_admin_group.py @@ -88,6 +88,29 @@ def test_key_admin_reads_encrypted_key(self): with self.assertRaises(AccessError): key.with_user(operator_user).encrypted_key # noqa: B018 + def test_system_admin_retains_key_access(self): + """System administrators keep key access without holding the key + admin group: the field gate and menu root both list + base.group_system, and the ACL grants system its own rows. This + pins the other half of the design - removing the escalation must + not cost real system admins anything.""" + key = self.env["spp.encryption.key"].create( + { + "key_id": "test-system-access", + "encrypted_key": "d3JhcHBlZA==", + } + ) + system_user = self._create_user( + "key_system_admin", + [self.env.ref("base.group_system")], + ) + # Access is retained via base.group_system, NOT via the key group. + self.assertFalse(system_user.has_group("spp_key_management.group_key_admin")) + self.assertEqual(key.with_user(system_user).encrypted_key, "d3JhcHBlZA==") + root = self.env.ref("spp_key_management.menu_key_management_root") + menus = self.env["ir.ui.menu"].with_user(system_user).load_menus(False) + self.assertIn(root.id, menus) + def test_key_admin_sees_key_management_menu(self): """A user holding only the key admin role must be able to reach the Key Management menu. Odoo drops menus whose ancestors are invisible,