From 472fac659ed33302158be8dd0ab5c588ddbea3d2 Mon Sep 17 00:00:00 2001
From: dja-odoo <dja@odoo.com>
Date: Tue, 4 May 2021 07:38:11 +0000
Subject: [PATCH] [IMP] mail: limit followers edition/removal for users with
 read access

Before this commit, if the user has read only access to a record:

 - User could see 'Add Followers' button, which raised AccessError
 - User could see edit and remove icons but performing these actions
  for followers other than one related to logged in user, it also
  raised AccessError.

With this commit, we are restricting it based on the user rights, if
the user has read only access to a record:

 - 'Add Followers', subtype edition(pencil icon) and follwer removal
   buttons will be visible only for the follower related to logged in
   user, not for others. This way we can avoid the unexpected
   AccessErrors.

Note that if user is having write access for the record, nothing
changes.

task-id-2447776

closes odoo/odoo#70575

Signed-off-by: Thibault Delavallee (tde) <tde@openerp.com>
---
 addons/mail/controllers/discuss.py            |  13 +-
 .../follower_list_menu/follower_list_menu.xml |  24 ++-
 addons/mail/static/src/models/follower.js     |  16 +-
 addons/mail/static/src/models/thread.js       |  10 +-
 .../tests/helpers/model_definitions_setup.js  |   3 -
 .../components/follow_button_tests.js         |   3 -
 .../components/follower_list_menu_tests.js    | 159 +++++++++++++++++-
 .../components/follower_subtype_tests.js      |   3 -
 .../components/follower_tests.js              |   7 +-
 9 files changed, 203 insertions(+), 35 deletions(-)

diff --git a/addons/mail/controllers/discuss.py b/addons/mail/controllers/discuss.py
index 3d07c56fdb7..0f3c88eec38 100644
--- a/addons/mail/controllers/discuss.py
+++ b/addons/mail/controllers/discuss.py
@@ -8,7 +8,7 @@ from psycopg2 import IntegrityError
 from psycopg2.errorcodes import UNIQUE_VIOLATION
 
 from odoo import http
-from odoo.exceptions import UserError
+from odoo.exceptions import AccessError, UserError
 from odoo.http import request
 from odoo.tools import consteq, file_open
 from odoo.tools.misc import get_lang
@@ -413,8 +413,14 @@ class DiscussController(http.Controller):
 
     @http.route('/mail/thread/data', methods=['POST'], type='json', auth='user')
     def mail_thread_data(self, thread_model, thread_id, request_list, **kwargs):
-        res = {}
+        res = {'hasWriteAccess': False}
         thread = request.env[thread_model].with_context(active_test=False).search([('id', '=', thread_id)])
+        try:
+            thread.check_access_rights("write")
+            thread.check_access_rule("write")
+            res['hasWriteAccess'] = True
+        except AccessError:
+            pass
         if 'activities' in request_list:
             res['activities'] = thread.activity_ids.activity_format()
         if 'attachments' in request_list:
@@ -427,9 +433,6 @@ class DiscussController(http.Controller):
                 'display_name': follower.display_name,
                 'email': follower.email,
                 'is_active': follower.is_active,
-                # When editing the followers, the "pencil" icon that leads to the edition of subtypes
-                # should be always be displayed and not only when "debug" mode is activated.
-                'is_editable': True,
                 'partner': follower.partner_id.mail_partner_format()[follower.partner_id],
             } for follower in thread.message_follower_ids]
         if 'suggestedRecipients' in request_list:
diff --git a/addons/mail/static/src/components/follower_list_menu/follower_list_menu.xml b/addons/mail/static/src/components/follower_list_menu/follower_list_menu.xml
index 6d5d420c06c..04c3edafc7d 100644
--- a/addons/mail/static/src/components/follower_list_menu/follower_list_menu.xml
+++ b/addons/mail/static/src/components/follower_list_menu/follower_list_menu.xml
@@ -12,20 +12,26 @@
 
                     <t t-if="state.isDropdownOpen">
                         <div class="o_FollowerListMenu_dropdown dropdown-menu dropdown-menu-right" role="menu">
