Merge pull request #3377 from esl/retract_on_stanza_ids

Retract on stanza ids

big_tests/tests/mam_SUITE.erl

@@ -1,5 +1,4 @@
 %%==============================================================================
-
 %% Copyright 2012 Erlang Solutions Ltd.
 %%
 %% Licensed under the Apache License, Version 2.0 (the "License");
@@ -44,6 +43,7 @@
          text_search_is_available/1,
          muc_message_with_stanzaid/1,
          retract_muc_message/1,
+         retract_muc_message_on_stanza_id/1,
          retract_wrong_muc_message/1,
          muc_text_search_request/1,
          muc_archive_request/1,
@@ -95,6 +95,7 @@
          archived/1,
          message_with_stanzaid/1,
          retract_message/1,
+         retract_message_on_stanza_id/1,
          retract_wrong_message/1,
          filter_forwarded/1,
          offline_message/1,
@@ -435,6 +436,7 @@ muc_stanzaid_cases() ->
 
 muc_retract_cases() ->
     [retract_muc_message,
+     retract_muc_message_on_stanza_id,
      retract_wrong_muc_message].
 
 disabled_muc_retract_cases() ->
@@ -449,7 +451,8 @@ muc_configurable_archiveid_cases() ->
 configurable_archiveid_cases() ->
     [no_elements,
      only_stanzaid,
-     same_stanza_id
+     same_stanza_id,
+     retract_message_on_stanza_id
     ].
 
 muc_light_cases() ->
@@ -911,6 +914,13 @@ init_per_testcase(C=archived, Config) ->
 init_per_testcase(C, Config) when C =:= retract_message;
                                   C =:= retract_wrong_message ->
     skip_if_retraction_not_supported(Config, fun() -> escalus:init_per_testcase(C, Config) end);
+init_per_testcase(C=retract_message_on_stanza_id, Config) ->
+    Init = fun() ->
+                   OrigVal = rpc(mim(), gen_mod, get_module_opt, [host_type(), mod_mam, same_mam_id_for_peers, false]),
+                   true = rpc(mim(), gen_mod, set_module_opt, [host_type(), mod_mam, same_mam_id_for_peers, true]),
+                   escalus:init_per_testcase(C, [{same_mam_id_for_peers, OrigVal} | Config])
+        end,
+    skip_if_retraction_not_supported(Config, Init);
 init_per_testcase(C=offline_message, Config) ->
     Config1 = escalus_fresh:create_users(Config, [{alice, 1}, {bob, 1}, {carol, 1}]),
     escalus:init_per_testcase(C, Config1);
@@ -959,6 +969,7 @@ init_per_testcase(C=muc_message_with_stanzaid, Config) ->
     Config1 = escalus_fresh:create_users(Config, [{alice, 1}, {bob, 1}]),
     escalus:init_per_testcase(C, start_alice_room(Config1));
 init_per_testcase(C, Config) when C =:= retract_muc_message;
+                                  C =:= retract_muc_message_on_stanza_id;
                                   C =:= retract_wrong_muc_message ->
     Init = fun() ->
                    Config1 = escalus_fresh:create_users(Config, [{alice, 1}, {bob, 1}]),
@@ -1092,7 +1103,9 @@ end_per_testcase(C=muc_querying_for_all_messages_with_jid, Config) ->
 end_per_testcase(C=muc_message_with_stanzaid, Config) ->
     destroy_room(Config),
     escalus:end_per_testcase(C, Config);
-end_per_testcase(C=retract_muc_message, Config) ->
+end_per_testcase(C, Config) when C =:= retract_muc_message;
+                                 C =:= retract_muc_message_on_stanza_id;
+                                 C =:= retract_wrong_muc_message ->
     destroy_room(Config),
     escalus:end_per_testcase(C, Config);
 end_per_testcase(C=muc_no_elements, Config) ->
@@ -1105,6 +1118,10 @@ end_per_testcase(C = muc_light_stored_in_pm_if_allowed_to, Config0) ->
     {value, {_, OrigVal}, Config1} = lists:keytake(archive_groupchats_backup, 1, Config0),
     true = rpc(mim(), gen_mod, set_module_opt, [host_type(), mod_mam, archive_groupchats, OrigVal]),
     escalus:end_per_testcase(C, Config1);
