From 4c7f6ffff976c8419ceaa29c2e9fbbc88bed53a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chrz=C4=85szcz?= Date: Tue, 15 Nov 2022 13:38:05 +0100 Subject: [PATCH 1/6] Get rid of the nested 'wait_until' In case of failure it would result in very long execution time. Pass the optional Check function to the inner loop. --- big_tests/tests/inbox_helper.erl | 57 +++++++++++++++++--------------- 1 file changed, 30 insertions(+), 27 deletions(-) diff --git a/big_tests/tests/inbox_helper.erl b/big_tests/tests/inbox_helper.erl index 02e9d87119a..c7294c49281 100644 --- a/big_tests/tests/inbox_helper.erl +++ b/big_tests/tests/inbox_helper.erl @@ -106,6 +106,9 @@ Expected :: binary(), AttrName :: binary()) -> any() | no_return()). +-type inbox_result() :: #{respond_iq := exml:element(), + respond_messages := [exml:element()]}. + -type conv() :: #conv{}. %% --------------------------------------------------------- @@ -177,43 +180,41 @@ foreach_check_inbox(Users, Unread, SenderJid, Msg) -> check_inbox(U, [#conv{unread = Unread, from = SenderJid, to = UserJid, content = Msg}]) end || U <- Users]. --spec check_inbox(Client :: escalus:client(), Convs :: [conv()]) -> ok | no_return(). +-spec check_inbox(Client :: escalus:client(), Convs :: [conv()]) -> inbox_result(). check_inbox(Client, Convs) -> check_inbox(Client, Convs, #{}, #{}). --spec check_inbox(escalus:client(), [conv()], inbox_query_params()) -> ok | no_return(). +-spec check_inbox(escalus:client(), [conv()], inbox_query_params()) -> inbox_result(). check_inbox(Client, Convs, QueryOpts) -> check_inbox(Client, Convs, QueryOpts, #{}). -spec check_inbox(Client :: escalus:client(), Convs :: [conv()], QueryOpts :: inbox_query_params(), - CheckOpts :: inbox_check_params()) -> ok | no_return(). + CheckOpts :: inbox_check_params()) -> inbox_result(). check_inbox(Client, Convs, QueryOpts, CheckOpts) -> - {ok, Res} = mongoose_helper:wait_until( - fun() -> do_check_inbox(Client, Convs, QueryOpts, CheckOpts) end, - ok, #{name => inbox_size, validator => fun(_) -> true end}), - Res. - -do_check_inbox(Client, Convs, QueryOpts, CheckOpts) -> ExpectedSortedConvs = case maps:get(order, QueryOpts, undefined) of asc -> lists:reverse(Convs); _ -> Convs end, - Inbox = #{respond_messages := ResultStanzas} - = get_inbox(Client, QueryOpts, #{count => length(ExpectedSortedConvs)}), + F = fun(#{respond_messages := ResultStanzas} = InboxResult) -> + do_check_inbox(Client, QueryOpts, CheckOpts, ResultStanzas, ExpectedSortedConvs), + InboxResult + end, + get_inbox(Client, QueryOpts, #{count => length(ExpectedSortedConvs)}, F). + +do_check_inbox(Client, QueryOpts, CheckOpts, ResultStanzas, ExpectedSortedConvs) -> try - check_inbox_result(Client, QueryOpts, CheckOpts, ResultStanzas, ExpectedSortedConvs), - Inbox + check_inbox_result(Client, QueryOpts, CheckOpts, ResultStanzas, ExpectedSortedConvs) catch _:Reason:StackTrace -> - ct:fail(#{ reason => inbox_mismatch, - inbox_items => lists:map(fun exml:to_binary/1, ResultStanzas), - expected_items => lists:map(fun pretty_conv/1, ExpectedSortedConvs), - query_params => QueryOpts, - check_params => CheckOpts, - error => Reason, - stacktrace => StackTrace }) + error(#{ reason => inbox_mismatch, + inbox_items => lists:map(fun exml:to_binary/1, ResultStanzas), + expected_items => lists:map(fun pretty_conv/1, ExpectedSortedConvs), + query_params => QueryOpts, + check_params => CheckOpts, + error => Reason, + stacktrace => StackTrace }) end. check_inbox_result(Client, QueryOpts, CheckOpts, ResultStanzas, MsgCheckList) -> @@ -252,21 +253,23 @@ maybe_check_queryid(Children, #{queryid := QueryId}) -> maybe_check_queryid(_Children, #{}) -> ok. --spec get_inbox(Client :: escalus:client(), - ExpectedResult :: inbox_result_params()) -> - #{respond_iq := exml:element(), respond_messages := [exml:element()]}. +-spec get_inbox(escalus:client(), inbox_result_params()) -> inbox_result(). get_inbox(Client, ExpectedResult) -> get_inbox(Client, #{}, ExpectedResult). +-spec get_inbox(escalus:client(), inbox_query_params(), inbox_result_params()) -> inbox_result(). +get_inbox(Client, GetParams, ExpectedResult) -> + get_inbox(Client, GetParams, ExpectedResult, fun(R) -> R end). + -spec get_inbox(Client :: escalus:client(), GetParams :: inbox_query_params(), - ExpectedResult :: inbox_result_params()) -> - #{respond_iq := exml:element(), respond_messages := [exml:element()]}. -get_inbox(Client, GetParams, #{count := ExpectedCount} = ExpectedResult) -> + ExpectedResult :: inbox_result_params(), + Check :: fun((inbox_result()) -> inbox_result())) -> inbox_result(). +get_inbox(Client, GetParams, #{count := ExpectedCount} = ExpectedResult, Check) -> GetInbox = make_inbox_stanza(GetParams), Validator = fun(#{respond_messages := Val}) -> ExpectedCount =:= length(Val) end, {ok, Inbox} = mongoose_helper:wait_until( - fun() -> do_get_inbox(Client, GetInbox) end, + fun() -> Check(do_get_inbox(Client, GetInbox)) end, ExpectedCount, #{validator => Validator, name => inbox_size}), #{respond_iq := ResIQ} = Inbox, ?assert(escalus_pred:is_iq_result(GetInbox, ResIQ)), From 1088b09d35f7a32c3f74e5a69d70e179e36ae0de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chrz=C4=85szcz?= Date: Tue, 15 Nov 2022 13:38:52 +0100 Subject: [PATCH 2/6] Test the cases when the domain is not string-prepped --- big_tests/tests/common_helper.erl | 2 ++ big_tests/tests/graphql_account_SUITE.erl | 23 +++++++++----- big_tests/tests/graphql_gdpr_SUITE.erl | 13 ++++++-- big_tests/tests/graphql_http_upload_SUITE.erl | 13 ++++++-- big_tests/tests/graphql_inbox_SUITE.erl | 17 ++++++++--- big_tests/tests/graphql_last_SUITE.erl | 15 ++++++---- big_tests/tests/graphql_muc_SUITE.erl | 30 +++++++++++++++++-- big_tests/tests/graphql_muc_light_SUITE.erl | 30 ++++++++++++++++++- big_tests/tests/graphql_offline_SUITE.erl | 21 +++++++++---- 9 files changed, 134 insertions(+), 30 deletions(-) diff --git a/big_tests/tests/common_helper.erl b/big_tests/tests/common_helper.erl index 565e166ea5f..6cfb55112e6 100644 --- a/big_tests/tests/common_helper.erl +++ b/big_tests/tests/common_helper.erl @@ -6,3 +6,5 @@ get_bjid(UserSpec) -> Server = proplists:get_value(server, UserSpec), <>. +unprep(Bin) when is_binary(Bin) -> + list_to_binary(string:titlecase(binary_to_list(Bin))). diff --git a/big_tests/tests/graphql_account_SUITE.erl b/big_tests/tests/graphql_account_SUITE.erl index cc808ab689e..93793e3aa6d 100644 --- a/big_tests/tests/graphql_account_SUITE.erl +++ b/big_tests/tests/graphql_account_SUITE.erl @@ -4,6 +4,7 @@ -compile([export_all, nowarn_export_all]). +-import(common_helper, [unprep/1]). -import(distributed_helper, [mim/0, require_rpc_nodes/1, rpc/4]). -import(graphql_helper, [execute_command/4, execute_user_command/5, get_listener_port/1, get_listener_config/1, get_ok_value/2, get_err_msg/1, @@ -199,9 +200,11 @@ admin_list_users(Config) -> Domain = domain_helper:domain(), Username = jid:nameprep(escalus_users:get_username(Config, alice)), JID = <>, - Resp2 = list_users(Domain, Config), - Users = get_ok_value([data, account, listUsers], Resp2), - ?assert(lists:member(JID, Users)). + Resp1 = list_users(Domain, Config), + Users = get_ok_value([data, account, listUsers], Resp1), + ?assert(lists:member(JID, Users)), + Resp2 = list_users(unprep(Domain), Config), + ?assertEqual(Users, get_ok_value([data, account, listUsers], Resp2)). admin_list_users_unknown_domain(Config) -> Resp = list_users(<<"unknown-domain">>, Config), @@ -210,8 +213,11 @@ admin_list_users_unknown_domain(Config) -> admin_count_users(Config) -> % A domain with at least one user Domain = domain_helper:domain(), - Resp2 = count_users(Domain, Config), - ?assert(0 < get_ok_value([data, account, countUsers], Resp2)). + Resp1 = count_users(Domain, Config), + Count = get_ok_value([data, account, countUsers], Resp1), + ?assert(0 < Count), + Resp2 = count_users(unprep(Domain), Config), + ?assertEqual(Count, get_ok_value([data, account, countUsers], Resp2)). admin_count_users_unknown_domain(Config) -> Resp = count_users(<<"unknown-domain">>, Config), @@ -306,9 +312,12 @@ admin_register_user(Config) -> % Try to register a user with existing name Resp2 = register_user(Domain, Username, Password, Config), ?assertNotEqual(nomatch, binary:match(get_err_msg(Resp2), <<"already registered">>)), + % Try again, this time with a domain name that is not stringprepped + Resp3 = register_user(unprep(Domain), Username, Password, Config), + ?assertNotEqual(nomatch, binary:match(get_err_msg(Resp3), <<"already registered">>)), % Try to register a user without any name - Resp3 = register_user(Domain, <<>>, Password, Config), - ?assertNotEqual(nomatch, binary:match(get_err_msg(Resp3), <<"Invalid JID">>)). + Resp4 = register_user(Domain, <<>>, Password, Config), + ?assertNotEqual(nomatch, binary:match(get_err_msg(Resp4), <<"Invalid JID">>)). admin_register_random_user(Config) -> Password = <<"my_password">>, diff --git a/big_tests/tests/graphql_gdpr_SUITE.erl b/big_tests/tests/graphql_gdpr_SUITE.erl index b1f7e65ee5c..e876c341494 100644 --- a/big_tests/tests/graphql_gdpr_SUITE.erl +++ b/big_tests/tests/graphql_gdpr_SUITE.erl @@ -2,6 +2,7 @@ -compile([export_all, nowarn_export_all]). +-import(common_helper, [unprep/1]). -import(domain_helper, [host_type/0, domain/0]). -import(distributed_helper, [mim/0, rpc/4, require_rpc_nodes/1]). -import(graphql_helper, [execute_command/4, execute_user_command/5, user_to_bin/1, @@ -25,6 +26,7 @@ groups() -> admin_gdpr_tests() -> [admin_retrieve_user_data, + admin_retrieve_user_data_for_unprepped_domain, admin_gdpr_no_user_test, admin_gdpr_empty_filename_test, admin_gdpr_access_denied_erofs, @@ -34,6 +36,7 @@ admin_gdpr_tests() -> domain_admin_gdpr_tests() -> [admin_retrieve_user_data, + admin_retrieve_user_data_for_unprepped_domain, admin_gdpr_no_user_test, domain_admin_retrieve_user_data_no_permission]. @@ -65,11 +68,17 @@ end_per_testcase(CaseName, Config) -> % Admin test cases admin_retrieve_user_data(Config) -> - escalus:fresh_story_with_config(Config, [{alice, 1}], fun admin_retrieve_user_data/2). + Config1 = [{domain, domain()} | Config], + escalus:fresh_story_with_config(Config1, [{alice, 1}], fun admin_retrieve_user_data/2). + +admin_retrieve_user_data_for_unprepped_domain(Config) -> + Config1 = [{domain, unprep(domain())} | Config], + escalus:fresh_story_with_config(Config1, [{alice, 1}], fun admin_retrieve_user_data/2). admin_retrieve_user_data(Config, Alice) -> Filename = random_filename(Config), - Res = admin_retrieve_personal_data(escalus_client:username(Alice), escalus_client:server(Alice), + Domain = ?config(domain, Config), + Res = admin_retrieve_personal_data(escalus_client:username(Alice), Domain, list_to_binary(Filename), Config), ParsedResult = get_ok_value([data, gdpr, retrievePersonalData], Res), ?assertEqual(<<"Data retrieved">>, ParsedResult), diff --git a/big_tests/tests/graphql_http_upload_SUITE.erl b/big_tests/tests/graphql_http_upload_SUITE.erl index ca35bee7ea2..eb0cba9aca3 100644 --- a/big_tests/tests/graphql_http_upload_SUITE.erl +++ b/big_tests/tests/graphql_http_upload_SUITE.erl @@ -2,6 +2,7 @@ -compile([export_all, nowarn_export_all]). +-import(common_helper, [unprep/1]). -import(distributed_helper, [mim/0, require_rpc_nodes/1]). -import(domain_helper, [host_type/0, domain/0, secondary_domain/0]). -import(graphql_helper, [execute_user_command/5, execute_command/4, get_ok_value/2, @@ -190,7 +191,11 @@ user_http_upload_not_configured(Config, Alice) -> % Admin test cases admin_get_url_test(Config) -> - Result = admin_get_url(domain(), <<"test">>, 123, <<"Test">>, 123, Config), + admin_get_url_test(Config, domain()), + admin_get_url_test(Config, unprep(domain())). + +admin_get_url_test(Config, Domain) -> + Result = admin_get_url(Domain, <<"test">>, 123, <<"Test">>, 123, Config), ParsedResult = get_ok_value([data, httpUpload, getUrl], Result), #{<<"PutUrl">> := PutURL, <<"GetUrl">> := GetURL, <<"Header">> := _Headers} = ParsedResult, ?assertMatch({_, _}, binary:match(PutURL, [?S3_HOSTNAME])), @@ -217,7 +222,11 @@ admin_get_url_no_domain(Config) -> ?assertEqual(<<"domain does not exist">>, get_err_msg(Result)). admin_http_upload_not_configured(Config) -> - Result = admin_get_url(domain(), <<"test">>, 123, <<"Test">>, 123, Config), + admin_http_upload_not_configured(Config, domain()), + admin_http_upload_not_configured(Config, unprep(domain())). + +admin_http_upload_not_configured(Config, Domain) -> + Result = admin_get_url(Domain, <<"test">>, 123, <<"Test">>, 123, Config), ?assertEqual(<<"deps_not_loaded">>, get_err_code(Result)), ?assertEqual(<<"Some of required modules or services are not loaded">>, get_err_msg(Result)). diff --git a/big_tests/tests/graphql_inbox_SUITE.erl b/big_tests/tests/graphql_inbox_SUITE.erl index 011d81d35e6..c4568d7a3b9 100644 --- a/big_tests/tests/graphql_inbox_SUITE.erl +++ b/big_tests/tests/graphql_inbox_SUITE.erl @@ -2,12 +2,14 @@ -compile([export_all, nowarn_export_all]). +-import(common_helper, [unprep/1]). -import(distributed_helper, [mim/0, require_rpc_nodes/1, rpc/4]). -import(domain_helper, [host_type/0, domain/0]). -import(graphql_helper, [execute_user_command/5, execute_command/4, user_to_bin/1, get_ok_value/2, get_err_msg/1, get_err_code/1, get_not_loaded/1, get_unauthorized/1]). +-include_lib("common_test/include/ct.hrl"). -include_lib("eunit/include/eunit.hrl"). -include("inbox.hrl"). @@ -55,6 +57,7 @@ admin_inbox_tests() -> admin_try_flush_nonexistent_user_bin, admin_try_flush_user_bin_nonexistent_domain, admin_flush_domain_bin, + admin_flush_unprepped_domain_bin, admin_try_flush_nonexistent_domain_bin, admin_flush_global_bin, admin_flush_global_bin_after_days, @@ -155,13 +158,18 @@ admin_try_flush_user_bin_nonexistent_domain(Config) -> ?assertErrCode(Res, domain_not_found). admin_flush_domain_bin(Config) -> - escalus:fresh_story_with_config(Config, [{alice, 1}, {alice_bis, 1}, {kate, 1}], + Config1 = [{domain, domain()} | Config], + escalus:fresh_story_with_config(Config1, [{alice, 1}, {alice_bis, 1}, {kate, 1}], + fun admin_flush_domain_bin/4). + +admin_flush_unprepped_domain_bin(Config) -> + Config1 = [{domain, unprep(domain())} | Config], + escalus:fresh_story_with_config(Config1, [{alice, 1}, {alice_bis, 1}, {kate, 1}], fun admin_flush_domain_bin/4). admin_flush_domain_bin(Config, Alice, AliceBis, Kate) -> RoomBinJID = create_room_and_make_users_leave(Alice, AliceBis, Kate), - Domain = domain_helper:domain(), - Res = flush_domain_bin(Domain, Config), + Res = flush_domain_bin(?config(domain, Config), Config), NumOfRows = get_ok_value(p(flushDomainBin), Res), ?assertEqual(1, NumOfRows), inbox_helper:check_inbox(Kate, [], #{box => bin}), @@ -213,7 +221,8 @@ admin_flush_user_bin_inbox_not_configured(Config, Alice) -> get_not_loaded(flush_user_bin(Alice, Config)). admin_flush_domain_bin_inbox_not_configured(Config) -> - get_not_loaded(flush_domain_bin(domain(), Config)). + get_not_loaded(flush_domain_bin(domain(), Config)), + get_not_loaded(flush_domain_bin(unprep(domain()), Config)). admin_flush_global_bin_inbox_not_configured(Config) -> get_not_loaded(flush_global_bin(host_type(), 10, Config)). diff --git a/big_tests/tests/graphql_last_SUITE.erl b/big_tests/tests/graphql_last_SUITE.erl index 8d936187ae6..3ac6d07ea1d 100644 --- a/big_tests/tests/graphql_last_SUITE.erl +++ b/big_tests/tests/graphql_last_SUITE.erl @@ -2,6 +2,7 @@ -compile([export_all, nowarn_export_all]). +-import(common_helper, [unprep/1]). -import(distributed_helper, [mim/0, require_rpc_nodes/1, rpc/4]). -import(graphql_helper, [execute_command/4, execute_user_command/5, user_to_bin/1, user_to_jid/1, get_ok_value/2, get_err_msg/1, get_err_code/1, get_unauthorized/1, @@ -311,6 +312,8 @@ admin_count_active_users_story(Config, Alice, Bob) -> set_last(Bob, now_dt_with_offset(10), Config), Res = admin_count_active_users(Domain, null, Config), ?assertEqual(2, get_ok_value(p(countActiveUsers), Res)), + Res1 = admin_count_active_users(unprep(Domain), null, Config), + ?assertEqual(2, get_ok_value(p(countActiveUsers), Res1)), Res2 = admin_count_active_users(Domain, now_dt_with_offset(30), Config), ?assertEqual(0, get_ok_value(p(countActiveUsers), Res2)). @@ -561,13 +564,13 @@ admin_get_last_not_configured_story(Config, Alice) -> admin_count_active_users_last_not_configured(Config) -> Domain = domain_helper:domain(), - Res = admin_count_active_users(Domain, null, Config), - get_not_loaded(Res). + get_not_loaded(admin_count_active_users(Domain, null, Config)), + get_not_loaded(admin_count_active_users(unprep(Domain), null, Config)). admin_remove_old_users_domain_last_not_configured(Config) -> Domain = domain_helper:domain(), - Res = admin_remove_old_users(Domain, now_dt_with_offset(150), Config), - get_not_loaded(Res). + get_not_loaded(admin_remove_old_users(Domain, now_dt_with_offset(150), Config)), + get_not_loaded(admin_remove_old_users(unprep(Domain), now_dt_with_offset(150), Config)). admin_remove_old_users_global_last_not_configured(Config) -> Res = admin_remove_old_users(null, now_dt_with_offset(150), Config), @@ -575,8 +578,8 @@ admin_remove_old_users_global_last_not_configured(Config) -> admin_list_old_users_domain_last_not_configured(Config) -> Domain = domain_helper:domain(), - Res = admin_list_old_users(Domain, now_dt_with_offset(150), Config), - get_not_loaded(Res). + get_not_loaded(admin_list_old_users(Domain, now_dt_with_offset(150), Config)), + get_not_loaded(admin_list_old_users(unprep(Domain), now_dt_with_offset(150), Config)). admin_list_old_users_global_last_not_configured(Config) -> Res = admin_list_old_users(null, now_dt_with_offset(150), Config), diff --git a/big_tests/tests/graphql_muc_SUITE.erl b/big_tests/tests/graphql_muc_SUITE.erl index 4c722127fc7..7aaaf7efe66 100644 --- a/big_tests/tests/graphql_muc_SUITE.erl +++ b/big_tests/tests/graphql_muc_SUITE.erl @@ -2,6 +2,7 @@ -compile([export_all, nowarn_export_all]). +-import(common_helper, [unprep/1]). -import(distributed_helper, [mim/0, require_rpc_nodes/1, rpc/4]). -import(graphql_helper, [execute_command/4, execute_user_command/5, get_ok_value/2, get_err_msg/1, get_coercion_err_msg/1, user_to_bin/1, user_to_full_bin/1, user_to_jid/1, @@ -40,6 +41,7 @@ admin_groups() -> user_muc_tests() -> [user_create_and_delete_room, + user_create_room_with_unprepped_domain, user_try_delete_nonexistent_room, user_try_delete_room_by_not_owner, user_try_create_instant_room_with_nonexistent_domain, @@ -97,6 +99,7 @@ user_muc_not_configured_tests() -> admin_muc_tests() -> [admin_create_and_delete_room, + admin_create_room_with_unprepped_domain, admin_try_create_instant_room_with_nonexistent_domain, admin_try_create_instant_room_with_nonexistent_user, admin_try_delete_nonexistent_room, @@ -151,6 +154,7 @@ admin_muc_not_configured_tests() -> domain_admin_muc_tests() -> [admin_create_and_delete_room, + admin_create_room_with_unprepped_domain, admin_try_create_instant_room_with_nonexistent_domain, admin_try_delete_nonexistent_room, domain_admin_create_and_delete_room_no_permission, @@ -287,8 +291,10 @@ admin_list_rooms_story(Config, Alice, Bob) -> BobRoom = rand_name(), muc_helper:create_instant_room(AliceRoom, AliceJID, <<"Ali">>, []), muc_helper:create_instant_room(BobRoom, BobJID, <<"Bob">>, [{public_list, false}]), - Res = list_rooms(muc_helper:muc_host(), Alice, null, null, Config), - #{<<"rooms">> := Rooms } = get_ok_value(?LIST_ROOMS_PATH, Res), + Res1 = list_rooms(muc_helper:muc_host(), Alice, null, null, Config), + #{<<"rooms">> := Rooms } = get_ok_value(?LIST_ROOMS_PATH, Res1), + Res2 = list_rooms(unprep(muc_helper:muc_host()), Alice, null, null, Config), + #{<<"rooms">> := Rooms } = get_ok_value(?LIST_ROOMS_PATH, Res2), ?assert(contain_room(AliceRoom, Rooms)), ?assert(contain_room(BobRoom, Rooms)). @@ -312,6 +318,15 @@ admin_create_and_delete_room_story(Config, Alice) -> Res4 = list_rooms(MUCServer, Alice, null, null, Config), ?assertNot(contain_room(Name, get_ok_value(?LIST_ROOMS_PATH, Res4))). +admin_create_room_with_unprepped_domain(Config) -> + FreshConfig = escalus_fresh:create_users(Config, [{alice, 1}]), + AliceJid = escalus_users:get_jid(FreshConfig, alice), + Name = rand_name(), + MUCServer = unprep(muc_helper:muc_host()), + Res = create_instant_room(MUCServer, Name, AliceJid, <<"Ali">>, FreshConfig), + ?assertMatch(#{<<"title">> := Name, <<"private">> := false, <<"usersNumber">> := 0}, + get_ok_value(?CREATE_INSTANT_ROOM_PATH, Res)). + admin_try_create_instant_room_with_nonexistent_domain(Config) -> escalus:fresh_story_with_config(Config, [{alice, 1}], fun admin_try_create_instant_room_with_nonexistent_domain_story/2). @@ -975,6 +990,17 @@ user_create_and_delete_room_story(Config, Alice) -> Res4 = user_list_rooms(Alice, MUCServer, null, null, Config), ?assertNot(contain_room(Name, get_ok_value(?LIST_ROOMS_PATH, Res4))). +user_create_room_with_unprepped_domain(Config) -> + escalus:fresh_story_with_config(Config, [{alice, 1}], + fun user_create_room_with_unprepped_domain_story/2). + +user_create_room_with_unprepped_domain_story(Config, Alice) -> + Name = rand_name(), + MUCServer = unprep(muc_helper:muc_host()), + Res = user_create_instant_room(Alice, MUCServer, Name, <<"Ali">>, Config), + ?assertMatch(#{<<"title">> := Name, <<"private">> := false, <<"usersNumber">> := 0}, + get_ok_value(?CREATE_INSTANT_ROOM_PATH, Res)). + user_try_create_instant_room_with_nonexistent_domain(Config) -> escalus:fresh_story_with_config(Config, [{alice, 1}], fun user_try_create_instant_room_with_nonexistent_domain_story/2). diff --git a/big_tests/tests/graphql_muc_light_SUITE.erl b/big_tests/tests/graphql_muc_light_SUITE.erl index 6b98374ea04..7e09b0517da 100644 --- a/big_tests/tests/graphql_muc_light_SUITE.erl +++ b/big_tests/tests/graphql_muc_light_SUITE.erl @@ -2,6 +2,7 @@ -compile([export_all, nowarn_export_all]). +-import(common_helper, [unprep/1]). -import(distributed_helper, [mim/0, require_rpc_nodes/1, rpc/4]). -import(graphql_helper, [execute_user_command/5, execute_command/4, get_listener_port/1, get_listener_config/1, get_ok_value/2, get_err_msg/1, @@ -48,7 +49,7 @@ groups() -> {admin_cli, [], admin_groups()}, {user_muc_light, [parallel], user_muc_light_tests()}, {user_muc_light_not_configured, [], user_muc_light_not_configured_tests()}, - {admin_muc_light, [parallel], admin_muc_light_tests()}, + {admin_muc_light, [], admin_muc_light_tests()}, {domain_admin_muc_light, [], domain_admin_muc_light_tests()}, {admin_muc_light_not_configured, [], admin_muc_light_not_configured_tests()}]. @@ -62,6 +63,7 @@ admin_groups() -> user_muc_light_tests() -> [user_create_room, + user_create_room_with_unprepped_domain, user_create_room_with_custom_fields, user_create_identified_room, user_change_room_config, @@ -94,6 +96,7 @@ user_muc_light_not_configured_tests() -> admin_muc_light_tests() -> [admin_create_room, + admin_create_room_with_unprepped_domain, admin_create_room_with_custom_fields, admin_create_identified_room, admin_change_room_config, @@ -122,6 +125,7 @@ admin_muc_light_tests() -> domain_admin_muc_light_tests() -> [admin_create_room, + admin_create_room_with_unprepped_domain, admin_create_room_with_custom_fields, domain_admin_create_room_no_permission, admin_create_identified_room, @@ -258,6 +262,19 @@ user_create_room_story(Config, Alice) -> Res2 = user_create_room(Alice, ?UNKNOWN_DOMAIN, Name, Subject, null, Config), ?assertNotEqual(nomatch, binary:match(get_err_msg(Res2), <<"not found">>)). +user_create_room_with_unprepped_domain(Config) -> + escalus:fresh_story_with_config(Config, [{alice, 1}], + fun user_create_room_with_unprepped_domain_story/2). + +user_create_room_with_unprepped_domain_story(Config, Alice) -> + MucServer = ?config(muc_light_host, Config), + Name = <<"room with unprepped domain">>, + Subject = <<"testing">>, + Res = user_create_room(Alice, unprep(MucServer), Name, Subject, null, Config), + #{<<"jid">> := JID, <<"name">> := Name, <<"subject">> := Subject} = + get_ok_value(?CREATE_ROOM_PATH, Res), + ?assertMatch(#jid{lserver = MucServer}, jid:from_binary_noprep(JID)). + user_create_room_with_custom_fields(Config) -> escalus:fresh_story_with_config(Config, [{alice, 1}], fun user_create_room_with_custom_fields_story/2). @@ -931,6 +948,17 @@ admin_create_room_story(Config, Alice) -> Res2 = create_room(?UNKNOWN_DOMAIN, Name, AliceBin, Subject, null, Config), ?assertNotEqual(nomatch, binary:match(get_err_msg(Res2), <<"not found">>)). +admin_create_room_with_unprepped_domain(Config) -> + FreshConfig = escalus_fresh:create_users(Config, [{alice, 1}]), + AliceBin = escalus_users:get_jid(FreshConfig, alice), + MucServer = ?config(muc_light_host, Config), + Name = <<"room with unprepped domain">>, + Subject = <<"testing">>, + Res = create_room(unprep(MucServer), Name, AliceBin, Subject, null, Config), + #{<<"jid">> := JID, <<"name">> := Name, <<"subject">> := Subject} = + get_ok_value(?CREATE_ROOM_PATH, Res), + ?assertMatch(#jid{lserver = MucServer}, jid:from_binary_noprep(JID)). + admin_create_room_with_custom_fields(Config) -> escalus:fresh_story_with_config(Config, [{alice, 1}], fun admin_create_room_with_custom_fields_story/2). diff --git a/big_tests/tests/graphql_offline_SUITE.erl b/big_tests/tests/graphql_offline_SUITE.erl index def5d6253b0..a5379cb1dfd 100644 --- a/big_tests/tests/graphql_offline_SUITE.erl +++ b/big_tests/tests/graphql_offline_SUITE.erl @@ -2,6 +2,7 @@ -compile([export_all, nowarn_export_all]). +-import(common_helper, [unprep/1]). -import(distributed_helper, [mim/0, require_rpc_nodes/1]). -import(domain_helper, [host_type/0, domain/0]). -import(graphql_helper, [execute_command/4, get_ok_value/2, get_err_code/1, user_to_bin/1, @@ -129,9 +130,13 @@ admin_delete_expired_messages2_test(Config) -> fun admin_delete_expired_messages2_test/3). admin_delete_expired_messages2_test(Config, JidMike, JidKate) -> + admin_delete_expired_messages2(Config, JidMike, JidKate, domain()), + admin_delete_expired_messages2(Config, JidMike, JidKate, unprep(domain())). + +admin_delete_expired_messages2(Config, JidMike, JidKate, Domain) -> generate_message(JidMike, JidKate, 2, 1), generate_message(JidMike, JidKate, 5, -1), % not expired yet - Result = delete_expired_messages(domain(), Config), + Result = delete_expired_messages(Domain, Config), ParsedResult = get_ok_value([data, offline, deleteExpiredMessages], Result), ?assertEqual(<<"Removed 1 messages">>, ParsedResult). @@ -140,10 +145,14 @@ admin_delete_old_messages2_test(Config) -> fun admin_delete_old_messages2_test/3). admin_delete_old_messages2_test(Config, JidMike, JidKate) -> + admin_delete_old_messages2(Config, JidMike, JidKate, domain()), + admin_delete_old_messages2(Config, JidMike, JidKate, unprep(domain())). + +admin_delete_old_messages2(Config, JidMike, JidKate, Domain) -> generate_message(JidMike, JidKate, 2, 1), % not old enough generate_message(JidMike, JidKate, 5, -1), generate_message(JidMike, JidKate, 7, 5), - Result = delete_old_messages(domain(), 3, Config), + Result = delete_old_messages(Domain, 3, Config), ParsedResult = get_ok_value([data, offline, deleteOldMessages], Result), ?assertEqual(<<"Removed 2 messages">>, ParsedResult). @@ -156,12 +165,12 @@ admin_delete_old_messages_no_domain_test(Config) -> ?assertEqual(<<"domain_not_found">>, get_err_code(Result)). admin_delete_expired_messages_offline_not_configured_test(Config) -> - Result = delete_expired_messages(domain(), Config), - get_not_loaded(Result). + get_not_loaded(delete_expired_messages(domain(), Config)), + get_not_loaded(delete_expired_messages(unprep(domain()), Config)). admin_delete_old_messages_offline_not_configured_test(Config) -> - Result = delete_old_messages(domain(), 2, Config), - get_not_loaded(Result). + get_not_loaded(delete_old_messages(domain(), 2, Config)), + get_not_loaded(delete_old_messages(unprep(domain()), 2, Config)). %% Domain admin test cases From 41609300af8fae96534b368f282446b17f6ec09e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chrz=C4=85szcz?= Date: Tue, 15 Nov 2022 13:39:33 +0100 Subject: [PATCH 3/6] Use DomainType to stringprep the domain --- priv/graphql/schemas/admin/account.gql | 6 +++--- priv/graphql/schemas/admin/admin_auth_status.gql | 2 +- priv/graphql/schemas/admin/gdpr.gql | 2 +- priv/graphql/schemas/admin/http_upload.gql | 2 +- priv/graphql/schemas/admin/inbox.gql | 2 +- priv/graphql/schemas/admin/last.gql | 6 +++--- priv/graphql/schemas/admin/muc.gql | 4 ++-- priv/graphql/schemas/admin/muc_light.gql | 2 +- priv/graphql/schemas/admin/offline.gql | 4 ++-- priv/graphql/schemas/user/muc.gql | 4 ++-- priv/graphql/schemas/user/muc_light.gql | 2 +- 11 files changed, 18 insertions(+), 18 deletions(-) diff --git a/priv/graphql/schemas/admin/account.gql b/priv/graphql/schemas/admin/account.gql index 43a8715f328..e7f83a3481c 100644 --- a/priv/graphql/schemas/admin/account.gql +++ b/priv/graphql/schemas/admin/account.gql @@ -3,10 +3,10 @@ Allow admin to get information about accounts. """ type AccountAdminQuery @protected{ "List users per domain" - listUsers(domain: String!): [String!] + listUsers(domain: DomainName!): [String!] @protected(type: DOMAIN, args: ["domain"]) "Get number of users per domain" - countUsers(domain: String!): Int + countUsers(domain: DomainName!): Int @protected(type: DOMAIN, args: ["domain"]) "Check if a password is correct" checkPassword(user: JID!, password: String!): CheckPasswordPayload @@ -24,7 +24,7 @@ Allow admin to manage user accounts. """ type AccountAdminMutation @protected{ "Register a user. Username will be generated when skipped" - registerUser(domain: String!, username: String, password: String!): UserPayload + registerUser(domain: DomainName!, username: String, password: String!): UserPayload @protected(type: DOMAIN, args: ["domain"]) "Remove the user's account along with all the associated personal data" removeUser(user: JID!): UserPayload diff --git a/priv/graphql/schemas/admin/admin_auth_status.gql b/priv/graphql/schemas/admin/admin_auth_status.gql index 799ab3aa0a6..e69e6308511 100644 --- a/priv/graphql/schemas/admin/admin_auth_status.gql +++ b/priv/graphql/schemas/admin/admin_auth_status.gql @@ -1,7 +1,7 @@ "Information about user request authorization" type AdminAuthInfo{ "Authorized for a domain" - domain: String + domain: DomainName "Authorization status" authStatus: AuthStatus! "Authorization as a " diff --git a/priv/graphql/schemas/admin/gdpr.gql b/priv/graphql/schemas/admin/gdpr.gql index eef5b5a317e..829b4f35f34 100644 --- a/priv/graphql/schemas/admin/gdpr.gql +++ b/priv/graphql/schemas/admin/gdpr.gql @@ -1,6 +1,6 @@ "Retrieve user's presonal data" type GdprAdminQuery @protected{ "Retrieves all personal data from MongooseIM for a given user" - retrievePersonalData(username: String!, domain: String!, resultFilepath: String!): String + retrievePersonalData(username: String!, domain: DomainName!, resultFilepath: String!): String @protected(type: DOMAIN, args: ["domain"]) } diff --git a/priv/graphql/schemas/admin/http_upload.gql b/priv/graphql/schemas/admin/http_upload.gql index fccffa1dca5..22b38c1f79b 100644 --- a/priv/graphql/schemas/admin/http_upload.gql +++ b/priv/graphql/schemas/admin/http_upload.gql @@ -3,6 +3,6 @@ Allow admin to generate upload/download URL for a file on user's behalf". """ type HttpUploadAdminMutation @use(modules: ["mod_http_upload"]) @protected{ "Allow admin to generate upload/download URLs for a file on user's behalf" - getUrl(domain: String!, filename: String!, size: Int!, contentType: String!, timeout: Int!): FileUrls + getUrl(domain: DomainName!, filename: String!, size: Int!, contentType: String!, timeout: Int!): FileUrls @use(arg: "domain") @protected(type: DOMAIN, args: ["domain"]) } diff --git a/priv/graphql/schemas/admin/inbox.gql b/priv/graphql/schemas/admin/inbox.gql index d34096ab23e..2517cf3834d 100644 --- a/priv/graphql/schemas/admin/inbox.gql +++ b/priv/graphql/schemas/admin/inbox.gql @@ -13,7 +13,7 @@ type InboxAdminMutation @use(modules: ["mod_inbox"]) @protected{ "Flush the whole domain bin and return the number of deleted rows" flushDomainBin( "Domain to be cleared" - domain: String!, + domain: DomainName!, "Remove older than given days or all if null" days: PosInt ): Int @use(arg: "domain") @protected(type: DOMAIN, args: ["domain"]) diff --git a/priv/graphql/schemas/admin/last.gql b/priv/graphql/schemas/admin/last.gql index ac003c17093..563c1868aa5 100644 --- a/priv/graphql/schemas/admin/last.gql +++ b/priv/graphql/schemas/admin/last.gql @@ -6,13 +6,13 @@ type LastAdminQuery @use(modules: ["mod_last"]) @protected{ getLast(user: JID!): LastActivity @use(arg: "user") @protected(type: DOMAIN, args: ["user"]) "Get the number of users active from the given timestamp" - countActiveUsers(domain: String!, timestamp: DateTime): Int + countActiveUsers(domain: DomainName!, timestamp: DateTime): Int @use(arg: "domain") @protected(type: DOMAIN, args: ["domain"]) """ List users that didn't log in the last days or have never logged in. Globally or for a specified domain """ - listOldUsers(domain: String, timestamp: DateTime!): [OldUser!] + listOldUsers(domain: DomainName, timestamp: DateTime!): [OldUser!] @use(arg: "domain") @protected(type: DOMAIN, args: ["domain"]) } @@ -27,7 +27,7 @@ type LastAdminMutation @use(modules: ["mod_last"]) @protected{ Delete users that didn't log in the last days or have never logged in. Globally or for a specified domain. Please use listOldUsers to check which users will be deleted """ - removeOldUsers(domain: String, timestamp: DateTime!): [OldUser!] + removeOldUsers(domain: DomainName, timestamp: DateTime!): [OldUser!] @use(arg: "domain") @protected(type: DOMAIN, args: ["domain"]) } diff --git a/priv/graphql/schemas/admin/muc.gql b/priv/graphql/schemas/admin/muc.gql index 78e788e6525..2679e5f686e 100644 --- a/priv/graphql/schemas/admin/muc.gql +++ b/priv/graphql/schemas/admin/muc.gql @@ -4,7 +4,7 @@ Allow admin to manage Multi-User Chat rooms. type MUCAdminMutation @protected @use(modules: ["mod_muc"]){ "Create a MUC room under the given XMPP hostname" #There is no @use directive because it is currently impossible to get HostType from mucDomain in directive code - createInstantRoom(mucDomain: String!, name: String!, owner: JID!, nick: String!): MUCRoomDesc + createInstantRoom(mucDomain: DomainName!, name: String!, owner: JID!, nick: String!): MUCRoomDesc @protected(type: DOMAIN, args: ["owner"]) "Invite a user to a MUC room" inviteUser(room: JID!, sender: JID!, recipient: JID!, reason: String): String @@ -44,7 +44,7 @@ Allow admin to get information about Multi-User Chat rooms. type MUCAdminQuery @protected @use(modules: ["mod_muc"]){ "Get MUC rooms under the given MUC domain" #There is no @use directive because it is currently impossible to get HostType from mucDomain in directive code - listRooms(mucDomain: String!, from: JID!, limit: Int, index: Int): MUCRoomsPayload! + listRooms(mucDomain: DomainName!, from: JID!, limit: Int, index: Int): MUCRoomsPayload! @protected(type: DOMAIN, args: ["from"]) "Get configuration of the MUC room" getRoomConfig(room: JID!): MUCRoomConfig diff --git a/priv/graphql/schemas/admin/muc_light.gql b/priv/graphql/schemas/admin/muc_light.gql index ae148a6e630..4f6a81adc80 100644 --- a/priv/graphql/schemas/admin/muc_light.gql +++ b/priv/graphql/schemas/admin/muc_light.gql @@ -4,7 +4,7 @@ Allow admin to manage Multi-User Chat Light rooms. type MUCLightAdminMutation @use(modules: ["mod_muc_light"]) @protected{ "Create a MUC light room under the given XMPP hostname" #There is no @use directive because it is currently impossible to get HostType from mucDomain in directive code - createRoom(mucDomain: String!, name: String!, owner: JID!, subject: String!, id: NonEmptyString, options: [RoomConfigDictEntryInput!]): Room + createRoom(mucDomain: DomainName!, name: String!, owner: JID!, subject: String!, id: NonEmptyString, options: [RoomConfigDictEntryInput!]): Room @protected(type: DOMAIN, args: ["owner"]) "Change configuration of a MUC Light room" changeRoomConfiguration(room: JID!, owner: JID!, name: String!, subject: String!, options: [RoomConfigDictEntryInput!]): Room diff --git a/priv/graphql/schemas/admin/offline.gql b/priv/graphql/schemas/admin/offline.gql index ca212fd5f28..a21fd1b49d7 100644 --- a/priv/graphql/schemas/admin/offline.gql +++ b/priv/graphql/schemas/admin/offline.gql @@ -3,9 +3,9 @@ Allow admin to delete offline messages from specified domain """ type OfflineAdminMutation @protected @use(modules: ["mod_offline"]){ "Delete offline messages whose date has expired" - deleteExpiredMessages(domain: String!): String @use(arg: "domain") + deleteExpiredMessages(domain: DomainName!): String @use(arg: "domain") @protected(type: DOMAIN, args: ["domain"]) "Delete messages at least as old as the number of days specified in the parameter" - deleteOldMessages(domain: String!, days: Int!): String + deleteOldMessages(domain: DomainName!, days: Int!): String @protected(type: DOMAIN, args: ["domain"]) @use(arg: "domain") } diff --git a/priv/graphql/schemas/user/muc.gql b/priv/graphql/schemas/user/muc.gql index 1430afbe9cf..d6dd3732c4c 100644 --- a/priv/graphql/schemas/user/muc.gql +++ b/priv/graphql/schemas/user/muc.gql @@ -4,7 +4,7 @@ Allow user to manage Multi-User Chat rooms. type MUCUserMutation @protected @use(modules: ["mod_muc"]){ "Create a MUC room under the given XMPP hostname" #There is no @use directive because it is currently impossible to get HostType from mucDomain in directive code - createInstantRoom(mucDomain: String!, name: String!, nick: String!): MUCRoomDesc + createInstantRoom(mucDomain: DomainName!, name: String!, nick: String!): MUCRoomDesc "Invite a user to a MUC room" inviteUser(room: JID!, recipient: JID!, reason: String): String @use(arg: "room") "Kick a user from a MUC room" @@ -33,7 +33,7 @@ Allow user to get information about Multi-User Chat rooms. type MUCUserQuery @protected @use(modules: ["mod_muc"]){ "Get MUC rooms under the given MUC domain" #There is no @use directive because it is currently impossible to get HostType from mucDomain in directive code - listRooms(mucDomain: String!, limit: Int, index: Int): MUCRoomsPayload! + listRooms(mucDomain: DomainName!, limit: Int, index: Int): MUCRoomsPayload! "Get configuration of the MUC room" getRoomConfig(room: JID!): MUCRoomConfig @use(arg: "room") "Get the user list of a given MUC room" diff --git a/priv/graphql/schemas/user/muc_light.gql b/priv/graphql/schemas/user/muc_light.gql index 3263fa692fb..34ae3495b14 100644 --- a/priv/graphql/schemas/user/muc_light.gql +++ b/priv/graphql/schemas/user/muc_light.gql @@ -4,7 +4,7 @@ Allow user to manage Multi-User Chat Light rooms. type MUCLightUserMutation @protected @use(modules: ["mod_muc_light"]){ "Create a MUC light room under the given XMPP hostname" #There is no @use directive because it is currently impossible to get HostType from mucDomain in directive code - createRoom(mucDomain: String!, name: String!, subject: String!, id: NonEmptyString, options: [RoomConfigDictEntryInput!]): Room + createRoom(mucDomain: DomainName!, name: String!, subject: String!, id: NonEmptyString, options: [RoomConfigDictEntryInput!]): Room "Change configuration of a MUC Light room" changeRoomConfiguration(room: JID!, name: String!, subject: String!, options: [RoomConfigDictEntryInput!]): Room @use(arg: "room") "Invite a user to a MUC Light room" From a4577fbf21491eb0e4040db8e74df699144dcabc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chrz=C4=85szcz?= Date: Wed, 16 Nov 2022 15:25:39 +0100 Subject: [PATCH 4/6] Update the expected printout in tests --- big_tests/tests/mongooseimctl_SUITE.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/big_tests/tests/mongooseimctl_SUITE.erl b/big_tests/tests/mongooseimctl_SUITE.erl index 72a27be9c29..ed57643e9f2 100644 --- a/big_tests/tests/mongooseimctl_SUITE.erl +++ b/big_tests/tests/mongooseimctl_SUITE.erl @@ -1227,7 +1227,7 @@ expect_existing_commands(Res) -> ?assertMatch({match, _}, re:run(Res, "countUsers")). expect_command_arguments(Res) -> - ?assertMatch({match, _}, re:run(Res, "domain\s+String!")). + ?assertMatch({match, _}, re:run(Res, "domain\s+DomainName!")). %%----------------------------------------------------------------- %% Help tests From 2c8ac74ee73749ea22a67718390f6c0c57d5a79e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chrz=C4=85szcz?= Date: Thu, 17 Nov 2022 18:40:38 +0100 Subject: [PATCH 5/6] Add tests for unprepped domains in session and stat commands Also: add a test verifying that the stats are counted only for the given domain, and not globally. --- big_tests/tests/graphql_session_SUITE.erl | 70 +++++++++++------------ big_tests/tests/graphql_stats_SUITE.erl | 44 ++++++++++---- 2 files changed, 66 insertions(+), 48 deletions(-) diff --git a/big_tests/tests/graphql_session_SUITE.erl b/big_tests/tests/graphql_session_SUITE.erl index f7ff33905e7..d67d4af521e 100644 --- a/big_tests/tests/graphql_session_SUITE.erl +++ b/big_tests/tests/graphql_session_SUITE.erl @@ -4,7 +4,9 @@ -compile([export_all, nowarn_export_all]). +-import(common_helper, [unprep/1]). -import(distributed_helper, [mim/0, require_rpc_nodes/1, rpc/4]). +-import(domain_helper, [domain/0]). -import(graphql_helper, [execute_user_command/5, execute_command/4, get_listener_port/1, get_listener_config/1, get_ok_value/2, get_err_msg/1, get_unauthorized/1]). @@ -224,7 +226,6 @@ domain_admin_list_users_with_status(Config) -> domain_admin_list_users_with_status_story(Config, Alice, _AliceB) -> AliceJID = escalus_client:full_jid(Alice), - Path = [data, session, listUsersWithStatus], AwayStatus = <<"away">>, AwayPresence = escalus_stanza:presence_show(AwayStatus), DndStatus = <<"dnd">>, @@ -234,10 +235,8 @@ domain_admin_list_users_with_status_story(Config, Alice, _AliceB) -> Res = list_users_with_status(null, AwayStatus, Config), get_unauthorized(Res), % List users with away status for a domain - Res2 = list_users_with_status(domain_helper:domain(), AwayStatus, Config), - StatusUsers = get_ok_value(Path, Res2), - ?assertEqual(1, length(StatusUsers)), - check_users([AliceJID], StatusUsers), + assert_list_users_with_status([AliceJID], domain(), AwayStatus, Config), + assert_list_users_with_status([AliceJID], unprep(domain()), AwayStatus, Config), % List users with away status for an external domain Res3 = list_users_with_status(domain_helper:secondary_domain(), AwayStatus, Config), get_unauthorized(Res3), @@ -246,10 +245,7 @@ domain_admin_list_users_with_status_story(Config, Alice, _AliceB) -> Res4 = list_users_with_status(null, DndStatus, Config), get_unauthorized(Res4), % List users with dnd status for a domain - Res5 = list_users_with_status(domain_helper:domain(), DndStatus, Config), - StatusUsers2 = get_ok_value(Path, Res5), - ?assertEqual(1, length(StatusUsers2)), - check_users([AliceJID], StatusUsers2), + assert_list_users_with_status([AliceJID], domain(), DndStatus, Config), % List users with dnd status for an external domain Res6 = list_users_with_status(domain_helper:secondary_domain(), AwayStatus, Config), get_unauthorized(Res6). @@ -259,7 +255,6 @@ domain_admin_count_users_with_status(Config) -> fun domain_admin_count_users_with_status_story/3). domain_admin_count_users_with_status_story(Config, Alice, _AliceB) -> - Path = [data, session, countUsersWithStatus], AwayStatus = <<"away">>, AwayPresence = escalus_stanza:presence_show(AwayStatus), DndStatus = <<"dnd">>, @@ -269,15 +264,14 @@ domain_admin_count_users_with_status_story(Config, Alice, _AliceB) -> Res = count_users_with_status(null, AwayStatus, Config), get_unauthorized(Res), % Count users with away status for a domain - Res2 = count_users_with_status(domain_helper:domain(), AwayStatus, Config), - ?assertEqual(1, get_ok_value(Path, Res2)), + assert_count_users_with_status(1, domain_helper:domain(), AwayStatus, Config), + assert_count_users_with_status(1, unprep(domain_helper:domain()), AwayStatus, Config), % Count users with dnd status globally escalus_client:send(Alice, DndPresence), Res3 = count_users_with_status(null, DndStatus, Config), get_unauthorized(Res3), % Count users with dnd status for a domain - Res4 = count_users_with_status(domain_helper:domain(), DndStatus, Config), - ?assertEqual(1, get_ok_value(Path, Res4)). + assert_count_users_with_status(1, domain_helper:domain(), DndStatus, Config). admin_list_sessions(Config) -> escalus:fresh_story_with_config(Config, [{alice, 1}, {alice_bis, 1}, {bob, 1}], @@ -293,7 +287,10 @@ admin_list_sessions_story(Config, _Alice, AliceB, _Bob) -> % List sessions for a domain Res2 = list_sessions(BisDomain, Config), Sessions2 = get_ok_value(Path, Res2), - ?assertEqual(1, length(Sessions2)). + ?assertEqual(1, length(Sessions2)), + Res3 = list_sessions(unprep(BisDomain), Config), + Sessions3 = get_ok_value(Path, Res3), + ?assertEqual(1, length(Sessions3)). admin_count_sessions(Config) -> escalus:fresh_story_with_config(Config, [{alice, 1}, {alice_bis, 1}, {bob, 1}], @@ -308,8 +305,9 @@ admin_count_sessions_story(Config, _Alice, AliceB, _Bob) -> ?assertEqual(3, Number), % Count sessions for a domain Res2 = count_sessions(BisDomain, Config), - Number2 = get_ok_value(Path, Res2), - ?assertEqual(1, Number2). + ?assertEqual(1, get_ok_value(Path, Res2)), + Res3 = count_sessions(unprep(BisDomain), Config), + ?assertEqual(1, get_ok_value(Path, Res3)). admin_list_user_sessions(Config) -> escalus:fresh_story_with_config(Config, [{alice, 2}, {bob, 1}], @@ -352,7 +350,6 @@ admin_count_users_with_status(Config) -> fun admin_count_users_with_status_story/3). admin_count_users_with_status_story(Config, Alice, AliceB) -> - Path = [data, session, countUsersWithStatus], AwayStatus = <<"away">>, AwayPresence = escalus_stanza:presence_show(AwayStatus), DndStatus = <<"dnd">>, @@ -360,15 +357,13 @@ admin_count_users_with_status_story(Config, Alice, AliceB) -> % Count users with away status globally escalus_client:send(Alice, AwayPresence), escalus_client:send(AliceB, AwayPresence), - Res = count_users_with_status(null, AwayStatus, Config), - ?assertEqual(2, get_ok_value(Path, Res)), + assert_count_users_with_status(2, null, AwayStatus, Config), % Count users with away status for a domain - Res2 = count_users_with_status(domain_helper:domain(), AwayStatus, Config), - ?assertEqual(1, get_ok_value(Path, Res2)), + assert_count_users_with_status(1, domain_helper:domain(), AwayStatus, Config), + assert_count_users_with_status(1, unprep(domain_helper:domain()), AwayStatus, Config), % Count users with dnd status globally escalus_client:send(AliceB, DndPresence), - Res3 = count_users_with_status(null, DndStatus, Config), - ?assertEqual(1, get_ok_value(Path, Res3)). + assert_count_users_with_status(1, null, DndStatus, Config). admin_list_users_with_status(Config) -> escalus:fresh_story_with_config(Config, [{alice, 1}, {alice_bis, 1}], @@ -377,7 +372,6 @@ admin_list_users_with_status(Config) -> admin_list_users_with_status_story(Config, Alice, AliceB) -> AliceJID = escalus_client:full_jid(Alice), AliceBJID = escalus_client:full_jid(AliceB), - Path = [data, session, listUsersWithStatus], AwayStatus = <<"away">>, AwayPresence = escalus_stanza:presence_show(AwayStatus), DndStatus = <<"dnd">>, @@ -385,21 +379,13 @@ admin_list_users_with_status_story(Config, Alice, AliceB) -> % List users with away status globally escalus_client:send(Alice, AwayPresence), escalus_client:send(AliceB, AwayPresence), - Res = list_users_with_status(null, AwayStatus, Config), - StatusUsers = get_ok_value(Path, Res), - ?assertEqual(2, length(StatusUsers)), - check_users([AliceJID, AliceBJID], StatusUsers), + assert_list_users_with_status([AliceJID, AliceBJID], null, AwayStatus, Config), % List users with away status for a domain - Res2 = list_users_with_status(domain_helper:domain(), AwayStatus, Config), - StatusUsers2 = get_ok_value(Path, Res2), - ?assertEqual(1, length(StatusUsers2)), - check_users([AliceJID], StatusUsers2), + assert_list_users_with_status([AliceJID], domain(), AwayStatus, Config), + assert_list_users_with_status([AliceJID], unprep(domain()), AwayStatus, Config), % List users with dnd status globally escalus_client:send(AliceB, DndPresence), - Res3 = list_users_with_status(null, DndStatus, Config), - StatusUsers3 = get_ok_value(Path, Res3), - ?assertEqual(1, length(StatusUsers3)), - check_users([AliceBJID], StatusUsers3). + assert_list_users_with_status([AliceBJID], null, DndStatus, Config). admin_kick_session(Config) -> escalus:fresh_story_with_config(Config, [{alice, 2}], fun admin_kick_session_story/3). @@ -525,6 +511,16 @@ set_presence(JID, Type, Show, Status, Priority, Config) -> %% Helpers +assert_list_users_with_status(ExpectedUsers, Domain, Status, Config) -> + Res = list_users_with_status(Domain, Status, Config), + Users = get_ok_value([data, session, listUsersWithStatus], Res), + check_users(ExpectedUsers, Users). + +assert_count_users_with_status(ExpectedCount, Domain, Status, Config) -> + Res = count_users_with_status(Domain, Status, Config), + Count = get_ok_value([data, session, countUsersWithStatus], Res), + ?assertEqual(ExpectedCount, Count). + -spec check_users([jid:literal_jid()], [#{user := jid:literal_jid()}]) -> boolean(). check_users(Expected, ActualUsers) -> ActualJIDs = [JID || #{<<"user">> := JID} <- ActualUsers], diff --git a/big_tests/tests/graphql_stats_SUITE.erl b/big_tests/tests/graphql_stats_SUITE.erl index 9d17f95335d..36f23a0cf86 100644 --- a/big_tests/tests/graphql_stats_SUITE.erl +++ b/big_tests/tests/graphql_stats_SUITE.erl @@ -2,11 +2,13 @@ -compile([export_all, nowarn_export_all]). +-import(common_helper, [unprep/1]). -import(distributed_helper, [mim/0, require_rpc_nodes/1]). -import(domain_helper, [host_type/0, domain/0, secondary_domain/0]). -import(graphql_helper, [execute_command/4, get_ok_value/2, get_unauthorized/1]). -import(mongooseimctl_helper, [mongooseimctl/3, rpc_call/3]). +-include_lib("common_test/include/ct.hrl"). -include_lib("eunit/include/eunit.hrl"). suite() -> @@ -26,16 +28,19 @@ admin_stats_tests() -> [admin_stats_global_test, admin_stats_global_with_users_test, admin_stats_domain_test, - admin_stats_domain_with_users_test]. + admin_stats_domain_with_users_test, + admin_stats_domain_without_users_test]. domain_admin_tests() -> [domain_admin_stats_global_test, admin_stats_domain_test, + admin_stats_domain_with_users_test, domain_admin_stats_domain_no_permission_test]. init_per_suite(Config) -> Config1 = ejabberd_node_utils:init(mim(), Config), - escalus:init_per_suite(Config1). + Config2 = [{auth_mods, mongoose_helper:auth_modules()} | Config1], + escalus:init_per_suite(Config2). end_per_suite(Config) -> escalus:end_per_suite(Config). @@ -51,6 +56,13 @@ end_per_group(_, _Config) -> graphql_helper:clean(), escalus_fresh:clean(). +init_per_testcase(CaseName = admin_stats_domain_without_users_test, Config) -> + case lists:member(ejabberd_auth_ldap, ?config(auth_mods, Config)) of + true -> + {skip, not_supported_with_ldap}; + false -> + escalus:init_per_testcase(CaseName, Config) + end; init_per_testcase(CaseName, Config) -> escalus:init_per_testcase(CaseName, Config). @@ -88,19 +100,28 @@ admin_stats_global_with_users_test(Config, _Alice) -> ?assert(is_not_negative_integer(OutgoingS2S)). admin_stats_domain_test(Config) -> - Result = get_ok_value([data, stat, domainStats], get_domain_stats(domain(), Config)), - #{<<"registeredUsers">> := RegisteredUsers, <<"onlineUsers">> := OnlineUsers} = Result, - ?assertEqual(0, RegisteredUsers), - ?assertEqual(0, OnlineUsers). + Result1 = get_ok_value([data, stat, domainStats], get_domain_stats(domain(), Config)), + ?assertMatch(#{<<"registeredUsers">> := 0, <<"onlineUsers">> := 0}, Result1), + Result2 = get_ok_value([data, stat, domainStats], get_domain_stats(unprep(domain()), Config)), + ?assertMatch(#{<<"registeredUsers">> := 0, <<"onlineUsers">> := 0}, Result2). admin_stats_domain_with_users_test(Config) -> escalus:fresh_story_with_config(Config, [{alice, 1}], fun admin_stats_domain_with_users_test/2). admin_stats_domain_with_users_test(Config, _Alice) -> - Result = get_ok_value([data, stat, domainStats], get_domain_stats(domain(), Config)), - #{<<"registeredUsers">> := RegisteredUsers, <<"onlineUsers">> := OnlineUsers} = Result, - ?assertEqual(1, RegisteredUsers), - ?assertEqual(1, OnlineUsers). + Result1 = get_ok_value([data, stat, domainStats], get_domain_stats(domain(), Config)), + ?assertMatch(#{<<"registeredUsers">> := 1, <<"onlineUsers">> := 1}, Result1), + Result2 = get_ok_value([data, stat, domainStats], get_domain_stats(unprep(domain()), Config)), + ?assertMatch(#{<<"registeredUsers">> := 1, <<"onlineUsers">> := 1}, Result2). + +admin_stats_domain_without_users_test(Config) -> + escalus:fresh_story_with_config(Config, [{alice, 1}], + fun admin_stats_domain_without_users_test/2). + +admin_stats_domain_without_users_test(Config, _Alice) -> + %% Alice's session is at domain, not secondary_domain + Result = get_ok_value([data, stat, domainStats], get_domain_stats(secondary_domain(), Config)), + ?assertMatch(#{<<"registeredUsers">> := 0, <<"onlineUsers">> := 0}, Result). % Domain admin test cases @@ -108,7 +129,8 @@ domain_admin_stats_global_test(Config) -> get_unauthorized(get_stats(Config)). domain_admin_stats_domain_no_permission_test(Config) -> - get_unauthorized(get_domain_stats(secondary_domain(), Config)). + get_unauthorized(get_domain_stats(secondary_domain(), Config)), + get_unauthorized(get_domain_stats(unprep(secondary_domain()), Config)). % Commands From b2007a7974a3138712c394749a88d7dce8f27145 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chrz=C4=85szcz?= Date: Thu, 17 Nov 2022 18:42:52 +0100 Subject: [PATCH 6/6] Use DomainName in the remaining GraphQL commands --- priv/graphql/schemas/admin/domain.gql | 2 +- priv/graphql/schemas/admin/session.gql | 8 ++++---- priv/graphql/schemas/admin/stats.gql | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/priv/graphql/schemas/admin/domain.gql b/priv/graphql/schemas/admin/domain.gql index 56f8b435cfd..2638ecbbb0b 100644 --- a/priv/graphql/schemas/admin/domain.gql +++ b/priv/graphql/schemas/admin/domain.gql @@ -1,6 +1,6 @@ type DomainAdminQuery @use(services: ["service_domain_db"]) @protected{ "Get all enabled domains by hostType. Only for global admin" - domainsByHostType(hostType: String!): [String!] + domainsByHostType(hostType: String!): [DomainName!] @protected(type: GLOBAL) @use "Get information about the domain" domainDetails(domain: DomainName!): Domain diff --git a/priv/graphql/schemas/admin/session.gql b/priv/graphql/schemas/admin/session.gql index 3e6724c97d5..1a528a94554 100644 --- a/priv/graphql/schemas/admin/session.gql +++ b/priv/graphql/schemas/admin/session.gql @@ -3,10 +3,10 @@ Allow admin to get information about sessions. """ type SessionAdminQuery @protected{ "Get the list of established sessions for a specified domain or globally" - listSessions(domain: String): [Session!] + listSessions(domain: DomainName): [Session!] @protected(type: DOMAIN, args: ["domain"]) "Get the number of established sessions for a specified domain or globally" - countSessions(domain: String): Int + countSessions(domain: DomainName): Int @protected(type: DOMAIN, args: ["domain"]) "Get information about all sessions of a user" listUserSessions(user: JID!): [Session!] @@ -18,10 +18,10 @@ type SessionAdminQuery @protected{ getUserResource(user: JID!, number: Int): String @protected(type: DOMAIN, args: ["user"]) "Get the list of logged users with this status for a specified domain or globally" - listUsersWithStatus(domain: String, status: String!): [UserStatus!] + listUsersWithStatus(domain: DomainName, status: String!): [UserStatus!] @protected(type: DOMAIN, args: ["domain"]) "Get the number of logged users with this status for a specified domain or globally" - countUsersWithStatus(domain: String, status: String!): Int + countUsersWithStatus(domain: DomainName, status: String!): Int @protected(type: DOMAIN, args: ["domain"]) } diff --git a/priv/graphql/schemas/admin/stats.gql b/priv/graphql/schemas/admin/stats.gql index a0dfecd78b7..24d7dd72c6a 100644 --- a/priv/graphql/schemas/admin/stats.gql +++ b/priv/graphql/schemas/admin/stats.gql @@ -4,7 +4,7 @@ type StatsAdminQuery @protected{ globalStats: GlobalStats @protected(type: GLOBAL) "Get statistics from a specific domain" - domainStats(domain: String!): DomainStats + domainStats(domain: DomainName!): DomainStats @protected(type: DOMAIN, args: ["domain"]) }