From e3cdff35fc912652e29d526fc798aa82cd3dcca8 Mon Sep 17 00:00:00 2001 From: Aleksander Lisiecki Date: Mon, 13 May 2019 08:56:46 +0200 Subject: [PATCH 1/5] Test remove personal data from mod_roster --- big_tests/tests/gdpr_SUITE.erl | 40 +++++++++++++++++++++++++++++----- 1 file changed, 35 insertions(+), 5 deletions(-) diff --git a/big_tests/tests/gdpr_SUITE.erl b/big_tests/tests/gdpr_SUITE.erl index f290872d404..5b3dfbd78b9 100644 --- a/big_tests/tests/gdpr_SUITE.erl +++ b/big_tests/tests/gdpr_SUITE.erl @@ -15,6 +15,7 @@ retrieve_vcard/1, remove_vcard/1, retrieve_roster/1, + remove_roster/1, retrieve_mam/1, retrieve_offline/1, retrieve_pubsub_payloads/1, @@ -95,11 +96,13 @@ groups() -> ]}, {remove_personal_data, [], [ % per type - remove_vcard + remove_vcard, + remove_roster ]}, {remove_personal_data_with_mods_disabled, [], [ % per type - remove_vcard + remove_vcard, + remove_roster ]} ]. @@ -154,6 +157,10 @@ init_per_testcase(retrieve_mam = CN, Config) -> dynamic_modules:ensure_modules(domain(), mam_required_modules(Backend)), escalus:init_per_testcase(CN, Config) end; +init_per_testcase(remove_roster = CN, Config) -> + dynamic_modules:ensure_modules(domain(), [{mod_roster, []}]), + escalus_fresh:clean(), + escalus:init_per_testcase(CN, Config); init_per_testcase(CN, Config) -> escalus:init_per_testcase(CN, Config). @@ -270,14 +277,35 @@ retrieve_roster(Config) -> escalus_story:make_all_clients_friends([Alice, Bob]), BobU = escalus_utils:jid_to_lower(escalus_client:username(Bob)), BobS = escalus_utils:jid_to_lower(escalus_client:server(Bob)), - ExpectedHeader = ["jid", "name", "subscription", - "ask", "groups", "askmessage", "xs"], ExpectedItems = [ #{ "jid" => [{contains, BobU}, {contains, BobS}] } ], maybe_stop_and_unload_module(mod_roster, mod_roster_backend, Config), retrieve_and_validate_personal_data( - Alice, Config, "roster", ExpectedHeader, ExpectedItems) + Alice, Config, "roster", expected_header(mod_roster), ExpectedItems) + end). + +remove_roster(Config) -> + escalus:fresh_story(Config, [{alice, 1}, {bob, 1}], fun(Alice, Bob) -> + escalus_story:make_all_clients_friends([Alice, Bob]), + AliceU = escalus_utils:jid_to_lower(escalus_client:username(Alice)), + AliceS = escalus_utils:jid_to_lower(escalus_client:server(Alice)), + ExpectedItems = [ + #{ "jid" => [{contains, AliceU}, {contains, AliceS}] } + ], + + maybe_stop_and_unload_module(mod_roster, mod_roster_backend, Config), + {0, _} = unregister(Alice, Config), + + mongoose_helper:wait_until( + fun() -> + mongoose_helper:successful_rpc(mod_roster, get_personal_data, + [AliceU, AliceS]) + end, + [{roster, expected_header(mod_roster), []}]), + retrieve_and_validate_personal_data( + Bob, Config, "roster", expected_header(mod_roster), ExpectedItems) + end). retrieve_mam(_Config) -> @@ -762,3 +790,5 @@ validate_time1(Time) -> check_list(List) -> lists:all(fun({V, L}) -> I = list_to_integer(V), I >= 0 andalso I < L end, List). +expected_header(mod_roster) -> ["jid", "name", "subscription", + "ask", "groups", "askmessage", "xs"]. From 95b149010e87a6d27b700354924ab3bc7a187884 Mon Sep 17 00:00:00 2001 From: Aleksander Lisiecki Date: Mon, 13 May 2019 15:23:49 +0200 Subject: [PATCH 2/5] Change remove user to work for all backends --- src/mod_roster.erl | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/mod_roster.erl b/src/mod_roster.erl index afbb8f77eff..4d5b445abad 100644 --- a/src/mod_roster.erl +++ b/src/mod_roster.erl @@ -884,8 +884,17 @@ remove_user(Acc, User, Server) -> LUser = jid:nodeprep(User), LServer = jid:nameprep(Server), Acc1 = send_unsubscription_to_rosteritems(Acc, LUser, LServer), - R = mod_roster_backend:remove_user(LUser, LServer), - mongoose_lib:log_if_backend_error(R, ?MODULE, ?LINE, {User, Server}), + lists:foreach(fun(Backend) -> + try + Backend:remove_user(LUser, LServer) + catch + Class:Reason -> + StackTrace = erlang:get_stacktrace(), + ?WARNING_MSG("event=cannot_delete_personal_data," + "backend=~p,class=~p,reason=~p,stacktrace=~p", + [Backend, Class, Reason, StackTrace]) + end + end, mongoose_lib:find_behaviour_implementations(mod_roster)), Acc1. %% For each contact with Subscription: From 522359983f229f7060cbedf63e97f2fcf676f086 Mon Sep 17 00:00:00 2001 From: Aleksander Lisiecki Date: Mon, 20 May 2019 11:40:03 +0200 Subject: [PATCH 3/5] Correct generating accumulator --- src/mod_roster.erl | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/mod_roster.erl b/src/mod_roster.erl index 4d5b445abad..dd12d5f8f5b 100644 --- a/src/mod_roster.erl +++ b/src/mod_roster.erl @@ -883,18 +883,28 @@ remove_user(User, Server) -> remove_user(Acc, User, Server) -> LUser = jid:nodeprep(User), LServer = jid:nameprep(Server), - Acc1 = send_unsubscription_to_rosteritems(Acc, LUser, LServer), + Acc1 = + try + send_unsubscription_to_rosteritems(Acc, LUser, LServer) + catch + E:R -> + S = erlang:get_stacktrace(), + ?WARNING_MSG("event=cannot_send_unsubscription_to_rosteritems," + "class=~p,reason=~p,stacktrace=~p", [E, R, S]), + Acc + end, + Backends = mongoose_lib:find_behaviour_implementations(mod_roster), lists:foreach(fun(Backend) -> try Backend:remove_user(LUser, LServer) catch Class:Reason -> StackTrace = erlang:get_stacktrace(), - ?WARNING_MSG("event=cannot_delete_personal_data," + ?WARNING_MSG("event=cannot_delete_personal_data,backends = ~p," "backend=~p,class=~p,reason=~p,stacktrace=~p", - [Backend, Class, Reason, StackTrace]) + [Backends, Backend, Class, Reason, StackTrace]) end - end, mongoose_lib:find_behaviour_implementations(mod_roster)), + end, Backends), Acc1. %% For each contact with Subscription: From bdb81db9b6895242f71e661398023def42499e2a Mon Sep 17 00:00:00 2001 From: Aleksander Lisiecki Date: Mon, 20 May 2019 11:44:21 +0200 Subject: [PATCH 4/5] Refactor user removal --- src/mod_roster.erl | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/mod_roster.erl b/src/mod_roster.erl index dd12d5f8f5b..0c437f8ccab 100644 --- a/src/mod_roster.erl +++ b/src/mod_roster.erl @@ -883,16 +883,7 @@ remove_user(User, Server) -> remove_user(Acc, User, Server) -> LUser = jid:nodeprep(User), LServer = jid:nameprep(Server), - Acc1 = - try - send_unsubscription_to_rosteritems(Acc, LUser, LServer) - catch - E:R -> - S = erlang:get_stacktrace(), - ?WARNING_MSG("event=cannot_send_unsubscription_to_rosteritems," - "class=~p,reason=~p,stacktrace=~p", [E, R, S]), - Acc - end, + Acc1 = maybe_send_unsubscription_to_rosteritems(Acc, LUser, LServer), Backends = mongoose_lib:find_behaviour_implementations(mod_roster), lists:foreach(fun(Backend) -> try @@ -907,6 +898,17 @@ remove_user(Acc, User, Server) -> end, Backends), Acc1. +maybe_send_unsubscription_to_rosteritems(Acc, LUser, LServer) -> + try + send_unsubscription_to_rosteritems(Acc, LUser, LServer) + catch + E:R -> + S = erlang:get_stacktrace(), + ?WARNING_MSG("event=cannot_send_unsubscription_to_rosteritems," + "class=~p,reason=~p,stacktrace=~p", [E, R, S]), + Acc + end. + %% For each contact with Subscription: %% Both or From, send a "unsubscribed" presence stanza; %% Both or To, send a "unsubscribe" presence stanza. From 2070e7bdd28f41826b11190ab953708391ee1470 Mon Sep 17 00:00:00 2001 From: Aleksander Lisiecki Date: Mon, 20 May 2019 15:22:22 +0200 Subject: [PATCH 5/5] Test different backends for mod_roster --- big_tests/tests/gdpr_SUITE.erl | 14 ++++++++++++-- src/mod_roster.erl | 4 ++-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/big_tests/tests/gdpr_SUITE.erl b/big_tests/tests/gdpr_SUITE.erl index 5b3dfbd78b9..c008ee8d3df 100644 --- a/big_tests/tests/gdpr_SUITE.erl +++ b/big_tests/tests/gdpr_SUITE.erl @@ -158,13 +158,14 @@ init_per_testcase(retrieve_mam = CN, Config) -> escalus:init_per_testcase(CN, Config) end; init_per_testcase(remove_roster = CN, Config) -> - dynamic_modules:ensure_modules(domain(), [{mod_roster, []}]), - escalus_fresh:clean(), + Backend = pick_backend_for_roster(), + dynamic_modules:ensure_modules(domain(), [{mod_roster, [{backend, Backend}]}]), escalus:init_per_testcase(CN, Config); init_per_testcase(CN, Config) -> escalus:init_per_testcase(CN, Config). end_per_testcase(CN, Config) -> + escalus_fresh:clean(), escalus:end_per_testcase(CN, Config). init_inbox(CN, Config) -> @@ -187,6 +188,15 @@ inbox_opts() -> {groupchat, [muclight]}, {markers, [displayed]}]. +pick_backend_for_roster() -> + IsRiak = mam_helper:is_riak_enabled(domain()), + IsRdbms = mongoose_helper:is_rdbms_enabled(domain()), + if + IsRiak -> riak; + IsRdbms -> rdbms; + true -> mnesia + end. + pick_backend_for_mam() -> BackendsList = [ {mam_helper:is_cassandra_enabled(domain()), cassandra}, diff --git a/src/mod_roster.erl b/src/mod_roster.erl index 0c437f8ccab..abe98ac8283 100644 --- a/src/mod_roster.erl +++ b/src/mod_roster.erl @@ -883,7 +883,7 @@ remove_user(User, Server) -> remove_user(Acc, User, Server) -> LUser = jid:nodeprep(User), LServer = jid:nameprep(Server), - Acc1 = maybe_send_unsubscription_to_rosteritems(Acc, LUser, LServer), + Acc1 = try_send_unsubscription_to_rosteritems(Acc, LUser, LServer), Backends = mongoose_lib:find_behaviour_implementations(mod_roster), lists:foreach(fun(Backend) -> try @@ -898,7 +898,7 @@ remove_user(Acc, User, Server) -> end, Backends), Acc1. -maybe_send_unsubscription_to_rosteritems(Acc, LUser, LServer) -> +try_send_unsubscription_to_rosteritems(Acc, LUser, LServer) -> try send_unsubscription_to_rosteritems(Acc, LUser, LServer) catch