Two plugins changes

- Remove the `plugins.remove` API method.
- Throw an error when attempting to register multiple plugins with the same name.

docs/source/security.rst

@@ -43,9 +43,9 @@ Restrict access to private code/data
 
 3. Only plugins are allowed to access the private API and the closured ``_converse`` object.
 4. TODO: Whitelist plugins that have access to the private API and closured ``_converse`` object.
-5. TODO: Prevent the removal of registered plugins (otherwise the whitelist could be circumvented).
-6. TODO: Throw an unrecoverable error when multiple plugins try to register under the
-   same name (otherwise the whitelist could be circumvented).
+5. Prevent the removal of registered plugins (otherwise the whitelist could be circumvented).
+6. Throw an error when multiple plugins try to register under the same name
+   (otherwise the whitelist could be circumvented).
 
 .. note::
     Care should be taken when using a custom build of Converse.js where some

spec/converse.js

@@ -4,9 +4,9 @@
         "mock",
         "test_utils"], factory);
 } (this, function (converse, mock, test_utils) {
-    var b64_sha1 = _converse.env.b64_sha1;
-    var _ = _converse.env._;
-    var $ = _converse.env.jQuery;
+    var b64_sha1 = converse.env.b64_sha1;
+    var _ = converse.env._;
+    var $ = converse.env.jQuery;
 
     describe("Converse", function() {
         
@@ -280,7 +280,7 @@
                     expect(chatboxview.$el.is(':visible')).toBeTruthy();
                     // Test for multiple JIDs
                     var jid2 = mock.cur_names[1].replace(/ /g,'.').toLowerCase() + '@localhost';
-                    var list = converse.chats.open([jid, jid2]);
+                    var list = _converse.api.chats.open([jid, jid2]);
                     expect(_.isArray(list)).toBeTruthy();
                     expect(list[0].get('box_id')).toBe(b64_sha1(jid));
                     expect(list[1].get('box_id')).toBe(b64_sha1(jid2));
@@ -290,17 +290,39 @@
 
         describe("The \"settings\" API", function() {
             it("has methods 'get' and 'set' to set configuration settings", mock.initConverse(function (_converse) {
-                expect(_.keys(converse.settings)).toEqual(["get", "set"]);
-                expect(converse.settings.get("play_sounds")).toBe(false);
-                converse.settings.set("play_sounds", true);
-                expect(converse.settings.get("play_sounds")).toBe(true);
-                converse.settings.set({"play_sounds": false});
-                expect(converse.settings.get("play_sounds")).toBe(false);
+                expect(_.keys(_converse.api.settings)).toEqual(["get", "set"]);
+                expect(_converse.api.settings.get("play_sounds")).toBe(false);
+                _converse.api.settings.set("play_sounds", true);
+                expect(_converse.api.settings.get("play_sounds")).toBe(true);
+                _converse.api.settings.set({"play_sounds": false});
+                expect(_converse.api.settings.get("play_sounds")).toBe(false);
                 // Only whitelisted settings allowed.
                 expect(typeof _converse.api.settings.get("non_existing")).toBe("undefined");
                 _converse.api.settings.set("non_existing", true);
                 expect(typeof _converse.api.settings.get("non_existing")).toBe("undefined");
             }));
         });
+
+        describe("The \"plugins\" API", function() {
+            it("only has a method 'add' for registering plugins", mock.initConverse(function (_converse) {
+                expect(_.keys(converse.plugins)).toEqual(["add"]);
+                // Cheating a little bit. We clear the plugins to test more easily.
+                var _old_plugins = _converse.pluggable.plugins;
+                _converse.pluggable.plugins = [];
+                converse.plugins.add('plugin1', {});
+                expect(_.keys(_converse.pluggable.plugins)).toEqual(['plugin1']);
+                converse.plugins.add('plugin2', {});
+                expect(_.keys(_converse.pluggable.plugins)).toEqual(['plugin1', 'plugin2']);
+                _converse.pluggable.plugins = _old_plugins;
+            }));
+
+            describe("The \"plugins.add\" method", function() {
+                it("throws an error when multiple plugins attempt to register with the same name", mock.initConverse(function (_converse) {
+                    converse.plugins.add('myplugin', {});
+                    var error = new TypeError('Error: plugin with name "myplugin" has already been registered!');
+                    expect(_.partial(converse.plugins.add, 'myplugin', {})).toThrow(error);
+                }));
+            });
+        });
     });
 }));

src/converse-api.js

@@ -193,11 +193,14 @@
         'plugins': {
             'add': function (name, plugin) {
                 plugin.__name__ = name;
-                _converse.pluggable.plugins[name] = plugin;
-            },
-            'remove': function (name) {
-                delete _converse.pluggable.plugins[name];
-            },
+                if (!_.isUndefined(_converse.pluggable.plugins[name])) {
+                    throw new TypeError(
+                        'Error: plugin with name "'+name+'" has already been '+
+                        'registered!');
+                } else {
+                    _converse.pluggable.plugins[name] = plugin;
+                }
+            }
         },
         'env': {
             '$build': strophe.$build,

src/converse-minimize.js

@@ -536,9 +536,9 @@
             });
 
             var logOut = function () {
-                converse.minimized_chats.remove();
+                _converse.minimized_chats.remove();
             };
-            converse.on('logout', logOut);
+            _converse.on('logout', logOut);
         }
     });
 }));

src/converse-muc.js

@@ -607,7 +607,7 @@
                     if (!_.isUndefined(member.reason)) {
                         iq.c("reason", member.reason);
                     }
-                    converse.connection.sendIQ(iq, deferred.resolve, deferred.reject);
+                    _converse.connection.sendIQ(iq, deferred.resolve, deferred.reject);
                     return deferred;
                 },
 
@@ -2377,9 +2377,9 @@
                  */
                 _.each(_converse.auto_join_rooms, function (room) {
                     if (_.isString(room)) {
-                        _converse.api.rooms.open(room);
+                        converse.api.rooms.open(room);
                     } else if (_.isObject(room)) {
-                        _converse_api.rooms.open(room.jid, room.nick);
+                        converse.api.rooms.open(room.jid, room.nick);
                     } else {
                         _converse.log('Invalid room criteria specified for "auto_join_rooms"', 'error');
                     }

src/converse-otr.js

@@ -283,7 +283,7 @@
                     this.model.on('showReceivedOTRMessage', function (text) {
                         this.showMessage({'message': text, 'sender': 'them'});
                     }, this);
-                    if ((_.contains([UNVERIFIED, VERIFIED], this.model.get('otr_status'))) || _converse.use_otr_by_default) {
+                    if ((_.includes([UNVERIFIED, VERIFIED], this.model.get('otr_status'))) || _converse.use_otr_by_default) {
                         this.model.initiateOTR();
                     }
                 },

src/converse-ping.js

@@ -17,7 +17,8 @@
 }(this, function (converse) {
     "use strict";
     // Strophe methods for building stanzas
-    var Strophe = converse.env.Strophe;
+    var Strophe = converse.env.Strophe,
+        _ = converse.env._;
     
     converse.plugins.add('converse-ping', {