fix EJAB-701 and EJAB-836

SVN Revision: 1786

diff --git a/ChangeLog b/ChangeLog
index 2042ff30b5f8a85dff0264f78f172748719bd426..98a20ae35de7fa401134a544a8897acacb21d91a 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2009-01-08  Christophe Romain <christophe.romain@process-one.net>
+
+       * src/mod_pubsub/mod_pubsub.erl: completely support subscription using
+       full JID (EJAB-701)
+       * src/mod_pubsub/node_default.erl: Likewise
+
+       * src/mod_pubsub/node_default.erl: any entity can retrieve item when
+       node access model is "open" (thanks to Myers Carpenter)(EJAB-836)
+
 2009-01-08  Mickael Remond  <mremond@process-one.net>
 
        * src/ejabberd_listener.erl: Define send timeout option to avoid

diff --git a/src/mod_pubsub/mod_pubsub.erl b/src/mod_pubsub/mod_pubsub.erl
index 4c5b661df487b139e1ac5126cb84a795c9a95c41..d7b05d32fc0461a417aba0ad78a3d6fe526f575e 100644 (file)
--- a/src/mod_pubsub/mod_pubsub.erl
+++ b/src/mod_pubsub/mod_pubsub.erl
@@ -462,12 +462,12 @@ handle_cast({presence, JID, Pid}, State) ->
     lists:foreach(fun(Type) ->
        {result, Subscriptions} = node_action(Type, get_entity_subscriptions, [Host, JID]),
        lists:foreach(
-           fun({Node, subscribed}) -> 
+           fun({Node, subscribed, SubJID}) ->
                case tree_action(Host, get_node, [Host, Node]) of
                    #pubsub_node{options = Options} ->
                        case get_option(Options, send_last_published_item) of
                            on_sub_and_presence ->
-                               send_last_item(Host, Node, LJID);
+                               send_last_item(Host, Node, SubJID);
                            _ ->
                                ok
                        end;
@@ -519,13 +519,11 @@ handle_cast({presence, JID, Pid}, State) ->
 handle_cast({remove_user, LUser, LServer}, State) ->
     Host = State#state.host,
     Owner = jlib:make_jid(LUser, LServer, ""),
-    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),
+           ({Node, subscribed, JID}) ->
                unsubscribe_node(Host, Node, Owner, JID, all);
            (_) ->
                ok
@@ -534,7 +532,7 @@ handle_cast({remove_user, LUser, LServer}, State) ->
     %% 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])),
+    end, tree_action(Host, get_nodes, [jlib:jid_tolower(Owner)])),
     %% remove user's nodes
     delete_node(Host, ["home", LServer, LUser], Owner),
     {noreply, State};
@@ -1428,11 +1426,13 @@ subscribe_node(Host, Node, From, JID) ->
 %%<li>The node does not exist.</li>
 %%<li>The request specifies a subscription ID that is not valid or current.</li>
 %%</ul>
-unsubscribe_node(Host, Node, From, JID, SubId) ->
+unsubscribe_node(Host, Node, From, JID, SubId) when is_list(JID) ->
     Subscriber = case jlib:string_to_jid(JID) of
                     error -> {"", "", ""};
                     J -> jlib:jid_tolower(J)
                 end,
+    unsubscribe_node(Host, Node, From, Subscriber, SubId);
+unsubscribe_node(Host, Node, From, Subscriber, SubId) ->
     case node_action(Host, Node, unsubscribe_node,
                     [Host, Node, From, Subscriber, SubId]) of
        {error, Error} ->
@@ -1910,7 +1910,8 @@ get_subscriptions(Host, JID, Plugins) when is_list(Plugins) ->
                               %% Service does not support retreive subscriptions
                               {{error, extended_error(?ERR_FEATURE_NOT_IMPLEMENTED, unsupported, "retrieve-subscriptions")}, Acc};
                           true ->
-                              {result, Subscriptions} = node_action(Type, get_entity_subscriptions, [Host, JID]),
+                              Subscriber = jlib:jid_remove_resource(JID),
+                              {result, Subscriptions} = node_action(Type, get_entity_subscriptions, [Host, Subscriber]),
                               {Status, [Subscriptions|Acc]}
                       end
               end, {ok, []}, Plugins),
@@ -2263,7 +2264,7 @@ broadcast_config_notification(Host, Node, Lang) ->
 
 broadcast_stanza(Host, NodeOpts, States, Stanza) ->
     PresenceDelivery = get_option(NodeOpts, presence_based_delivery),