+end_per_testcase(C = retract_message_on_stanza_id, Config0) ->
+    {value, {_, OrigVal}, Config1} = lists:keytake(same_mam_id_for_peers, 1, Config0),
+    true = rpc(mim(), gen_mod, set_module_opt, [host_type(), mod_mam, same_mam_id_for_peers, OrigVal]),
+    escalus:end_per_testcase(C, Config1);
 end_per_testcase(C = muc_light_chat_markers_are_archived_if_enabled, Config) ->
     restore_module_opts(mod_mam_muc, Config),
     escalus:end_per_testcase(C, Config);
@@ -1824,6 +1841,9 @@ message_with_stanzaid(Config) ->
     end,
     escalus:story(Config, [{alice, 1}, {bob, 1}], F).
 
+retract_message_on_stanza_id(Config) ->
+    retract_message([{retract_on, stanza_id} | Config]).
+
 retract_wrong_message(Config) ->
     retract_message([{origin_id, <<"wrong-id">>} | Config]).
 
@@ -1836,12 +1856,17 @@ retract_message(Config) ->
         Msg = #xmlel{children = Children} = escalus_stanza:chat_to(Bob, Body),
         escalus:send(Alice, Msg#xmlel{children = Children ++ [OriginIdElement]}),
 
+        mam_helper:wait_for_archive_size(Alice, 1),
+        escalus:send(Alice, stanza_archive_request(P, <<"q1">>)),
+        Result = wait_archive_respond(Alice),
+        [AliceCopyOfMessage] = respond_messages(Result),
+
         %% ... and Bob receives the message
         RecvMsg = escalus:wait_for_stanza(Bob),
         ?assert_equal(OriginIdElement, exml_query:subelement(RecvMsg, <<"origin-id">>)),
 
         %% WHEN Alice retracts the message
-        ApplyToElement = apply_to_element(origin_id_to_retract(Config)),
+        ApplyToElement = apply_to_element(Config, AliceCopyOfMessage),
         RetractMsg = retraction_message(<<"chat">>, escalus_utils:get_jid(Bob), ApplyToElement),
         escalus:send(Alice, RetractMsg),
 
@@ -1963,6 +1988,9 @@ muc_message_with_stanzaid(Config) ->
         end,
     escalus:story(Config, [{alice, 1}, {bob, 1}], F).
 
+retract_muc_message_on_stanza_id(Config) ->
+    retract_muc_message([{retract_on, stanza_id} | Config]).
+
 retract_wrong_muc_message(Config) ->
     retract_muc_message([{origin_id, <<"wrong-id">>} | Config]).
 
@@ -1973,25 +2001,27 @@ retract_muc_message(Config) ->
         RoomAddr = room_address(Room),
         escalus:send(Alice, stanza_muc_enter_room(Room, nick(Alice))),
         escalus:send(Bob, stanza_muc_enter_room(Room, nick(Bob))),
-
         %% Bob received presences.
         escalus:wait_for_stanzas(Bob, 2),
-
         %% Bob received the room's subject.
         escalus:wait_for_stanzas(Bob, 1),
+        %% Alice receives all the messages Bob did as well
+        escalus:wait_for_stanzas(Alice, 3),
 
         %% GIVEN Alice sends a message with 'origin-id' to the chat room ...
         Body = <<"Hi, Bob!">>,
         OriginIdElement = origin_id_element(origin_id()),
         Msg = #xmlel{children = Children} = escalus_stanza:groupchat_to(RoomAddr, Body),
         escalus:send(Alice, Msg#xmlel{children = Children ++ [OriginIdElement]}),
 
+        AliceCopyOfMessage = escalus:wait_for_stanza(Alice),
+
         %% ... and Bob receives the message
         RecvMsg = escalus:wait_for_stanza(Bob),
         ?assert_equal(OriginIdElement, exml_query:subelement(RecvMsg, <<"origin-id">>)),
 
         %% WHEN Alice retracts the message
-        ApplyToElement = apply_to_element(origin_id_to_retract(Config)),
+        ApplyToElement = apply_to_element(Config, AliceCopyOfMessage),
         RetractMsg = retraction_message(<<"groupchat">>, RoomAddr, ApplyToElement),
         escalus:send(Alice, RetractMsg),
 
@@ -3131,7 +3161,11 @@ check_archive_after_retraction(Config, Client, ApplyToElement, Body) ->
     end.
 
 message_should_be_retracted(Config) ->
-    message_retraction_is_enabled(Config) andalso origin_id_to_retract(Config) =:= origin_id().
+    message_retraction_is_enabled(Config) andalso
+    (retract_on_stanza_id(Config) orelse origin_id_to_retract(Config) =:= origin_id()).
+
+retract_on_stanza_id(Config) ->
+    stanza_id =:= ?config(retract_on, Config).
 
 message_retraction_is_enabled(Config) ->
     BasicGroup = ?config(basic_group, Config),
@@ -3161,16 +3195,29 @@ origin_id_element(OriginId) ->
            attrs = [{<<"xmlns">>, <<"urn:xmpp:sid:0">>},
                     {<<"id">>, OriginId}]}.
 
-apply_to_element(OriginId) ->
+apply_to_element(Config, Copy) ->
+    {RetractOn, Id} = case ?config(retract_on, Config) of
+                    stanza_id -> {stanza_id, stanza_id_from_msg(Copy)};
+                    _ -> {origin_id, origin_id_to_retract(Config)}
+                end,
     #xmlel{name = <<"apply-to">>,
-           attrs = [{<<"id">>, OriginId},
+           attrs = [{<<"id">>, Id},
                     {<<"xmlns">>, <<"urn:xmpp:fasten:0">>}],
-           children = [retract_element()]
+           children = [retract_element(RetractOn)]
           }.
 
-retract_element() ->
+stanza_id_from_msg(Msg) ->
+    case exml_query:path(Msg, [{element, <<"stanza-id">>}, {attr, <<"id">>}]) of
+        undefined -> exml_query:path(Msg, [{element, <<"result">>}, {attr, <<"id">>}]);
+        Id -> Id
+    end.
+
+retract_element(origin_id) ->
+    #xmlel{name = <<"retract">>,
+           attrs = [{<<"xmlns">>, <<"urn:xmpp:message-retract:0">>}]};
+retract_element(stanza_id) ->
     #xmlel{name = <<"retract">>,
