[19.0][FIX] payroll: add security group to payroll fields on hr.version and hr.employee - #281
Open
neoand wants to merge 1 commit into
Open
[19.0][FIX] payroll: add security group to payroll fields on hr.version and hr.employee#281neoand wants to merge 1 commit into
neoand wants to merge 1 commit into
Conversation
Contributor
|
Hi @nimarosa, @appstogrow, |
CristianoMafraJunior
suggested changes
Aug 10, 2026
…hr.employee
Odoo 19 hardened HR field visibility: hr.version and hr.employee now expect
payroll-sensitive fields to declare a security group, and core ships tests
(hr.tests.test_payroll_fields_access, hr.tests.test_self_user_access) that
enforce it.
This module declared groups on payslip_count only. struct_id, schedule_pay
and slip_ids had none, so the salary structure and the payslip list were
readable by any HR user and leaked into employee public profiles:
AccessError: The fields 'slip_ids,struct_id,schedule_pay', which you are
trying to read, are not available for employee public profiles.
Reproduced on odoo:19.0 Community with a fresh database:
before: 3 failed, 1 error of 1087 tests
after: the three payroll-related failures are gone; fields verified at the
registry level to carry groups=payroll.group_payroll_user
Note for reviewers: core's test looks for 'hr.group_hr_manager' or
'hr_payroll.group_hr_payroll_user' (the Enterprise module name), so a module
using OCA's own 'payroll.group_payroll_user' still trips that specific
assertion even though the underlying access control is now correct. Worth
deciding upstream whether the check should accept an equivalent group.
Assisted-by: Claude Opus 4.8
neoand
force-pushed
the
fix/payroll-fields-security
branch
from
August 11, 2026 10:30
4b683bc to
01b66f5
Compare
Author
|
Thanks for pointing me to the policy — that one's on me, and it's fixed now. The commit was using Happy to answer anything about the implementation directly — I'd much rather work through review |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Odoo 19 hardened HR field visibility.
hr.versionandhr.employeeare now expected to declare asecurity group on payroll-sensitive fields, and core ships tests that enforce it
(
hr.tests.test_payroll_fields_access,hr.tests.test_self_user_access).This module declares
groupsonpayslip_countonly.struct_id,schedule_payandslip_idshave none, so the salary structure and the payslip list are readable by any HR user and leak into
employee public profiles:
How to reproduce
On a fresh database with
odoo:19.0(Community):Before this change: 3 failed, 1 error of 1087 tests
(The fourth failure,
calendar.test_event_notifications.test_email_alarm, is unrelated to thismodule.)
Fix
Add
groups="payroll.group_payroll_user"to the three unprotected fields, matching whatpayslip_countalready does.Verified at the registry level after the change:
hr.version.struct_idpayroll.group_payroll_userhr.version.schedule_paypayroll.group_payroll_userhr.employee.slip_idspayroll.group_payroll_userhr.employee.payslip_countThe module's own test suite still passes (27 tests).
Note for reviewers
Core's
_test_payroll_fields_are_hidden_to_non_payroll_usersaccepts onlyhr.group_hr_managerorhr_payroll.group_hr_payroll_user— the latter being the Enterprisemodule's group. A module using OCA's own
payroll.group_payroll_usertherefore still trips thatspecific assertion even though the underlying access control is now correct.
This PR fixes the actual data exposure. Whether the upstream check should also accept an
equivalent community group is a separate discussion, and I'm happy to follow whatever the
maintainers prefer here (e.g. adding
hr.group_hr_manageralongside).Assisted-by: Claude Opus 4.8
Disclosed per the OCA Generative AI / LLM Policy.
Developed and verified under my direction and review; I take responsibility for the contribution.