From 7a49880ffc56fc478e7a41fb0f52001e3f4602cf Mon Sep 17 00:00:00 2001 From: Christophe Romain Date: Tue, 22 Jul 2008 23:41:44 +0000 Subject: [PATCH] pubsub improvement, fixes EJAB-684 EJAB-675 EJAB-663 SVN Revision: 1472 --- ChangeLog | 11 ++++++++ src/mod_pubsub/mod_pubsub.erl | 45 ++++++++++++++++++++------------- src/mod_pubsub/node_default.erl | 16 +++++++----- src/mod_pubsub/node_pep.erl | 23 ++++++++++++++--- src/mod_pubsub/node_zoo.erl | 19 +++++++------- 5 files changed, 77 insertions(+), 37 deletions(-) diff --git a/ChangeLog b/ChangeLog index be5f8c044..5c9c0762b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,14 @@ +2008-07-23 Christophe Romain + + * src/mod_pubsub/mod_pubsub.erl: remove_user hook removes + subscriptions (EJAB-684), send the last published and not the + first published item (EJAB-675), remove the pubsub/nodes tree. + * src/mod_pubsub/node_pep.erl: added acl and jid match on node + creation permission (EJAB-663) + * src/mod_pubsub/node_default.erl: fix node creation permission + issue for service + * src/mod_pubsub/node_zoo.erl: Likewise + 2008-07-22 Badlop * src/ejabberd_config.erl: If syntax mistake in config file, show diff --git a/src/mod_pubsub/mod_pubsub.erl b/src/mod_pubsub/mod_pubsub.erl index e83e0170f..a993af71b 100644 --- a/src/mod_pubsub/mod_pubsub.erl +++ b/src/mod_pubsub/mod_pubsub.erl @@ -226,8 +226,6 @@ terminate_plugins(Host, ServerHost, Plugins, TreePlugin) -> ok. init_nodes(Host, ServerHost) -> - create_node(Host, ServerHost, ["pubsub"], service_jid(Host), ?STDNODE), - create_node(Host, ServerHost, ["pubsub", "nodes"], service_jid(Host), ?STDNODE), create_node(Host, ServerHost, ["home"], service_jid(Host), ?STDNODE), create_node(Host, ServerHost, ["home", ServerHost], service_jid(Host), ?STDNODE), ok. @@ -416,7 +414,7 @@ remove_user(User, Server) -> LUser = jlib:nodeprep(User), LServer = jlib:nameprep(Server), Proc = gen_mod:get_module_proc(Server, ?PROCNAME), - gen_server:cast(Proc, {remove, LUser, LServer}). + gen_server:cast(Proc, {remove_user, LUser, LServer}). %%-------------------------------------------------------------------- %% Function: @@ -513,10 +511,27 @@ handle_cast({presence, JID, Pid}, State) -> end, {noreply, State}; -handle_cast({remove, User, Server}, State) -> - Owner = jlib:make_jid(User, Server, ""), - delete_nodes(Server, Owner), - {noreply, State}; +handle_cast({remove_user, User, Host}, State) -> + Owner = jlib:make_jid(User, Host, ""), + OwnerKey = jlib:jid_tolower(jlib:jid_remove_resource(Owner)), + %% remove user's subscriptions + lists:foreach(fun(Type) -> + {result, Subscriptions} = node_action(Type, get_entity_subscriptions, [Host, Owner]), + lists:foreach(fun + ({Node, subscribed}) -> + JID = jlib:jid_to_string(Owner), + unsubscribe_node(Host, Node, Owner, JID, all); + (_) -> + ok + end, Subscriptions) + end, State#state.plugins), + %% remove user's PEP nodes + lists:foreach(fun(#pubsub_node{nodeid={NodeKey, NodeName}}) -> + delete_node(NodeKey, NodeName, Owner) + end, tree_action(Host, get_nodes, [OwnerKey])), + %% remove user's nodes + delete_node(Host, ["home", Host, User], Owner), + {noreply, State}; handle_cast(_Msg, State) -> {noreply, State}. @@ -1295,12 +1310,6 @@ delete_node(Host, Node, Owner) -> {result, Result} -> {result, Result} end. -delete_nodes(Host, Owner) -> - %% This removes only PEP nodes when user is removed - OwnerKey = jlib:jid_tolower(jlib:jid_remove_resource(Owner)), - lists:foreach(fun(#pubsub_node{nodeid={NodeKey, NodeName}}) -> - delete_node(NodeKey, NodeName, Owner) - end, tree_action(Host, get_nodes, [OwnerKey])). %% @spec (Host, Node, From, JID) -> %% {error, Reason::stanzaError()} | @@ -1725,9 +1734,9 @@ send_last_item(Host, Node, LJID) -> send_items(Host, Node, LJID, Number) -> Items = get_items(Host, Node), ToSend = case Number of - last -> lists:sublist(Items, 1); + last -> hd(lists:reverse(Items)); all -> Items; - N when N > 0 -> lists:sublist(Items, N); + N when N > 0 -> lists:nthtail(length(Items)-N, Items); _ -> Items end, ItemsEls = lists:map( @@ -2414,9 +2423,11 @@ get_default(Host, _Node, _From, Lang) -> %% The result depend of the node type plugin system. get_option([], _) -> false; get_option(Options, Var) -> + get_option(Options, Var, false). +get_option(Options, Var, Def) -> case lists:keysearch(Var, 1, Options) of {value, {_Val, Ret}} -> Ret; - _ -> false + _ -> Def end. %% Get default options from the module plugin. @@ -2461,7 +2472,7 @@ max_items(Options) -> -define(STRING_CONFIG_FIELD(Label, Var), ?STRINGXFIELD(Label, "pubsub#" ++ atom_to_list(Var), - get_option(Options, Var))). + get_option(Options, Var, ""))). -define(INTEGER_CONFIG_FIELD(Label, Var), ?STRINGXFIELD(Label, "pubsub#" ++ atom_to_list(Var), diff --git a/src/mod_pubsub/node_default.erl b/src/mod_pubsub/node_default.erl index 491676269..af8ad9dce 100644 --- a/src/mod_pubsub/node_default.erl +++ b/src/mod_pubsub/node_default.erl @@ -193,21 +193,23 @@ features() -> %% module by implementing this function like this: %% ```check_create_user_permission(Host, Node, Owner, Access) -> %% node_default:check_create_user_permission(Host, Node, Owner, Access).'''

-create_node_permission(_Host, ServerHost, Node, _ParentNode, Owner, Access) -> +create_node_permission(Host, ServerHost, Node, _ParentNode, Owner, Access) -> LOwner = jlib:jid_tolower(Owner), {User, Server, _Resource} = LOwner, - Allowed = case acl:match_rule(ServerHost, Access, LOwner) of + Allowed = case LOwner of + {"", Host, ""} -> + true; % pubsub service always allowed + _ -> + case acl:match_rule(ServerHost, Access, LOwner) of allow -> case Node of ["home", Server, User | _] -> true; _ -> false end; _ -> - case Owner of - {jid, "", _, "", "", _, ""} -> true; - _ -> false - end - end, + false + end + end, {result, Allowed}. %% @spec (Host, Node, Owner) -> diff --git a/src/mod_pubsub/node_pep.erl b/src/mod_pubsub/node_pep.erl index 2c648148d..c986ef61a 100644 --- a/src/mod_pubsub/node_pep.erl +++ b/src/mod_pubsub/node_pep.erl @@ -110,10 +110,25 @@ features() -> "subscribe" %* ]. -create_node_permission(_Host, _ServerHost, _Node, _ParentNode, _Owner, _Access) -> - %% TODO may we check bare JID match ? - {result, true}. - +create_node_permission(Host, ServerHost, _Node, _ParentNode, Owner, Access) -> + LOwner = jlib:jid_tolower(Owner), + {User, Server, _Resource} = LOwner, + Allowed = case LOwner of + {"", Host, ""} -> + true; % pubsub service always allowed + _ -> + case acl:match_rule(ServerHost, Access, LOwner) of + allow -> + case Host of + {User, Server, _} -> true; + _ -> false + end; + _ -> + false + end + end, + {result, Allowed}. + create_node(Host, Node, Owner) -> case node_default:create_node(Host, Node, Owner) of {result, _} -> {result, []}; diff --git a/src/mod_pubsub/node_zoo.erl b/src/mod_pubsub/node_zoo.erl index 45e601ed7..520a006a9 100644 --- a/src/mod_pubsub/node_zoo.erl +++ b/src/mod_pubsub/node_zoo.erl @@ -91,18 +91,19 @@ features() -> %% use same code as node_default, but do not limite node to %% the home/localhost/user/... hierarchy %% any node is allowed -create_node_permission(_Host, ServerHost, _Node, _ParentNode, Owner, Access) -> +create_node_permission(Host, ServerHost, _Node, _ParentNode, Owner, Access) -> LOwner = jlib:jid_tolower(Owner), - %%{_User, _Server, _Resource} = LOwner, - Allowed = case acl:match_rule(ServerHost, Access, LOwner) of + Allowed = case LOwner of + {"", Host, ""} -> + true; % pubsub service always allowed + _ -> + case acl:match_rule(ServerHost, Access, LOwner) of allow -> true; - _ -> - case Owner of - {jid, "", _, "", "", _, ""} -> true; - _ -> false - end - end, + _ -> + false + end + end, {result, Allowed}. create_node(Host, Node, Owner) ->