-                            <t t-if="thread.channel_type !== 'chat'">
+                            <t t-if="thread.model !== 'channel' and thread.hasWriteAccess">
                                 <a class="o_FollowerListMenu_addFollowersButton dropdown-item" href="#" role="menuitem" t-on-click="_onClickAddFollowers">
                                     Add Followers
                                 </a>
+                                <t t-if="thread.followers.length > 0">
+                                    <div role="separator" class="dropdown-divider"/>
+                                </t>
                             </t>
                             <t t-if="thread.followers.length > 0">
-                                <div role="separator" class="dropdown-divider"/>
-                                <t t-foreach="thread.followers" t-as="follower" t-key="follower.localId">
-                                    <Follower
-                                        className="'o_FollowerMenu_follower dropdown-item'"
-                                        followerLocalId="follower.localId"
-                                        onClick="_onClickFollower"
-                                    />
-                                </t>
+                                <Follower
+                                    t-foreach="thread.followers" t-as="follower" t-key="follower.localId"
+                                    className="'o_FollowerMenu_follower dropdown-item'"
+                                    followerLocalId="follower.localId"
+                                    onClick="_onClickFollower"
+                                />
+                            </t>
+                            <t t-elif="!thread.hasWriteAccess">
+                                <div class="o_FollowerListMenu_noFollowers dropdown-item disabled">
+                                    No Followers
+                                </div>
                             </t>
                         </div>
                     </t>
