Skip to content

Commit c9bb0f0

Browse files
committed
fix(spp_base_common): gate Apps menu on base.group_system via existing menuitem override (OP#951)
Earlier commits in this PR attempted the gating via spp_security: first as an XML record override (e70a983), then as a post_init_hook (cc11557). Both produced empty group_ids on a freshly installed DB — verified on port 32774 with spp_security at 19.0.2.0.1. Root cause: spp_base_common's views/main_view.xml already overrides base.menu_management with its own <menuitem> (to set the OpenSPP web icon). spp_base_common loads AFTER spp_security in the dep graph, and in Odoo 19 a <menuitem> without an explicit `groups` attribute resets group_ids on the target record. So any group_ids that spp_security's hook or XML override writes earlier gets clobbered later by spp_base_common's reload. Fix: gate the menu where it's already declared. Add `groups="base.group_system"` to the existing menuitem in spp_base_common, and revert the spp_security gymnastics (version bump, HISTORY entry, _gate_apps_menu hook, migration script). System Admin is the only OpenSPP role that pulls in base.group_system (via spp_user_roles.global_role_admin → Command.link(base.group_system)), so this single attribute hides Apps from every other role and matches the OP#951 audit's `Apps: no` rows. Modules touched: - spp_base_common: + groups attr on Apps menuitem; bump 19.0.2.0.0 → 19.0.2.0.1 + HISTORY entry - spp_security: revert version bump, drop _gate_apps_menu hook and associated comment, no migration script needed
1 parent cc11557 commit c9bb0f0

7 files changed

Lines changed: 18 additions & 47 deletions

File tree

spp_base_common/__manifest__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
{
66
"name": "OpenSPP Base (Common)",
77
"category": "OpenSPP/Core",
8-
"version": "19.0.2.0.0",
8+
"version": "19.0.2.0.1",
99
"sequence": 1,
1010
"author": "OpenSPP.org",
1111
"website": "https://github.com/OpenSPP/OpenSPP2",

spp_base_common/readme/HISTORY.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
### 19.0.2.0.1
2+
3+
- fix(security): add `groups="base.group_system"` to the existing `<menuitem id="base.menu_management" />` override in `views/main_view.xml`. Out of the box the Apps top-level menu has no group restriction and is visible to every logged-in user, violating the OP#951 audit's `Apps: no` rows. The override here is the single authoritative declaration for this menu's attributes in the OpenSPP install (sequence, custom OpenSPP icon, and now group_ids); doing the gating anywhere upstream (e.g. a `post_init_hook` in `spp_security`) is unreliable because this `<menuitem>` reload re-writes the record without a `groups` attribute and resets `group_ids` to empty.
4+
15
### 19.0.2.0.0
26

37
- Initial migration to OpenSPP2

spp_base_common/views/main_view.xml

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,22 @@
77
sequence="-1"
88
/>
99

10+
<!--
11+
Gate the Apps top-level menu on base.group_system per the OP#951
12+
role/menu audit (only System Admin should see Apps). spp_base_common
13+
already overrides this menuitem to set the OpenSPP web_icon; adding
14+
the groups attribute here is reliable because this <menuitem> runs
15+
AFTER any upstream module's post_init_hook on fresh install AND on
16+
every subsequent module upgrade. Doing the same gating from a
17+
post_init_hook in an upstream module (e.g. spp_security) does not
18+
survive because a later `<menuitem>` reload re-writes the record
19+
without a `groups` attribute, resetting group_ids to empty.
20+
-->
1021
<menuitem
1122
id="base.menu_management"
1223
name="Apps"
1324
web_icon="spp_base_common,static/description/OpenSPP-Icons-App.png"
25+
groups="base.group_system"
1426
/>
1527
<menuitem
1628
id="base.menu_administration"

spp_security/__manifest__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
"name": "OpenSPP Security",
55
"summary": "Central security definitions for OpenSPP modules",
66
"category": "OpenSPP/Core",
7-
"version": "19.0.2.0.1",
7+
"version": "19.0.2.0.0",
88
"author": "OpenSPP.org",
99
"website": "https://github.com/OpenSPP/OpenSPP2",
1010
"license": "LGPL-3",

spp_security/hooks.py

Lines changed: 0 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ def post_init_hook(env_or_cr, registry=None):
3131
env = env_or_cr
3232

3333
_setup_admin_group_inheritance(env)
34-
_gate_apps_menu(env)
3534

3635

3736
def _setup_admin_group_inheritance(env):
@@ -64,33 +63,3 @@ def _setup_admin_group_inheritance(env):
6463

6564
except Exception as e:
6665
_logger.warning("Failed to set up admin group inheritance: %s", str(e))
67-
68-
69-
def _gate_apps_menu(env):
70-
"""
71-
Restrict the "Apps" top-level menu (base.menu_management) to base.group_system.
72-
73-
Out of the box this menuitem has no `groups` attribute, so every logged-in
74-
user sees it. Per the OP#951 role/menu audit only System Admin should see
75-
Apps; everyone else should not. base.group_system is the only group System
76-
Admin pulls in transitively (via spp_user_roles.global_role_admin), so a
77-
single Many2many write here enforces the audit for every role.
78-
79-
Idempotent — re-applies on every install/upgrade of spp_security. Uses a
80-
hook rather than a `<record id="base.menu_management">` XML override
81-
because cross-module Many2many writes via data XML can be wiped on
82-
subsequent base-module upgrades.
83-
"""
84-
try:
85-
apps_menu = env.ref("base.menu_management", raise_if_not_found=False)
86-
system_admin = env.ref("base.group_system", raise_if_not_found=False)
87-
if not apps_menu or not system_admin:
88-
_logger.warning("Could not gate Apps menu: base.menu_management or base.group_system not found")
89-
return
90-
if apps_menu.group_ids == system_admin:
91-
_logger.debug("Apps menu gating already configured")
92-
return
93-
apps_menu.write({"group_ids": [(6, 0, [system_admin.id])]})
94-
_logger.info("Gated Apps menu (base.menu_management) on base.group_system")
95-
except Exception as e:
96-
_logger.warning("Failed to gate Apps menu: %s", str(e))

spp_security/readme/HISTORY.md

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,3 @@
1-
### 19.0.2.0.1
2-
3-
- fix(security): gate the Odoo-stock **Apps** top-level menu (`base.menu_management`) on `base.group_system` via a new `_gate_apps_menu` hook called from `post_init_hook`. Out of the box the menu had no `groups` restriction and was visible to every logged-in user, so the OP#951 audit's `Apps: no` rows were silently violated. System Admin is the only OpenSPP role that pulls in `base.group_system`, so this single Many2many write hides Apps from every other role without touching any individual role definition. Hook is idempotent and re-applies on every install/upgrade.
4-
51
### 19.0.2.0.0
62

73
- Initial migration to OpenSPP2

spp_security/security/groups_admin.xml

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -39,16 +39,6 @@ Used for field agents and limited-access users.</field>
3939
<field name="implied_ids" eval="[Command.link(ref('group_spp_admin'))]" />
4040
</record>
4141

42-
<!--
43-
Note: the "Apps" top-level menu (base.menu_management) is gated on
44-
base.group_system at install / upgrade time by the post_init_hook
45-
(see hooks.py:_gate_apps_menu). A hook is used instead of an
46-
`<record id="base.menu_management" ...>` override because the XML
47-
override is unreliable for cross-module Many2many writes (it can
48-
be wiped on subsequent base-module upgrades). The hook idempotently
49-
re-applies the gating on every install/upgrade of spp_security.
50-
-->
51-
5242
<!--
5343
How domain modules extend this:
5444
===============================

0 commit comments

Comments
 (0)