Skip to content

Commit 90556c2

Browse files
Merge pull request OCA#2 from QuatraInternational/OD-46
[FIX] auditlog: prevent removal of x2many values from inaccessible companies
2 parents 3d16422 + 593576a commit 90556c2

File tree

5 files changed

+121
-2
lines changed

5 files changed

+121
-2
lines changed

auditlog/models/rule.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -387,6 +387,9 @@ def write_full(self, vals, **kwargs):
387387
.with_context(prefetch_fields=False)
388388
.read(fields_list)
389389
}
390+
# Prevent the cache of modified fields from being poisoned by
391+
# x2many items inaccessible to the current user.
392+
self.invalidate_recordset(vals.keys())
390393
result = write_full.origin(self, vals, **kwargs)
391394
new_values = {
392395
d["id"]: d

auditlog/tests/__init__.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
11
# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl).
22
from . import test_auditlog
33
from . import test_autovacuum
4+
from . import test_multi_company
Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
from unittest.mock import patch
2+
3+
from odoo.fields import Command
4+
from odoo.tests.common import TransactionCase
5+
6+
from odoo.addons.base.models.res_users import Groups
7+
8+
9+
class TestMultiCompany(TransactionCase):
10+
@classmethod
11+
def setUpClass(cls):
12+
super().setUpClass()
13+
# Disarm any existing auditing rules.
14+
cls.env["auditlog.rule"].search([]).unlink()
15+
cls.env["auditlog.log"].search([]).unlink()
16+
# Set up a group with two share users from different companies
17+
cls.company1 = cls.env["res.company"].create({"name": "c1"})
18+
cls.company2 = cls.env["res.company"].create({"name": "c2"})
19+
cls.group = cls.env["res.groups"].create({"name": "g1", "share": True})
20+
cls.user1 = cls.env["res.users"].create(
21+
{
22+
"name": "u1",
23+
"login": "u1",
24+
"groups_id": [Command.set(cls.group.ids)],
25+
"company_ids": [Command.set(cls.company1.ids)],
26+
"company_id": cls.company1.id,
27+
}
28+
)
29+
cls.user2 = cls.env["res.users"].create(
30+
{
31+
"name": "u2",
32+
"login": "u2",
33+
"groups_id": [Command.set(cls.group.ids)],
34+
"company_ids": [Command.set(cls.company2.ids)],
35+
"company_id": cls.company2.id,
36+
}
37+
)
38+
# We will test with a user that has access to only one of the companies
39+
cls.user_demo = cls.env.ref("base.user_demo")
40+
cls.user_demo.write(
41+
{
42+
"company_ids": [Command.set(cls.company2.ids)],
43+
"company_id": cls.company2.id,
44+
"groups_id": [Command.link(cls.env.ref("base.group_system").id)],
45+
}
46+
)
47+
48+
def test_group_set_users(self): # pylint: disable=missing-return
49+
"""Writing x2many values does not wipe values from inaccessible companies."""
50+
self.assertEqual(
51+
self.group.users,
52+
self.user1 + self.user2,
53+
)
54+
self.group.invalidate_recordset()
55+
group_with_user = self.group.with_user(self.user_demo)
56+
self.assertEqual(group_with_user.users, self.user2)
57+
58+
# The issue arises when `users` is missing from the cache and is first
59+
# read as the superuser when fetching the full values for the auditlog.
60+
# To emulate this, we want the field missing from the cache at the
61+
# moment of writing. To prevent various overrides from populating the
62+
# cache even earlier on when fetching other fields we preemptively fill
63+
# the cache of the other fields.
64+
#
65+
# All of this is undermined by res.users's own `write` method which
66+
# wipes the cache just in time, so we avoid this override with a patch.
67+
#
68+
# The issue is reproduceable on the product.template model without this
69+
# trickery but this module does not depend on the product module so the
70+
# model is not available.
71+
self.group.read()
72+
self.group.invalidate_recordset(["users"])
73+
74+
def write(self, vals):
75+
"""Avoid the cache invalidation in this particular override.
76+
77+
With the faulty behaviour, values from all companies are already
78+
present in the cache at this point, leading to the deletion of the
79+
value from the company that is inaccessible to the current user.
80+
"""
81+
return super(Groups, self).write(vals)
82+
83+
# Do the write.
84+
with patch.object(Groups, "write", side_effect=write, autospec=True):
85+
group_with_user.write({"users": [Command.set(self.user2.ids)]})
86+
self.assertEqual(group_with_user.users, self.user2)
87+
# Ensure that the users of the other companies are still there.
88+
self.env.invalidate_all()
89+
self.assertEqual(
90+
self.group.users,
91+
self.user1 + self.user2,
92+
)
93+
94+
def test_group_set_users_with_auditlog(self):
95+
"""Repeat the test above with an auditlog on the groups model"""
96+
rule = (
97+
self.env["auditlog.rule"]
98+
.sudo()
99+
.create(
100+
{
101+
"name": "Test rule for groups",
102+
"model_id": self.env["ir.model"]._get("res.groups").id,
103+
"log_read": False,
104+
"log_create": False,
105+
"log_write": True,
106+
"log_unlink": False,
107+
"log_type": "full",
108+
"state": "subscribed",
109+
}
110+
)
111+
)
112+
try:
113+
self.test_group_set_users()
114+
finally:
115+
rule.unlink()

sentry/README.rst

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ Sentry
77
!! This file is generated by oca-gen-addon-readme !!
88
!! changes will be overwritten. !!
99
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
10-
!! source digest: sha256:88c5722f68e393e581f3bd3dfc08a4350731602583fdce2d407f6efcede43443
10+
!! source digest: sha256:e43ec67df852d6f647f71e1bcfccb9bbeabcaae5072636a2aa2a5bef7a68eb80
1111
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
1212
1313
.. |badge1| image:: https://img.shields.io/badge/maturity-Beta-yellow.png

sentry/static/description/index.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -367,7 +367,7 @@ <h1 class="title">Sentry</h1>
367367
!! This file is generated by oca-gen-addon-readme !!
368368
!! changes will be overwritten. !!
369369
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
370-
!! source digest: sha256:88c5722f68e393e581f3bd3dfc08a4350731602583fdce2d407f6efcede43443
370+
!! source digest: sha256:e43ec67df852d6f647f71e1bcfccb9bbeabcaae5072636a2aa2a5bef7a68eb80
371371
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! -->
372372
<p><a class="reference external image-reference" href="https://odoo-community.org/page/development-status"><img alt="Beta" src="https://img.shields.io/badge/maturity-Beta-yellow.png" /></a> <a class="reference external image-reference" href="http://www.gnu.org/licenses/agpl-3.0-standalone.html"><img alt="License: AGPL-3" src="https://img.shields.io/badge/licence-AGPL--3-blue.png" /></a> <a class="reference external image-reference" href="https://github.com/OCA/server-tools/tree/16.0/sentry"><img alt="OCA/server-tools" src="https://img.shields.io/badge/github-OCA%2Fserver--tools-lightgray.png?logo=github" /></a> <a class="reference external image-reference" href="https://translation.odoo-community.org/projects/server-tools-16-0/server-tools-16-0-sentry"><img alt="Translate me on Weblate" src="https://img.shields.io/badge/weblate-Translate%20me-F47D42.png" /></a> <a class="reference external image-reference" href="https://runboat.odoo-community.org/builds?repo=OCA/server-tools&amp;target_branch=16.0"><img alt="Try me on Runboat" src="https://img.shields.io/badge/runboat-Try%20me-875A7B.png" /></a></p>
373373
<p>This module allows painless <a class="reference external" href="https://sentry.io/">Sentry</a> integration with

0 commit comments

Comments
 (0)