Merge branch '0614'

Ben Brown · Ben Brown · commit 8fe85d4e318d · 2018-04-30T12:32:56.000-05:00
diff --git a/__test__/lib/Slackbot_worker.test.js b/__test__/lib/Slackbot_worker.test.js
@@ -0,0 +1,108 @@
+'use strict';
+
+const util = require('util');
+const EventEmitter = require('events').EventEmitter;
+
+let mockWs;
+function mockWebSocket(url) {
+    this.url = url;
+    this.close = function() { };
+    this.ping = function() { };
+    mockWs = this;
+}
+util.inherits(mockWebSocket, EventEmitter);
+
+jest.mock('ws', () => mockWebSocket);
+
+let mockResponse;
+let mockRequest = {
+    post: jest.fn().mockImplementation((params, cb) => {
+        cb(null, mockResponse, mockResponse.body);
+    })
+};
+jest.mock('request', () => mockRequest);
+
+// these need to be required after mocks are set up
+const CoreBot = require('../../lib/CoreBot');
+const Slackbot_worker = require('../../lib/Slackbot_worker');
+
+beforeEach(() => {
+    jest.clearAllMocks();
+});
+
+describe('startRTM', () => {
+    it('connects to rtm url and calls callback function', () => {
+        const botkit = CoreBot({});
+        const bot = Slackbot_worker(botkit, {});
+
+        mockResponse = {
+            statusCode: 200,
+            body: JSON.stringify({
+                ok: true,
+                url: 'http://mockurl',
+                self: {
+                    name: 'botkit'
+                }
+            })
+        };
+
+        const cb = jest.fn();
+        bot.startRTM(cb);
+        mockWs.emit('open');
+        expect(mockRequest.post).toHaveBeenCalledTimes(1);
+        expect(mockWs.url).toBe('http://mockurl');
+        expect(cb).toHaveBeenCalledTimes(1);
+
+        bot.closeRTM();
+
+        const errorArg = cb.mock.calls[0][0];
+        expect(errorArg).toBeNull();
+    });
+    it('handles Slack API rtm.connect errors', () => {
+        const botkit = CoreBot({});
+        const bot = Slackbot_worker(botkit, {});
+
+        mockResponse = {
+            statusCode: 200,
+            body: JSON.stringify({
+                ok: false,
+                error: 'test_error'
+            })
+        };
+
+        const cb = jest.fn();
+        bot.startRTM(cb);
+        expect(mockRequest.post).toHaveBeenCalledTimes(1);
+        expect(cb).toHaveBeenCalledTimes(1);
+        bot.closeRTM();
+
+        const errorArg = cb.mock.calls[0][0];
+        expect(errorArg).toBe('test_error');
+    });
+    it('handles websocket connection errors', () => {
+        const botkit = CoreBot({});
+        const bot = Slackbot_worker(botkit, {});
+
+        mockResponse = {
+            statusCode: 200,
+            body: JSON.stringify({
+                ok: true,
+                url: 'http://mockurl',
+                self: {
+                    name: 'botkit'
+                }
+            })
+        };
+
+        const cb = jest.fn();
+        bot.startRTM(cb);
+        mockWs.emit('error', 'test websocket error');
+        expect(mockRequest.post).toHaveBeenCalledTimes(1);
+        expect(cb).toHaveBeenCalledTimes(1);
+
+        bot.closeRTM();
+
+        const errorArg = cb.mock.calls[0][0];
+        expect(errorArg).toBe('test websocket error');
+    });
+});
diff --git a/changelog.md b/changelog.md
@@ -4,6 +4,18 @@
 
 [Want to contribute? Read our guide!](https://github.com/howdyai/botkit/blob/master/CONTRIBUTING.md)
 
+# 0.6.14
+
+* Fix for require_delivery option in Facebook bots. [PR #1312](https://github.com/howdyai/botkit/pull/1312)
+
+* Errors encountered during Slack RTM connection process will now be reprorted to the callback function [PR #1335](https://github.com/howdyai/botkit/pull/1335)
+
+* Updated methodology used to validate email addresses when restricting access to Cisco Spark bots
+
+* Updated dependencies to latest stable versions
+
+
+
 # 0.6.13
 
 * Fix bugs and refactor handling of message actions, particularly as they relate to Botkit Studio scripts
diff --git a/lib/CiscoSparkbot.js b/lib/CiscoSparkbot.js
@@ -157,9 +157,12 @@ function Sparkbot(configuration) {
                 domains = controller.config.limit_to_domain;
             }
 
+            var addressParser = require('email-addresses');
+            var a = addressParser.parseOneAddress(message.raw_message.data.personEmail);
+
             var allowed = false;
             for (var d = 0; d < domains.length; d++) {
-                if (message.user.toLowerCase().indexOf(domains[d]) >= 0) {
+                if (a.domain.toLowerCase() == domains[d]) {
                     allowed = true;
                 }
             }
diff --git a/lib/Facebook.js b/lib/Facebook.js
@@ -489,24 +489,19 @@ function Facebookbot(configuration) {
     facebook_botkit.middleware.receive.use(function handleDelivery(bot, message, next) {
 
         if (message.type === 'message_delivered' && facebook_botkit.config.require_delivery) {
-            // get list of mids in this message
-            for (var m = 0; m < message.delivery.mids.length; m++) {
-                var mid = message.delivery.mids[m];
-
-                // loop through all active conversations this bot is having
-                // and mark messages in conversations as delivered = true
-                // note: we don't pass the real event in here because message_delivered events are excluded from conversations and won't ever match!
-                bot.findConversation({user: message.user}, function(convo) {
-                    if (convo) {
-                        for (var s = 0; s < convo.sent.length; s++) {
-                            if (convo.sent[s].sent_timestamp <= message.delivery.watermark ||
-                          (convo.sent[s].api_response && convo.sent[s].api_response.message_id == mid)) {
-                                convo.sent[s].delivered = true;
-                            }
+            // loop through all active conversations this bot is having
+            // and mark messages in conversations as delivered = true
+            // note: we don't pass the real event in here because message_delivered events are excluded from conversations and won't ever match!
+            // also note: we only use message.delivery.watermark since message.delivery.mids can sometimes not be in the payload (#1311)
+            bot.findConversation({user: message.user}, function(convo) {
+                if (convo) {
+                    for (var s = 0; s < convo.sent.length; s++) {
+                        if (convo.sent[s].sent_timestamp <= message.delivery.watermark) {
+                            convo.sent[s].delivered = true;
                         }
                     }
-                });
-            }
+                }
+            });
         }
 
         next();
diff --git a/lib/Slackbot_worker.js b/lib/Slackbot_worker.js
@@ -128,15 +128,22 @@ module.exports = function(botkit, config) {
         botkit.shutdown();
     };
 
-    bot.startRTM = function(cb) {
+    bot.startRTM = function(userCb) {
+        var userCbCalled = false;
+        var cb = function() {
+            if (!userCbCalled) {
+                userCbCalled = true;
+                userCb && userCb.apply(this, arguments);
+            }
+        };
         var lastPong = 0;
         bot.api.rtm.connect({}, function(err, res) {
             if (err) {
-                return cb && cb(err);
+                return cb(err);
             }
 
             if (!res) {
-                return cb && cb('Invalid response from rtm.start');
+                return cb('Invalid response from rtm.start');
             }
 
             bot.identity = res.self;
@@ -206,14 +213,16 @@ module.exports = function(botkit, config) {
 
                 botkit.startTicking();
 
-                cb && cb(null, bot, res);
+                cb(null, bot, res);
             });
 
             bot.rtm.on('error', function(err) {
                 botkit.log.error('RTM websocket error!', err);
                 if (pingTimeoutId) {
                     clearTimeout(pingTimeoutId);
                 }
+
+                cb(err);
                 botkit.trigger('rtm_close', [bot, err]);
             });
 
@@ -222,6 +231,8 @@ module.exports = function(botkit, config) {
                 if (pingTimeoutId) {
                     clearTimeout(pingTimeoutId);
                 }
+
+                cb(message);
                 botkit.trigger('rtm_close', [bot]);
 
                 /**
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json

Original file line number	Diff line number	Diff line change
`@@ -157,9 +157,12 @@ function Sparkbot(configuration) {`
`157`	`157`	`domains = controller.config.limit_to_domain;`
`158`	`158`	`}`
`159`	`159`
	`160`	`+ var addressParser = require('email-addresses');`
	`161`	`+ var a = addressParser.parseOneAddress(message.raw_message.data.personEmail);`
	`162`	`+`
`160`	`163`	`var allowed = false;`
`161`	`164`	`for (var d = 0; d < domains.length; d++) {`
`162`		`- if (message.user.toLowerCase().indexOf(domains[d]) >= 0) {`
	`165`	`+ if (a.domain.toLowerCase() == domains[d]) {`
`163`	`166`	`allowed = true;`
`164`	`167`	`}`
`165`	`168`	`}`