From c443ee4f4a56b9490427a8723b83dc9bcd6c76be Mon Sep 17 00:00:00 2001 From: Christophe Romain Date: Mon, 8 Dec 2008 19:50:50 +0000 Subject: [PATCH] several pubsub improvements SVN Revision: 1715 --- ChangeLog | 20 +++ src/mod_pubsub/gen_pubsub_nodetree.erl | 2 + src/mod_pubsub/mod_pubsub.erl | 118 +++++++++++------- .../{node_zoo.erl => node_flat.erl} | 11 +- src/mod_pubsub/nodetree_default.erl | 8 ++ src/mod_pubsub/nodetree_virtual.erl | 8 ++ src/mod_pubsub/pubsub.hrl | 10 -- 7 files changed, 117 insertions(+), 60 deletions(-) rename src/mod_pubsub/{node_zoo.erl => node_flat.erl} (97%) diff --git a/ChangeLog b/ChangeLog index c873fd729..f7cf256c2 100644 --- a/ChangeLog +++ b/ChangeLog @@ -4,6 +4,26 @@ * src/mod_pubsub/mod_pubsub.erl: Likewise * src/mod_caps.erl: Likewise + * src/mod_pubsub/mod_pubsub.erl: ignore unknown configuration fields + (EJAB-762) (thanks to Jack Moffitt) + + * src/mod_pubsub/mod_pubsub.erl: fix node authorization bug (EJAB-798) + send authorization update event (EJAB-799) (thanks to Brian Cully) + + * src/mod_pubsub/mod_pubsub.erl: add nodetree filtering and + authorization (EJAB-813) (thanks to Brian Cully) + * src/mod_pubsub/nodetree_default.erl: Likewise + * src/mod_pubsub/nodetree_virtual.erl: Likewise + * src/mod_pubsub/gen_pubsub_nodetree.erl: Likewise + + * src/mod_pubsub/mod_pubsub.erl: prevent publish items with invalid + XML schema (EJAB-699) + + * src/mod_pubsub/pubsub.hrl: remove unused pubsub_presence record + + * src/mod_pubsub/node_flat.erl: renamed from node_zoo + + 2008-12-08 Mickael Remond * src/ejabberd_c2s.erl: Enforce client stanza from attribute diff --git a/src/mod_pubsub/gen_pubsub_nodetree.erl b/src/mod_pubsub/gen_pubsub_nodetree.erl index ebdeca450..c249ff43d 100644 --- a/src/mod_pubsub/gen_pubsub_nodetree.erl +++ b/src/mod_pubsub/gen_pubsub_nodetree.erl @@ -42,7 +42,9 @@ behaviour_info(callbacks) -> {terminate, 2}, {options, 0}, {set_node, 1}, + {get_node, 3}, {get_node, 2}, + {get_nodes, 2}, {get_nodes, 1}, {get_subnodes, 3}, {get_subnodes_tree, 2}, diff --git a/src/mod_pubsub/mod_pubsub.erl b/src/mod_pubsub/mod_pubsub.erl index 1c5483cda..8a7c199c3 100644 --- a/src/mod_pubsub/mod_pubsub.erl +++ b/src/mod_pubsub/mod_pubsub.erl @@ -355,11 +355,11 @@ disco_sm_features(Acc, From, To, Node, _Lang) -> Acc end. -disco_sm_items(Acc, _From, To, [], _Lang) -> +disco_sm_items(Acc, From, To, [], _Lang) -> %% TODO, use iq_disco_items(Host, [], From) Host = To#jid.lserver, LJID = jlib:jid_tolower(jlib:jid_remove_resource(To)), - case tree_action(Host, get_nodes, [Host]) of + case tree_action(Host, get_nodes, [Host, From]) of [] -> Acc; Nodes -> @@ -461,7 +461,7 @@ handle_cast({presence, JID, Pid}, State) -> {result, Subscriptions} = node_action(Type, get_entity_subscriptions, [Host, JID]), lists:foreach( fun({Node, subscribed}) -> - case tree_action(Host, get_node, [Host, Node]) of + case tree_action(Host, get_node, [Host, Node, JID]) of #pubsub_node{options = Options} -> case get_option(Options, send_last_published_item) of on_sub_and_presence -> @@ -507,7 +507,7 @@ handle_cast({presence, JID, Pid}, State) -> _ -> ok end - end, tree_action(Host, get_nodes, [Owner])) + end, tree_action(Host, get_nodes, [Owner, JID])) end, Contacts); _ -> ok @@ -785,7 +785,7 @@ iq_disco_items(Host, Item, From) -> Node = string_to_node(SNode), %% Note: Multiple Node Discovery not supported (mask on pubsub#type) %% TODO this code is also back-compatible with pubsub v1.8 (for client issue) - %% TODO make it pubsub v1.10 compliant (this breaks client compatibility) + %% TODO make it pubsub v1.12 compliant (breaks client compatibility ?) %% TODO That is, remove name attribute Action = fun(#pubsub_node{type = Type}) -> @@ -1022,7 +1022,7 @@ send_authorization_request(Host, Node, Subscriber) -> {"type", "boolean"}, {"label", translate:translate(Lang, "Allow this JID to subscribe to this pubsub node?")}], [{xmlelement, "value", [], [{xmlcdata, "false"}]}]}]}]}, - case tree_action(Host, get_node, [Host, Node]) of + case tree_action(Host, get_node, [Host, Node, Subscriber]) of #pubsub_node{owners = Owners} -> lists:foreach( fun(Owner) -> @@ -1055,20 +1055,39 @@ find_authorization_response(Packet) -> [invalid] -> invalid; [] -> none; [XFields] when is_list(XFields) -> + ?DEBUG("XFields: ~p", [XFields]), case lists:keysearch("FORM_TYPE", 1, XFields) of - {value, {_, ?NS_PUBSUB_SUB_AUTH}} -> + {value, {_, [?NS_PUBSUB_SUB_AUTH]}} -> XFields; _ -> invalid end end. +%% @spec (Host, JID, Node, Subscription) -> void +%% Host = mod_pubsub:host() +%% JID = jlib:jid() +%% Node = string() +%% Subscription = atom() +%% Plugins = [Plugin::string()] +%% @doc Send a message to JID with the supplied Subscription +send_authorization_approval(Host, JID, Node, Subscription) -> + Stanza = {xmlelement, "message", + [], + [{xmlelement, "event", [{"xmlns", ?NS_PUBSUB_EVENT}], + [{xmlelement, "subscription", + [{"node", Node}, + {"jid", jlib:jid_to_string(JID)}, + {"subscription", subscription_to_string(Subscription)}], + []}]}]}, + ejabberd_router ! {route, service_jid(Host), JID, Stanza}. + handle_authorization_response(Host, From, To, Packet, XFields) -> case {lists:keysearch("pubsub#node", 1, XFields), lists:keysearch("pubsub#subscriber_jid", 1, XFields), lists:keysearch("pubsub#allow", 1, XFields)} of - {{value, {_, SNode}}, {value, {_, SSubscriber}}, - {value, {_, SAllow}}} -> + {{value, {_, [SNode]}}, {value, {_, [SSubscriber]}}, + {value, {_, [SAllow]}}} -> Node = case Host of {_, _, _} -> [SNode]; _ -> string:tokens(SNode, "/") @@ -1083,7 +1102,7 @@ handle_authorization_response(Host, From, To, Packet, XFields) -> %%options = Options, owners = Owners}) -> IsApprover = lists:member(jlib:jid_tolower(jlib:jid_remove_resource(From)), Owners), - Subscription = node_call(Type, get_subscription, [Host, Node, Subscriber]), + {result, Subscription} = node_call(Type, get_subscription, [Host, Node, Subscriber]), if not IsApprover -> {error, ?ERR_FORBIDDEN}; @@ -1094,6 +1113,7 @@ handle_authorization_response(Host, From, To, Packet, XFields) -> true -> subscribed; false -> none end, + send_authorization_approval(Host, Subscriber, SNode, NewSubscription), node_call(Type, set_subscription, [Host, Node, Subscriber, NewSubscription]) end end, @@ -1457,34 +1477,38 @@ publish_item(Host, ServerHost, Node, Publisher, "", Payload) -> publish_item(Host, ServerHost, Node, Publisher, uniqid(), Payload); publish_item(Host, ServerHost, Node, Publisher, ItemId, Payload) -> Action = fun(#pubsub_node{options = Options, type = Type}) -> - Features = features(Type), - PublishFeature = lists:member("publish", Features), - PublishModel = get_option(Options, publish_model), - MaxItems = max_items(Options), - PayloadSize = size(term_to_binary(Payload)), - PayloadMaxSize = get_option(Options, max_payload_size), - if - not PublishFeature -> - %% Node does not support item publication - {error, extended_error(?ERR_FEATURE_NOT_IMPLEMENTED, unsupported, "publish")}; - PayloadSize > PayloadMaxSize -> - %% Entity attempts to publish very large payload - {error, extended_error(?ERR_NOT_ACCEPTABLE, "payload-too-big")}; - %%?? -> iq_pubsub just does that matchs - %% % Entity attempts to publish item with multiple payload elements or namespace does not match - %% {error, extended_error(?ERR_BAD_REQUEST, "invalid-payload")}; - %% % Publisher attempts to publish to persistent node with no item - %% {error, extended_error(?ERR_BAD_REQUEST, "item-required")}; - Payload == "" -> - %% Publisher attempts to publish to payload node with no payload - {error, extended_error(?ERR_BAD_REQUEST, "payload-required")}; - %%?? -> - %% % Publisher attempts to publish to transient notification node with item - %% {error, extended_error(?ERR_BAD_REQUEST, "item-forbidden")}; - true -> - node_call(Type, publish_item, [Host, Node, Publisher, PublishModel, MaxItems, ItemId, Payload]) - end - end, + Features = features(Type), + PublishFeature = lists:member("publish", Features), + PublishModel = get_option(Options, publish_model), + MaxItems = max_items(Options), + PayloadCount = payload_elements(xmlelement, Payload), + PayloadSize = size(term_to_binary(Payload)), + PayloadMaxSize = get_option(Options, max_payload_size), + % pubsub#deliver_payloads true + % pubsub#persist_items true -> 1 item; false -> 0 item + if + not PublishFeature -> + %% Node does not support item publication + {error, extended_error(?ERR_FEATURE_NOT_IMPLEMENTED, unsupported, "publish")}; + PayloadSize > PayloadMaxSize -> + %% Entity attempts to publish very large payload + {error, extended_error(?ERR_NOT_ACCEPTABLE, "payload-too-big")}; + PayloadCount > 1 -> + %% Entity attempts to publish item with multiple payload elements + {error, extended_error(?ERR_BAD_REQUEST, "invalid-payload")}; + Payload == "" -> + %% Publisher attempts to publish to payload node with no payload + {error, extended_error(?ERR_BAD_REQUEST, "payload-required")}; + (MaxItems == 0) and (PayloadSize > 0) -> + % Publisher attempts to publish to transient notification node with item + {error, extended_error(?ERR_BAD_REQUEST, "item-forbidden")}; + (MaxItems > 0) and (PayloadSize == 0) -> + % Publisher attempts to publish to persistent node with no item + {error, extended_error(?ERR_BAD_REQUEST, "item-required")}; + true -> + node_call(Type, publish_item, [Host, Node, Publisher, PublishModel, MaxItems, ItemId, Payload]) + end + end, ejabberd_hooks:run(pubsub_publish_item, ServerHost, [ServerHost, Node, Publisher, service_jid(Host), ItemId, Payload]), Reply = [], case transaction(Host, Node, Action, sync_dirty) of @@ -1556,7 +1580,7 @@ delete_item(Host, Node, Publisher, ItemId, ForceNotify) -> PersistentFeature = lists:member("persistent-items", Features), DeleteFeature = lists:member("delete-nodes", Features), if - %%?? -> iq_pubsub just does that matchs + %%-> iq_pubsub just does that matchs %% %% Request does not specify an item %% {error, extended_error(?ERR_BAD_REQUEST, "item-required")}; not PersistentFeature -> @@ -1735,7 +1759,7 @@ send_items(Host, Node, LJID, Number) -> []; Items -> case Number of - last -> lists:sublist(lists:reverse(Items), 1); + last -> lists:last(Items); all -> Items; N when N > 0 -> lists:nthtail(length(Items)-N, Items); _ -> Items @@ -2095,6 +2119,15 @@ is_to_delivered({User, Server, _}, _, true) -> end, false, Ss) end. +%% @spec (Elem, Payload) -> int() +%% Elem = atom() +%% Payload = term() +%% @doc

