From 0d87b1ccea07b952d9079008cfa3cddaf1ccdbee Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alexandre=20K=C3=BChn?= <aku@odoo.com>
Date: Mon, 21 Jan 2019 15:20:03 +0000
Subject: [PATCH] [FIX] mail: do not open document chat window without access
 rights

Before this commit, a user could unintentionally discard a
notification from a document that he does not have access
rights at the moment.

Steps to reproduce:

1. Setup:
- Odoo with multi-company
- Two companies: "MyCompany" and "SuperCompany"
- Two users "Admin" and "Demo", both have access to being in
  both companies
- "Demo" has Notification management in Odoo in the user preferences.
- "Admin" is logged in "MyCompany"
- "Demo" is logged in "SuperCompany"

2. Scenario:
- "Admin" mentions "Demo" in a log of a document from "MyCompany"
- "Demo" receives the notification and click on the notification
  from messaging menu.
> Dialog prompts that Demo does not have access to the document (OK)
> Empty chat window is open (not OK)
- "Demo" closes the chat window
> Notification is marked as read (not OK)

This commit fixes the issue by aborting the opening of a
chat window if the user does not have the access rights
at the moment. It should prevent marking the notification
as read by mistake by closing the chat window.

Task-ID 1931313

closes odoo/odoo#30414
---
 .../src/js/services/mail_window_manager.js    | 44 +++++++++++--------
 .../tests/document_thread_window_tests.js     | 31 +++++++++++++
 2 files changed, 56 insertions(+), 19 deletions(-)

diff --git a/addons/mail/static/src/js/services/mail_window_manager.js b/addons/mail/static/src/js/services/mail_window_manager.js
index 008b6cf7a6d..eace1e8659c 100644
--- a/addons/mail/static/src/js/services/mail_window_manager.js
+++ b/addons/mail/static/src/js/services/mail_window_manager.js
@@ -101,31 +101,37 @@ MailManager.include({
         // valid threadID, therefore no check
         var thread = this.getThread(threadID);
         var threadWindow = this._getThreadWindow(threadID);
+        var def = $.when();
         if (!threadWindow) {
-            threadWindow = this._makeNewThreadWindow(thread, options);
-            this._placeNewThreadWindow(threadWindow, options.passively);
-
-            threadWindow.appendTo($(this.THREAD_WINDOW_APPENDTO))
-                .then(function () {
-                    self._repositionThreadWindows();
-                    return thread.fetchMessages();
-                }).then(function () {
-                    threadWindow.render();
-                    threadWindow.scrollToBottom();
-                    if (
-                        !self._areAllThreadWindowsHidden() &&
-                        !thread.isFolded() &&
-                        !threadWindow.isPassive()
-                    ) {
-                        thread.markAsRead();
-                    }
-                });
+            def = thread.fetchMessages().then(function () {
+                threadWindow = self._makeNewThreadWindow(thread, options);
+                self._placeNewThreadWindow(threadWindow, options.passively);
+                return threadWindow.appendTo($(self.THREAD_WINDOW_APPENDTO));
+            }).then(function () {
+                self._repositionThreadWindows();
+                threadWindow.render();
+                threadWindow.scrollToBottom();
+                if (
+                    !self._areAllThreadWindowsHidden() &&
+                    !thread.isFolded() &&
+                    !threadWindow.isPassive()
+                ) {
+                    thread.markAsRead();
+                }
+            }).fail(function () {
+                // thread window could not be open, which may happen due to
+                // access error while fetching messages to the document.
+                // abort opening the thread window in this case.
+                thread.close();
+            });
         } else if (!options.passively) {
             if (threadWindow.isHidden()) {
                 this._makeThreadWindowVisible(threadWindow);
             }
         }
-        threadWindow.updateVisualFoldState();
+        def.then(function () {
+            threadWindow.updateVisualFoldState();
+        });
     },
     /**
      * Called when a thread has its window state that has been changed, so its
diff --git a/addons/mail/static/tests/document_thread_window_tests.js b/addons/mail/static/tests/document_thread_window_tests.js
index d7064b8695d..084a7fb32a3 100644
--- a/addons/mail/static/tests/document_thread_window_tests.js
+++ b/addons/mail/static/tests/document_thread_window_tests.js
@@ -271,6 +271,37 @@ QUnit.test('open, fold, unfold and close a document thread window', function (as
     messagingMenu.destroy();
 });
 
+QUnit.test('do not open thread window on fetch message failure', function (assert) {
+    // this may happen when the user receives a notification from a document
+    // that he does not have access rights at the moment.
+    assert.expect(4);
+
+    var messagingMenu = new MessagingMenu();
+    testUtils.addMockEnvironment(messagingMenu, {
+        services: this.services,
+        data: this.data,
+        session: this.session,
+        mockRPC: function (route, args) {
+            if (args.method === 'read' && args.model === 'some.res.model' && args.args[0][0] === 1) {
+                assert.step('some.res.model/1/read');
+                return $.Deferred().reject(); // simulate failure
+            }
+            return this._super.apply(this, arguments);
+        },
+    });
+    messagingMenu.appendTo($('#qunit-fixture'));
+
+    testUtils.dom.click(messagingMenu.$('.dropdown-toggle'));
+    assert.containsOnce(messagingMenu, '.o_mail_preview');
+
+    testUtils.dom.click(messagingMenu.$('.o_mail_preview'));
+    assert.verifySteps(['some.res.model/1/read']);
+    assert.strictEqual($('.o_thread_window').length, 0,
+        "should not have open the DocumentThread in a thread window on fetch messages failure");
+
+    messagingMenu.destroy();
+});
+
 });
 });
 });