From d8186bad2456f9f89d598759ee70b1fb8ef16cf9 Mon Sep 17 00:00:00 2001 From: "jaspreet.chhabra" Date: Sun, 20 Oct 2024 20:07:02 +0530 Subject: [PATCH 01/12] multiple owner support in muc_light. --- big_tests/tests/muc_light_SUITE.erl | 17 +++++++++++++++++ src/muc_light/mod_muc_light_utils.erl | 22 +--------------------- 2 files changed, 18 insertions(+), 21 deletions(-) diff --git a/big_tests/tests/muc_light_SUITE.erl b/big_tests/tests/muc_light_SUITE.erl index 23ca53c3073..6ae7e3dbf1d 100644 --- a/big_tests/tests/muc_light_SUITE.erl +++ b/big_tests/tests/muc_light_SUITE.erl @@ -53,6 +53,7 @@ deny_config_change_that_conflicts_with_schema/1, assorted_config_doesnt_lead_to_duplication/1, remove_and_add_users/1, + multiple_owner_change/1, explicit_owner_change/1, implicit_owner_change/1, edge_case_owner_change/1, @@ -179,6 +180,7 @@ groups() -> assorted_config_doesnt_lead_to_duplication, remove_and_add_users, explicit_owner_change, + multiple_owner_change, implicit_owner_change, edge_case_owner_change, adding_wrongly_named_user_triggers_infinite_loop @@ -792,6 +794,21 @@ remove_and_add_users(Config) -> assert_cache_hit_event(TS, 1, room_bin_jid(?ROOM)) end). +multiple_owner_change(Config) -> + escalus:story(Config, [{alice, 1}, {bob, 1}, {kate, 1}], fun(Alice, Bob, Kate) -> + TS = instrument_helper:timestamp(), + AffUsersChanges1 = [{Bob, none}, {Kate, none}], + escalus:send(Alice, stanza_aff_set(?ROOM, AffUsersChanges1)), + verify_aff_bcast([{Alice, owner}], AffUsersChanges1), + escalus:assert(is_iq_result, escalus:wait_for_stanza(Alice)), + AffUsersChanges2 = [{Bob, owner}, {Kate, owner}], + escalus:send(Alice, stanza_aff_set(?ROOM, AffUsersChanges2)), + verify_aff_bcast([{Alice, owner}, {Bob, owner}, {Kate, owner}], AffUsersChanges2), + escalus:assert(is_iq_result, escalus:wait_for_stanza(Alice)), + assert_cache_miss_event(TS, 1, room_bin_jid(?ROOM)), + assert_cache_hit_event(TS, 1, room_bin_jid(?ROOM)) + end). + explicit_owner_change(Config) -> escalus:story(Config, [{alice, 1}, {bob, 1}, {kate, 1}], fun(Alice, Bob, Kate) -> AffUsersChanges1 = [{Bob, none}, {Alice, none}, {Kate, owner}], diff --git a/src/muc_light/mod_muc_light_utils.erl b/src/muc_light/mod_muc_light_utils.erl index bb99cab80de..5c6afcadd81 100644 --- a/src/muc_light/mod_muc_light_utils.erl +++ b/src/muc_light/mod_muc_light_utils.erl @@ -68,8 +68,7 @@ change_aff_users(AffUsers, AffUsersChangesAssorted) -> _ -> lists:foldl(fun(F, Acc) -> F(Acc) end, apply_aff_users_change(AffUsers, AffUsersChangesAssorted), - [fun maybe_demote_old_owner/1, - fun maybe_select_new_owner/1]) + [fun maybe_select_new_owner/1]) end. -spec aff2b(Aff :: aff()) -> binary(). @@ -208,25 +207,6 @@ select_promotion(_OldMembers, _JoiningUsers, [U | _]) -> select_promotion(_, _, _) -> false. --spec maybe_demote_old_owner(ChangeResult :: change_aff_success() | {error, bad_request()}) -> - change_aff_success() | {error, bad_request()}. -maybe_demote_old_owner({ok, AU, AUC, JoiningUsers, LeavingUsers}) -> - Owners = [U || {U, owner} <- AU], - PromotedOwners = [U || {U, owner} <- AUC], - OldOwners = Owners -- PromotedOwners, - case {Owners, OldOwners} of - _ when length(Owners) =< 1 -> - {ok, AU, AUC, JoiningUsers, LeavingUsers}; - {[_, _], [OldOwner]} -> - NewAU = lists:keyreplace(OldOwner, 1, AU, {OldOwner, member}), - NewAUC = [{OldOwner, member} | AUC], - {ok, NewAU, NewAUC, JoiningUsers, LeavingUsers}; - _ -> - {error, {bad_request, <<"Failed to demote old owner">>}} - end; -maybe_demote_old_owner(Error) -> - Error. - -spec apply_aff_users_change(AffUsers :: aff_users(), AffUsersChanges :: aff_users()) -> change_aff_success() | {error, bad_request()}. From 746165851fc4ebe3f31ac55080fc707121e45ae1 Mon Sep 17 00:00:00 2001 From: "jaspreet.chhabra" Date: Sun, 27 Oct 2024 13:57:54 +0530 Subject: [PATCH 02/12] multiple admin in muc_light is optional. --- src/config/mongoose_config_spec.erl | 1 + src/hooks/mongoose_hooks.erl | 9 ++- src/muc_light/mod_muc_light.erl | 8 +-- src/muc_light/mod_muc_light_db_mnesia.erl | 25 ++++---- src/muc_light/mod_muc_light_db_rdbms.erl | 2 +- .../mod_muc_light_multiple_admin.erl | 60 +++++++++++++++++++ src/muc_light/mod_muc_light_utils.erl | 33 ++++++++-- test/muc_light_SUITE.erl | 6 +- 8 files changed, 120 insertions(+), 24 deletions(-) create mode 100644 src/muc_light/mod_muc_light_multiple_admin.erl diff --git a/src/config/mongoose_config_spec.erl b/src/config/mongoose_config_spec.erl index 943a3635d5e..3a68a861f49 100644 --- a/src/config/mongoose_config_spec.erl +++ b/src/config/mongoose_config_spec.erl @@ -760,6 +760,7 @@ configurable_modules() -> mod_mam, mod_muc, mod_muc_light, + mod_muc_light_multiple_admin, mod_muc_log, mod_offline, mod_offline_chatmarkers, diff --git a/src/hooks/mongoose_hooks.erl b/src/hooks/mongoose_hooks.erl index 475178c77e1..11287d024fa 100644 --- a/src/hooks/mongoose_hooks.erl +++ b/src/hooks/mongoose_hooks.erl @@ -35,7 +35,8 @@ user_available/2, user_ping_response/5, vcard_set/4, - xmpp_send_element/3]). + xmpp_send_element/3, + should_allow_multiple_admin/1]). %% sasl2 handlers -export([sasl2_stream_features/2, @@ -1376,6 +1377,12 @@ mod_global_distrib_unknown_recipient(GlobalHost, Info) -> run_hook_for_host_type(mod_global_distrib_unknown_recipient, GlobalHost, Info, #{}). +-spec should_allow_multiple_admin(HostType) -> Result when + HostType :: mongooseim:host_type(), + Result :: boolean(). +should_allow_multiple_admin(HostType) -> + run_hook_for_host_type(should_allow_multiple_admin, HostType, true, #{}). + %%%---------------------------------------------------------------------- %%% Internal functions %%%---------------------------------------------------------------------- diff --git a/src/muc_light/mod_muc_light.erl b/src/muc_light/mod_muc_light.erl index 2ecd39db2f4..7037080c541 100644 --- a/src/muc_light/mod_muc_light.erl +++ b/src/muc_light/mod_muc_light.erl @@ -658,7 +658,7 @@ process_create_aff_users_if_valid(HostType, Creator, AffUsers) -> case lists:any(fun ({User, _}) when User =:= Creator -> true; ({_, Aff}) -> Aff =:= none end, AffUsers) of false -> - process_create_aff_users(Creator, AffUsers, equal_occupants(HostType)); + process_create_aff_users(HostType, Creator, AffUsers, equal_occupants(HostType)); true -> {error, bad_request} end. @@ -666,11 +666,11 @@ process_create_aff_users_if_valid(HostType, Creator, AffUsers) -> equal_occupants(HostType) -> gen_mod:get_module_opt(HostType, ?MODULE, equal_occupants). --spec process_create_aff_users(Creator :: jid:simple_bare_jid(), AffUsers :: aff_users(), +-spec process_create_aff_users(HostType :: host_type(), Creator :: jid:simple_bare_jid(), AffUsers :: aff_users(), EqualOccupants :: boolean()) -> {ok, aff_users()} | {error, bad_request}. -process_create_aff_users(Creator, AffUsers, EqualOccupants) -> - case mod_muc_light_utils:change_aff_users([{Creator, creator_aff(EqualOccupants)}], AffUsers) of +process_create_aff_users(HostType, Creator, AffUsers, EqualOccupants) -> + case mod_muc_light_utils:change_aff_users(HostType, [{Creator, creator_aff(EqualOccupants)}], AffUsers) of {ok, FinalAffUsers, _ChangedAffUsers, _JoiningUsers, _LeavingUsers} -> {ok, FinalAffUsers}; Error -> Error end. diff --git a/src/muc_light/mod_muc_light_db_mnesia.erl b/src/muc_light/mod_muc_light_db_mnesia.erl index 3d06e0a7df1..4f726a95bf2 100644 --- a/src/muc_light/mod_muc_light_db_mnesia.erl +++ b/src/muc_light/mod_muc_light_db_mnesia.erl @@ -126,9 +126,9 @@ get_user_rooms_count(_HostType, UserUS) -> UserUS :: jid:simple_bare_jid(), Version :: binary()) -> mod_muc_light_db_backend:remove_user_return() | {error, term()}. -remove_user(_HostType, UserUS, Version) -> +remove_user(HostType, UserUS, Version) -> mnesia:dirty_delete(muc_light_blocking, UserUS), - {atomic, Res} = mnesia:transaction(fun remove_user_transaction/2, [UserUS, Version]), + {atomic, Res} = mnesia:transaction(fun remove_user_transaction/3, [HostType, UserUS, Version]), Res. -spec remove_domain(mongooseim:host_type(), jid:lserver(), jid:lserver()) -> ok. @@ -210,9 +210,9 @@ get_aff_users(_HostType, RoomUS) -> ExternalCheck :: external_check_fun(), Version :: binary()) -> mod_muc_light_db_backend:modify_aff_users_return(). -modify_aff_users(_HostType, RoomUS, AffUsersChanges, ExternalCheck, Version) -> - {atomic, Res} = mnesia:transaction(fun modify_aff_users_transaction/4, - [RoomUS, AffUsersChanges, ExternalCheck, Version]), +modify_aff_users(HostType, RoomUS, AffUsersChanges, ExternalCheck, Version) -> + {atomic, Res} = mnesia:transaction(fun modify_aff_users_transaction/5, + [HostType, RoomUS, AffUsersChanges, ExternalCheck, Version]), Res. %% ------------------------ Misc ------------------------ @@ -311,13 +311,15 @@ destroy_room_transaction(RoomUS) -> mnesia:delete({muc_light_room, RoomUS}) end. --spec remove_user_transaction(UserUS :: jid:simple_bare_jid(), Version :: binary()) -> +-spec remove_user_transaction(HostType :: mongooseim:host_type(), + UserUS :: jid:simple_bare_jid(), + Version :: binary()) -> mod_muc_light_db_backend:remove_user_return(). -remove_user_transaction(UserUS, Version) -> +remove_user_transaction(HostType, UserUS, Version) -> lists:map( fun(#muc_light_user_room{ room = RoomUS }) -> {RoomUS, modify_aff_users_transaction( - RoomUS, [{UserUS, none}], fun(_, _) -> ok end, Version)} + HostType, RoomUS, [{UserUS, none}], fun(_, _) -> ok end, Version)} end, mnesia:read(muc_light_user_room, UserUS)). %% ------------------------ Configuration manipulation ------------------------ @@ -345,17 +347,18 @@ dirty_get_blocking_raw(UserUS) -> %% ------------------------ Affiliations manipulation ------------------------ --spec modify_aff_users_transaction(RoomUS :: jid:simple_bare_jid(), +-spec modify_aff_users_transaction(HostType :: mongooseim:host_type(), + RoomUS :: jid:simple_bare_jid(), AffUsersChanges :: aff_users(), ExternalCheck :: external_check_fun(), Version :: binary()) -> mod_muc_light_db_backend:modify_aff_users_return(). -modify_aff_users_transaction(RoomUS, AffUsersChanges, ExternalCheck, Version) -> +modify_aff_users_transaction(HostType, RoomUS, AffUsersChanges, ExternalCheck, Version) -> case mnesia:wread({muc_light_room, RoomUS}) of [] -> {error, not_exists}; [#muc_light_room{ aff_users = AffUsers } = RoomRec] -> - case mod_muc_light_utils:change_aff_users(AffUsers, AffUsersChanges) of + case mod_muc_light_utils:change_aff_users(HostType, AffUsers, AffUsersChanges) of {ok, NewAffUsers, _, _, _} = ChangeResult -> verify_externally_and_submit( RoomUS, RoomRec, ChangeResult, ExternalCheck(RoomUS, NewAffUsers), Version); diff --git a/src/muc_light/mod_muc_light_db_rdbms.erl b/src/muc_light/mod_muc_light_db_rdbms.erl index 48bad0ea1f7..249e75ff66b 100644 --- a/src/muc_light/mod_muc_light_db_rdbms.erl +++ b/src/muc_light/mod_muc_light_db_rdbms.erl @@ -752,7 +752,7 @@ modify_aff_users_transaction(HostType, RoomUS, RoomID, AffUsersChanges, CheckFun, PrevVersion, Version) -> {selected, AffUsersDB} = select_affs_by_room_id(HostType, RoomID), AffUsers = decode_affs(AffUsersDB), - case mod_muc_light_utils:change_aff_users(AffUsers, AffUsersChanges) of + case mod_muc_light_utils:change_aff_users(HostType, AffUsers, AffUsersChanges) of {ok, NewAffUsers, AffUsersChanged, JoiningUsers, _LeavingUsers} -> case CheckFun(RoomUS, NewAffUsers) of ok -> diff --git a/src/muc_light/mod_muc_light_multiple_admin.erl b/src/muc_light/mod_muc_light_multiple_admin.erl new file mode 100644 index 00000000000..bd57802fe59 --- /dev/null +++ b/src/muc_light/mod_muc_light_multiple_admin.erl @@ -0,0 +1,60 @@ +%%%------------------------------------------------------------------- +%%% @author jaspreet.chhabra +%%% @copyright (C) 2024, +%%% @doc +%%% +%%% @end +%%% Created : 27. Oct 2024 10:00 AM +%%%------------------------------------------------------------------- +-module(mod_muc_light_multiple_admin). +-author("jaspreet.chhabra"). + +-behaviour(gen_mod). + +-include("mongoose.hrl"). +-include("mongoose_config_spec.hrl"). +%% ------------------------------------------------------------------ +%% gen_mod +%% ------------------------------------------------------------------ +-export([start/2, stop/1, hooks/1, config_spec/0]). +%% ------------------------------------------------------------------ +%% Hook handlers +%% ------------------------------------------------------------------ +-export([should_allow_multiple_admin/3]). + +%% ------------------------------------------------------------------ +%% Macros +%%-------------------------------------------------------------------- +-define(DEFAULT_ALLOW_MULTIPLE_ADMINS, true). + +-spec start(mongooseim:host_type(), gen_mod:module_opts()) -> ok. +start(_HostType, _Opts) -> + ok. + +-spec stop(mongooseim:host_type()) -> ok. +stop(_HostType) -> + ok. + +-spec hooks(mongooseim:host_type()) -> gen_hook:hook_list(). +hooks(HostType) -> + [{should_allow_multiple_admin, HostType, fun ?MODULE:should_allow_multiple_admin/3, #{}, 98}]. + +-spec config_spec() -> mongoose_config_spec:config_section(). +config_spec() -> + #section{ + items = #{<<"allow_multiple_admin">> => #option{type = boolean}}, + defaults = #{<<"allow_multiple_admin">> => ?DEFAULT_ALLOW_MULTIPLE_ADMINS} + }. + +%%==================================================================== +%% Hook handlers +%%==================================================================== +-spec should_allow_multiple_admin(Acc, Params, Extra) -> {ok | stop, Acc} when + Acc :: boolean(), + Params :: #{}, + Extra :: gen_hook:extra(). +should_allow_multiple_admin(_InAcc, #{}, #{host_type := HostType}) -> + AllowMultipleAdmin = gen_mod:get_module_opt(HostType, ?MODULE, allow_multiple_admin), + {ok, AllowMultipleAdmin}. + + diff --git a/src/muc_light/mod_muc_light_utils.erl b/src/muc_light/mod_muc_light_utils.erl index 5c6afcadd81..e5ee27bbe1d 100644 --- a/src/muc_light/mod_muc_light_utils.erl +++ b/src/muc_light/mod_muc_light_utils.erl @@ -24,7 +24,7 @@ -author('piotr.nosek@erlang-solutions.com'). %% API --export([change_aff_users/2]). +-export([change_aff_users/3]). -export([b2aff/1, aff2b/1]). -export([light_aff_to_muc_role/1]). -export([room_limit_reached/2]). @@ -57,20 +57,26 @@ %% API %%==================================================================== --spec change_aff_users(CurrentAffUsers :: aff_users(), AffUsersChangesAssorted :: aff_users()) -> +-spec change_aff_users(HostType :: mongooseim:host_type(), CurrentAffUsers :: aff_users(), AffUsersChangesAssorted :: aff_users()) -> change_aff_success() | {error, bad_request()}. -change_aff_users(AffUsers, AffUsersChangesAssorted) -> +change_aff_users(HostType, AffUsers, AffUsersChangesAssorted) -> case {lists:keyfind(owner, 2, AffUsers), lists:keyfind(owner, 2, AffUsersChangesAssorted)} of {false, false} -> % simple, no special cases apply_aff_users_change(AffUsers, AffUsersChangesAssorted); {false, {_, _}} -> % ownerless room! {error, {bad_request, <<"Ownerless room">>}}; _ -> + MultipleAdmin = mongoose_hooks:should_allow_multiple_admin(HostType), lists:foldl(fun(F, Acc) -> F(Acc) end, apply_aff_users_change(AffUsers, AffUsersChangesAssorted), - [fun maybe_select_new_owner/1]) + change_aff_functions(MultipleAdmin)) end. +change_aff_functions(false)-> + [fun maybe_demote_old_owner/1, fun maybe_select_new_owner/1]; +change_aff_functions(true)-> + [fun maybe_select_new_owner/1]. + -spec aff2b(Aff :: aff()) -> binary(). aff2b(owner) -> <<"owner">>; aff2b(member) -> <<"member">>; @@ -207,6 +213,25 @@ select_promotion(_OldMembers, _JoiningUsers, [U | _]) -> select_promotion(_, _, _) -> false. +-spec maybe_demote_old_owner(ChangeResult :: change_aff_success() | {error, bad_request()}) -> + change_aff_success() | {error, bad_request()}. +maybe_demote_old_owner({ok, AU, AUC, JoiningUsers, LeavingUsers}) -> + Owners = [U || {U, owner} <- AU], + PromotedOwners = [U || {U, owner} <- AUC], + OldOwners = Owners -- PromotedOwners, + case {Owners, OldOwners} of + _ when length(Owners) =< 1 -> + {ok, AU, AUC, JoiningUsers, LeavingUsers}; + {[_, _], [OldOwner]} -> + NewAU = lists:keyreplace(OldOwner, 1, AU, {OldOwner, member}), + NewAUC = [{OldOwner, member} | AUC], + {ok, NewAU, NewAUC, JoiningUsers, LeavingUsers}; + _ -> + {error, {bad_request, <<"Failed to demote old owner">>}} + end; +maybe_demote_old_owner(Error) -> + Error. + -spec apply_aff_users_change(AffUsers :: aff_users(), AffUsersChanges :: aff_users()) -> change_aff_success() | {error, bad_request()}. diff --git a/test/muc_light_SUITE.erl b/test/muc_light_SUITE.erl index 740e2d89c78..1a33a20d8b2 100644 --- a/test/muc_light_SUITE.erl +++ b/test/muc_light_SUITE.erl @@ -164,7 +164,7 @@ filter_room_packet_handler(Acc, _Params, _Extra) -> prop_aff_change_success() -> ?FORALL({AffUsers, Changes, Joining, Leaving, WithOwner}, change_aff_params(), begin - case mod_muc_light_utils:change_aff_users(AffUsers, Changes) of + case mod_muc_light_utils:change_aff_users(<<"localhost">>, AffUsers, Changes) of {ok, NewAffUsers0, AffUsersChanged, Joining0, Leaving0} -> Joining = lists:sort(Joining0), Leaving = lists:sort(Leaving0), @@ -178,7 +178,7 @@ prop_aff_change_success() -> % are there no owners or there is exactly one? true = validate_owner(NewAffUsers0, false, WithOwner), % changes list applied to old list should produce the same result - {ok, NewAffUsers1, _, _, _} = mod_muc_light_utils:change_aff_users(AffUsers, AffUsersChanged), + {ok, NewAffUsers1, _, _, _} = mod_muc_light_utils:change_aff_users(<<"localhost">>, AffUsers, AffUsersChanged), NewAffUsers0 = NewAffUsers1, true; _ -> @@ -198,7 +198,7 @@ prop_aff_change_bad_request() -> ?FORALL({AffUsers, Changes}, bad_change_aff(), begin {error, {bad_request, _}} = - mod_muc_light_utils:change_aff_users(AffUsers, Changes), + mod_muc_light_utils:change_aff_users(<<"localhost">>, AffUsers, Changes), true end). From d79c5f14e98d0897b4fc59af6ed86b3a58290a92 Mon Sep 17 00:00:00 2001 From: "jaspreet.chhabra" Date: Wed, 30 Oct 2024 23:27:43 +0530 Subject: [PATCH 03/12] avoid using new mod and managed muc_light multiple admin support with muc_light config only. --- big_tests/tests/muc_light_SUITE.erl | 2 + include/mod_muc_light.hrl | 1 + src/config/mongoose_config_spec.erl | 1 - src/muc_light/mod_muc_light.erl | 2 + .../mod_muc_light_multiple_admin.erl | 60 ------------------- src/muc_light/mod_muc_light_utils.erl | 5 +- test/common/config_parser_helper.erl | 2 + 7 files changed, 11 insertions(+), 62 deletions(-) delete mode 100644 src/muc_light/mod_muc_light_multiple_admin.erl diff --git a/big_tests/tests/muc_light_SUITE.erl b/big_tests/tests/muc_light_SUITE.erl index 6ae7e3dbf1d..00fdcf56239 100644 --- a/big_tests/tests/muc_light_SUITE.erl +++ b/big_tests/tests/muc_light_SUITE.erl @@ -300,6 +300,8 @@ muc_light_opts(block_user) -> #{all_can_invite => true}; muc_light_opts(blocking_disabled) -> #{blocking => false}; +muc_light_opts(allow_multiple_admin) -> + #{allow_multiple_admin => true}; muc_light_opts(_) -> #{}. diff --git a/include/mod_muc_light.hrl b/include/mod_muc_light.hrl index 6e93f6395c2..f6c5850bb9c 100644 --- a/include/mod_muc_light.hrl +++ b/include/mod_muc_light.hrl @@ -15,6 +15,7 @@ -define(DEFAULT_MAX_OCCUPANTS, infinity). -define(DEFAULT_ROOMS_PER_PAGE, 10). -define(DEFAULT_ROOMS_IN_ROSTERS, false). +-define(DEFAULT_ALLOW_MULTIPLE_ADMIN, true). -type aff() :: owner | member | none. -type aff_user() :: {jid:simple_bare_jid(), aff()}. diff --git a/src/config/mongoose_config_spec.erl b/src/config/mongoose_config_spec.erl index 3a68a861f49..943a3635d5e 100644 --- a/src/config/mongoose_config_spec.erl +++ b/src/config/mongoose_config_spec.erl @@ -760,7 +760,6 @@ configurable_modules() -> mod_mam, mod_muc, mod_muc_light, - mod_muc_light_multiple_admin, mod_muc_log, mod_offline, mod_offline_chatmarkers, diff --git a/src/muc_light/mod_muc_light.erl b/src/muc_light/mod_muc_light.erl index 7037080c541..0ef27ca6cc5 100644 --- a/src/muc_light/mod_muc_light.erl +++ b/src/muc_light/mod_muc_light.erl @@ -231,6 +231,7 @@ config_spec() -> <<"rooms_per_page">> => #option{type = int_or_infinity, validate = positive}, <<"rooms_in_rosters">> => #option{type = boolean}, + <<"allow_multiple_admin">> => #option{type = boolean}, <<"config_schema">> => #list{items = config_schema_spec(), process = fun ?MODULE:process_config_schema/1} }, @@ -245,6 +246,7 @@ config_spec() -> <<"max_occupants">> => ?DEFAULT_MAX_OCCUPANTS, <<"rooms_per_page">> => ?DEFAULT_ROOMS_PER_PAGE, <<"rooms_in_rosters">> => ?DEFAULT_ROOMS_IN_ROSTERS, + <<"allow_multiple_admin">> => ?DEFAULT_ALLOW_MULTIPLE_ADMIN, <<"config_schema">> => default_schema()} }. diff --git a/src/muc_light/mod_muc_light_multiple_admin.erl b/src/muc_light/mod_muc_light_multiple_admin.erl deleted file mode 100644 index bd57802fe59..00000000000 --- a/src/muc_light/mod_muc_light_multiple_admin.erl +++ /dev/null @@ -1,60 +0,0 @@ -%%%------------------------------------------------------------------- -%%% @author jaspreet.chhabra -%%% @copyright (C) 2024, -%%% @doc -%%% -%%% @end -%%% Created : 27. Oct 2024 10:00 AM -%%%------------------------------------------------------------------- --module(mod_muc_light_multiple_admin). --author("jaspreet.chhabra"). - --behaviour(gen_mod). - --include("mongoose.hrl"). --include("mongoose_config_spec.hrl"). -%% ------------------------------------------------------------------ -%% gen_mod -%% ------------------------------------------------------------------ --export([start/2, stop/1, hooks/1, config_spec/0]). -%% ------------------------------------------------------------------ -%% Hook handlers -%% ------------------------------------------------------------------ --export([should_allow_multiple_admin/3]). - -%% ------------------------------------------------------------------ -%% Macros -%%-------------------------------------------------------------------- --define(DEFAULT_ALLOW_MULTIPLE_ADMINS, true). - --spec start(mongooseim:host_type(), gen_mod:module_opts()) -> ok. -start(_HostType, _Opts) -> - ok. - --spec stop(mongooseim:host_type()) -> ok. -stop(_HostType) -> - ok. - --spec hooks(mongooseim:host_type()) -> gen_hook:hook_list(). -hooks(HostType) -> - [{should_allow_multiple_admin, HostType, fun ?MODULE:should_allow_multiple_admin/3, #{}, 98}]. - --spec config_spec() -> mongoose_config_spec:config_section(). -config_spec() -> - #section{ - items = #{<<"allow_multiple_admin">> => #option{type = boolean}}, - defaults = #{<<"allow_multiple_admin">> => ?DEFAULT_ALLOW_MULTIPLE_ADMINS} - }. - -%%==================================================================== -%% Hook handlers -%%==================================================================== --spec should_allow_multiple_admin(Acc, Params, Extra) -> {ok | stop, Acc} when - Acc :: boolean(), - Params :: #{}, - Extra :: gen_hook:extra(). -should_allow_multiple_admin(_InAcc, #{}, #{host_type := HostType}) -> - AllowMultipleAdmin = gen_mod:get_module_opt(HostType, ?MODULE, allow_multiple_admin), - {ok, AllowMultipleAdmin}. - - diff --git a/src/muc_light/mod_muc_light_utils.erl b/src/muc_light/mod_muc_light_utils.erl index e5ee27bbe1d..8ebcbbcbdd4 100644 --- a/src/muc_light/mod_muc_light_utils.erl +++ b/src/muc_light/mod_muc_light_utils.erl @@ -66,7 +66,7 @@ change_aff_users(HostType, AffUsers, AffUsersChangesAssorted) -> {false, {_, _}} -> % ownerless room! {error, {bad_request, <<"Ownerless room">>}}; _ -> - MultipleAdmin = mongoose_hooks:should_allow_multiple_admin(HostType), + MultipleAdmin = allow_multiple_admin(HostType), lists:foldl(fun(F, Acc) -> F(Acc) end, apply_aff_users_change(AffUsers, AffUsersChangesAssorted), change_aff_functions(MultipleAdmin)) @@ -322,3 +322,6 @@ run_forget_room_hook({Room, MucHost}) -> ?LOG_ERROR(#{what => run_forget_room_hook_skipped, room => Room, muc_host => MucHost}) end. + +allow_multiple_admin(HostType) -> + gen_mod:get_module_opt(HostType, mod_muc_light, allow_multiple_admin). \ No newline at end of file diff --git a/test/common/config_parser_helper.erl b/test/common/config_parser_helper.erl index 7d76c7f79e4..c8166b3f0ed 100644 --- a/test/common/config_parser_helper.erl +++ b/test/common/config_parser_helper.erl @@ -570,6 +570,7 @@ all_modules() -> max_occupants => 50, rooms_in_rosters => true, rooms_per_page => 5, + allow_multiple_admin => true, rooms_per_user => 10}), mod_push_service_mongoosepush => #{api_version => <<"v3">>, @@ -976,6 +977,7 @@ default_mod_config(mod_muc_light) -> max_occupants => infinity, rooms_per_page => 10, rooms_in_rosters => false, + allow_multiple_admin => true, config_schema => [{<<"roomname">>, <<"Untitled">>, roomname, binary}, {<<"subject">>, <<>>, subject, binary}]}; default_mod_config(mod_muc_log) -> From 8aa5f42b21080bd204b2a736052286e6526402c5 Mon Sep 17 00:00:00 2001 From: "jaspreet.chhabra" Date: Wed, 30 Oct 2024 23:32:35 +0530 Subject: [PATCH 04/12] added doc for modules.mod_muc_light.allow_multiple_admin. --- doc/modules/mod_muc_light.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/doc/modules/mod_muc_light.md b/doc/modules/mod_muc_light.md index bd752427e90..05067a75c0c 100644 --- a/doc/modules/mod_muc_light.md +++ b/doc/modules/mod_muc_light.md @@ -109,6 +109,14 @@ Specifies maximal number of rooms returned for a single Disco request. When enabled, rooms the user occupies are included in their roster. +### `modules.mod_muc_light.allow_multiple_admin` +* **Syntax:** boolean +* **Default:** `true` +* **Example:** `allow_multiple_admin = false` + +When enabled, admin will be able to add one or more person as admin. +If disabled, only admin can have admin rites! + ### `modules.mod_muc_light.config_schema` * **Syntax:** an array of `config_schema` items, as described below * **Default:** From 9a56a229dd6d6853bb21bf3307129dbd0699d4b2 Mon Sep 17 00:00:00 2001 From: "jaspreet.chhabra" Date: Wed, 30 Oct 2024 23:53:00 +0530 Subject: [PATCH 05/12] added test in config_parser_SUITE for allow_multiple_admin in mod_muc_light. --- test/config_parser_SUITE.erl | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/config_parser_SUITE.erl b/test/config_parser_SUITE.erl index f9bfe4ae7de..73cbcfd3b2b 100644 --- a/test/config_parser_SUITE.erl +++ b/test/config_parser_SUITE.erl @@ -2344,6 +2344,8 @@ mod_muc_light(_Config) -> T(#{<<"rooms_per_page">> => 10})), ?cfgh(P ++ [rooms_in_rosters], true, T(#{<<"rooms_in_rosters">> => true})), + ?cfgh(P ++ [allow_multiple_admin], true, + T(#{<<"allow_multiple_admin">> => true})), ?errh(T(#{<<"backend">> => <<"frontend">>})), ?errh(T(#{<<"host">> => <<"what is a domain?!">>})), ?errh(T(#{<<"host">> => <<"invalid..com">>})), @@ -2359,6 +2361,7 @@ mod_muc_light(_Config) -> ?errh(T(#{<<"all_can_invite">> => #{}})), ?errh(T(#{<<"max_occupants">> => <<"seven">>})), ?errh(T(#{<<"rooms_per_page">> => false})), + ?errh(T(#{<<"allow_multiple_admin">> => true})), ?errh(T(#{<<"rooms_in_rosters">> => [1, 2, 3]})). mod_muc_light_config_schema(_Config) -> From bb5a2a2711b57c849a1a0cedb295856528e9f7c2 Mon Sep 17 00:00:00 2001 From: "jaspreet.chhabra" Date: Sun, 3 Nov 2024 14:01:39 +0530 Subject: [PATCH 06/12] Changed allow_multiple_admin default value to false and code style issues fixed. --- big_tests/tests/muc_light_SUITE.erl | 26 +++++++++++++------------- doc/modules/mod_muc_light.md | 4 ++-- include/mod_muc_light.hrl | 2 +- src/hooks/mongoose_hooks.erl | 9 +-------- src/muc_light/mod_muc_light_utils.erl | 2 +- test/muc_light_SUITE.erl | 6 +++--- 6 files changed, 21 insertions(+), 28 deletions(-) diff --git a/big_tests/tests/muc_light_SUITE.erl b/big_tests/tests/muc_light_SUITE.erl index 00fdcf56239..82c87eaea81 100644 --- a/big_tests/tests/muc_light_SUITE.erl +++ b/big_tests/tests/muc_light_SUITE.erl @@ -797,19 +797,19 @@ remove_and_add_users(Config) -> end). multiple_owner_change(Config) -> - escalus:story(Config, [{alice, 1}, {bob, 1}, {kate, 1}], fun(Alice, Bob, Kate) -> - TS = instrument_helper:timestamp(), - AffUsersChanges1 = [{Bob, none}, {Kate, none}], - escalus:send(Alice, stanza_aff_set(?ROOM, AffUsersChanges1)), - verify_aff_bcast([{Alice, owner}], AffUsersChanges1), - escalus:assert(is_iq_result, escalus:wait_for_stanza(Alice)), - AffUsersChanges2 = [{Bob, owner}, {Kate, owner}], - escalus:send(Alice, stanza_aff_set(?ROOM, AffUsersChanges2)), - verify_aff_bcast([{Alice, owner}, {Bob, owner}, {Kate, owner}], AffUsersChanges2), - escalus:assert(is_iq_result, escalus:wait_for_stanza(Alice)), - assert_cache_miss_event(TS, 1, room_bin_jid(?ROOM)), - assert_cache_hit_event(TS, 1, room_bin_jid(?ROOM)) - end). + escalus:story(Config, [{alice, 1}, {bob, 1}, {kate, 1}], fun(Alice, Bob, Kate) -> + TS = instrument_helper:timestamp(), + AffUsersChanges1 = [{Bob, none}, {Kate, none}], + escalus:send(Alice, stanza_aff_set(?ROOM, AffUsersChanges1)), + verify_aff_bcast([{Alice, owner}], AffUsersChanges1), + escalus:assert(is_iq_result, escalus:wait_for_stanza(Alice)), + AffUsersChanges2 = [{Bob, owner}, {Kate, owner}], + escalus:send(Alice, stanza_aff_set(?ROOM, AffUsersChanges2)), + verify_aff_bcast([{Alice, owner}, {Bob, owner}, {Kate, owner}], AffUsersChanges2), + escalus:assert(is_iq_result, escalus:wait_for_stanza(Alice)), + assert_cache_miss_event(TS, 1, room_bin_jid(?ROOM)), + assert_cache_hit_event(TS, 1, room_bin_jid(?ROOM)) + end). explicit_owner_change(Config) -> escalus:story(Config, [{alice, 1}, {bob, 1}, {kate, 1}], fun(Alice, Bob, Kate) -> diff --git a/doc/modules/mod_muc_light.md b/doc/modules/mod_muc_light.md index 05067a75c0c..bf5ee903c10 100644 --- a/doc/modules/mod_muc_light.md +++ b/doc/modules/mod_muc_light.md @@ -111,11 +111,11 @@ When enabled, rooms the user occupies are included in their roster. ### `modules.mod_muc_light.allow_multiple_admin` * **Syntax:** boolean -* **Default:** `true` +* **Default:** `false` * **Example:** `allow_multiple_admin = false` When enabled, admin will be able to add one or more person as admin. -If disabled, only admin can have admin rites! +If disabled, only admin can have admin rights! ### `modules.mod_muc_light.config_schema` * **Syntax:** an array of `config_schema` items, as described below diff --git a/include/mod_muc_light.hrl b/include/mod_muc_light.hrl index f6c5850bb9c..8e2c9a59707 100644 --- a/include/mod_muc_light.hrl +++ b/include/mod_muc_light.hrl @@ -15,7 +15,7 @@ -define(DEFAULT_MAX_OCCUPANTS, infinity). -define(DEFAULT_ROOMS_PER_PAGE, 10). -define(DEFAULT_ROOMS_IN_ROSTERS, false). --define(DEFAULT_ALLOW_MULTIPLE_ADMIN, true). +-define(DEFAULT_ALLOW_MULTIPLE_ADMIN, false). -type aff() :: owner | member | none. -type aff_user() :: {jid:simple_bare_jid(), aff()}. diff --git a/src/hooks/mongoose_hooks.erl b/src/hooks/mongoose_hooks.erl index 11287d024fa..475178c77e1 100644 --- a/src/hooks/mongoose_hooks.erl +++ b/src/hooks/mongoose_hooks.erl @@ -35,8 +35,7 @@ user_available/2, user_ping_response/5, vcard_set/4, - xmpp_send_element/3, - should_allow_multiple_admin/1]). + xmpp_send_element/3]). %% sasl2 handlers -export([sasl2_stream_features/2, @@ -1377,12 +1376,6 @@ mod_global_distrib_unknown_recipient(GlobalHost, Info) -> run_hook_for_host_type(mod_global_distrib_unknown_recipient, GlobalHost, Info, #{}). --spec should_allow_multiple_admin(HostType) -> Result when - HostType :: mongooseim:host_type(), - Result :: boolean(). -should_allow_multiple_admin(HostType) -> - run_hook_for_host_type(should_allow_multiple_admin, HostType, true, #{}). - %%%---------------------------------------------------------------------- %%% Internal functions %%%---------------------------------------------------------------------- diff --git a/src/muc_light/mod_muc_light_utils.erl b/src/muc_light/mod_muc_light_utils.erl index 8ebcbbcbdd4..b919927778f 100644 --- a/src/muc_light/mod_muc_light_utils.erl +++ b/src/muc_light/mod_muc_light_utils.erl @@ -324,4 +324,4 @@ run_forget_room_hook({Room, MucHost}) -> end. allow_multiple_admin(HostType) -> - gen_mod:get_module_opt(HostType, mod_muc_light, allow_multiple_admin). \ No newline at end of file + gen_mod:get_module_opt(HostType, mod_muc_light, allow_multiple_admin). diff --git a/test/muc_light_SUITE.erl b/test/muc_light_SUITE.erl index 1a33a20d8b2..189dd861971 100644 --- a/test/muc_light_SUITE.erl +++ b/test/muc_light_SUITE.erl @@ -164,7 +164,7 @@ filter_room_packet_handler(Acc, _Params, _Extra) -> prop_aff_change_success() -> ?FORALL({AffUsers, Changes, Joining, Leaving, WithOwner}, change_aff_params(), begin - case mod_muc_light_utils:change_aff_users(<<"localhost">>, AffUsers, Changes) of + case mod_muc_light_utils:change_aff_users(host_type(), AffUsers, Changes) of {ok, NewAffUsers0, AffUsersChanged, Joining0, Leaving0} -> Joining = lists:sort(Joining0), Leaving = lists:sort(Leaving0), @@ -178,7 +178,7 @@ prop_aff_change_success() -> % are there no owners or there is exactly one? true = validate_owner(NewAffUsers0, false, WithOwner), % changes list applied to old list should produce the same result - {ok, NewAffUsers1, _, _, _} = mod_muc_light_utils:change_aff_users(<<"localhost">>, AffUsers, AffUsersChanged), + {ok, NewAffUsers1, _, _, _} = mod_muc_light_utils:change_aff_users(host_type(), AffUsers, AffUsersChanged), NewAffUsers0 = NewAffUsers1, true; _ -> @@ -198,7 +198,7 @@ prop_aff_change_bad_request() -> ?FORALL({AffUsers, Changes}, bad_change_aff(), begin {error, {bad_request, _}} = - mod_muc_light_utils:change_aff_users(<<"localhost">>, AffUsers, Changes), + mod_muc_light_utils:change_aff_users(host_type(), AffUsers, Changes), true end). From 1de972e0b2da8f875e40229c5ef35a4480744c49 Mon Sep 17 00:00:00 2001 From: Gustaw Lippa Date: Wed, 6 Nov 2024 17:19:09 +0100 Subject: [PATCH 07/12] Fix test of default value --- test/common/config_parser_helper.erl | 4 ++-- test/config_parser_SUITE.erl | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/common/config_parser_helper.erl b/test/common/config_parser_helper.erl index c8166b3f0ed..2370adbd706 100644 --- a/test/common/config_parser_helper.erl +++ b/test/common/config_parser_helper.erl @@ -570,7 +570,7 @@ all_modules() -> max_occupants => 50, rooms_in_rosters => true, rooms_per_page => 5, - allow_multiple_admin => true, + allow_multiple_admin => false, rooms_per_user => 10}), mod_push_service_mongoosepush => #{api_version => <<"v3">>, @@ -977,7 +977,7 @@ default_mod_config(mod_muc_light) -> max_occupants => infinity, rooms_per_page => 10, rooms_in_rosters => false, - allow_multiple_admin => true, + allow_multiple_admin => false, config_schema => [{<<"roomname">>, <<"Untitled">>, roomname, binary}, {<<"subject">>, <<>>, subject, binary}]}; default_mod_config(mod_muc_log) -> diff --git a/test/config_parser_SUITE.erl b/test/config_parser_SUITE.erl index 73cbcfd3b2b..ca9dd362a35 100644 --- a/test/config_parser_SUITE.erl +++ b/test/config_parser_SUITE.erl @@ -2361,7 +2361,7 @@ mod_muc_light(_Config) -> ?errh(T(#{<<"all_can_invite">> => #{}})), ?errh(T(#{<<"max_occupants">> => <<"seven">>})), ?errh(T(#{<<"rooms_per_page">> => false})), - ?errh(T(#{<<"allow_multiple_admin">> => true})), + ?errh(T(#{<<"allow_multiple_admin">> => <<"true">>})), ?errh(T(#{<<"rooms_in_rosters">> => [1, 2, 3]})). mod_muc_light_config_schema(_Config) -> From 312b510ae85be94df3836945fd9f966b344376fc Mon Sep 17 00:00:00 2001 From: Gustaw Lippa Date: Thu, 7 Nov 2024 10:20:27 +0100 Subject: [PATCH 08/12] Configure option for test --- big_tests/tests/muc_light_SUITE.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/big_tests/tests/muc_light_SUITE.erl b/big_tests/tests/muc_light_SUITE.erl index 82c87eaea81..f34dba67dea 100644 --- a/big_tests/tests/muc_light_SUITE.erl +++ b/big_tests/tests/muc_light_SUITE.erl @@ -300,7 +300,7 @@ muc_light_opts(block_user) -> #{all_can_invite => true}; muc_light_opts(blocking_disabled) -> #{blocking => false}; -muc_light_opts(allow_multiple_admin) -> +muc_light_opts(multiple_owner_change) -> #{allow_multiple_admin => true}; muc_light_opts(_) -> #{}. From 2b53c7cc7e3d2af45e1865750a6c9ebde38b2e38 Mon Sep 17 00:00:00 2001 From: Gustaw Lippa Date: Thu, 7 Nov 2024 13:40:07 +0100 Subject: [PATCH 09/12] Rename "admin" to "owner" This is more in line with the MUC Light nomenclature. --- big_tests/tests/muc_light_SUITE.erl | 2 +- doc/modules/mod_muc_light.md | 15 +++++++++++---- include/mod_muc_light.hrl | 2 +- src/muc_light/mod_muc_light.erl | 4 ++-- src/muc_light/mod_muc_light_utils.erl | 8 ++++---- test/common/config_parser_helper.erl | 4 ++-- test/config_parser_SUITE.erl | 6 +++--- 7 files changed, 24 insertions(+), 17 deletions(-) diff --git a/big_tests/tests/muc_light_SUITE.erl b/big_tests/tests/muc_light_SUITE.erl index f34dba67dea..cb5abed9712 100644 --- a/big_tests/tests/muc_light_SUITE.erl +++ b/big_tests/tests/muc_light_SUITE.erl @@ -301,7 +301,7 @@ muc_light_opts(block_user) -> muc_light_opts(blocking_disabled) -> #{blocking => false}; muc_light_opts(multiple_owner_change) -> - #{allow_multiple_admin => true}; + #{allow_multiple_owners => true}; muc_light_opts(_) -> #{}. diff --git a/doc/modules/mod_muc_light.md b/doc/modules/mod_muc_light.md index bf5ee903c10..f6be164854d 100644 --- a/doc/modules/mod_muc_light.md +++ b/doc/modules/mod_muc_light.md @@ -109,13 +109,20 @@ Specifies maximal number of rooms returned for a single Disco request. When enabled, rooms the user occupies are included in their roster. -### `modules.mod_muc_light.allow_multiple_admin` +### `modules.mod_muc_light.allow_multiple_owners` * **Syntax:** boolean * **Default:** `false` -* **Example:** `allow_multiple_admin = false` +* **Example:** `allow_multiple_owners = true` -When enabled, admin will be able to add one or more person as admin. -If disabled, only admin can have admin rights! +When enabled, owner can add one or more people as owners. +If disabled, there can only be one owner. +This option may be useful for creating a subset of users with admin rights, instead of giving rights for all members, +which can be done with the [all_can_configure](#modulesmod_muc_lightall_can_configure) and +[all_can_invite](#modulesmod_muc_lightall_can_invite) options. + +!!! Warning + This is a custom option, not compatible with our [MUC Light XEP](../open-extensions/muc_light.md). + If a client is adhering to the XEP, its behaviour may be unexpected, and this option should not be enabled. ### `modules.mod_muc_light.config_schema` * **Syntax:** an array of `config_schema` items, as described below diff --git a/include/mod_muc_light.hrl b/include/mod_muc_light.hrl index 8e2c9a59707..682c2c31020 100644 --- a/include/mod_muc_light.hrl +++ b/include/mod_muc_light.hrl @@ -15,7 +15,7 @@ -define(DEFAULT_MAX_OCCUPANTS, infinity). -define(DEFAULT_ROOMS_PER_PAGE, 10). -define(DEFAULT_ROOMS_IN_ROSTERS, false). --define(DEFAULT_ALLOW_MULTIPLE_ADMIN, false). +-define(DEFAULT_ALLOW_MULTIPLE_OWNERS, false). -type aff() :: owner | member | none. -type aff_user() :: {jid:simple_bare_jid(), aff()}. diff --git a/src/muc_light/mod_muc_light.erl b/src/muc_light/mod_muc_light.erl index 0ef27ca6cc5..96489d520a2 100644 --- a/src/muc_light/mod_muc_light.erl +++ b/src/muc_light/mod_muc_light.erl @@ -231,7 +231,7 @@ config_spec() -> <<"rooms_per_page">> => #option{type = int_or_infinity, validate = positive}, <<"rooms_in_rosters">> => #option{type = boolean}, - <<"allow_multiple_admin">> => #option{type = boolean}, + <<"allow_multiple_owners">> => #option{type = boolean}, <<"config_schema">> => #list{items = config_schema_spec(), process = fun ?MODULE:process_config_schema/1} }, @@ -246,7 +246,7 @@ config_spec() -> <<"max_occupants">> => ?DEFAULT_MAX_OCCUPANTS, <<"rooms_per_page">> => ?DEFAULT_ROOMS_PER_PAGE, <<"rooms_in_rosters">> => ?DEFAULT_ROOMS_IN_ROSTERS, - <<"allow_multiple_admin">> => ?DEFAULT_ALLOW_MULTIPLE_ADMIN, + <<"allow_multiple_owners">> => ?DEFAULT_ALLOW_MULTIPLE_OWNERS, <<"config_schema">> => default_schema()} }. diff --git a/src/muc_light/mod_muc_light_utils.erl b/src/muc_light/mod_muc_light_utils.erl index b919927778f..af5b1269625 100644 --- a/src/muc_light/mod_muc_light_utils.erl +++ b/src/muc_light/mod_muc_light_utils.erl @@ -66,10 +66,10 @@ change_aff_users(HostType, AffUsers, AffUsersChangesAssorted) -> {false, {_, _}} -> % ownerless room! {error, {bad_request, <<"Ownerless room">>}}; _ -> - MultipleAdmin = allow_multiple_admin(HostType), + MultipleOwners = allow_multiple_owners(HostType), lists:foldl(fun(F, Acc) -> F(Acc) end, apply_aff_users_change(AffUsers, AffUsersChangesAssorted), - change_aff_functions(MultipleAdmin)) + change_aff_functions(MultipleOwners)) end. change_aff_functions(false)-> @@ -323,5 +323,5 @@ run_forget_room_hook({Room, MucHost}) -> room => Room, muc_host => MucHost}) end. -allow_multiple_admin(HostType) -> - gen_mod:get_module_opt(HostType, mod_muc_light, allow_multiple_admin). +allow_multiple_owners(HostType) -> + gen_mod:get_module_opt(HostType, mod_muc_light, allow_multiple_owners). diff --git a/test/common/config_parser_helper.erl b/test/common/config_parser_helper.erl index 2370adbd706..590cacfb0b0 100644 --- a/test/common/config_parser_helper.erl +++ b/test/common/config_parser_helper.erl @@ -570,7 +570,7 @@ all_modules() -> max_occupants => 50, rooms_in_rosters => true, rooms_per_page => 5, - allow_multiple_admin => false, + allow_multiple_owners => false, rooms_per_user => 10}), mod_push_service_mongoosepush => #{api_version => <<"v3">>, @@ -977,7 +977,7 @@ default_mod_config(mod_muc_light) -> max_occupants => infinity, rooms_per_page => 10, rooms_in_rosters => false, - allow_multiple_admin => false, + allow_multiple_owners => false, config_schema => [{<<"roomname">>, <<"Untitled">>, roomname, binary}, {<<"subject">>, <<>>, subject, binary}]}; default_mod_config(mod_muc_log) -> diff --git a/test/config_parser_SUITE.erl b/test/config_parser_SUITE.erl index ca9dd362a35..d0de5346008 100644 --- a/test/config_parser_SUITE.erl +++ b/test/config_parser_SUITE.erl @@ -2344,8 +2344,8 @@ mod_muc_light(_Config) -> T(#{<<"rooms_per_page">> => 10})), ?cfgh(P ++ [rooms_in_rosters], true, T(#{<<"rooms_in_rosters">> => true})), - ?cfgh(P ++ [allow_multiple_admin], true, - T(#{<<"allow_multiple_admin">> => true})), + ?cfgh(P ++ [allow_multiple_owners], true, + T(#{<<"allow_multiple_owners">> => true})), ?errh(T(#{<<"backend">> => <<"frontend">>})), ?errh(T(#{<<"host">> => <<"what is a domain?!">>})), ?errh(T(#{<<"host">> => <<"invalid..com">>})), @@ -2361,7 +2361,7 @@ mod_muc_light(_Config) -> ?errh(T(#{<<"all_can_invite">> => #{}})), ?errh(T(#{<<"max_occupants">> => <<"seven">>})), ?errh(T(#{<<"rooms_per_page">> => false})), - ?errh(T(#{<<"allow_multiple_admin">> => <<"true">>})), + ?errh(T(#{<<"allow_multiple_owners">> => <<"true">>})), ?errh(T(#{<<"rooms_in_rosters">> => [1, 2, 3]})). mod_muc_light_config_schema(_Config) -> From ea690895e5c1d77a4b805c800cc3effa2e67b9ca Mon Sep 17 00:00:00 2001 From: Gustaw Lippa Date: Thu, 7 Nov 2024 14:03:14 +0100 Subject: [PATCH 10/12] Move helper down in file --- src/muc_light/mod_muc_light_utils.erl | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/muc_light/mod_muc_light_utils.erl b/src/muc_light/mod_muc_light_utils.erl index af5b1269625..fac7a428cb3 100644 --- a/src/muc_light/mod_muc_light_utils.erl +++ b/src/muc_light/mod_muc_light_utils.erl @@ -72,10 +72,6 @@ change_aff_users(HostType, AffUsers, AffUsersChangesAssorted) -> change_aff_functions(MultipleOwners)) end. -change_aff_functions(false)-> - [fun maybe_demote_old_owner/1, fun maybe_select_new_owner/1]; -change_aff_functions(true)-> - [fun maybe_select_new_owner/1]. -spec aff2b(Aff :: aff()) -> binary(). aff2b(owner) -> <<"owner">>; @@ -161,6 +157,12 @@ filter_out_loop(_HostType, _FromUS, _MUCServer, _BlockingQuery, _RoomsPerUser, [ %% ---------------- Affiliations manipulation ---------------- +-spec change_aff_functions(AllowMultipleOwners :: boolean()) -> [fun(), ...]. +change_aff_functions(_AllowMultipleOwners = false)-> + [fun maybe_demote_old_owner/1, fun maybe_select_new_owner/1]; +change_aff_functions(_AllowMultipleOwners = true)-> + [fun maybe_select_new_owner/1]. + -spec maybe_select_new_owner(ChangeResult :: change_aff_success() | {error, bad_request()}) -> change_aff_success() | {error, bad_request()}. maybe_select_new_owner({ok, AU, AUC, JoiningUsers, LeavingUsers} = _AffRes) -> From 23f84ca6e91bb66cecbb17fe150a1a81f1419770 Mon Sep 17 00:00:00 2001 From: Gustaw Lippa Date: Fri, 8 Nov 2024 11:06:44 +0100 Subject: [PATCH 11/12] Reformat code --- src/muc_light/mod_muc_light_db_mnesia.erl | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/muc_light/mod_muc_light_db_mnesia.erl b/src/muc_light/mod_muc_light_db_mnesia.erl index 4f726a95bf2..543be13a675 100644 --- a/src/muc_light/mod_muc_light_db_mnesia.erl +++ b/src/muc_light/mod_muc_light_db_mnesia.erl @@ -317,10 +317,12 @@ destroy_room_transaction(RoomUS) -> mod_muc_light_db_backend:remove_user_return(). remove_user_transaction(HostType, UserUS, Version) -> lists:map( - fun(#muc_light_user_room{ room = RoomUS }) -> - {RoomUS, modify_aff_users_transaction( - HostType, RoomUS, [{UserUS, none}], fun(_, _) -> ok end, Version)} - end, mnesia:read(muc_light_user_room, UserUS)). + fun(#muc_light_user_room{room = RoomUS}) -> + Res = modify_aff_users_transaction( + HostType, RoomUS, [{UserUS, none}], fun(_, _) -> ok end, Version), + {RoomUS, Res} + end, + mnesia:read(muc_light_user_room, UserUS)). %% ------------------------ Configuration manipulation ------------------------ From abf624566586a182c7cbaae24c9def144313f0f9 Mon Sep 17 00:00:00 2001 From: Gustaw Lippa Date: Tue, 12 Nov 2024 12:34:14 +0100 Subject: [PATCH 12/12] Add owner handover test and simplify tests As per code review comments, simplify multiple_owner_change case, and remove the allow_multiple_owners option from config_parser_helper. --- big_tests/tests/muc_light_SUITE.erl | 25 +++++++++++++++---------- test/common/config_parser_helper.erl | 1 - 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/big_tests/tests/muc_light_SUITE.erl b/big_tests/tests/muc_light_SUITE.erl index cb5abed9712..7e6094a7524 100644 --- a/big_tests/tests/muc_light_SUITE.erl +++ b/big_tests/tests/muc_light_SUITE.erl @@ -55,6 +55,7 @@ remove_and_add_users/1, multiple_owner_change/1, explicit_owner_change/1, + explicit_owner_handover/1, implicit_owner_change/1, edge_case_owner_change/1, adding_wrongly_named_user_triggers_infinite_loop/1 @@ -180,6 +181,7 @@ groups() -> assorted_config_doesnt_lead_to_duplication, remove_and_add_users, explicit_owner_change, + explicit_owner_handover, multiple_owner_change, implicit_owner_change, edge_case_owner_change, @@ -798,17 +800,10 @@ remove_and_add_users(Config) -> multiple_owner_change(Config) -> escalus:story(Config, [{alice, 1}, {bob, 1}, {kate, 1}], fun(Alice, Bob, Kate) -> - TS = instrument_helper:timestamp(), - AffUsersChanges1 = [{Bob, none}, {Kate, none}], - escalus:send(Alice, stanza_aff_set(?ROOM, AffUsersChanges1)), - verify_aff_bcast([{Alice, owner}], AffUsersChanges1), - escalus:assert(is_iq_result, escalus:wait_for_stanza(Alice)), - AffUsersChanges2 = [{Bob, owner}, {Kate, owner}], + AffUsersChanges2 = [{Bob, owner}], escalus:send(Alice, stanza_aff_set(?ROOM, AffUsersChanges2)), - verify_aff_bcast([{Alice, owner}, {Bob, owner}, {Kate, owner}], AffUsersChanges2), - escalus:assert(is_iq_result, escalus:wait_for_stanza(Alice)), - assert_cache_miss_event(TS, 1, room_bin_jid(?ROOM)), - assert_cache_hit_event(TS, 1, room_bin_jid(?ROOM)) + verify_aff_bcast([{Alice, owner}, {Bob, owner}, {Kate, member}], AffUsersChanges2), + escalus:assert(is_iq_result, escalus:wait_for_stanza(Alice)) end). explicit_owner_change(Config) -> @@ -819,6 +814,16 @@ explicit_owner_change(Config) -> escalus:assert(is_iq_result, escalus:wait_for_stanza(Alice)) end). +explicit_owner_handover(Config) -> + % check that Alice looses ownership when Kate is set to owner + escalus:story(Config, [{alice, 1}, {bob, 1}, {kate, 1}], fun(Alice, Bob, Kate) -> + ReqAffUsersChanges = [{Kate, owner}], + escalus:send(Alice, stanza_aff_set(?ROOM, ReqAffUsersChanges)), + ExpectedAffUserChanges = [{Alice, member} | ReqAffUsersChanges], + verify_aff_bcast([{Kate, owner}, {Alice, member}, {Bob, member}], ExpectedAffUserChanges), + escalus:assert(is_iq_result, escalus:wait_for_stanza(Alice)) + end). + implicit_owner_change(Config) -> escalus:story(Config, [{alice, 1}, {bob, 1}, {kate, 1}], fun(Alice, Bob, Kate) -> AffUsersChanges1 = [{Bob, none}, {Alice, member}], diff --git a/test/common/config_parser_helper.erl b/test/common/config_parser_helper.erl index 590cacfb0b0..6d101e949cd 100644 --- a/test/common/config_parser_helper.erl +++ b/test/common/config_parser_helper.erl @@ -570,7 +570,6 @@ all_modules() -> max_occupants => 50, rooms_in_rosters => true, rooms_per_page => 5, - allow_multiple_owners => false, rooms_per_user => 10}), mod_push_service_mongoosepush => #{api_version => <<"v3">>,