mix: Multiple features and fixes

- Implement disco#info against the MIX host
  - Access Control for the #channel-create feature
- Replace "urn:xmpp:mix:core:1" with mix_core_xmlns
- Prevent a user from joining twice
- Return an error when no node could be subscribed during join
- Generate Adhoc Channel JIDs until a unique one was found

mod_mix/mod_mix.lua

@@ -12,17 +12,23 @@ local time = require("util.time");
 local serialization = require("util.serialization");
 local pep = module:depends("pep");
 
+-- XML namespaces
+local mix_core_xmlns = "urn:xmpp:mix:core:1";
+
+-- Persistent data
 local persistent_channels = module:open_store("mix_channels", "keyval");
 local persistent_channel_data = module:open_store("mix_data", "keyval");
 
--- Default values
+-- Configuration
 local default_channel_description = module:get_option("default_description", "A MIX channel for chatting");
 local default_channel_name = module:get_option("default_name", "MIX channel");
+local restrict_local_channel_creation = module:get_option("restrict_local_channels", true);
 
 module:depends("disco");
+-- module:depends("mam"); TODO: Once message sending works
 module:add_identity("conference", "mix", module:get_option("name", "Prosody MIX service");
 module:add_feature("http://jabber.org/protocol/disco#info");
-module:add_feature("urn:xmpp:mix:core:1");
+module:add_feature(mix_core_xmlns);
 
 Participant = {};
 Participant.__index = Participant;
@@ -52,6 +58,7 @@ function Channel:from(config)
 end
 
 function Channel:get_spid(jid)
+    -- Returns the Stable Participant ID for the *BARE* jid
     return self.spid[jid];
 end
 
@@ -81,11 +88,15 @@ end
 local channels = {};
 
 function get_channel(jid)
+    -- Return the channel object from the channels array for which the
+    -- JID matches. If none is found, returns -1, nil
     for i, channel in pairs(channels) do
         if channel.jid == jid then
-            return i, channel
+            return i, channel;
         end
     end
+
+    return -1, nil;
 end
 
 function publish_participant(service, spid, participant)
@@ -93,7 +104,7 @@ function publish_participant(service, spid, participant)
                     true,
                     spid,
                     st.stanza("item", { id = spid, xmlns = "http://jabber.org/protocol/pubsub" })
-                        :tag("participant", { xmlns = "urn:xmpp:mix:core:1" })
+                        :tag("participant", { xmlns = mix_core_xmlns })
                             :tag("nick"):text(participant["nick"]):up()
                             :tag("jid"):text(participant["jid"]));
 end