Count occurence of given element in payload.

+payload_elements(Elem, Payload) -> payload_elements(Elem, Payload, 0). +payload_elements(_, [], Count) -> Count; +payload_elements(Elem, [Elem|Tail], Count) -> payload_elements(Elem, Tail, Count+1); +payload_elements(Elem, [_|Tail], Count) -> payload_elements(Elem, Tail, Count). + %%%%%% broadcast functions broadcast_publish_item(Host, Node, ItemId, _From, Payload) -> @@ -2643,8 +2676,9 @@ set_xoption([{"pubsub#type", Value} | Opts], NewOpts) -> ?SET_STRING_XOPT(type, Value); set_xoption([{"pubsub#body_xslt", Value} | Opts], NewOpts) -> ?SET_STRING_XOPT(body_xslt, Value); -set_xoption([_ | _Opts], _NewOpts) -> - {error, ?ERR_NOT_ACCEPTABLE}. +set_xoption([_ | Opts], NewOpts) -> + % skip unknown field + set_xoption(Opts, NewOpts). %%%% plugin handling diff --git a/src/mod_pubsub/node_zoo.erl b/src/mod_pubsub/node_flat.erl similarity index 97% rename from src/mod_pubsub/node_zoo.erl rename to src/mod_pubsub/node_flat.erl index 12a69c58f..c71fefd47 100644 --- a/src/mod_pubsub/node_zoo.erl +++ b/src/mod_pubsub/node_flat.erl @@ -22,7 +22,7 @@ %%% @end %%% ==================================================================== --module(node_zoo). +-module(node_flat). -author('christophe.romain@process-one.net'). -include("pubsub.hrl"). @@ -69,7 +69,7 @@ terminate(Host, ServerHost) -> node_default:terminate(Host, ServerHost). options() -> - [{node_type, zoo}, + [{node_type, flat}, {deliver_payloads, true}, {notify_config, false}, {notify_delete, false}, @@ -97,12 +97,7 @@ create_node_permission(Host, ServerHost, _Node, _ParentNode, Owner, Access) -> {"", Host, ""} -> true; % pubsub service always allowed _ -> - case acl:match_rule(ServerHost, Access, LOwner) of - allow -> - true; - _ -> - false - end + acl:match_rule(ServerHost, Access, LOwner) =:= allow end, {result, Allowed}. diff --git a/src/mod_pubsub/nodetree_default.erl b/src/mod_pubsub/nodetree_default.erl index 700397edb..05dd87746 100644 --- a/src/mod_pubsub/nodetree_default.erl +++ b/src/mod_pubsub/nodetree_default.erl @@ -45,7 +45,9 @@ terminate/2, options/0, set_node/1, + get_node/3, get_node/2, + get_nodes/2, get_nodes/1, get_subnodes/3, get_subnodes_tree/2, @@ -97,6 +99,9 @@ set_node(_) -> %% @spec (Host, Node) -> pubsubNode() | {error, Reason} %% Host = mod_pubsub:host() %% Node = mod_pubsub:pubsubNode() +get_node(Host, Node, _From) -> + get_node(Host, Node). + get_node(Host, Node) -> case catch mnesia:read({pubsub_node, {Host, Node}}) of [Record] when is_record(Record, pubsub_node) -> Record; @@ -106,6 +111,9 @@ get_node(Host, Node) -> %% @spec (Key) -> [pubsubNode()] | {error, Reason} %% Key = mod_pubsub:host() | mod_pubsub:jid() +get_nodes(Key, _From) -> + get_nodes(Key). + get_nodes(Key) -> mnesia:match_object(#pubsub_node{nodeid = {Key, '_'}, _ = '_'}). diff --git a/src/mod_pubsub/nodetree_virtual.erl b/src/mod_pubsub/nodetree_virtual.erl index 7b057a0fd..a122c4b1d 100644 --- a/src/mod_pubsub/nodetree_virtual.erl +++ b/src/mod_pubsub/nodetree_virtual.erl @@ -43,7 +43,9 @@ terminate/2, options/0, set_node/1, + get_node/3, get_node/2, + get_nodes/2, get_nodes/1, get_subnodes/3, get_subnodes_tree/2, @@ -86,6 +88,9 @@ set_node(_NodeRecord) -> %% Node = mod_pubsub:pubsubNode() %% @doc

Virtual node tree does not handle a node database. Any node is considered %% as existing. Node record contains default values.

+get_node(Host, Node, _From) -> + get_node(Host, Node). + get_node(Host, Node) -> #pubsub_node{nodeid = {Host, Node}}. @@ -93,6 +98,9 @@ get_node(Host, Node) -> %% Host = mod_pubsub:host() | mod_pubsub:jid() %% @doc

Virtual node tree does not handle a node database. Any node is considered %% as existing. Nodes list can not be determined.

+get_nodes(Key, _From) -> + get_nodes(Key). + get_nodes(_Key) -> []. diff --git a/src/mod_pubsub/pubsub.hrl b/src/mod_pubsub/pubsub.hrl index f763909a7..b4ceed4fa 100644 --- a/src/mod_pubsub/pubsub.hrl +++ b/src/mod_pubsub/pubsub.hrl @@ -119,13 +119,3 @@ payload = [] }). - -%% @type pubsubPresence() = #pubsub_presence{ -%% key = {Host::host(), User::string(), Server::string()}, -%% presence = list()}. -%%%

This is the format of the published presence table. The type of the -%%% table is: set,ram.

--record(pubsub_presence, {key, - resource - }). -