-           attrs = [{<<"xmlns">>, <<"urn:xmpp:message-retract:0">>}]}.
+           attrs = [{<<"xmlns">>, <<"urn:esl:message-retract-by-stanza-id:0">>}]}.
 
 origin_id_to_retract(Config) ->
     proplists:get_value(origin_id, Config, origin_id()).

big_tests/tests/mam_helper.erl

@@ -94,6 +94,7 @@
          mam_ns_binary_v04/0,
          mam_ns_binary_v06/0,
          retract_ns/0,
+         retract_esl_ns/0,
          retract_tombstone_ns/0,
          make_alice_and_bob_friends/2,
          run_prefs_case/6,
@@ -231,12 +232,14 @@ namespaces() ->
     [mam_ns_binary_v04(),
      mam_ns_binary_v06(),
      retract_ns(),
+     retract_esl_ns(),
      retract_tombstone_ns()].
 
 mam_ns_binary() -> mam_ns_binary_v04().
 mam_ns_binary_v04() -> <<"urn:xmpp:mam:1">>.
 mam_ns_binary_v06() -> <<"urn:xmpp:mam:2">>.
 retract_ns() -> <<"urn:xmpp:message-retract:0">>.
+retract_esl_ns() -> <<"urn:esl:message-retract-by-stanza-id:0">>.
 retract_tombstone_ns() -> <<"urn:xmpp:message-retract:0#tombstone">>.
 
 skip_undefined(Xs) ->

big_tests/tests/muc_SUITE.erl

@@ -2902,11 +2902,8 @@ disco_features_with_mam(Config) ->
                                   ?NS_MUC_UNIQUE,
                                   <<"jabber:iq:register">>,
                                   ?NS_RSM,
