From 0b2ee16885bfa03b1df577a0d9533ffe4c7b5bad Mon Sep 17 00:00:00 2001 From: Martin Trigaux Date: Tue, 18 Nov 2014 15:09:13 +0100 Subject: [PATCH] [FIX] project: access rights and followers For privacy_visibility 'followers' or 'portal', the user should be follower of the project (not the task). Remove public access to portal task Fixes #2372 If no project on the task (or other rule), an employee (not a portal) can access if is follower of the task. Follower rule is not enough as a user creating a rule will subscribe to the rule but to subscribe to record, the user should have access to it in the first place. To make sure the snake does not bit its tail, fallback to give access on task where the user is reponsible (user_id = user.id). Fixes #139 Adapted the tests to the new behaviour (removed not relevant and added some on creation) --- .../security/portal_security.xml | 59 ++++++++++++------- .../tests/test_access_rights.py | 43 ++++++++------ addons/project/security/project_security.xml | 20 ++++--- 3 files changed, 76 insertions(+), 46 deletions(-) diff --git a/addons/portal_project/security/portal_security.xml b/addons/portal_project/security/portal_security.xml index f6adf6b1391..b35c8be87d5 100644 --- a/addons/portal_project/security/portal_security.xml +++ b/addons/portal_project/security/portal_security.xml @@ -16,12 +16,17 @@ Project: portal users: public, portal or following - ['|', - ('privacy_visibility', 'in', ['public', 'portal']), - '&', - ('privacy_visibility', '=', 'followers'), - ('message_follower_ids', 'in', [user.partner_id.id]), - ] + [ + '|', + '|', + ('privacy_visibility', '=', 'public'), + '&', + ('privacy_visibility', '=', 'portal'), + ('message_follower_ids', 'child_of', [user.partner_id.commercial_partner_id.id]), + '&', + ('privacy_visibility', '=', 'followers'), + ('message_follower_ids', 'in', [user.partner_id.id]) + ] @@ -34,26 +39,38 @@ Project/Task: employees: public, portal, employee or (followers and following) - ['|', - ('project_id.privacy_visibility', 'in', ['public', 'portal', 'employees']), - '&', - ('project_id.privacy_visibility', '=', 'followers'), - ('message_follower_ids', 'in', [user.partner_id.id]), - ] + [ + '|', + ('project_id.privacy_visibility', 'in', ['public', 'portal', 'employees']), + '|', + '&', + ('project_id.privacy_visibility', '=', 'followers'), + ('project_id.message_follower_ids', 'in', [user.partner_id.id]), + '|', + ('message_follower_ids', 'in', [user.partner_id.id]), + ('user_id', '=', user.id) + ] Project/Task: portal users: public or (portal and colleagues following) or (followers and following) - ['|', '|', - ('project_id.privacy_visibility', 'in', ['public']), - '&', - ('project_id.privacy_visibility', '=', 'portal'), - ('message_follower_ids', 'child_of', [user.partner_id.commercial_partner_id.id]), - '&', - ('project_id.privacy_visibility', '=', 'followers'), - ('message_follower_ids', 'in', [user.partner_id.id]), - ] + [ + '|', + '|', + '|', + ('project_id.privacy_visibility', '=', 'public'), + '&', + ('project_id.privacy_visibility', '=', 'portal'), + ('project_id.message_follower_ids', 'child_of', [user.partner_id.commercial_partner_id.id]), + '&', + ('project_id.privacy_visibility', '=', 'followers'), + ('project_id.message_follower_ids', 'in', [user.partner_id.id]), + '&', + # on employee project can receive messages but not access the object + ('project_id.privacy_visibility', '!=', 'employees'), + ('message_follower_ids', 'in', [user.partner_id.id]) + ] diff --git a/addons/portal_project/tests/test_access_rights.py b/addons/portal_project/tests/test_access_rights.py index e36df90b652..a4f29735a69 100644 --- a/addons/portal_project/tests/test_access_rights.py +++ b/addons/portal_project/tests/test_access_rights.py @@ -149,15 +149,11 @@ class TestPortalProject(TestPortalProjectBase): self.assertRaises(AccessError, self.project_task.search, cr, self.user_none_id, [('project_id', '=', pigs_id)]) # Data: task follower + self.project_project.message_subscribe_users(cr, self.user_manager_id, [pigs_id], [self.user_portal_id]) self.project_task.message_subscribe_users(cr, self.user_projectuser_id, [self.task_1_id, self.task_3_id], [self.user_portal_id]) - # Do: Chell reads project -> ok (portal ok public) + # Do: Chell reads project -> ok (portal ok portal) self.project_project.read(cr, self.user_portal_id, [pigs_id], ['state']) - # Test: only followed project tasks visible + assigned - task_ids = self.project_task.search(cr, self.user_portal_id, [('project_id', '=', pigs_id)]) - test_task_ids = set([self.task_1_id, self.task_3_id, self.task_5_id]) - self.assertEqual(set(task_ids), test_task_ids, - 'access rights: portal user should see the followed tasks of a portal project') # Do: Donovan reads project -> ko (public ko portal) self.assertRaises(except_orm, self.project_project.read, cr, self.user_public_id, [pigs_id], ['state']) @@ -197,6 +193,12 @@ class TestPortalProject(TestPortalProjectBase): task_ids = self.project_task.search(cr, self.user_public_id, [('project_id', '=', pigs_id)]) self.assertFalse(task_ids, 'access rights: public user should not see tasks of an employees project') + # Do: project user is employee and can create a task + tmp_task_id = self.project_task.create(cr, self.user_projectuser_id, { + 'name': 'Pigs task', 'project_id': pigs_id + }, {'mail_create_nolog': True}) + self.project_task.unlink(cr, self.user_projectuser_id, [tmp_task_id]) + # ---------------------------------------- # CASE4: followers project # ---------------------------------------- @@ -214,7 +216,8 @@ class TestPortalProject(TestPortalProjectBase): # Do: Bert reads project -> crash, no group self.assertRaises(AccessError, self.project_project.read, cr, self.user_none_id, [pigs_id], ['state']) - # Do: Chell reads project -> ko (portal ko employee) + # Do: Chell reads project -> ko (portal ko followers) + self.project_project.message_unsubscribe_users(cr, self.user_portal_id, [pigs_id], [self.user_portal_id]) self.assertRaises(except_orm, self.project_project.read, cr, self.user_portal_id, [pigs_id], ['state']) # Test: no project task visible task_ids = self.project_task.search(cr, self.user_portal_id, [('project_id', '=', pigs_id)]) @@ -234,19 +237,25 @@ class TestPortalProject(TestPortalProjectBase): # Do: Alfred reads project -> ok (follower ok followers) self.project_project.read(cr, self.user_projectuser_id, [pigs_id], ['state']) - # Test: followed + assigned tasks visible - task_ids = self.project_task.search(cr, self.user_projectuser_id, [('project_id', '=', pigs_id)]) - test_task_ids = set([self.task_1_id, self.task_3_id, self.task_4_id]) - self.assertEqual(set(task_ids), test_task_ids, - 'access rights: employee user should not see followed + assigned tasks of a follower project') # Do: Chell reads project -> ok (follower ok follower) self.project_project.read(cr, self.user_portal_id, [pigs_id], ['state']) - # Test: followed + assigned tasks visible - task_ids = self.project_task.search(cr, self.user_portal_id, [('project_id', '=', pigs_id)]) - test_task_ids = set([self.task_1_id, self.task_3_id, self.task_5_id]) - self.assertEqual(set(task_ids), test_task_ids, - 'access rights: employee user should not see followed + assigned tasks of a follower project') # Do: Donovan reads project -> ko (public ko follower even if follower) self.assertRaises(except_orm, self.project_project.read, cr, self.user_public_id, [pigs_id], ['state']) + + # Do: project user is follower of the project and can create a task + self.project_task.create(cr, self.user_projectuser_id, { + 'name': 'Pigs task', 'project_id': pigs_id + }, {'mail_create_nolog': True}) + + # not follower user should not be able to create a task + self.project_project.message_unsubscribe_users(cr, self.user_projectuser_id, [pigs_id], [self.user_projectuser_id]) + self.assertRaises(except_orm, + self.project_task.create, cr, self.user_projectuser_id, {'name': 'Pigs task', 'project_id': pigs_id}, {'mail_create_nolog': True} + ) + + # Do: project user can create a task without project + self.project_task.create(cr, self.user_projectuser_id, { + 'name': 'Pigs task', 'project_id': False + }, {'mail_create_nolog': True}) diff --git a/addons/project/security/project_security.xml b/addons/project/security/project_security.xml index 444a9d03b40..e1c2a8f997a 100644 --- a/addons/project/security/project_security.xml +++ b/addons/project/security/project_security.xml @@ -79,14 +79,18 @@ Project/Task: employees: public or employee or (followers and following) - ['|', - ('project_id.privacy_visibility', 'in', ['public', 'employees']), - '&', - '|', - ('project_id', '=', False), - ('project_id.privacy_visibility', '=', 'followers'), - ('message_follower_ids', 'in', [user.partner_id.id]), - ] + [ + '|', + ('project_id.privacy_visibility', 'in', ['public', 'employees']), + '|', + '&', + ('project_id.privacy_visibility', '=', 'followers'), + ('project_id.message_follower_ids', 'in', [user.partner_id.id]), + '|', + ('message_follower_ids', 'in', [user.partner_id.id]), + # to subscribe check access to the record, follower is not enough at creation + ('user_id', '=', user.id) + ]