-    BroadcastAll = get_option(NodeOpts, broadcast_all_resources),
+    BroadcastAll = get_option(NodeOpts, broadcast_all_resources), %% XXX this is not standard
     From = service_jid(Host),
     lists:foreach(fun(#pubsub_state{stateid = {LJID, _}, subscription = Subs}) ->
        case is_to_deliver(LJID, Subs, PresenceDelivery) of

diff --git a/src/mod_pubsub/node_default.erl b/src/mod_pubsub/node_default.erl
index e1ca6c41026b73ae3b4eb2a384694392f20611e5..fa0e24d54a879b69609c9ec8948ff3be064596ca 100644 (file)
--- a/src/mod_pubsub/node_default.erl
+++ b/src/mod_pubsub/node_default.erl
@@ -157,7 +157,6 @@ features() ->
     ["create-nodes",
      "auto-create",
      "delete-nodes",
-     "delete-any",
      "instant-nodes",
      "manage-subscriptions",
      "modify-affiliations",
@@ -274,11 +273,16 @@ delete_node(Host, Removed) ->
 %% <p>In the default plugin module, the record is unchanged.</p>
 subscribe_node(Host, Node, Sender, Subscriber, AccessModel,
               SendLast, PresenceSubscription, RosterGroup) ->
-    SubscriberKey = jlib:jid_tolower(jlib:jid_remove_resource(Subscriber)),
-    Authorized = (jlib:jid_tolower(jlib:jid_remove_resource(Sender)) == SubscriberKey),
-    State = get_state(Host, Node, SubscriberKey),
-    #pubsub_state{affiliation = Affiliation,
-                 subscription = Subscription} = State,
+    SubKey = jlib:jid_tolower(Subscriber),
+    GenKey = jlib:jid_remove_resource(SubKey),
+    Authorized = (jlib:jid_tolower(jlib:jid_remove_resource(Sender)) == GenKey),
+    GenState = get_state(Host, Node, GenKey),
+    SubState = case SubKey of
+       GenKey -> GenState;
+       _ -> get_state(Host, Node, SubKey)
+       end,
+    Affiliation = GenState#pubsub_state.affiliation,
+    Subscription = SubState#pubsub_state.subscription,
     if
        not Authorized ->
            %% JIDs do not match
@@ -317,7 +321,7 @@ subscribe_node(Host, Node, Sender, Subscriber, AccessModel,
                    true ->
                        subscribed
                end,
-           set_state(State#pubsub_state{subscription = NewSubscription}),
+           set_state(SubState#pubsub_state{subscription = NewSubscription}),
            case NewSubscription of
                subscribed ->
                    case SendLast of
@@ -339,9 +343,16 @@ subscribe_node(Host, Node, Sender, Subscriber, AccessModel,
 %%      Reason = mod_pubsub:stanzaError()
 %% @doc <p>Unsubscribe the <tt>Subscriber</tt> from the <tt>Node</tt>.</p>
 unsubscribe_node(Host, Node, Sender, Subscriber, _SubId) ->
-    SubscriberKey = jlib:jid_tolower(jlib:jid_remove_resource(Subscriber)),
-    Authorized = (jlib:jid_tolower(jlib:jid_remove_resource(Sender)) == SubscriberKey),
-    State = get_state(Host, Node, SubscriberKey),
+    SubKey = jlib:jid_tolower(Subscriber),
+    GenKey = jlib:jid_remove_resource(SubKey),
+    Authorized = (jlib:jid_tolower(jlib:jid_remove_resource(Sender)) == GenKey),
+    GenState = get_state(Host, Node, GenKey),
+    SubState = case SubKey of
+       GenKey -> GenState;
+       _ -> get_state(Host, Node, SubKey)
+       end,
+    Affiliation = GenState#pubsub_state.affiliation,
+    Subscription = SubState#pubsub_state.subscription,
     if
        %% Entity did not specify SubID
        %%SubID == "", ?? ->
@@ -350,17 +361,18 @@ unsubscribe_node(Host, Node, Sender, Subscriber, _SubId) ->
        %%InvalidSubID ->
        %%      {error, ?ERR_EXTENDED(?ERR_NOT_ACCEPTABLE, "invalid-subid")};
        %% Requesting entity is not a subscriber
-       State#pubsub_state.subscription == none ->
+       Subscription == none ->
            {error, ?ERR_EXTENDED(?ERR_UNEXPECTED_REQUEST, "not-subscribed")};
        %% Requesting entity is prohibited from unsubscribing entity
-       (not Authorized) and (State#pubsub_state.affiliation =/= owner) ->
+       (not Authorized) and (Affiliation =/= owner) ->
            {error, ?ERR_FORBIDDEN};
        %% Was just subscriber, remove the record
-       State#pubsub_state.affiliation == none ->
-           del_state(State#pubsub_state.stateid),
+       Affiliation == none ->
+           del_state(SubState#pubsub_state.stateid),
            {result, default};
        true ->
-           set_state(State#pubsub_state{subscription = none}),
+           %% TODO, may require better clean
+           set_state(SubState#pubsub_state{subscription = none}),
            {result, default}
     end.
 
@@ -404,10 +416,15 @@ unsubscribe_node(Host, Node, Sender, Subscriber, _SubId) ->
 %% </p>
 %% <p>In the default plugin module, the record is unchanged.</p>
 publish_item(Host, Node, Publisher, PublishModel, MaxItems, ItemId, Payload) ->
-    PublisherKey = jlib:jid_tolower(jlib:jid_remove_resource(Publisher)),
-    State = get_state(Host, Node, PublisherKey),
-    #pubsub_state{affiliation = Affiliation,
-                 subscription = Subscription} = State,
+    SubKey = jlib:jid_tolower(Publisher),
+    GenKey = jlib:jid_remove_resource(SubKey),
+    GenState = get_state(Host, Node, GenKey),
+    SubState = case SubKey of
+       GenKey -> GenState;
+       _ -> get_state(Host, Node, SubKey)
+       end,
+    Affiliation = GenState#pubsub_state.affiliation,
+    Subscription = SubState#pubsub_state.subscription,
     if
        not ((PublishModel == open)
             or ((PublishModel == publishers)
@@ -417,24 +434,24 @@ publish_item(Host, Node, Publisher, PublishModel, MaxItems, ItemId, Payload) ->
            %% Entity does not have sufficient privileges to publish to node
            {error, ?ERR_FORBIDDEN};
        true ->
-           PubId = {PublisherKey, now()},
+           PubId = {SubKey, now()},
            Item = case get_item(Host, Node, ItemId) of
                       {result, OldItem} ->
                           OldItem#pubsub_item{modification = PubId,
                                               payload = Payload};
                       _ ->
                           #pubsub_item{itemid = {ItemId, {Host, Node}},
-                                       creation = PubId,
+                                       creation = {GenKey, now()},
                                        modification = PubId,
                                        payload = Payload}
                   end,
-           Items = [ItemId | State#pubsub_state.items--[ItemId]],
+           Items = [ItemId | GenState#pubsub_state.items--[ItemId]],
            {result, {NI, OI}} = remove_extra_items(
                                   Host, Node, MaxItems, Items),
            if MaxItems > 0 -> set_item(Item);
               true -> ok
            end,
-           set_state(State#pubsub_state{items = NI}),
+           set_state(GenState#pubsub_state{items = NI}),
            {result, {default, broadcast, OI}}
     end.
 
@@ -475,12 +492,13 @@ remove_extra_items(Host, Node, MaxItems, ItemIds) ->
 %% <p>Default plugin: The user performing the deletion must be the node owner
 %% or a publisher.</p>
 delete_item(Host, Node, Publisher, ItemId) ->
-    PublisherKey = jlib:jid_tolower(jlib:jid_remove_resource(Publisher)),
-    State = get_state(Host, Node, PublisherKey),
-    #pubsub_state{affiliation = Affiliation, items = Items} = State,
+    SubKey = jlib:jid_tolower(Publisher),
+    GenKey = jlib:jid_remove_resource(SubKey),
+    GenState = get_state(Host, Node, GenKey),
+    #pubsub_state{affiliation = Affiliation, items = Items} = GenState,
     Allowed = (Affiliation == publisher) orelse (Affiliation == owner)
        orelse case get_item(Host, Node, ItemId) of
-                  {result, #pubsub_item{creation = {PublisherKey, _}}} -> true;
+                  {result, #pubsub_item{creation = {GenKey, _}}} -> true;
                   _ -> false
               end,
     if
@@ -492,7 +510,7 @@ delete_item(Host, Node, Publisher, ItemId) ->
                {result, _} ->
                    del_item(Host, Node, ItemId),
                    NewItems = lists:delete(ItemId, Items),
-                   set_state(State#pubsub_state{items = NewItems}),
+                   set_state(GenState#pubsub_state{items = NewItems}),
                    {result, {default, broadcast}};
                _ ->
                    %% Non-existent node or item
@@ -509,8 +527,10 @@ delete_item(Host, Node, Publisher, ItemId) ->
 %% @doc <p>Purge all node items.</p>
 %% <p>Default plugin: The user performing the deletion must be the node owner.</p>
 purge_node(Host, Node, Owner) ->
-    OwnerKey = jlib:jid_tolower(jlib:jid_remove_resource(Owner)),
-    case get_state(Host, Node, OwnerKey) of
+    SubKey = jlib:jid_tolower(Owner),
+    GenKey = jlib:jid_remove_resource(SubKey),
+    GenState = get_state(Host, Node, GenKey),
+    case GenState of
        #pubsub_state{items = Items, affiliation = owner} ->
            del_items(Host, Node, Items),
            {result, {default, broadcast}};
@@ -530,9 +550,10 @@ purge_node(Host, Node, Owner) ->
 %% that will be added to the affiliation stored in the main
 %% <tt>pubsub_state</tt> table.</p>
 get_entity_affiliations(Host, Owner) ->
-    OwnerKey = jlib:jid_tolower(jlib:jid_remove_resource(Owner)),
+    SubKey = jlib:jid_tolower(Owner),
+    GenKey = jlib:jid_remove_resource(SubKey),
     States = mnesia:match_object(
-              #pubsub_state{stateid = {OwnerKey, {Host, '_'}}, _ = '_'}),
+              #pubsub_state{stateid = {GenKey, {Host, '_'}}, _ = '_'}),
     Tr = fun(#pubsub_state{stateid = {_, {_, N}}, affiliation = A}) ->
                 {N, A}
         end,
@@ -547,14 +568,16 @@ get_node_affiliations(Host, Node) ->
     {result, lists:map(Tr, States)}.
 
 get_affiliation(Host, Node, Owner) ->
-    OwnerKey = jlib:jid_tolower(jlib:jid_remove_resource(Owner)),
-    State = get_state(Host, Node, OwnerKey),
-    {result, State#pubsub_state.affiliation}.
+    SubKey = jlib:jid_tolower(Owner),
+    GenKey = jlib:jid_remove_resource(SubKey),
+    GenState = get_state(Host, Node, GenKey),
+    {result, GenState#pubsub_state.affiliation}.
 
 set_affiliation(Host, Node, Owner, Affiliation) ->
-    OwnerKey = jlib:jid_tolower(jlib:jid_remove_resource(Owner)),
-    State = get_state(Host, Node, OwnerKey),
-    set_state(State#pubsub_state{affiliation = Affiliation}),
+    SubKey = jlib:jid_tolower(Owner),
+    GenKey = jlib:jid_remove_resource(SubKey),
+    GenState = get_state(Host, Node, GenKey),
+    set_state(GenState#pubsub_state{affiliation = Affiliation}),
     ok.
 
 %% @spec (Host, Ownner) -> [{Node,Subscription}]
@@ -568,11 +591,16 @@ set_affiliation(Host, Node, Owner, Affiliation) ->
 %% that will be added to the affiliation stored in the main
 %% <tt>pubsub_state</tt> table.</p>
 get_entity_subscriptions(Host, Owner) ->
-    OwnerKey = jlib:jid_tolower(jlib:jid_remove_resource(Owner)),
-    States = mnesia:match_object(
-              #pubsub_state{stateid = {OwnerKey, {Host, '_'}}, _ = '_'}),
-    Tr = fun(#pubsub_state{stateid = {_, {_, N}}, subscription = S}) ->
-                {N, S}
+    States = case jlib:jid_tolower(Owner) of
+       {U, D, ""} -> mnesia:match_object(
+              #pubsub_state{stateid = {{U, D, '_'}, {Host, '_'}}, _ = '_'});
+       {U, D, R} -> mnesia:match_object(
+              #pubsub_state{stateid = {{U, D, ""}, {Host, '_'}}, _ = '_'})
+           ++ mnesia:match_object(
+              #pubsub_state{stateid = {{U, D, R}, {Host, '_'}}, _ = '_'})
+    end,
+    Tr = fun(#pubsub_state{stateid = {J, {_, N}}, subscription = S}) ->
+                {N, S, J}
         end,
     {result, lists:map(Tr, States)}.
 
@@ -585,14 +613,14 @@ get_node_subscriptions(Host, Node) ->
     {result, lists:map(Tr, States)}.
 
 get_subscription(Host, Node, Owner) ->
-    OwnerKey = jlib:jid_tolower(jlib:jid_remove_resource(Owner)),
-    State = get_state(Host, Node, OwnerKey),
-    {result, State#pubsub_state.subscription}.
+    SubKey = jlib:jid_tolower(Owner),
+    SubState = get_state(Host, Node, SubKey),
+    {result, SubState#pubsub_state.subscription}.
 
 set_subscription(Host, Node, Owner, Subscription) ->
-    OwnerKey = jlib:jid_tolower(jlib:jid_remove_resource(Owner)),
-    State = get_state(Host, Node, OwnerKey),
-    set_state(State#pubsub_state{subscription = Subscription}),
+    SubKey = jlib:jid_tolower(Owner),
+    SubState = get_state(Host, Node, SubKey),
+    set_state(SubState#pubsub_state{subscription = Subscription}),
     ok.
 
 %% @spec (Host, Node) -> [States] | []
@@ -660,10 +688,10 @@ get_items(Host, Node) ->
              #pubsub_item{itemid = {'_', {Host, Node}}, _ = '_'}),
     {result, Items}.
 get_items(Host, Node, JID, AccessModel, PresenceSubscription, RosterGroup, _SubId) ->
-    State = get_state(Host, Node, jlib:jid_tolower(jlib:jid_remove_resource(JID))),
-    #pubsub_state{affiliation = Affiliation,
-                 subscription = Subscription} = State,
-    Subscribed = not ((Subscription == none) or (Subscription == pending)),
+    SubKey = jlib:jid_tolower(JID),
+    GenKey = jlib:jid_remove_resource(SubKey),
+    GenState = get_state(Host, Node, GenKey),
+    Affiliation = GenState#pubsub_state.affiliation,
     if
        %%SubID == "", ?? ->
            %% Entity has multiple subscriptions to the node but does not specify a subscription ID
@@ -674,9 +702,6 @@ get_items(Host, Node, JID, AccessModel, PresenceSubscription, RosterGroup, _SubI
        Affiliation == outcast ->
            %% Requesting entity is blocked
            {error, ?ERR_FORBIDDEN};
-       (AccessModel == open) and (not Subscribed) ->
-           %% Entity is not subscribed
-           {error, ?ERR_EXTENDED(?ERR_NOT_AUTHORIZED, "not-subscribed")};
        (AccessModel == presence) and (not PresenceSubscription) ->
            %% Entity is not authorized to create a subscription (presence subscription required)
            {error, ?ERR_EXTENDED(?ERR_NOT_AUTHORIZED, "presence-subscription-required")};
@@ -710,10 +735,10 @@ get_item(Host, Node, ItemId) ->
            {error, ?ERR_ITEM_NOT_FOUND}
     end.
 get_item(Host, Node, ItemId, JID, AccessModel, PresenceSubscription, RosterGroup, _SubId) ->
-    State = get_state(Host, Node, jlib:jid_tolower(jlib:jid_remove_resource(JID))),
-    #pubsub_state{affiliation = Affiliation,
-                 subscription = Subscription} = State,
-    Subscribed = not ((Subscription == none) or (Subscription == pending)),
+    SubKey = jlib:jid_tolower(JID),
+    GenKey = jlib:jid_remove_resource(SubKey),
+    GenState = get_state(Host, Node, GenKey),
+    Affiliation = GenState#pubsub_state.affiliation,
     if
        %%SubID == "", ?? ->
            %% Entity has multiple subscriptions to the node but does not specify a subscription ID
@@ -724,9 +749,6 @@ get_item(Host, Node, ItemId, JID, AccessModel, PresenceSubscription, RosterGroup
        Affiliation == outcast ->
            %% Requesting entity is blocked
            {error, ?ERR_FORBIDDEN};
-       (AccessModel == open) and (not Subscribed) ->
-           %% Entity is not subscribed
-           {error, ?ERR_EXTENDED(?ERR_NOT_AUTHORIZED, "not-subscribed")};
        (AccessModel == presence) and (not PresenceSubscription) ->
            %% Entity is not authorized to create a subscription (presence subscription required)
            {error, ?ERR_EXTENDED(?ERR_NOT_AUTHORIZED, "presence-subscription-required")};