-                                  <<"vcard-temp">>,
-                                  mam_helper:mam_ns_binary_v04(),
-                                  mam_helper:mam_ns_binary_v06(),
-                                  mam_helper:retract_ns(),
-                                  mam_helper:retract_tombstone_ns()]).
+                                  <<"vcard-temp">> |
+                                  mam_helper:namespaces()]).
 
 disco_rooms(Config) ->
     escalus:fresh_story(Config, [{alice, 1}], fun(Alice) ->

big_tests/tests/muc_light_SUITE.erl

@@ -338,12 +338,10 @@ check_features(Stanza, HasMAM) ->
                                                {attr, <<"var">>}]),
     ?assertEqual(ExpectedFeatures, ActualFeatures).
 
-expected_features(HasMAM) ->
-    MamFeatures = case HasMAM of
-                      true -> mam_helper:namespaces();
-                      false -> []
-                  end,
-    lists:sort([?NS_MUC_LIGHT | MamFeatures]).
+expected_features(true) ->
+    lists:sort([?NS_MUC_LIGHT | mam_helper:namespaces()]);
+expected_features(false) ->
+    [?NS_MUC_LIGHT].
 
 %% The room list is empty. Rooms_per_page set to `infinity`
 disco_rooms_empty_page_infinity(Config) ->

doc/modules/mod_mam.md

@@ -17,14 +17,32 @@ Configure MAM with different storage backends:
 `mod_mam_meta` is a meta-module that ensures all relevant `mod_mam_*` modules are loaded and properly configured.
 
 ### Message retraction
