diff --git a/big_tests/tests/muc_light_SUITE.erl b/big_tests/tests/muc_light_SUITE.erl index 23ca53c3073..7e6094a7524 100644 --- a/big_tests/tests/muc_light_SUITE.erl +++ b/big_tests/tests/muc_light_SUITE.erl @@ -53,7 +53,9 @@ 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, + explicit_owner_handover/1, implicit_owner_change/1, edge_case_owner_change/1, adding_wrongly_named_user_triggers_infinite_loop/1 @@ -179,6 +181,8 @@ 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, adding_wrongly_named_user_triggers_infinite_loop @@ -298,6 +302,8 @@ muc_light_opts(block_user) -> #{all_can_invite => true}; muc_light_opts(blocking_disabled) -> #{blocking => false}; +muc_light_opts(multiple_owner_change) -> + #{allow_multiple_owners => true}; muc_light_opts(_) -> #{}. @@ -792,6 +798,14 @@ 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) -> + AffUsersChanges2 = [{Bob, owner}], + escalus:send(Alice, stanza_aff_set(?ROOM, AffUsersChanges2)), + 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) -> escalus:story(Config, [{alice, 1}, {bob, 1}, {kate, 1}], fun(Alice, Bob, Kate) -> AffUsersChanges1 = [{Bob, none}, {Alice, none}, {Kate, owner}], @@ -800,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/doc/modules/mod_muc_light.md b/doc/modules/mod_muc_light.md index bd752427e90..f6be164854d 100644 --- a/doc/modules/mod_muc_light.md +++ b/doc/modules/mod_muc_light.md @@ -109,6 +109,21 @@ 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_owners` +* **Syntax:** boolean +* **Default:** `false` +* **Example:** `allow_multiple_owners = true` + +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 * **Default:** diff --git a/include/mod_muc_light.hrl b/include/mod_muc_light.hrl index 6e93f6395c2..682c2c31020 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_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 2ecd39db2f4..96489d520a2 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_owners">> => #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_owners">> => ?DEFAULT_ALLOW_MULTIPLE_OWNERS, <<"config_schema">> => default_schema()} }. @@ -658,7 +660,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 +668,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..543be13a675 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,14 +311,18 @@ 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)} - 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 ------------------------ @@ -345,17 +349,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_utils.erl b/src/muc_light/mod_muc_light_utils.erl index bb99cab80de..fac7a428cb3 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,21 +57,22 @@ %% 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">>}}; _ -> + MultipleOwners = allow_multiple_owners(HostType), 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]) + change_aff_functions(MultipleOwners)) end. + -spec aff2b(Aff :: aff()) -> binary(). aff2b(owner) -> <<"owner">>; aff2b(member) -> <<"member">>; @@ -156,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) -> @@ -317,3 +324,6 @@ run_forget_room_hook({Room, MucHost}) -> ?LOG_ERROR(#{what => run_forget_room_hook_skipped, room => Room, muc_host => MucHost}) end. + +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 7d76c7f79e4..6d101e949cd 100644 --- a/test/common/config_parser_helper.erl +++ b/test/common/config_parser_helper.erl @@ -976,6 +976,7 @@ default_mod_config(mod_muc_light) -> max_occupants => infinity, rooms_per_page => 10, rooms_in_rosters => 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 f9bfe4ae7de..d0de5346008 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_owners], true, + T(#{<<"allow_multiple_owners">> => 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_owners">> => <<"true">>})), ?errh(T(#{<<"rooms_in_rosters">> => [1, 2, 3]})). mod_muc_light_config_schema(_Config) -> diff --git a/test/muc_light_SUITE.erl b/test/muc_light_SUITE.erl index 740e2d89c78..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(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(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(AffUsers, Changes), + mod_muc_light_utils:change_aff_users(host_type(), AffUsers, Changes), true end).