On 01.05.2017 14:48, Lennart Sauerbeck wrote:
> Attached is a patch implementing atomic changes for both affiliation and
> roles change requests.
While taking a look at issue #337 I noticed that while the XSD for
muc#admin allows changes to both affiliations and roles in the same
request it is explicitly forbidden by XEP-0045 in section 16.4.4.
I added a check to the patch to return the required 'bad-request' to the
client in the case of both affiliation and role changes in a single request.
Best Regards
Lennart
--
You received this message because you are subscribed to the Google Groups
"prosody-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to prosody-dev+unsubscr...@googlegroups.com.
To post to this group, send email to prosody-dev@googlegroups.com.
Visit this group at https://groups.google.com/group/prosody-dev.
For more options, visit https://groups.google.com/d/optout.
# HG changeset patch
# User Lennart Sauerbeck <de...@lennart.sauerbeck.org>
# Date 1489859248 -3600
# Sat Mar 18 18:47:28 2017 +0100
# Node ID b5b443401bb8c51c8695952c49f62062c1ba2457
# Parent 484ea6201c6c28793c6d1a94fea5264811e12889
muc: Allow clients to change multiple affiliations or roles at once (#345)
According to XEP-0045 sections 9.2, 9.5 and 9.8 affiliation lists and role
lists should allow mass-modification. Prosody however would just use the
first entry of the list and ignore the rest. This is fixed by introducing
a `for` loop to `set` stanzas of the respective `muc#admin` namespace.
The change requested by the client will be treated as atomic: Either the
whole change can be applied, or nothing is applied. The way this is
implemented is to first check if any of the requested affiliation or role
changes would be inconsistent and only if that is not the case all of them
are applied.
In XEP-0045 section 16.4.4 the standard disallows clients to change
both roles and affiliations in the same request. This is checked at the
end right before applying the changes.
References: #345
diff -r 484ea6201c6c -r b5b443401bb8 plugins/muc/muc.lib.lua
--- a/plugins/muc/muc.lib.lua Thu Apr 27 12:41:53 2017 +0200
+++ b/plugins/muc/muc.lib.lua Sat Mar 18 18:47:28 2017 +0100
@@ -773,10 +773,16 @@
local affiliation = self:get_affiliation(actor);
local current_nick = self._jid_nick[actor];
local role = current_nick and self._occupants[current_nick].role or self:get_default_role(affiliation);
- local item = stanza.tags[1].tags[1];
- if item and item.name == "item" then
- if type == "set" then
- local callback = function() origin.send(st.reply(stanza)); end
+ if type == "set" then
+ local at_least_one_item_provided = false;
+ local callback = function() origin.send(st.reply(stanza)); end
+
+ -- Gather all changes to affiliations and roles
+ local jid_affiliation = {};
+ local jidnick_role = {};
+ for item in stanza.tags[1]:childtags("item") do
+ at_least_one_item_provided = true;
+
if item.attr.jid then -- Validate provided JID
item.attr.jid = jid_prep(item.attr.jid);
if not item.attr.jid then
@@ -791,17 +797,55 @@
local nick = self._jid_nick[item.attr.jid];
if nick then item.attr.nick = select(3, jid_split(nick)); end
end
+
local reason = item.tags[1] and item.tags[1].name == "reason" and #item.tags[1] == 1 and item.tags[1][1];
if item.attr.affiliation and item.attr.jid and not item.attr.role then
- local success, errtype, err = self:set_affiliation(actor, item.attr.jid, item.attr.affiliation, callback, reason);
- if not success then origin.send(st.error_reply(stanza, errtype, err)); end
+ jid_affiliation[item.attr.jid] = { ["affiliation"] = item.attr.affiliation, ["reason"] = reason };
elseif item.attr.role and item.attr.nick and not item.attr.affiliation then
- local success, errtype, err = self:set_role(actor, self.jid.."/"..item.attr.nick, item.attr.role, callback, reason);
- if not success then origin.send(st.error_reply(stanza, errtype, err)); end
+ jidnick_role[item.attr.jid.."/"..item.attr.nick] = { ["role"] = item.attr.role, ["reason"] = reason };
else
origin.send(st.error_reply(stanza, "cancel", "bad-request"));
+ return;
end
- elseif type == "get" then
+ end
+
+ if not at_least_one_item_provided then
+ origin.send(st.error_reply(stanza, "cancel", "bad-request"));
+ return;
+ else
+ local can_set_affiliations, errtype_aff, err_aff = self:can_set_affiliations(actor, jid_affiliation)
+ local can_set_roles, errtype_role, err_role = self:can_set_roles(actor, jidnick_role)
+
+ if can_set_affiliations and can_set_roles then
+ local nb_affiliation_changes = 0;
+ for _ in pairs(jid_affiliation) do nb_affiliation_changes = nb_affiliation_changes + 1; end
+ local nb_role_changes = 0;
+ for _ in pairs(jidnick_role) do nb_role_changes = nb_role_changes + 1; end
+
+ if nb_affiliation_changes > 0 and nb_role_changes > 0 then
+ origin.send(st.error_reply(stanza, "cancel", "bad-request"));
+ end
+ if nb_affiliation_changes > 0 then
+ self:set_affiliations(actor, jid_affiliation, callback);
+ end
+ if nb_role_changes > 0 then
+ self:set_roles(actor, jidnick_role, callback);
+ end
+ else
+ if not can_set_affiliations then
+ origin.send(st.error_reply(stanza, ertype_aff, err_aff));
+ elseif not can_set_roles then
+ origin.send(st.error_reply(stanza, errtype_role, err_role));
+ else
+ origin.send(st.error_reply(stanza, "cancel", "bad-request"));
+ end
+
+ return;
+ end
+ end
+ elseif type == "get" then
+ local item = stanza.tags[1].tags[1];
+ if item and item.name == "item" then
local _aff = item.attr.affiliation;
local _rol = item.attr.role;
if _aff and not _rol then
@@ -839,9 +883,9 @@
else
origin.send(st.error_reply(stanza, "cancel", "bad-request"));
end
+ else
+ origin.send(st.error_reply(stanza, "cancel", "bad-request"));
end
- elseif type == "set" or type == "get" then
- origin.send(st.error_reply(stanza, "cancel", "bad-request"));
end
elseif xmlns == "http://jabber.org/protocol/muc#owner"; and (type == "get" or type == "set") and stanza.tags[1].name == "query" then
if self:get_affiliation(stanza.attr.from) ~= "owner" then
@@ -969,76 +1013,119 @@
if not result and self._affiliations[host] == "outcast" then result = "outcast"; end -- host banned
return result;
end
-function room_mt:set_affiliation(actor, jid, affiliation, callback, reason)
- jid = jid_bare(jid);
- if affiliation == "none" then affiliation = nil; end
- if affiliation and affiliation ~= "outcast" and affiliation ~= "owner" and affiliation ~= "admin" and affiliation ~= "member" then
- return nil, "modify", "not-acceptable";
- end
+--- Checks whether the given affiliation changes in jid_affiliation can be applied by actor.
+-- Note: Empty tables can always be applied and won't have any effect.
+function room_mt:can_set_affiliations(actor, jid_affiliation)
+ local actor_affiliation;
if actor ~= true then
- local actor_affiliation = self:get_affiliation(actor);
- local target_affiliation = self:get_affiliation(jid);
- if target_affiliation == affiliation then -- no change, shortcut
- if callback then callback(); end
- return true;
- end
- if actor_affiliation ~= "owner" then
- if affiliation == "owner" or affiliation == "admin" or actor_affiliation ~= "admin" or target_affiliation == "owner" or target_affiliation == "admin" then
- return nil, "cancel", "not-allowed";
- end
- elseif target_affiliation == "owner" and jid_bare(actor) == jid then -- self change
- local is_last = true;
- for j, aff in pairs(self._affiliations) do if j ~= jid and aff == "owner" then is_last = false; break; end end
- if is_last then
- return nil, "cancel", "conflict";
- end
- end
+ actor_affiliation = self:get_affiliation(actor);
end
- self._affiliations[jid] = affiliation;
- local role = self:get_default_role(affiliation);
- local x = st.stanza("x", {xmlns = "http://jabber.org/protocol/muc#user"})
- :tag("item", {affiliation=affiliation or "none", role=role or "none"})
- :tag("reason"):text(reason or ""):up()
- :up();
- local presence_type = nil;
- if not role then -- getting kicked
- presence_type = "unavailable";
- if affiliation == "outcast" then
- x:tag("status", {code="301"}):up(); -- banned
- else
- x:tag("status", {code="321"}):up(); -- affiliation change
+
+ -- First let's see if there are any problems with the affiliations given
+ -- in jid_affiliation
+ for jid, value in pairs(jid_affiliation) do
+ local affiliation = value["affiliation"];
+
+ jid = jid_bare(jid);
+ if affiliation == "none" then affiliation = nil; end
+ if affiliation and affiliation ~= "outcast" and affiliation ~= "owner" and affiliation ~= "admin" and affiliation ~= "member" then
+ return false, "modify", "not-acceptable";
end
- end
- local modified_nicks = {};
- for nick, occupant in pairs(self._occupants) do
- if jid_bare(occupant.jid) == jid then
- if not role then -- getting kicked
- self._occupants[nick] = nil;
- else
- occupant.affiliation, occupant.role = affiliation, role;
- end
- for jid,pres in pairs(occupant.sessions) do -- remove for all sessions of the nick
- if not role then self._jid_nick[jid] = nil; end
- local p = st.clone(pres);
- p.attr.from = nick;
- p.attr.type = presence_type;
- p.attr.to = jid;
- p:add_child(x);
- self:_route_stanza(p);
- if occupant.jid == jid then
- modified_nicks[nick] = p;
+
+ local target_affiliation = self:get_affiliation(jid);
+ if target_affiliation == affiliation then
+ -- no change, no error checking necessary
+ else
+ if actor ~= true and actor_affiliation ~= "owner" then
+ if affiliation == "owner" or affiliation == "admin" or actor_affiliation ~= "admin" or target_affiliation == "owner" or target_affiliation == "admin" then
+ return false, "cancel", "not-allowed";
+ end
+ elseif target_affiliation == "owner" and jid_bare(actor) == jid then -- self change
+ local is_last = true;
+ for j, aff in pairs(self._affiliations) do if j ~= jid and aff == "owner" then is_last = false; break; end end
+ if is_last then
+ return false, "cancel", "conflict";
end
end
end
end
- if self.save then self:save(); end
- if callback then callback(); end
+
+ return true;
+end
+--- Updates the room affiliations by applying the ones given here.
+-- Takes the affiliations given in jid_affiliation and applies them to
+-- the room, overwriting a potentially existing affiliation for any given
+-- jid.
+-- @param jid_affiliation A table associating a jid with a table consisting
+-- of two subkeys: `affilation` and `reason`. The jids
+-- within must not be malformed.
+function room_mt:set_affiliations(actor, jid_affiliation, callback)
+ local can_set, err_type, err_condition = self:can_set_affiliations(actor, jid_affiliation)
+
+ if not can_set then
+ return false, err_type, err_condition;
+ end
+
+ local modified_nicks = {};
+ local nb_modified_nicks = 0;
+ -- Now we can be sure that jid_affiliation causes no problems
+ -- We can actually set them
+ for jid, value in pairs(jid_affiliation) do
+ local affiliation = value["affiliation"];
+ local reason = value["reason"];
+
+ self._affiliations[jid] = affiliation;
+ local role = self:get_default_role(affiliation);
+ local x = st.stanza("x", {xmlns = "http://jabber.org/protocol/muc#user"})
+ :tag("item", {affiliation=affiliation or "none", role=role or "none"})
+ :tag("reason"):text(reason or ""):up()
+ :up();
+ local presence_type = nil;
+ if not role then -- getting kicked
+ presence_type = "unavailable";
+ if affiliation == "outcast" then
+ x:tag("status", {code="301"}):up(); -- banned
+ else
+ x:tag("status", {code="321"}):up(); -- affiliation change
+ end
+ end
+ for nick, occupant in pairs(self._occupants) do
+ if jid_bare(occupant.jid) == jid then
+ if not role then -- getting kicked
+ self._occupants[nick] = nil;
+ else
+ occupant.affiliation, occupant.role = affiliation, role;
+ end
+ for jid,pres in pairs(occupant.sessions) do -- remove for all sessions of the nick
+ if not role then self._jid_nick[jid] = nil; end
+ local p = st.clone(pres);
+ p.attr.from = nick;
+ p.attr.type = presence_type;
+ p.attr.to = jid;
+ p:add_child(x);
+ self:_route_stanza(p);
+ if occupant.jid == jid then
+ modified_nicks[nick] = p;
+ nb_modified_nicks = nb_modified_nicks + 1;
+ end
+ end
+ end
+ end
+ end
+
+ if nb_modified_nicks > 0 then
+ if self.save then self:save(); end
+ if callback then callback(); end
+ end
for nick,p in pairs(modified_nicks) do
p.attr.from = nick;
self:broadcast_except_nick(p, nick);
end
return true;
end
+function room_mt:set_affiliation(actor, jid, affiliation, callback, reason)
+ return self.set_affiliations(actor, { [jid] = { ["affiliation"] = affiliation, ["reason"] = reason } }, callback)
+end
function room_mt:get_role(nick)
local session = self._occupants[nick];
@@ -1062,42 +1149,74 @@
end
return nil, "cancel", "not-allowed";
end
-function room_mt:set_role(actor, occupant_jid, role, callback, reason)
- if role == "none" then role = nil; end
- if role and role ~= "moderator" and role ~= "participant" and role ~= "visitor" then return nil, "modify", "not-acceptable"; end
- local allowed, err_type, err_condition = self:can_set_role(actor, occupant_jid, role);
- if not allowed then return allowed, err_type, err_condition; end
- local occupant = self._occupants[occupant_jid];
- local x = st.stanza("x", {xmlns = "http://jabber.org/protocol/muc#user"})
- :tag("item", {affiliation=occupant.affiliation or "none", nick=select(3, jid_split(occupant_jid)), role=role or "none"})
- :tag("reason"):text(reason or ""):up()
- :up();
- local presence_type = nil;
- if not role then -- kick
- presence_type = "unavailable";
- self._occupants[occupant_jid] = nil;
- for jid in pairs(occupant.sessions) do -- remove for all sessions of the nick
- self._jid_nick[jid] = nil;
- end
- x:tag("status", {code = "307"}):up();
- else
- occupant.role = role;
- end
- local bp;
- for jid,pres in pairs(occupant.sessions) do -- send to all sessions of the nick
- local p = st.clone(pres);
- p.attr.from = occupant_jid;
- p.attr.type = presence_type;
- p.attr.to = jid;
- p:add_child(x);
- self:_route_stanza(p);
- if occupant.jid == jid then
- bp = p;
+
+--- Checks whether the given role changes in jidnick_role can be applied by actor.
+-- Note: Empty tables can always be applied and won't have any effect.
+function room_mt:can_set_roles(actor, jidnick_role)
+ for jidnick, role in pairs(jidnick_role) do
+ if role == "none" then role = nil; end
+ if role and role ~= "moderator" and role ~= "participant" and role ~= "visitor" then return false, "modify", "not-acceptable"; end
+ local can_set, err_type, err_condition = self:can_set_role(actor, jidnick, role)
+ if not can_set then
+ return false, err_type, err_condition;
end
end
- if callback then callback(); end
- if bp then
- self:broadcast_except_nick(bp, occupant_jid);
+
+ return true;
+end
+--- Updates the room roles by applying the ones given here.
+-- Takes the roles given in jidnick_role and applies them to
+-- the room, overwriting a potentially existing role for any given
+-- jid.
+-- @param jidnick_role A table associating a jid/nick with a table consisting
+-- of two subkeys: `role` and `reason`. The jids within
+-- must not be malformed.
+function room_mt:set_roles(actor, jidnick_role, callback)
+ local allowed, err_type, err_condition = self:can_set_roles(actor, jidnick_role);
+ if not allowed then return allowed, err_type, err_condition; end
+
+ local modified_nicks = {};
+ local nb_modified_nicks = 0;
+ for jidnick, value in pairs(jidnick_role) do
+ local occupant_jid = jidnick;
+ local role = value["role"];
+ local reason = value["reason"];
+
+ local occupant = self._occupants[occupant_jid];
+ local x = st.stanza("x", {xmlns = "http://jabber.org/protocol/muc#user"})
+ :tag("item", {affiliation=occupant.affiliation or "none", nick=select(3, jid_split(occupant_jid)), role=role or "none"})
+ :tag("reason"):text(reason or ""):up()
+ :up();
+ local presence_type = nil;
+ if not role then -- kick
+ presence_type = "unavailable";
+ self._occupants[occupant_jid] = nil;
+ for jid in pairs(occupant.sessions) do -- remove for all sessions of the nick
+ self._jid_nick[jid] = nil;
+ end
+ x:tag("status", {code = "307"}):up();
+ else
+ occupant.role = role;
+ end
+ for jid,pres in pairs(occupant.sessions) do -- send to all sessions of the nick
+ local p = st.clone(pres);
+ p.attr.from = occupant_jid;
+ p.attr.type = presence_type;
+ p.attr.to = jid;
+ p:add_child(x);
+ self:_route_stanza(p);
+ if occupant.jid == jid then
+ modified_nicks[occupant_jid] = p;
+ nb_modified_nicks = nb_modified_nicks + 1;
+ end
+ end
+ end
+
+ if nb_modified_nicks > 0 then
+ if callback then callback(); end
+ end
+ for nick,p in pairs(modified_nicks) do
+ self:broadcast_except_nick(p, nick);
end
return true;
end