-This module supports [XEP-0424: Message Retraction](http://xmpp.org/extensions/xep-0424.html) with RDBMS storage backends. When a [retraction message](https://xmpp.org/extensions/xep-0424.html#example-4) is received, the MAM module finds the message to retract and replaces it with a tombstone. The following criteria are used to find the original message:
+This module supports [XEP-0424: Message Retraction](http://xmpp.org/extensions/xep-0424.html) with RDBMS storage backends. When a [retraction message](https://xmpp.org/extensions/xep-0424.html#example-4) is received, the MAM module finds the message to retract and replaces it with a tombstone.
 
-* The `id` attribute specified in the `apply-to` element of the retraction message has to be the same as the `id` attribute of the `origin-id` element of the original message.
+The following criteria are used to find the original message:
+* The `id` attribute specified in the `apply-to` element of the retraction message has to be the same as the `id` attribute of the `origin-id` (or `stanza-id` when configured, see [below](#retraction-on-the-stanza-id)) element of the original message.
 * Both messages need to originate from the same user.
 * Both messages need to be addressed to the same user.
 
 If more than one message matches the criteria, only the most recent one is retracted. To avoid this case, it is recommended to use a unique identifier (UUID) as the origin ID.
 
+#### Retraction on the stanza-id
+This module also implements an extension to the XEP, where it allows to specify the [`stanza-id`](https://xmpp.org/extensions/xep-0359.html#stanza-id) as [created by](https://xmpp.org/extensions/xep-0313.html#archives_id) the server's MAM, instead of the `origin-id` that the original [XEP-0424](https://xmpp.org/extensions/xep-0424.html) specifies. It announces this capability under the namespace `urn:esl:message-retract-by-stanza-id:0`. This is specially useful in groupchats where the `stanza-id` of a message is shared and known for all participants.
+
+In this case, to use such functionality,
+```xml
+<apply-to id="origin-id-1" xmlns="urn:xmpp:fasten:0">
+  <retract xmlns='urn:xmpp:message-retract:0'/>
+</apply-to>
+```
+turns into
+```xml
+<apply-to id="stanza-id-1" xmlns="urn:xmpp:fasten:0">
+  <retract xmlns='urn:esl:message-retract:0'/>
+</apply-to>
+```
+and likewise, the answer would be tagged by the mentioned `esl` namespace.
+
 ### Full Text Search
 This module allows message filtering by their text body (if enabled, see *Common backend options*).
 This means that an XMPP client, while requesting messages from the archive may not only specify standard form fields (`with`, `start`, `end`), but also `full-text-search` (of type `text-single`).
@@ -132,7 +150,7 @@ When enabled, MAM will store groupchat messages in recipients' individual archiv
 * **Default:** `false`
 * **Example:** `modules.mod_mam_meta.pm.same_mam_id_for_peers = true`
 
-When enabled, MAM will set the same MAM ID for both sender and recipient. Note that this might not work with clients across federation, as the recipient might not implement the same retraction, nor the same IDs.
+When enabled, MAM will set the same MAM ID for both sender and recipient. This can be useful in combination with [retraction on the stanza-id](#retraction-on-the-stanza-id). Note that this might not work with clients across federation, as the recipient might not implement the same retraction, nor the same IDs.
 
 ### Enable MUC message archive
 

include/mongoose_ns.hrl

@@ -105,6 +105,9 @@
 
 -define(JINGLE_NS, <<"urn:xmpp:jingle:1">>).
 
+%% Custom extension to accept stanza-ids as retraction IDs
+-define(NS_ESL_RETRACT,         <<"urn:esl:message-retract-by-stanza-id:0">>).
+
 %% Erlang Solutions custom extension - token based authentication
 -define(NS_ESL_TOKEN_AUTH,      <<"erlang-solutions.com:xmpp:token-auth:0">>).
 

src/mam/mam_decoder.erl

@@ -2,15 +2,18 @@
 -export([decode_row/2]).
 -export([decode_muc_row/2]).
 -export([decode_muc_gdpr_row/2]).
--export([decode_retraction_info/2]).
+-export([decode_retraction_info/3]).
 
 -type ext_mess_id() :: non_neg_integer() | binary().
 -type env_vars() :: mod_mam_rdbms_arch:env_vars().
 -type db_row() :: {ext_mess_id(), ExtSrcJID :: binary(), ExtData :: binary()}.
 -type db_muc_row() :: {ext_mess_id(), Nick :: binary(), ExtData :: binary()}.
 -type db_muc_gdpr_row() :: {ext_mess_id(), ExtData :: binary()}.
 -type decoded_muc_gdpr_row() :: {ext_mess_id(), exml:element()}.
--type retraction_info() :: #{packet := exml:element(), message_id := mod_mam:message_id()}.
+-type retraction_info() :: #{retract_on := origin_id | stanza_id,
+                             packet := exml:element(),
+                             message_id := mod_mam:message_id(),
+                             origin_id := binary()}.
 
 -spec decode_row(db_row(), env_vars()) -> mod_mam:message_row().
 decode_row({ExtMessID, ExtSrcJID, ExtData}, Env) ->
@@ -31,13 +34,19 @@ decode_muc_gdpr_row({ExtMessID, ExtData}, Env) ->
     Packet = decode_packet(ExtData, Env),
     {ExtMessID, Packet}.
 
--spec decode_retraction_info(env_vars(), [] | [{mod_mam:message_id(), binary()}]) ->
+-spec decode_retraction_info(env_vars(),
+                             [{binary(), mod_mam:message_id(), binary()}],
+                             mod_mam_utils:retraction_id()) ->
     skip | retraction_info().
-decode_retraction_info(_Env, []) -> skip;
-decode_retraction_info(Env, [{ResMessID, Data}]) ->
+decode_retraction_info(_Env, [], _) -> skip;
+decode_retraction_info(Env, [{ResMessID, Data}], {origin_id, OriginID}) ->
     Packet = decode_packet(Data, Env),
     MessID = mongoose_rdbms:result_to_integer(ResMessID),
-    #{packet => Packet, message_id => MessID}.
+    #{retract_on => origin_id, packet => Packet, message_id => MessID, origin_id => OriginID};
+decode_retraction_info(Env, [{OriginID, Data}], {stanza_id, StanzaID}) ->
+    Packet = decode_packet(Data, Env),
+    MessID = mod_mam_utils:external_binary_to_mess_id(StanzaID),
+    #{retract_on => stanza_id, packet => Packet, message_id => MessID, origin_id => OriginID}.
 
 -spec decode_jid(binary(), env_vars()) -> jid:jid().
 decode_jid(ExtJID, #{db_jid_codec := Codec, archive_jid := ArcJID}) ->