From 765b0950350b2f6d6d6261dd44f2b45402a4d05f Mon Sep 17 00:00:00 2001 From: gsinghpal Date: Sun, 24 May 2026 12:47:26 -0400 Subject: [PATCH] =?UTF-8?q?fix(shopfloor):=20Phase=20B=20review=20findings?= =?UTF-8?q?=20=E2=80=94=20C1/I1/I2/I3/M1?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit C1: Add placeholder fp_tablet_cron.xml + fp_tablet_session_event_views.xml so the module is installable now (real content lands in Phase C task C4 and Phase E task E1 respectively). I1: test_tablet_pin_auth_manager now passes {} (not self.env) as the env arg to _check_credentials — matches what request.session.authenticate provides and what the base implementation expects. I2: Auth manager role check now uses user_sudo.all_group_ids (transitive) instead of group_ids (direct) per CLAUDE.md rules 13l + 23. Owner users who hold Owner directly still match all 5 shop-branch xmlids via the implication chain. I3: fp.tablet.session.event gains Python-layer write() + unlink() overrides that always raise AccessError unless the explicit fp_tablet_audit_admin_write / fp_tablet_audit_admin_purge context flag is set. Closes the gap between the model's append-only docstring and its actual enforcement (ACL-only previously). M1: Hoisted 'from odoo.exceptions import AccessDenied' to top-of-file imports next to existing UserError import. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../data/fp_tablet_cron.xml | 6 +++++ .../models/fp_tablet_session_event.py | 27 ++++++++++++++++++- .../models/res_users.py | 10 ++++--- .../tests/test_tablet_pin_auth_manager.py | 4 +-- .../views/fp_tablet_session_event_views.xml | 7 +++++ 5 files changed, 47 insertions(+), 7 deletions(-) create mode 100644 fusion_plating/fusion_plating_shopfloor/data/fp_tablet_cron.xml create mode 100644 fusion_plating/fusion_plating_shopfloor/views/fp_tablet_session_event_views.xml diff --git a/fusion_plating/fusion_plating_shopfloor/data/fp_tablet_cron.xml b/fusion_plating/fusion_plating_shopfloor/data/fp_tablet_cron.xml new file mode 100644 index 00000000..47b82d2a --- /dev/null +++ b/fusion_plating/fusion_plating_shopfloor/data/fp_tablet_cron.xml @@ -0,0 +1,6 @@ + + + + diff --git a/fusion_plating/fusion_plating_shopfloor/models/fp_tablet_session_event.py b/fusion_plating/fusion_plating_shopfloor/models/fp_tablet_session_event.py index 9fac6412..039c8e0d 100644 --- a/fusion_plating/fusion_plating_shopfloor/models/fp_tablet_session_event.py +++ b/fusion_plating/fusion_plating_shopfloor/models/fp_tablet_session_event.py @@ -7,7 +7,8 @@ write/unlink/edit are forbidden to anyone except root via direct SQL. Spec section 4: docs/superpowers/specs/2026-05-24-tablet-pin-session-redesign-design.md """ -from odoo import api, fields, models +from odoo import _, api, fields, models +from odoo.exceptions import AccessError class FpTabletSessionEvent(models.Model): @@ -88,3 +89,27 @@ class FpTabletSessionEvent(models.Model): ) notes = fields.Text(readonly=True) + + # ------------------------------------------------------------------ + # Append-only enforcement at the Python layer + # ACL grants ONLY read on the model (Phase A CSV); these overrides + # block write/unlink even for sudo() callers that would bypass ACL. + # The only legitimate path for purging the audit log is a + # context-flagged admin sweep (e.g. a future retention cron). + # ------------------------------------------------------------------ + def write(self, vals): + if not self.env.context.get('fp_tablet_audit_admin_write'): + raise AccessError(_( + "fp.tablet.session.event rows are append-only. " + "Use the audit log; do not modify historical entries." + )) + return super().write(vals) + + def unlink(self): + if not self.env.context.get('fp_tablet_audit_admin_purge'): + raise AccessError(_( + "fp.tablet.session.event rows are append-only. " + "Delete only via the retention cron with the explicit " + "admin-purge context flag." + )) + return super().unlink() diff --git a/fusion_plating/fusion_plating_shopfloor/models/res_users.py b/fusion_plating/fusion_plating_shopfloor/models/res_users.py index 9710afb3..61182f95 100644 --- a/fusion_plating/fusion_plating_shopfloor/models/res_users.py +++ b/fusion_plating/fusion_plating_shopfloor/models/res_users.py @@ -12,7 +12,7 @@ import hashlib import secrets from odoo import _, fields, models -from odoo.exceptions import UserError +from odoo.exceptions import AccessDenied, UserError # PBKDF2 iteration count. ~50ms verify on entech-class hardware. Safe # against brute-force even if the DB leaks. @@ -151,7 +151,6 @@ class ResUsers(models.Model): See docs/superpowers/specs/2026-05-24-tablet-pin-session-redesign-design.md Section 2 — Auth path. """ - from odoo.exceptions import AccessDenied if isinstance(credential, dict) and credential.get('type') == 'fp_tablet_pin': login = credential.get('login') pin = credential.get('pin') @@ -160,7 +159,10 @@ class ResUsers(models.Model): user_sudo = self.sudo().search([('login', '=', login)], limit=1) if not user_sudo or not user_sudo.active: raise AccessDenied() - # Must hold a shop-branch role (otherwise they can't operate the tablet) + # Must hold a shop-branch role (transitively — all_group_ids follows + # the implication chain so users who hold Owner directly still match + # the Technician/Manager checks below). Matches has_group() semantics + # and is futureproof against role-graph edits (CLAUDE.md rules 13l + 23). shop_branch_xmlids = ( 'fusion_plating.group_fp_technician', 'fusion_plating.group_fp_shop_manager_v2', @@ -174,7 +176,7 @@ class ResUsers(models.Model): for x in shop_branch_xmlids ) if g } - user_group_ids = set(user_sudo.group_ids.ids) + user_group_ids = set(user_sudo.all_group_ids.ids) if not (shop_branch_ids & user_group_ids): raise AccessDenied() # Verify the PIN hash. verify_tablet_pin already exists. diff --git a/fusion_plating/fusion_plating_shopfloor/tests/test_tablet_pin_auth_manager.py b/fusion_plating/fusion_plating_shopfloor/tests/test_tablet_pin_auth_manager.py index 193485f7..8a93e5ce 100644 --- a/fusion_plating/fusion_plating_shopfloor/tests/test_tablet_pin_auth_manager.py +++ b/fusion_plating/fusion_plating_shopfloor/tests/test_tablet_pin_auth_manager.py @@ -28,7 +28,7 @@ class TestTabletPinAuthManager(TransactionCase): def _check(self, login, pin): return self.env['res.users'].sudo()._check_credentials( {'type': 'fp_tablet_pin', 'login': login, 'pin': pin}, - self.env, + {}, ) def test_correct_pin_succeeds(self): @@ -72,7 +72,7 @@ class TestTabletPinAuthManager(TransactionCase): self.env['res.users'].sudo()._check_credentials( {'type': 'password', 'login': 'authmgr_tech@example.com', 'password': 'wrong'}, - self.env, + {}, ) except AccessDenied: pass # expected — wrong password diff --git a/fusion_plating/fusion_plating_shopfloor/views/fp_tablet_session_event_views.xml b/fusion_plating/fusion_plating_shopfloor/views/fp_tablet_session_event_views.xml new file mode 100644 index 00000000..b4f7b62f --- /dev/null +++ b/fusion_plating/fusion_plating_shopfloor/views/fp_tablet_session_event_views.xml @@ -0,0 +1,7 @@ + + + +