diff --git a/big_tests/tests/gdpr_SUITE.erl b/big_tests/tests/gdpr_SUITE.erl index 0daeb41769f..64b7787f7bb 100644 --- a/big_tests/tests/gdpr_SUITE.erl +++ b/big_tests/tests/gdpr_SUITE.erl @@ -14,7 +14,6 @@ -export([ retrieve_vcard/1, remove_vcard/1, - try_remove_data_of_non_existing_user/1, retrieve_roster/1, retrieve_mam/1, retrieve_offline/1, @@ -51,7 +50,8 @@ all() -> {group, retrieve_personal_data}, {group, retrieve_personal_data_with_mods_disabled}, {group, retrieve_negative}, - {group, remove_personal_data} + {group, remove_personal_data}, + {group, remove_personal_data_with_mods_disabled} ]. groups() -> @@ -93,9 +93,12 @@ groups() -> {retrieve_negative, [], [ data_is_not_retrieved_for_missing_user ]}, - {remove_personal_data, [parallel], [ + {remove_personal_data, [], [ + % per type + remove_vcard + ]}, + {remove_personal_data_with_mods_disabled, [], [ % per type - try_remove_data_of_non_existing_user, remove_vcard ]} ]. @@ -113,6 +116,8 @@ end_per_suite(Config) -> init_per_group(retrieve_personal_data_with_mods_disabled, Config) -> dynamic_modules:ensure_modules(domain(), pubsub_required_modules()), [{disable_module, true} | Config]; +init_per_group(remove_personal_data_with_mods_disabled, Config) -> + [{disable_module, true} | Config]; init_per_group(retrieve_personal_data_pubsub, Config) -> dynamic_modules:ensure_modules(domain(), pubsub_required_modules()), Config; @@ -138,6 +143,7 @@ init_per_testcase(remove_vcard = CN, Config) -> true -> {skip, skipped_for_simplicity_for_now}; % TODO: Fix the case for LDAP as well _ -> + vcard_started(), escalus:init_per_testcase(CN, Config) end; init_per_testcase(retrieve_mam = CN, Config) -> @@ -190,6 +196,9 @@ pick_backend_for_mam() -> mam_required_modules(Backend) -> [{mod_mam_meta, [{backend, Backend}, {pm, []}]}]. +vcard_required_modules() -> + [{mod_vcard, [{backend, mnesia}]}]. + pubsub_required_modules() -> [{mod_caps, []}, {mod_pubsub, [ {backend, mongoose_helper:mnesia_or_rdbms_backend()}, @@ -199,13 +208,14 @@ pubsub_required_modules() -> ] }]. +vcard_started() -> + dynamic_modules:ensure_modules(domain(), vcard_required_modules()). + %% ------------------------------------------------------------- %% Test cases %% ------------------------------------------------------------- %% ------------------------- Data retrieval - per type verification ------------------------- -try_remove_data_of_non_existing_user(Config) -> - {"Error: \"User does not exist\"\n", 1} = ejabberdctl("remove_personal_data", [<<"user_not_exist">>, <<"localhost">>], Config). retrieve_vcard(Config) -> escalus:fresh_story(Config, [{alice, 1}], fun(Alice) -> @@ -233,9 +243,18 @@ remove_vcard(Config) -> AliceSetResultStanza = escalus:send_and_wait(Alice, escalus_stanza:vcard_update(AliceFields)), escalus:assert(is_iq_result, AliceSetResultStanza), + AliceU = escalus_utils:jid_to_lower(escalus_client:username(Alice)), + AliceS = escalus_utils:jid_to_lower(escalus_client:server(Alice)), + + maybe_stop_and_unload_module(mod_vcard, mod_vcard_backend, Config), + {0, _} = unregister(Alice, Config), + + mongoose_helper:wait_until( + fun() -> + mongoose_helper:successful_rpc(mod_vcard, get_personal_data, + [AliceU, AliceS]) + end, [{vcard,["jid","vcard"],[]}]) - {0, _} = remove_personal_data(Alice, Config), - retrieve_and_validate_personal_data(Alice, Config, "vcard", ["jid", "vcard"], []) end). retrieve_roster(Config) -> @@ -643,10 +662,10 @@ retrieve_personal_data(User, Domain, Config) -> {CommandOutput, Code} = ejabberdctl("retrieve_personal_data", [User, Domain, Filename], Config), {Filename, Code, CommandOutput}. -remove_personal_data(Client, Config) -> +unregister(Client, Config) -> User = escalus_client:username(Client), Domain = escalus_client:server(Client), - {CommandOutput, Code} = ejabberdctl("remove_personal_data", [User, Domain], Config), + {CommandOutput, Code} = ejabberdctl("unregister", [User, Domain], Config), {Code, CommandOutput}. random_filename(Config) -> diff --git a/src/admin_extra/service_admin_extra_gdpr.erl b/src/admin_extra/service_admin_extra_gdpr.erl index 3b84d545f77..b7303b56e20 100644 --- a/src/admin_extra/service_admin_extra_gdpr.erl +++ b/src/admin_extra/service_admin_extra_gdpr.erl @@ -3,7 +3,7 @@ -include("ejabberd_commands.hrl"). -include("mongoose_logger.hrl"). --export([commands/0, retrieve_all/3, remove_all/2]). +-export([commands/0, retrieve_all/3]). % Exported for RPC call -export([retrieve_logs/2]). @@ -19,14 +19,6 @@ commands() -> [ module = ?MODULE, function = retrieve_all, args = [{username, binary}, {domain, binary}, {path, binary}], - result = {res, rescode}}, - #ejabberd_commands{name = remove_personal_data, tags = [gdpr], - desc = "Remove all user's presonal data.", - longdesc = "Removes all personal data from MongooseIM for a given user. Example:\n" - " mongooseimctl remove_personal_data alice localhost ", - module = ?MODULE, - function = remove_all, - args = [{username, binary}, {domain, binary}], result = {res, rescode}} ]. @@ -56,15 +48,6 @@ retrieve_all(Username, Domain, ResultFilePath) -> {error, "User does not exist"} end. --spec remove_all(jid:user(), jid:server()) -> ok | {error, Reason :: any()}. -remove_all(Username, Domain) -> - case user_exists(Username, Domain) of - true -> - remove_data_from_modules(Username, Domain); - false -> - {error, "User does not exist"} - end. - %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% %%% Private funs %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% @@ -90,13 +73,6 @@ try_get_data_from_module(Module, Username, Domain) -> [] end. --spec remove_data_from_modules(jid:user(), jid:server()) -> ok. -remove_data_from_modules(Username, Domain) -> -%%TODO: when all modules ready use the one from below -%% Modules = modules_with_personal_data(), - Modules = [mod_vcard], - lists:foreach(fun(M) -> M:remove_personal_data(Username, Domain) end, Modules). - -spec to_csv_file(CsvFilename :: binary(), gdpr:schema(), gdpr:entities(), file:name()) -> ok. to_csv_file(Filename, DataSchema, DataRows, TmpDir) -> FilePath = <<(list_to_binary(TmpDir))/binary, "/", Filename/binary>>, diff --git a/src/auth/ejabberd_auth.erl b/src/auth/ejabberd_auth.erl index 1ed0cd30eb1..222bbf0b7c4 100644 --- a/src/auth/ejabberd_auth.erl +++ b/src/auth/ejabberd_auth.erl @@ -544,8 +544,22 @@ do_remove_user(LUser, LServer) -> lserver => LServer, element => undefined }), ejabberd_hooks:run_fold(remove_user, LServer, Acc, [LUser, LServer]), + %% We need to take care about cleaning the data in modules that are currently disabled. + %% Mich duplicate Module:remove_user/2 calls for enabled modules + gdpr_remove_user_from_all_modules(LUser, LServer), ok. +gdpr_remove_user_from_all_modules(LUser, LServer) -> + Modules = mongoose_lib:find_behaviour_implementations(gdpr), + lists:foreach(fun(M) -> try_remove_user_from_module(M, LUser, LServer) end, Modules). + +try_remove_user_from_module(Module, LUser, LServer) -> + try + Module:remove_user(LUser, LServer) + catch + _:_ -> [] + end. + %% @doc Try to remove user if the provided password is correct. %% The removal is attempted in each auth method provided: %% when one returns 'ok' the loop stops; diff --git a/src/mod_vcard.erl b/src/mod_vcard.erl index 68f20eb4acb..b5baf5163a3 100644 --- a/src/mod_vcard.erl +++ b/src/mod_vcard.erl @@ -75,7 +75,7 @@ -export([config_change/4]). %% GDPR related --export([get_personal_data/2, remove_personal_data/2]). +-export([get_personal_data/2]). -define(PROCNAME, ejabberd_mod_vcard). @@ -148,21 +148,6 @@ get_personal_data(Username, Server) -> end, mongoose_lib:find_behaviour_implementations(mod_vcard)), [{vcard, Schema, Entries}]. --spec remove_personal_data(jid:user(), jid:server()) -> ok. -remove_personal_data(Username, Server) -> - LUser = jid:nodeprep(Username), - LServer = jid:nameprep(Server), - lists:foreach(fun(B) -> try_remove_personal_data(LUser, LServer, B) end, mongoose_lib:find_behaviour_implementations(mod_vcard)). - -try_remove_personal_data(LUser, LServer, Module) -> - try - Module:remove_user(LUser, LServer) - catch - _:_ -> - ok - end. - - -spec default_search_fields() -> list(). default_search_fields() -> [{<<"User">>, <<"user">>}, @@ -430,7 +415,13 @@ remove_user(Acc, User, Server) -> remove_user(User, Server) -> LUser = jid:nodeprep(User), LServer = jid:nodeprep(Server), - mod_vcard_backend:remove_user(LUser, LServer). + lists:foreach(fun(B) -> + try + B:remove_user(LUser, LServer) + catch + _:_ -> + ok + end end, mongoose_lib:find_behaviour_implementations(mod_vcard)). %% react to "global" config change config_change(Acc, Host, ldap, _NewConfig) ->