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

@@ -302,5 +302,27 @@
                 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(Object.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(Object.keys(_converse.pluggable.plugins)).toEqual(['plugin1']);
+                converse.plugins.add('plugin2', {});
+                expect(Object.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

@@ -192,11 +192,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,