Don't send markers to someone not on your roster

Updates #324

.eslintrc.json

@@ -117,7 +117,7 @@
             "error",
             "except-parens"
         ],
-        "no-confusing-arrow": "error",
+        "no-confusing-arrow": "off",
         "no-continue": "off",
         "no-div-regex": "error",
         "no-duplicate-imports": "error",

CHANGES.md

@@ -6,6 +6,7 @@
 - New language supported: Esperanto
 - Accessibility: Tag the chat-content as an ARIA live region, for screen readers
 - Set releases URL to new Github repo
+- Rudimentary support for XEP-0333 chat markers
 - #1369 Don't wrongly interpret message with `subject` as a topic change.
 - #1405 Status of contacts list are not displayed properly
 - #1408 New config option `roomconfig_whitelist`

dist/converse.js

@@ -61659,7 +61659,7 @@ _converse_core__WEBPACK_IMPORTED_MODULE_2__["default"].plugins.add('converse-cha
         _converse.api.send(stanza);
       },
 
-      handleChatMarker(stanza, from_jid, is_carbon) {
+      handleChatMarker(stanza, from_jid, is_carbon, is_roster_contact) {
         const to_bare_jid = Strophe.getBareJidFromJid(stanza.getAttribute('to'));
 
         if (to_bare_jid !== _converse.bare_jid) {
@@ -61679,8 +61679,11 @@ _converse_core__WEBPACK_IMPORTED_MODULE_2__["default"].plugins.add('converse-cha
         } else {
           const marker = markers.pop();
 
-          if (marker.nodeName === 'markable' && !is_carbon) {
+          if (marker.nodeName === 'markable') {
+            if (is_roster_contact && !is_carbon) {
               this.sendMarker(from_jid, stanza.getAttribute('id'), 'received');
+            }
+
             return false;
           } else {
             const msgid = marker && marker.getAttribute('id'),
@@ -62221,7 +62224,8 @@ _converse_core__WEBPACK_IMPORTED_MODULE_2__["default"].plugins.add('converse-cha
         const from_bare_jid = Strophe.getBareJidFromJid(from_jid),
               from_resource = Strophe.getResourceFromJid(from_jid),
               is_me = from_bare_jid === _converse.bare_jid;
-        let contact_jid;
+        let contact_jid,
+            is_roster_contact = false;
 
         if (is_me) {
           // I am the sender, so this must be a forwarded message...
@@ -62233,10 +62237,9 @@ _converse_core__WEBPACK_IMPORTED_MODULE_2__["default"].plugins.add('converse-cha
         } else {
           contact_jid = from_bare_jid;
           await _converse.api.waitUntil('rosterContactsFetched');
+          is_roster_contact = !_.isUndefined(_converse.roster.get(contact_jid));
 
-          const roster_item = _converse.roster.get(contact_jid);
-
-          if (_.isUndefined(roster_item) && !_converse.allow_non_roster_messaging) {
+          if (!is_roster_contact && !_converse.allow_non_roster_messaging) {
             return;
           }
         } // Get chat box, but only create when the message has something to show to the user
@@ -62248,7 +62251,7 @@ _converse_core__WEBPACK_IMPORTED_MODULE_2__["default"].plugins.add('converse-cha
         },
               chatbox = this.getChatBox(contact_jid, chatbox_attrs, has_body);
 
-        if (chatbox && !chatbox.handleMessageCorrection(stanza) && !chatbox.handleReceipt(stanza, from_jid, is_carbon, is_me) && !chatbox.handleChatMarker(stanza, from_jid, is_carbon)) {
+        if (chatbox && !chatbox.handleMessageCorrection(stanza) && !chatbox.handleReceipt(stanza, from_jid, is_carbon, is_me) && !chatbox.handleChatMarker(stanza, from_jid, is_carbon, is_roster_contact)) {
           const attrs = await chatbox.getMessageAttributesFromStanza(stanza, original_stanza);
 
           if (attrs['chat_state'] || !u.isEmptyMessage(attrs)) {

spec/messages.js

@@ -2030,12 +2030,13 @@
 
     describe("A XEP-0333 Chat Marker", function () {
 
-        it("is sent when a markable message is received",
+        it("is sent when a markable message is received from a roster contact",
             mock.initConverse(
                 null, ['rosterGroupsFetched'], {},
                 async function (done, _converse) {
 
             test_utils.createContacts(_converse, 'current', 1);
+            _converse.emit('rosterContactsFetched');
             const contact_jid = mock.cur_names[0].replace(/ /g,'.').toLowerCase() + '@localhost';
             await test_utils.openChatBoxFor(_converse, contact_jid);
             const view = await _converse.api.chatviews.get(contact_jid);
@@ -2063,6 +2064,42 @@
             done();
         }));
 
+        it("is not sent when a markable message is received from someone not on the roster",
+            mock.initConverse(
+                null, ['rosterGroupsFetched'], {'allow_non_roster_messaging': true},
+                async function (done, _converse) {
+
+            _converse.emit('rosterContactsFetched');
+            const contact_jid = 'someone@localhost';
+            const msgid = u.getUniqueId();
+            const stanza = u.toStanza(`
+                <message from='${contact_jid}'
+                    id='${msgid}'
+                    type="chat"
+                    to='${_converse.jid}'>
+                  <body>My lord, dispatch; read o'er these articles.</body>
+                  <markable xmlns='urn:xmpp:chat-markers:0'/>
+                </message>`);
+
+            const sent_stanzas = [];
+            spyOn(_converse.connection, 'send').and.callFake(s => sent_stanzas.push(s));
+            _converse.connection._dataRecv(test_utils.createRequest(stanza));
+            await test_utils.waitUntil(() => _converse.api.chats.get().length == 2);
+            const sent_messages = sent_stanzas
+                .map(s => _.isElement(s) ? s : s.nodeTree)
+                .filter(e => e.nodeName === 'message');
+
+            // Only one message is sent out, and it's not a chat marker
+            expect(sent_messages.length).toBe(1);
+            expect(Strophe.serialize(sent_messages[0])).toBe(
+                `<message id="${sent_messages[0].getAttribute('id')}" to="someone@localhost" type="chat" xmlns="jabber:client">`+
+                    `<active xmlns="http://jabber.org/protocol/chatstates"/>`+
+                    `<no-store xmlns="urn:xmpp:hints"/>`+
+                    `<no-permanent-store xmlns="urn:xmpp:hints"/>`+
+                `</message>`);
+            done();
+        }));
+
         it("is ignored if it's a carbon copy of one that I sent from a different client",
             mock.initConverse(
                 null, ['rosterGroupsFetched'], {},

src/headless/converse-chatboxes.js

@@ -324,7 +324,7 @@ converse.plugins.add('converse-chatboxes', {
                 _converse.api.send(stanza);
             },
 
-            handleChatMarker (stanza, from_jid, is_carbon) {
+            handleChatMarker (stanza, from_jid, is_carbon, is_roster_contact) {
                 const to_bare_jid = Strophe.getBareJidFromJid(stanza.getAttribute('to'));
                 if (to_bare_jid !== _converse.bare_jid) {
                     return false;
@@ -341,8 +341,10 @@ converse.plugins.add('converse-chatboxes', {
                     return false;
                 } else {
                     const marker = markers.pop();
-                    if (marker.nodeName === 'markable' && !is_carbon) {
+                    if (marker.nodeName === 'markable') {
+                        if (is_roster_contact && !is_carbon) {
                             this.sendMarker(from_jid, stanza.getAttribute('id'), 'received');
+                        }
                         return false;
                     } else {
                         const msgid = marker && marker.getAttribute('id'),
@@ -820,7 +822,8 @@ converse.plugins.add('converse-chatboxes', {
                       from_resource = Strophe.getResourceFromJid(from_jid),
                       is_me = from_bare_jid === _converse.bare_jid;
 
-                let contact_jid;
+                let contact_jid,
+                    is_roster_contact = false;
                 if (is_me) {
                     // I am the sender, so this must be a forwarded message...
                     if (_.isNull(to_jid)) {
@@ -833,8 +836,8 @@ converse.plugins.add('converse-chatboxes', {
                 } else {
                     contact_jid = from_bare_jid;
                     await _converse.api.waitUntil('rosterContactsFetched');
-                    const roster_item = _converse.roster.get(contact_jid);
-                    if (_.isUndefined(roster_item) && !_converse.allow_non_roster_messaging) {
+                    is_roster_contact = !_.isUndefined(_converse.roster.get(contact_jid));
+                    if (!is_roster_contact && !_converse.allow_non_roster_messaging) {
                         return;
                     }
                 }
@@ -846,7 +849,7 @@ converse.plugins.add('converse-chatboxes', {
                 if (chatbox &&
                         !chatbox.handleMessageCorrection(stanza) &&
                         !chatbox.handleReceipt (stanza, from_jid, is_carbon, is_me) &&
-                        !chatbox.handleChatMarker(stanza, from_jid, is_carbon)) {
+                        !chatbox.handleChatMarker(stanza, from_jid, is_carbon, is_roster_contact)) {
 
                     const attrs = await chatbox.getMessageAttributesFromStanza(stanza, original_stanza);
                     if (attrs['chat_state'] || !u.isEmptyMessage(attrs)) {