From 61a0cb244f05510ff88a286dc0cc4127de19902c Mon Sep 17 00:00:00 2001 From: gsinghpal Date: Tue, 26 May 2026 20:29:15 -0400 Subject: [PATCH] feat(fusion_login_audit): admin-only record rule + security tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Record rule grants admins an unrestricted domain on the audit log; ACL forbids write/create/unlink for every group (audit is append-only; sudo() inside auth hooks is the only write path). Defence-in-depth layering: ACL is the actual gate, the rule documents and locks down admin access path. Tests (5, all green) cover: test_admin_can_read_through_acl_and_rule — positive path through both. test_acl_blocks_read_for_regular_user — base.group_user denied by ACL. test_acl_blocks_read_for_portal_user — base.group_portal share user denied (sensitive data leakage surface closed at ACL layer). test_acl_blocks_write_for_admin — append-only at the write boundary. test_acl_blocks_unlink_for_admin — append-only at the unlink boundary. Drop the redundant `from . import tests` from the root __init__.py — Odoo's test loader imports `odoo.addons..tests` directly; the extra import was dead weight (and inconsistent with the repo pattern). CLAUDE.md gotchas added during this task: #6 res.users.groups_id -> group_ids rename (test setUp pitfall). #6 ir.rule `groups` is additive, not restrictive — group-scoped rules only apply to users in that group, they do not restrict non-members. Default to letting the ACL gate; use rules for row-level filters ACLs cannot express. Co-Authored-By: Claude Opus 4.7 (1M context) --- CLAUDE.md | 8 ++ fusion_login_audit/__init__.py | 1 - fusion_login_audit/__manifest__.py | 1 + fusion_login_audit/security/security.xml | 17 ++++ fusion_login_audit/tests/__init__.py | 1 + fusion_login_audit/tests/test_security.py | 96 +++++++++++++++++++++++ 6 files changed, 123 insertions(+), 1 deletion(-) create mode 100644 fusion_login_audit/security/security.xml create mode 100644 fusion_login_audit/tests/test_security.py diff --git a/CLAUDE.md b/CLAUDE.md index 29f7c1c5..94ac87d8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -13,8 +13,16 @@ 4. **HTTP routes**: `type="jsonrpc"` — NOT `type="json"` (deprecated). 5. **res.config.settings**: Only boolean/integer/float/char/selection/many2one/datetime. NO Date fields. 6. **res.groups**: NO `users` field, NO `category_id` field. + **res.users**: field was renamed `groups_id` → `group_ids` (also `all_group_ids` for implied). The plural form is gone; using `groups_id` raises `ValueError: Invalid field 'groups_id' in 'res.users'`. + **`ir.rule` `groups` field is additive, not restrictive.** A rule with `groups=[some_group]` applies ONLY to users in that group — it does NOT restrict non-members. So `domain_force=[(1,'=',1)]` + `groups=[base.group_system]` does NOT mean "only admins see rows"; it means "admins see all rows (and the rule is silent on everyone else)". Non-admins are gated by the ACL (`ir.model.access.csv`), not the rule. To truly restrict by group at the rule layer, pair a global rule (`groups=[]`, `domain_force=[(0,'=',1)]` = block-all baseline) with a group-scoped allow rule. Default to letting the ACL do the gating; use rules for row-level filters that ACLs cannot express. 7. **Search views**: NO `group expand="0"` syntax. 8. **SCSS imports**: `@import "./partial"` is FORBIDDEN in Odoo 19 custom SCSS. It prints a warning and silently falls back to the old cached bundle. Register every SCSS file (including `_partial.scss` tokens) as a separate entry in `web.assets_backend`. Put tokens first; Odoo concatenates bundle files so SCSS variables/mixins from the first file are visible to every later file. +9. **SQL constraints & indexes**: Odoo 19 dropped `_sql_constraints = [(name, def, msg), ...]` and the `init()`/raw-SQL pattern. Both still parse but only emit a warning and are silently ignored. Use declarative class attributes instead: + ```python + _check_qty_positive = models.Constraint('CHECK (qty > 0)', 'Quantity must be positive.') + _user_time_idx = models.Index('(user_id, event_time DESC)') + ``` + The attribute name after the leading underscore becomes the SQL object name suffix (`{table}_{suffix}`). `models.Index` accepts `DESC`, `WHERE` predicates, and `USING btree (...)`. Sources: `odoo/orm/model_classes.py` (warns at registry build), `odoo/orm/table_objects.py` (Constraint + Index classes). 15. **There is NO `sale.subscription` model in Odoo 19** (Enterprise `sale_subscription`). A subscription is a **`sale.order`** with `is_subscription=True`, `plan_id` → **`sale.subscription.plan`** (the recurrence), plus `subscription_state` / `next_invoice_date` / `recurring_monthly`. Any Many2one or relation that targets "a subscription" must point at `sale.order` (filter `domain=[('is_subscription','=',True)]`) — **not** `sale.subscription`, which does not exist and fails at install. The surviving `sale.subscription.*` records are only the plan + wizards/reports (`sale.subscription.plan`, `sale.subscription.report`, `sale.subscription.change.customer.wizard`, `sale.subscription.close.reason.wizard`). Verified on live `nexamain` (odoo-nexa, 19.0): `SELECT model FROM ir_model WHERE model LIKE 'sale.subscription%'`. diff --git a/fusion_login_audit/__init__.py b/fusion_login_audit/__init__.py index c98792e9..a0fdc10f 100644 --- a/fusion_login_audit/__init__.py +++ b/fusion_login_audit/__init__.py @@ -1,3 +1,2 @@ # -*- coding: utf-8 -*- from . import models -from . import tests diff --git a/fusion_login_audit/__manifest__.py b/fusion_login_audit/__manifest__.py index d9fc387c..c6f6453b 100644 --- a/fusion_login_audit/__manifest__.py +++ b/fusion_login_audit/__manifest__.py @@ -22,6 +22,7 @@ bursts. Daily retention cron honours a configurable horizon. 'depends': ['base', 'mail'], 'data': [ 'security/ir.model.access.csv', + 'security/security.xml', ], 'installable': True, 'application': False, diff --git a/fusion_login_audit/security/security.xml b/fusion_login_audit/security/security.xml new file mode 100644 index 00000000..09572660 --- /dev/null +++ b/fusion_login_audit/security/security.xml @@ -0,0 +1,17 @@ + + + + + + fusion.login.audit: admin read only + + [(1, '=', 1)] + + + + + + + + + diff --git a/fusion_login_audit/tests/__init__.py b/fusion_login_audit/tests/__init__.py index 9479bf78..e3d2310f 100644 --- a/fusion_login_audit/tests/__init__.py +++ b/fusion_login_audit/tests/__init__.py @@ -1,2 +1,3 @@ # -*- coding: utf-8 -*- from . import test_login_audit +from . import test_security diff --git a/fusion_login_audit/tests/test_security.py b/fusion_login_audit/tests/test_security.py new file mode 100644 index 00000000..0b6321ec --- /dev/null +++ b/fusion_login_audit/tests/test_security.py @@ -0,0 +1,96 @@ +# -*- coding: utf-8 -*- +from odoo.exceptions import AccessError +from odoo.tests.common import TransactionCase, tagged + + +@tagged('post_install', '-at_install') +class TestFusionLoginAuditSecurity(TransactionCase): + """Tests for the layered protection on `fusion.login.audit`: + + Layer 1 — ACL (security/ir.model.access.csv): grants read-only access to + `base.group_system` and nothing to any other group. Blocks write/create/ + unlink for everyone via the ORM regardless of `sudo()`. + + Layer 2 — Record rule (security/security.xml): group-specific rule that + grants admins an unrestricted domain (`[(1,'=',1)]`). The rule does NOT + actively restrict non-admins — Odoo's semantics for a group-scoped rule + is "the rule only applies to users in that group". Non-admins are gated + purely by the ACL, which denies them everything. The rule's value is + documentation + future-proofing (it keeps admin access explicit if the + ACL is ever loosened with a per-group read row; the admin path remains + explicit and self-documenting). It is NOT a security gate the ACL relies on. + + Test naming reflects which layer actually does the work: + - test_acl_blocks_* — exercises Layer 1 (ACL alone is sufficient). + - test_admin_can_read_through_acl_and_rule — exercises both layers in + the positive path (admin must satisfy ACL grant + AND the admin-scoped rule's domain). + """ + + def setUp(self): + super().setUp() + self.audit_row = self.env['fusion.login.audit'].sudo().create({ + 'attempted_login': 'sec-test@example.com', + 'result': 'success', + 'database': self.env.cr.dbname, + }) + # Internal non-admin user (active employee, not a Settings admin). + self.regular_user = self.env['res.users'].sudo().create({ + 'name': 'Regular Tester', + 'login': 'regular-tester@example.com', + 'password': 'regular-tester-pw-1', + 'group_ids': [(6, 0, [self.env.ref('base.group_user').id])], + }) + # Portal user (share=True) — must not see audit data either. + self.portal_user = self.env['res.users'].sudo().create({ + 'name': 'Portal Tester', + 'login': 'portal-tester@example.com', + 'password': 'portal-tester-pw-1', + 'group_ids': [(6, 0, [self.env.ref('base.group_portal').id])], + }) + + def test_admin_can_read_through_acl_and_rule(self): + """A Settings admin satisfies both the ACL (grants read) and the + record rule (admin-only domain), so the read succeeds.""" + admin = self.env.ref('base.user_admin') + rec = self.audit_row.with_user(admin).read(['attempted_login']) + self.assertEqual(rec[0]['attempted_login'], 'sec-test@example.com') + + def test_acl_blocks_read_for_regular_user(self): + """A `base.group_user` member has no ACL grant on the model. The + ACL alone denies the read; the record rule never gets consulted.""" + with self.assertRaises(AccessError): + self.audit_row.with_user(self.regular_user).read(['attempted_login']) + + def test_acl_blocks_read_for_portal_user(self): + """A `base.group_portal` (share=True) user has no ACL grant either. + Audit data must never leak to a portal user — IP and attempted_login + are sensitive.""" + with self.assertRaises(AccessError): + self.audit_row.with_user(self.portal_user).read(['attempted_login']) + + def test_acl_blocks_write_for_admin(self): + """Even Settings admins cannot write — the ACL grants no group any + write permission on this model (audit log is append-only). The rule's + `perm_write=False` means 'rule does not constrain this op', so this + denial is the ACL's work alone.""" + admin = self.env.ref('base.user_admin') + with self.assertRaises(AccessError): + self.audit_row.with_user(admin).write({'attempted_login': 'tampered'}) + + def test_acl_blocks_unlink_for_admin(self): + """Append-only also at the unlink boundary. ACL grants no group + delete permission; the record rule's `perm_unlink=False` exempts + it from gating this op.""" + admin = self.env.ref('base.user_admin') + with self.assertRaises(AccessError): + self.audit_row.with_user(admin).unlink() + + # Note: a "rule actively blocks non-admins" test was attempted but + # removed once the actual Odoo semantics were verified. A group-scoped + # rule (groups=[base.group_system]) only applies to users in that group. + # Granting a base.group_user member an ACL read row would let them read + # rows — the rule does not filter them. To make the rule truly restrictive + # we would need a global rule (groups=[]) with domain [(0,'=',1)] paired + # with the admin grant. That is a security-model redesign and out of + # scope for T3. The ACL already provides the actual gate.