@@ -174,7 +185,29 @@ module:hook("iq/bare/http://jabber.org/protocol/disco#items:query", function(eve
     return true;
 end);
 
-module:hook("iq/bare/http://jabber.org/protocol/disco#info:query", function(event)
+module:hook("iq-get/host/http://jabber.org/protocol/disco#info:query", function(event)
+    module:log("debug", "IQ-GET host disco#info");
+    
+    local origin, stanza = event.origin, event.stanza;
+    local reply = st.reply(stanza)
+                    :tag("query", { xmlns = "http://jabber.org/protocol/disco#info" })
+                        -- TODO: Name
+                        :tag("identity", { category = "conference", type = "mix", name = "" }):up()
+                        :tag("feature", { var = "http://jabber.org/protocol/disco#info" }):up()
+                        :tag("feature", { var = mix_core_xmlns }):up();
+
+    -- TODO: This should also check for admin and an array
+    if restrict_channel_creation == "local" then
+        -- NOTE: Taken from plugins/muc/mod_muc.lua
+        local host_suffix = host:gsub("^[^%.]+%.", "");
+        module:log("debug", "Comparing %s (Sender) to %s (Host)", jid.host(stanza.attr.from), host_suffix);
+        if jid.host(stanza.attr.from) == host_suffix then 
+            reply:tag("feature", { var = mix_core_xmlns.."#create-channel" }):up();
+        end
+    end
+end);
+
+module:hook("iq-get/bare/http://jabber.org/protocol/disco#info:query", function(event)
     module:log("debug", "IQ-GET disco#info");
 
     local origin, stanza = event.origin, event.stanza;
@@ -187,13 +220,15 @@ module:hook("iq/bare/http://jabber.org/protocol/disco#info:query", function(even
     reply:tag("feature", { var = "http://jabber.org/protocol/disco#info" }):up();
     reply:tag("identity", { category = "conference", name = channel.name, type = "mix" }):up();
 
-    reply:tag("feature", { var = "urn:xmpp:mix:core:1" }):up();
+    reply:tag("feature", { var = mix_core_xmlns }):up();
-    -- TODO: Check permissions
+    -- TODO: Once message sending works, uncomment this
-    reply:tag("feature", { var = "urn:xmpp:mix:core:1#create-channel" }):up();
+    -- reply:tag("feature", { var = "urn:xmpp:mam:2" }):up();
+
     origin.send(reply);
     return true;
 end);
 
+-- TODO: Make this a function of Channel:
 function find_participant(table, jid)
     for i, v in pairs(table) do
         if v.jid == jid then
@@ -204,7 +239,7 @@ function find_participant(table, jid)
     return -1;
 end
 
-module:hook("iq-set/bare/urn:xmpp:mix:core:1:leave", function(event)
+module:hook("iq-set/bare/"..mix_core_xmlns..":leave", function(event)
     module:log("debug", "MIX leave received");
     local origin, stanza = event.origin, event.stanza;
     local from = jid.bare(stanza.attr.from);
@@ -233,51 +268,72 @@ module:hook("iq-set/bare/urn:xmpp:mix:core:1:leave", function(event)
         module:log("debug", "Unsubscribed %s from %s on %s", from, node, channel.jid);
     end
     channels[i].subscriptions[from] = nil;
-    -- Retracting the participation
+    -- Retracting the participatio)
-    srv:retract("urn:xmpp:mix:nodes:participants", true, channel.spid[from], true);
+    srv:retract("urn:xmpp:mix:nodes:participants", true, channel:get_spid(from), true);
     -- Removing the user
     table.remove(channels[i].participants, participant_index);
     channel:save_state();
 
-    origin.send(st.reply(stanza):tag("leave", { xmlns = "urn:xmpp:mix:core:1" }));
+    origin.send(st.reply(stanza):tag("leave", { xmlns = mix_core_xmlns }));
     return true;
 end);
 
-module:hook("iq-set/bare/urn:xmpp:mix:core:1:join", function(event)
+module:hook("iq-set/bare/"..mix_core_xmlns..":join", function(event)
     module:log("debug", "MIX join received");
 
     local origin, stanza = event.origin, event.stanza;
+    local from = jid.bare(stanza.attr.from);
     local i, channel = get_channel(stanza.attr.to);
     if not channel then
         origin.send(channel_not_found(stanza));
         return;
     end
-    local from = jid.bare(stanza.attr.from);
+    
-    local spid = channel.spid[from] or uuid.generate(); -- Stable Participant ID
+    -- Prevent the user from joining multiple times
+    local participant_index = find_participant(channel.participants, from);
+    if participant_index ~= -1 then
+        origin.send(st.error_reply(stanza, "cancel", "bad-request", "User already joined"));
+        return;
+    end
+
+    local spid = channel:get_spid(from) or uuid.generate(); -- Stable Participant ID
     local reply = st.reply(stanza)
-                    :tag("join", { xmlns = "urn:xmpp:mix:core:1", id = spid });
+                    :tag("join", { xmlns = mix_core_xmlns, id = spid });
     local srv = pep.get_pep_service(jid.node(stanza.attr.to));
-    local join = stanza:get_child("join", "urn:xmpp:mix:core:1");
+    local join = stanza:get_child("join", mix_core_xmlns);
     local nick = join:get_child("nick");
     module:log("debug", "User joining as nick %s", nick:get_text());
     
     local nodes = {};
-    -- TODO: The spec says that an error sould be returned when no nodes can be subscribed to
+    local has_subscribed_once = false;
+    local first_error = nil;
     for subscribe in join:childtags("subscribe") do
         module:log("debug", "Subscribing user to node %s", subscribe.attr.node);
         local ok, err = srv:add_subscription(subscribe.attr.node, true, from);
         if not ok then
             module:log("debug", "Error during subscription: %s", err);
+
+            -- MIX-CORE says that the first error should be returned when
+            -- no of the requested nodes could be subscribed to
+            if first_error ~= nil then
+                first_error = err;
+            end
         else
             table.insert(nodes, subscribe.attr.node);
             reply:tag("subscribe", { node = subscribe.attr.node }):up();
+            has_subscribed_once = true;
         end
     end
-    channels[i].subscriptions[from] = nodes;
+
-    
+    if not has_subscribed_once then
+        origin.send(st.error_reply(stanza, "cancel", first_error));
+        return;
+    end
+
     local participant = Participant:new(jid.bare(from), nick:get_text())
+    channels[i].subscriptions[from] = nodes;
     table.insert(channels[i].participants, participant)
-    channels[i].spid[jid.bare(stanza.attr.from)] = spid;
+    channels[i]:get_spid(jid.bare(stanza.attr.from)) = spid;
     publish_participant(srv, spid, participant);
     channels[i]:save_state();
 
@@ -286,7 +342,7 @@ module:hook("iq-set/bare/urn:xmpp:mix:core:1:join", function(event)
     return true
 end);
 
-module:hook("iq-set/bare/urn:xmpp:mix:core:1:setnick", function(event)
+module:hook("iq-set/bare/"..mix_core_xmlns..":setnick", function(event)
     module:log("debug", "MIX setnick received");
     local origin, stanza = event.origin, event.stanza;
     local from = jid.bare(stanza.attr.from);
@@ -302,21 +358,25 @@ module:hook("iq-set/bare/urn:xmpp:mix:core:1:setnick", function(event)
         module:log("debug", "%s is not a participant in %s", from, channel.jid);
         return;
     end
-
-    local setnick = stanza:get_child("setnick", "urn:xmpp:mix:core:1");
-    -- TODO: Error handling
-    local nick = setnick:get_child("nick"):get_text();
     
+    -- NOTE: Prosody should guarantee us that the setnick stanza exists
+    local setnick = stanza:get_child("setnick", mix_core_xmlns);
+    local nick = setnick:get_child("nick");
+    if nick_stanza == nil then
+        origin.send(st.error_reply(stanza, "cancel", "bad-request", "Missing <nick>"));
+        return;
+    end
+
     -- Change the nick
-    channels[i].participants[participant_index].nick = nick;
+    channels[i].participants[participant_index].nick = nick:get_text();
     -- Inform all other members
     local srv = pep.get_pep_service(channel.jid);
     local participant = channel.participants[participant_index];
     publish_participant(srv, channel:get_spid(participant.jid), participant);
 
     origin.send(st.reply(stanza)
-                    :tag("setnick", { xmlns = "urn:xmpp:mix:core:1" })
+                    :tag("setnick", { xmlns = mix_core_xmlns })
-                        :tag("nick"):text(nick));
+                        :tag("nick"):text(nick:get_text()));
     channel:save_state();
     return true;
 end);
@@ -326,7 +386,7 @@ function Channel:publish_info(srv)
                 st.stanza("item", { id = timestamp, xmlns = "http://jabber.org/protocol/pubsub" })
                     :tag("x", { xmlns = "jabber:x:data", type = "result" })
                         :tag("field", { var = "FORM_TYPE", type = "hidden" })
-                            :tag("value"):text("urn:xmpp:mix:core:1"):up():up()
+                            :tag("value"):text(mix_core_xmlns):up():up()
                         :tag("field", { var = "Name" })
                             :tag("value"):text(self.name):up()
                         :tag("field", { var = "Contact" })
@@ -352,12 +412,12 @@ function create_channel(node, creator, adhoc)
     table.insert(channels, channel);
 end
 
-module:hook("iq-set/host/urn:xmpp:mix:core:1:create", function(event)
+module:hook("iq-set/host/"..mix_core_xmlns..":create", function(event)
     module:log("debug", "MIX create received");
     local origin, stanza = event.origin, event.stanza;
     local from = jid.bare(stanza.attr.from);
 
-    local create = stanza:get_child("create", "urn:xmpp:mix:core:1");
+    local create = stanza:get_child("create", mix_core_xmlns);
     local node;
     if create.attr.channel ~= nil then
         -- Create non-adhoc channel
@@ -374,24 +434,30 @@ module:hook("iq-set/host/urn:xmpp:mix:core:1:create", function(event)
         create_channel(create.attr.channel, from, false);
     else
         -- Create adhoc channel
-        -- TODO: Check for a collision
+        while (true) do
-        node = id.short();
+            node = id.short();
+            local _, channel = get_channel(string.format("%s@%s", node, host));
+            if channel == nil then
+                break;
+            end
+        end
+        
         create_channel(node, from, true);
     end
     module:log("debug", "Channel %s created with %s as owner", node, from);
 
     origin.send(st.reply(stanza)
-                :tag("create", { xmlns = "urn:xmpp:mix:core:1", channel = node }));
+                :tag("create", { xmlns = mix_core_xmlns, channel = node }));
     save_channels();
     return true;
 end);
 
-module:hook("iq-set/host/urn:xmpp:mix:core:1:destroy", function(event)
+module:hook("iq-set/host/"..mix_core_xmlns..":destroy", function(event)
     module:log("debug", "MIX destroy received");
     local origin, stanza = event.origin, event.stanza;
     local from = jid.bare(stanza.attr.from);
 
-    local destroy = stanza:get_child("create", "urn:xmpp:mix:core:1");
+    local destroy = stanza:get_child("create", mix_core_xmlns);
     local node = destory.attr.channel;
     local node_jid = string.format("%s@%s", node, host);
     local i, channel = get_channel(node_jid);
@@ -440,10 +506,10 @@ module:hook("message/bare", function(event)
     local participant = channel.participants[participant_index];
 
     local msg = st.clone(stanza);
-    msg:add_child(st.stanza("mix", { xmlns = "urn:xmpp:mix:core:1" })
+    msg:add_child(st.stanza("mix", { xmlns = mix_core_xmlns })
                     :tag("nick"):text(participant.nick):up()
                     :tag("jid"):text(participant.jid):up());
-    msg.attr.from = channel.jid.."/"..channel.spid[from];
+    msg.attr.from = channel.jid.."/"..channel:get_spid(from);
     for _, p in pairs(channel.participants) do
         if p.jid ~= participant.jid then
             msg.attr.to = p.jid;