diff --git a/addons/mail/static/src/models/follower.js b/addons/mail/static/src/models/follower.js
index ab8e3738c7f..a028149c914 100644
--- a/addons/mail/static/src/models/follower.js
+++ b/addons/mail/static/src/models/follower.js
@@ -20,9 +20,6 @@ registerModel({
             if ('is_active' in data) {
                 data2.isActive = data.is_active;
             }
-            if ('is_editable' in data) {
-                data2.isEditable = data.is_editable;
-            }
             if ('partner_id' in data) {
                 if (!data.partner_id) {
                     data2.partner = unlinkAll();
@@ -134,6 +131,14 @@ registerModel({
             }
             this.closeSubtypes();
         },
+        /**
+         * @private
+         * @returns {boolean}
+         */
+        _computeIsEditable() {
+            const hasWriteAccess = this.followedThread ? this.followedThread.hasWriteAccess : false;
+            return this.messaging.currentPartner === this.partner ? true : hasWriteAccess;
+        },
     },
     fields: {
         followedThread: one('Thread', {
@@ -150,8 +155,11 @@ registerModel({
         isActive: attr({
             default: true,
         }),
+        /**
+         * States whether the follower's subtypes are editable by current user.
+         */
         isEditable: attr({
-            default: false,
+            compute: '_computeIsEditable',
         }),
         partner: one('Partner', {
             required: true,
diff --git a/addons/mail/static/src/models/thread.js b/addons/mail/static/src/models/thread.js
index f8844728ff3..ba1b4d8da8d 100644
--- a/addons/mail/static/src/models/thread.js
+++ b/addons/mail/static/src/models/thread.js
@@ -635,6 +635,7 @@ registerModel({
                 activities: activitiesData,
                 attachments: attachmentsData,
                 followers: followersData,
+                hasWriteAccess,
                 suggestedRecipients: suggestedRecipientsData,
             } = await this.env.services.rpc({
                 route: '/mail/thread/data',
@@ -647,7 +648,7 @@ registerModel({
             if (!this.exists()) {
                 return;
             }
-            const values = {};
+            const values = { hasWriteAccess };
             if (activitiesData) {
                 Object.assign(values, {
                     activities: insertAndReplace(activitiesData.map(activityData =>
@@ -2011,6 +2012,13 @@ registerModel({
             compute: '_computeHasSeenIndicators',
             default: false,
         }),
+        /**
+         * States whether current user has write access for the record. If yes, few other operations
+         * (like adding other followers to the thread) are enabled for the user.
+         */
+        hasWriteAccess: attr({
+            default: false,
+        }),
         id: attr({
             readonly: true,
             required: true,
diff --git a/addons/mail/static/tests/helpers/model_definitions_setup.js b/addons/mail/static/tests/helpers/model_definitions_setup.js
index 686b07f0944..32a12776ddf 100644
--- a/addons/mail/static/tests/helpers/model_definitions_setup.js
+++ b/addons/mail/static/tests/helpers/model_definitions_setup.js
@@ -58,9 +58,6 @@ insertModelFields('mail.channel', {
     state: { default: 'open', string: "FoldState", type: "char" },
     uuid: { default: () => _.uniqueId('mail.channel_uuid-') },
 });
-insertModelFields('mail.followers', {
-    is_editable: { type: 'boolean' },
-});
 insertModelFields('mail.message', {
     history_partner_ids: { relation: 'res.partner', string: "Partners with History", type: 'many2many' },
     is_discussion: { string: 'Discussion', type: 'boolean' },
diff --git a/addons/mail/static/tests/qunit_suite_tests/components/follow_button_tests.js b/addons/mail/static/tests/qunit_suite_tests/components/follow_button_tests.js
index bff8cde6e2e..e95e99e8cfe 100644
--- a/addons/mail/static/tests/qunit_suite_tests/components/follow_button_tests.js
+++ b/addons/mail/static/tests/qunit_suite_tests/components/follow_button_tests.js
@@ -80,7 +80,6 @@ QUnit.test('hover following button', async function (assert) {
     this.data['mail.followers'].records.push({
         id: 1,
         is_active: true,
-        is_editable: true,
         partner_id: this.data.currentPartnerId,
         res_id: 100,
         res_model: 'res.partner',
@@ -148,7 +147,6 @@ QUnit.test('click on "follow" button', async function (assert) {
     this.data['mail.followers'].records.push({
         id: 1,
         is_active: true,
-        is_editable: true,
         partner_id: this.data.currentPartnerId,
         res_id: 100,
         res_model: 'res.partner',
@@ -203,7 +201,6 @@ QUnit.test('click on "unfollow" button', async function (assert) {
     this.data['mail.followers'].records.push({
         id: 1,
         is_active: true,
-        is_editable: true,
         partner_id: this.data.currentPartnerId,
         res_id: 100,
         res_model: 'res.partner',
diff --git a/addons/mail/static/tests/qunit_suite_tests/components/follower_list_menu_tests.js b/addons/mail/static/tests/qunit_suite_tests/components/follower_list_menu_tests.js
index c98d1823614..9be84a917d2 100644
--- a/addons/mail/static/tests/qunit_suite_tests/components/follower_list_menu_tests.js
+++ b/addons/mail/static/tests/qunit_suite_tests/components/follower_list_menu_tests.js
@@ -70,6 +70,7 @@ QUnit.test('base rendering editable', async function (assert) {
     const thread = messaging.models['Thread'].create({
         id: 100,
         model: 'res.partner',
+        hasWriteAccess: true,
     });
     await this.createFollowerListMenuComponent(thread, widget.el);
 
@@ -141,13 +142,13 @@ QUnit.test('click on "add followers" button', async function (assert) {
         email: "bla@bla.bla",
         id: 1,
         is_active: true,
-        is_editable: true,
         name: "François Perusse",
         res_id: 100,
         res_model: 'res.partner',
     });
     const { messaging, widget } = await start({ data: this.data, env: { bus } });
     const thread = messaging.models['Thread'].create({
+        hasWriteAccess: true,
         id: 100,
         model: 'res.partner',
     });
@@ -241,7 +242,6 @@ QUnit.test('click on remove follower', async function (assert) {
         followedThread: link(thread),
         id: 2,
         isActive: true,
-        isEditable: true,
         partner: insert({
             email: "bla@bla.bla",
             id: messaging.currentPartner.id,
@@ -278,5 +278,160 @@ QUnit.test('click on remove follower', async function (assert) {
     );
 });
 
+QUnit.test('Hide "Add follower" and subtypes edition/removal buttons except own user on read only record', async function (assert) {
+    assert.expect(5);
+
+    this.data['res.partner'].records.push({ id: 100 }, { id: 11 });
+    this.data['mail.followers'].records.push(
+        {
+            id: 1,
+            name: "Jean Michang",
+            is_active: true,
+            partner_id: this.data.currentPartnerId,
+            res_id: 100,
+            res_model: 'res.partner',
+        }, {
+            id: 2,
+            name: "Eden Hazard",
+            is_active: true,
+            partner_id: 11,
+            res_id: 100,
+            res_model: 'res.partner',
+        },
+    );
+    const { click, createChatterContainerComponent } = await start({
+        data: this.data,
+        async mockRPC(route, args) {
+            if (route === '/mail/thread/data') {
+                // mimic user with no write access
+                const res = await this._super(...arguments);
+                res['hasWriteAccess'] = false;
+                return res;
+            }
+            return this._super(...arguments);
+        },
+    });
+    await createChatterContainerComponent({
+        threadId: 100,
+        threadModel: 'res.partner',
+    });
+
+    await click('.o_FollowerListMenu_buttonFollowers');
+    assert.containsNone(
+        document.body,
+        '.o_FollowerListMenu_addFollowersButton',
+        "'Add followers' button should not be displayed for a readonly record",
+    );
+    const followersList = document.querySelectorAll('.o_Follower');
+    assert.containsOnce(
+        followersList[0],
+        '.o_Follower_editButton',
+        "should display edit button for a follower related to current user",
+    );
+    assert.containsOnce(
+        followersList[0],
+        '.o_Follower_removeButton',
+        "should display remove button for a follower related to current user",
+    );
+    assert.containsNone(
+        followersList[1],
+        '.o_Follower_editButton',
+        "should not display edit button for other followers on a readonly record",
+    );
+    assert.containsNone(
+        followersList[1],
+        '.o_Follower_removeButton',
+        "should not display remove button for others on a readonly record",
+    );
+});
+
+QUnit.test('Show "Add follower" and subtypes edition/removal buttons on all followers if user has write access', async function (assert) {
+    assert.expect(5);
+
+    this.data['res.partner'].records.push({ id: 100 }, { id: 11 });
+    this.data['mail.followers'].records.push(
+        {
+            id: 1,
+            name: "Jean Michang",
+            is_active: true,
+            partner_id: this.data.currentPartnerId,
+            res_id: 100,
+            res_model: 'res.partner',
+        }, {
+            id: 2,
+            name: "Eden Hazard",
+            is_active: true,
+            partner_id: 11,
+            res_id: 100,
+            res_model: 'res.partner',
+        },
+    );
+    const { click, createChatterContainerComponent } = await start({
+        data: this.data,
+        async mockRPC(route, args) {
+            if (route === '/mail/thread/data') {
+                // mimic user with write access
+                const res = await this._super(...arguments);
+                res['hasWriteAccess'] = true;
+                return res;
+            }
+            return this._super(...arguments);
+        },
+    });
+    await createChatterContainerComponent({
+        threadId: 100,
+        threadModel: 'res.partner',
+    });
+
+    await click('.o_FollowerListMenu_buttonFollowers');
+    assert.containsOnce(
+        document.body,
+        '.o_FollowerListMenu_addFollowersButton',
+        "'Add followers' button should be displayed for the writable record",
+    );
+    const followersList = document.querySelectorAll('.o_Follower');
+    assert.containsOnce(
+        followersList[0],
+        '.o_Follower_editButton',
+        "should display edit button for a follower related to current user",
+    );
+    assert.containsOnce(
+        followersList[0],
+        '.o_Follower_removeButton',
+        "should display remove button for a follower related to current user",
+    );
+    assert.containsOnce(
+        followersList[1],
+        '.o_Follower_editButton',
+        "should display edit button for other followers also on the writable record",
+    );
+    assert.containsOnce(
+        followersList[1],
+        '.o_Follower_removeButton',
+        "should display remove button for other followers also on the writable record",
+    );
+});
+
+QUnit.test('Show "No Followers" dropdown-item if there are no followers and user dose not have write access', async function (assert) {
+    assert.expect(1);
+
+    const { messaging, widget } = await start({ data: this.data });
+    const thread = messaging.models['Thread'].create({
+        id: 100,
+        model: 'res.partner',
+        hasWriteAccess: false,
+    });
+
+    await this.createFollowerListMenuComponent(thread, widget.el);
+    await afterNextRender(() => {
+        document.querySelector('.o_FollowerListMenu_buttonFollowers').click();
+    });
+    assert.containsOnce(
+        document.body,
+        '.o_FollowerListMenu_noFollowers.disabled',
+        "should display 'No Followers' dropdown-item",
+    );
+});
+
 });
 });
diff --git a/addons/mail/static/tests/qunit_suite_tests/components/follower_subtype_tests.js b/addons/mail/static/tests/qunit_suite_tests/components/follower_subtype_tests.js
index 849425b833e..7159d0ddded 100644
--- a/addons/mail/static/tests/qunit_suite_tests/components/follower_subtype_tests.js
+++ b/addons/mail/static/tests/qunit_suite_tests/components/follower_subtype_tests.js
@@ -44,7 +44,6 @@ QUnit.test('simplest layout of a followed subtype', async function (assert) {
         followedThread: link(thread),
         id: 2,
         isActive: true,
-        isEditable: true,
     });
     const followerSubtype = messaging.models['FollowerSubtype'].create({
         id: 1,
@@ -105,7 +104,6 @@ QUnit.test('simplest layout of a not followed subtype', async function (assert)
         followedThread: link(thread),
         id: 2,
         isActive: true,
-        isEditable: true,
     });
     const followerSubtype = messaging.models['FollowerSubtype'].create({
         id: 1,
@@ -163,7 +161,6 @@ QUnit.test('toggle follower subtype checkbox', async function (assert) {
         followedThread: link(thread),
         id: 2,
         isActive: true,
-        isEditable: true,
     });
     const followerSubtype = messaging.models['FollowerSubtype'].create({
         id: 1,
diff --git a/addons/mail/static/tests/qunit_suite_tests/components/follower_tests.js b/addons/mail/static/tests/qunit_suite_tests/components/follower_tests.js
index d8b3a68d622..a7bccbaa53d 100644
--- a/addons/mail/static/tests/qunit_suite_tests/components/follower_tests.js
+++ b/addons/mail/static/tests/qunit_suite_tests/components/follower_tests.js
@@ -32,6 +32,7 @@ QUnit.test('base rendering not editable', async function (assert) {
     const { messaging, widget } = await start({ data: this.data });
 
     const thread = messaging.models['Thread'].create({
+        hasWriteAccess: false,
         id: 100,
         model: 'res.partner',
     });
@@ -43,7 +44,6 @@ QUnit.test('base rendering not editable', async function (assert) {
         followedThread: link(thread),
         id: 2,
         isActive: true,
-        isEditable: false,
     });
     await this.createFollowerComponent(follower, widget.el);
     assert.containsOnce(
@@ -78,6 +78,7 @@ QUnit.test('base rendering editable', async function (assert) {
 
     const { messaging, widget } = await start({ data: this.data });
     const thread = messaging.models['Thread'].create({
+        hasWriteAccess: true,
         id: 100,
         model: 'res.partner',
     });
@@ -89,7 +90,6 @@ QUnit.test('base rendering editable', async function (assert) {
         followedThread: link(thread),
         id: 2,
         isActive: true,
-        isEditable: true,
     });
     await this.createFollowerComponent(follower, widget.el);
     assert.containsOnce(
@@ -158,7 +158,6 @@ QUnit.test('click on partner follower details', async function (assert) {
         followedThread: link(thread),
         id: 2,
         isActive: true,
-        isEditable: true,
         partner: insert({
             email: "bla@bla.bla",
             id: messaging.currentPartner.id,
@@ -192,7 +191,6 @@ QUnit.test('click on edit follower', async function (assert) {
     this.data['mail.followers'].records.push({
         id: 2,
         is_active: true,
-        is_editable: true,
         partner_id: this.data.currentPartnerId,
         res_id: 100,
         res_model: 'res.partner',
@@ -266,7 +264,6 @@ QUnit.test('edit follower and close subtype dialog', async function (assert) {
         followedThread: link(thread),
         id: 2,
         isActive: true,
-        isEditable: true,
         partner: insert({
             email: "bla@bla.bla",
             id: messaging.currentPartner.id,