From ebddc1484b57adae93bdc9ab6fb7bedef2abd0f0 Mon Sep 17 00:00:00 2001 From: Adrien Schoffeniels Date: Mon, 27 Jun 2022 13:59:32 +0000 Subject: [PATCH] [FIX] knowledge: fix inheritance of internal permissions Description of the issue: ========================= Suppose we have 3 articles with the following structure: root L child L grand child When restricting the internal permission in 'child', the inherited permission parent id and the inherited permission of 'grand child' are not updated, and the share panel shows that the internal permission of grand child is based on 'root'. The 2 fields of grand child (and of any other child of 'child' at any depth) should be recomputed, and the share panel should show that the internal permission of these children is based on 'child'. Solution: ========= Add 'parent_id.inherited_permission_parent_id' as field on which '_compute_inherited_permission' depends on, so that 'inherited permission' and 'inherited permission parent id' of the children of an article are recomputed when the internal permission of this article changes. The scenario described in the issue description has been added to the tests. One now ensures that if an article gets its internal permission downgraded (either to 'read' or 'none'), the child and the grand-child of this article have their inherited permission parent id equal to the article and their inherited permission equal to the internal permission of the article. Task-2866950 X-original-commit: ea2d1b64c5f30c6d0e6a704b4f09c43084371b62 Part-of: odoo/odoo#94834 --- addons/knowledge/models/knowledge_article.py | 13 +++++++++++-- addons/knowledge/tests/common.py | 6 ++++++ .../test_knowledge_article_permissions.py | 18 ++++++++++++++++++ 3 files changed, 35 insertions(+), 2 deletions(-) diff --git a/addons/knowledge/models/knowledge_article.py b/addons/knowledge/models/knowledge_article.py index d016879f2fd..0935bd35d08 100644 --- a/addons/knowledge/models/knowledge_article.py +++ b/addons/knowledge/models/knowledge_article.py @@ -202,13 +202,22 @@ class Article(models.Model): parent = parent.parent_id articles.root_article_id = ancestors[-1:] - @api.depends('parent_id', 'internal_permission') + @api.depends('parent_id', 'parent_id.inherited_permission_parent_id', 'internal_permission') def _compute_inherited_permission(self): """ Computed inherited internal permission. We go up ancestors until finding an article with an internal permission set, or a root article (without parent) or until finding a desynchronized article which serves as permission ancestor. Desynchronized articles break the - permission tree finding. """ + permission tree finding. + + 'parent_id.inherited_permission_parent_id' needs to be in the trigger + as we will need to update this article's inherited permissions if our parent + changes itself from which article it's inheriting. + This allows cascading changes "downwards" when we modify the + internal_permission of an article in the chain. + + It is however not directly used as we optimize the batching and group all + articles by their parent_id.""" self_inherit = self.filtered(lambda article: article.internal_permission) for article in self_inherit: article.inherited_permission = article.internal_permission diff --git a/addons/knowledge/tests/common.py b/addons/knowledge/tests/common.py index 750efccc00b..2473c5aafd6 100644 --- a/addons/knowledge/tests/common.py +++ b/addons/knowledge/tests/common.py @@ -206,6 +206,7 @@ class KnowledgeArticlePermissionsCase(KnowledgeCommon): # - Readonly read (+manager-write) # - Writable " (w) (+portal-read) # - Writable child " (w) + # - Child " # - Nyarlathotep DESYNC read, manager-write # - Child " # READABLE ROOT read (+manager-write) @@ -313,6 +314,11 @@ class KnowledgeArticlePermissionsCase(KnowledgeCommon): 'parent_id': cls.article_write_contents[2].id, }, ]) + cls.article_write_contents_children += cls.env['knowledge.article'].create([ + {'name': 'Child of child of writable through inheritance', + 'parent_id': cls.article_write_contents_children[0].id, + }, + ]) cls.article_write_desync = cls.env['knowledge.article'].create([ # Community/Writable {'article_member_ids': [ diff --git a/addons/knowledge/tests/test_knowledge_article_permissions.py b/addons/knowledge/tests/test_knowledge_article_permissions.py index 94a317f303d..7d55120b359 100644 --- a/addons/knowledge/tests/test_knowledge_article_permissions.py +++ b/addons/knowledge/tests/test_knowledge_article_permissions.py @@ -196,6 +196,10 @@ class TestKnowledgeArticlePermissionsTools(KnowledgeArticlePermissionsCase): writable_as1 = self.article_write_contents[2].with_env(self.env) writable_as2 = writable_as1.with_user(self.user_employee2) self.assertEqual(writable_as2.user_has_access, True) + writable_root = self.article_roots[0].with_env(self.env) + writable_children = self.article_write_contents_children.with_env(self.env) + for child in writable_children: + self.assertEqual(child.inherited_permission_parent_id, writable_root) # downgrade write global perm to read writable_as1._set_internal_permission('none') @@ -214,11 +218,20 @@ class TestKnowledgeArticlePermissionsTools(KnowledgeArticlePermissionsCase): with self.assertRaises(exceptions.AccessError): writable_as2.body # trigger ACLs + # check children inherits downgraded permissions from article + for child in writable_children: + self.assertEqual(child.inherited_permission, 'none', 'Permission: lowering permission should lower the permission of the children') + self.assertEqual(child.inherited_permission_parent_id, writable_as1, 'Permission: lowering permission should make the children inherit the permission from this article') + @users('employee') def test_downgrade_internal_permission_read(self): writable_as1 = self.article_write_contents[2].with_env(self.env) writable_as2 = writable_as1.with_user(self.user_employee2) self.assertEqual(writable_as2.user_has_access, True) + writable_root = self.article_roots[0].with_env(self.env) + writable_children = self.article_write_contents_children.with_env(self.env) + for child in writable_children: + self.assertEqual(child.inherited_permission_parent_id, writable_root) # downgrade write global perm to read writable_as1._set_internal_permission('read') @@ -234,6 +247,11 @@ class TestKnowledgeArticlePermissionsTools(KnowledgeArticlePermissionsCase): self.assertFalse(writable_as2.user_has_write_access) self.assertTrue(writable_as2.user_has_access) + # check children inherits downgraded permissions from article + for child in writable_children: + self.assertEqual(child.inherited_permission, 'read', 'Permission: lowering permission should lower the permission of the children') + self.assertEqual(child.inherited_permission_parent_id, writable_as1, 'Permission: lowering permission should make the children inherit the permission from this article') + @mute_logger('odoo.addons.base.models.ir_rule', 'odoo.models.unlink') @users('employee') def test_remove_member_inherited_rights(self):