Add various improvements to mod_muc_light

big_tests/tests/muc_light_SUITE.erl

@@ -31,13 +31,16 @@
          change_subject/1,
          change_roomname/1,
          all_can_configure/1,
+         all_can_configure_opt/1,
          set_config_deny/1,
          get_room_config/1,
          custom_default_config_works/1,
          get_room_occupants/1,
          get_room_info/1,
          leave_room/1,
-         change_other_aff_deny/1
+         change_other_aff_deny/1,
+         all_can_invite/1,
+         all_can_invite_opt/1
         ]).
 -export([ % owner
          create_room/1,
@@ -57,6 +60,7 @@
          explicit_owner_change/1,
          explicit_owner_handover/1,
          implicit_owner_change/1,
+         implicit_owner_change_disabled/1,
          edge_case_owner_change/1,
          adding_wrongly_named_user_triggers_infinite_loop/1
         ]).
@@ -157,13 +161,16 @@ groups() ->
                                  change_subject,
                                  change_roomname,
                                  all_can_configure,
+                                 all_can_configure_opt,
                                  set_config_deny,
                                  get_room_config,
                                  custom_default_config_works,
                                  get_room_occupants,
                                  get_room_info,
                                  leave_room,
-                                 change_other_aff_deny
+                                 change_other_aff_deny,
+                                 all_can_invite,
+                                 all_can_invite_opt
                                 ]},
          {owner, [sequence], [
                               create_room,
@@ -183,6 +190,7 @@ groups() ->
                               explicit_owner_handover,
                               multiple_owner_change,
                               implicit_owner_change,
+                              implicit_owner_change_disabled,
                               edge_case_owner_change,
                               adding_wrongly_named_user_triggers_infinite_loop
                              ]},
@@ -291,6 +299,16 @@ muc_light_opts(CaseName) when CaseName =:= disco_rooms_empty_page_infinity;
     #{rooms_per_page => infinity};
 muc_light_opts(all_can_configure) ->
     #{all_can_configure => true};
+muc_light_opts(all_can_configure_opt) ->
+    #{config_schema => [{<<"all_can_configure">>, false, all_can_configure, boolean},
+                        {<<"roomname">>, <<"Untitled">>, roomname, binary},
+                        {<<"subject">>, <<>>, subject, binary}]};
+muc_light_opts(all_can_invite) ->
+    #{all_can_invite => true};
+muc_light_opts(all_can_invite_opt) ->
+    #{config_schema => [{<<"all_can_invite">>, false, all_can_invite, boolean},
+                        {<<"roomname">>, <<"Untitled">>, roomname, binary},
+                        {<<"subject">>, <<>>, subject, binary}]};
 muc_light_opts(create_room_with_equal_occupants) ->
     #{equal_occupants => true};
 muc_light_opts(rooms_per_user) ->
@@ -303,6 +321,8 @@ muc_light_opts(blocking_disabled) ->
     #{blocking => false};
 muc_light_opts(multiple_owner_change) ->
     #{allow_multiple_owners => true};
+muc_light_opts(implicit_owner_change_disabled) ->
+    #{promote_on_last_owner_leave => false};
 muc_light_opts(_) ->
     #{}.
 
@@ -563,6 +583,21 @@ all_can_configure(Config) ->
             foreach_occupant([Alice, Bob, Kate], Stanza, config_msg_verify_fun(ConfigChange))
         end).
 
+all_can_configure_opt(Config) ->
+    escalus:story(Config, [{alice, 1}, {bob, 1}, {kate, 1}], fun(Alice, Bob, Kate) ->
+            RoomNameChange = [{<<"roomname">>, <<"new subject">>}],
+            RoomNameStanza = stanza_config_set(?ROOM, RoomNameChange),
+            escalus:send(Kate, RoomNameStanza),
+            escalus:assert(is_error, [<<"cancel">>, <<"not-allowed">>],
+                           escalus:wait_for_stanza(Kate)),
+            verify_no_stanzas([Alice, Bob, Kate]),
+
+            AllCanConfigureChange = [{<<"all_can_configure">>, <<"true">>}],
+            change_room_config(Alice, [Alice, Bob, Kate], AllCanConfigureChange),
+
+            foreach_occupant([Alice, Bob, Kate], RoomNameStanza, config_msg_verify_fun(RoomNameChange))
+        end).
+
 set_config_deny(Config) ->
     escalus:story(Config, [{alice, 1}, {bob, 1}, {kate, 1}], fun(Alice, Bob, Kate) ->
             ConfigChange = [{<<"roomname">>, <<"new subject">>}],
@@ -655,6 +690,47 @@ change_other_aff_deny(Config) ->
             verify_no_stanzas([Alice, Bob, Kate, Mike])
         end).
 
+all_can_invite(Config) ->
+    escalus:story(Config, [{alice, 1}, {bob, 1}, {kate, 1}, {mike, 1}],
+                  fun(Alice, Bob, Kate, Mike) ->
+            AffUsersChanges1 = [{Bob, none}],
+            escalus:send(Kate, stanza_aff_set(?ROOM, AffUsersChanges1)),
+            escalus:assert(is_error, [<<"cancel">>, <<"not-allowed">>],
+                           escalus:wait_for_stanza(Kate)),
+
+            AffUsersChanges2 = [{Alice, member}, {Kate, owner}],
+            escalus:send(Kate, stanza_aff_set(?ROOM, AffUsersChanges2)),
+            escalus:assert(is_error, [<<"cancel">>, <<"not-allowed">>],
+                           escalus:wait_for_stanza(Kate)),
+
+            AffUsersChanges3 = [{Mike, member}],
+            escalus:send(Kate, stanza_aff_set(?ROOM, AffUsersChanges3)),
+            verify_aff_bcast([{Alice, owner}, {Bob, member}, {Kate, member}, {Mike, member}],
+                             AffUsersChanges3),
+            escalus:assert(is_iq_result,
+                           escalus:wait_for_stanza(Kate))
+        end).
+
+all_can_invite_opt(Config) ->
+    escalus:story(Config, [{alice, 1}, {bob, 1}, {kate, 1}, {mike, 1}],
+                  fun(Alice, Bob, Kate, Mike) ->
+            AffUsersChanges1 = [{Mike, member}],
+            escalus:send(Kate, stanza_aff_set(?ROOM, AffUsersChanges1)),
+            escalus:assert(is_error, [<<"cancel">>, <<"not-allowed">>],
+                           escalus:wait_for_stanza(Kate)),
+            verify_no_stanzas([Alice, Bob, Kate, Mike]),
+
+            AllCanConfigureChange = [{<<"all_can_invite">>, <<"true">>}],
+            change_room_config(Alice, [Alice, Bob, Kate], AllCanConfigureChange),
+
+            AffUsersChanges2 = [{Mike, member}],
+            escalus:send(Kate, stanza_aff_set(?ROOM, AffUsersChanges2)),
+            verify_aff_bcast([{Alice, owner}, {Bob, member}, {Kate, member}, {Mike, member}],
+                             AffUsersChanges2),
+            escalus:assert(is_iq_result,
+                           escalus:wait_for_stanza(Kate))
+        end).
+
 %% ---------------------- owner ----------------------
 
 create_room(Config) ->
@@ -831,6 +907,14 @@ implicit_owner_change(Config) ->
             escalus:assert(is_iq_result, escalus:wait_for_stanza(Alice))
         end).
 
+implicit_owner_change_disabled(Config) ->
+    escalus:story(Config, [{alice, 1}, {bob, 1}], fun(Alice, Bob) ->
+            AffUsersChanges1 = [{Bob, none}, {Alice, member}],
+            escalus:send(Alice, stanza_aff_set(?ROOM, AffUsersChanges1)),
+            escalus:assert(is_error, [<<"modify">>, <<"bad-request">>],
+                           escalus:wait_for_stanza(Alice))
+        end).
+
 edge_case_owner_change(Config) ->
     escalus:story(Config, [{alice, 1}, {bob, 1}, {kate, 1}], fun(Alice, Bob, Kate) ->
             AffUsersChanges1 = [{Alice, member}, {Bob, none}, {Kate, none}],

doc/modules/mod_muc_light.md

@@ -127,6 +127,14 @@ which can be done with the [all_can_configure](#modulesmod_muc_lightall_can_conf
     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.promote_on_last_owner_leave`
+* **Syntax**: boolean
+* **Default**: `true`
+* **Example**: `promote_on_last_owner_leave = false`
+
+When enabled, one of the group members is promoted to owner when the last owner leaves the room.
+Otherwise, leaving the room is rejected.
+
 ### `modules.mod_muc_light.config_schema`
   * **Syntax:** an array of `config_schema` items, as described below
   * **Default:**
@@ -150,19 +158,24 @@ which can be done with the [all_can_configure](#modulesmod_muc_lightall_can_conf
           internal_key = "display_lines"
 ```
 
-Defines fields allowed in the room configuration.
-
-Each `config_schema` item is a TOML table with the following keys:
+Defines fields allowed in the room configuration. Each `config_schema` item is a TOML table with the following keys:
 
 * `field` - mandatory, non-empty string - field name.
-* `string_value`, `integer_value`, `float_value` - exactly one of them has to be present, depending on the type of the field:
+* `string_value`, `integer_value`, `float_value`, `boolean_value` - exactly one of them has to be present, depending on the type of the field:
     * `string_value` - string,
     * `integer_value` - integer,
-    * `float_value` - floating-point number.
+    * `float_value` - floating-point number,
+    * `boolean_value` - boolean.
 * `internal_key` - optional, non-empty string - field name used in the internal representation, useful only for debugging or custom applications. By default it is the same as `field`.
 
-!!! WARNING
-    Lack of the `roomname` field will cause room names in Disco results and Roster items be set to the room username.
+The following internal fields are reserved and have special meaning:
+
+* `roomname` (`string_value`) - specifies the room name displayed in Disco results and Roster items. If not configured, the room username is used.
+* `all_can_configure` (`boolean_value`) - overrides the `all_can_configure` module option for the room.
+* `all_can_invite` (`boolean_value`) - overrides the `all_can_invite` module option for the room.
+
+!!! NOTE
+  Internal fields can be specified either explicitly using `internal_key` or implicitly by the value of `field`.
 
 ## Example Configuration
 

doc/open-extensions/muc_light.md

@@ -19,9 +19,9 @@ Here are some high-level features required from a new variant of MUC
 1. The full occupant list is always available to all occupants.
 1. The occupant is always visible on the list, even if they do not have any resources online.
 1. Occupants can only have two affiliations: owner and member.
-1. There MUST be at most one owner in the room (the service can choose to treat all users equally).
+1. There MUST be at least one owner in the room (the service can choose to treat all users equally).
 1. If the room becomes empty, it is destroyed.
-1. Occupants cannot hide behind nicks. Their real bare JID is always visible to everyone
+1. Occupants cannot hide behind nicks. Their real bare JID is always visible to everyone.
 1. No exchange of any `<presence/>` stanza inside the room.
 1. The user MUST be able to retrieve the list of rooms they occupy.
 1. The owner can modify the room configuration at any time; members may also be allowed to set configuration.
@@ -915,8 +915,8 @@ The occupant list is modified by a direct affiliation change. Following rules ap
 1. Every occupant can change its own affiliation to none in order to leave the room.
 1. The only way to join the room is being added by other occupant.
 1. The owner can change affiliations at will.
-1. If the owner leaves, the server MAY use any strategy to choose a new one.
-1. The room can have at most one owner. Giving someone else the 'owner' status effectively causes the current one to lose it.
+1. If the owner leaves, the server MAY use any strategy to choose a new one or reject the change.
+1. The room MAY be configured to have either a single owner or multiple owners. When configured for a single owner, giving someone else the 'owner' status effectively causes the current one to lose it.
 1. The owner can choose a new owner when leaving by including both 'none' and 'owner' items in affiliation change request.
 1. Every user JID can be used in the request at most once.
 1. A single request MAY change multiple affiliations.

include/mod_muc_light.hrl

@@ -16,6 +16,7 @@
 -define(DEFAULT_ROOMS_PER_PAGE, 10).
 -define(DEFAULT_ROOMS_IN_ROSTERS, false).
 -define(DEFAULT_ALLOW_MULTIPLE_OWNERS, false).
+-define(DEFAULT_PROMOTE_ON_LAST_OWNER_LEAVE, true).
 
 -type aff() :: owner | member | none.
 -type aff_user() :: {jid:simple_bare_jid(), aff()}.

src/muc_light/mod_muc_light.erl

@@ -241,6 +241,7 @@ config_spec() ->
                                                  validate = positive},
                  <<"rooms_in_rosters">> => #option{type = boolean},
                  <<"allow_multiple_owners">> => #option{type = boolean},
+                 <<"promote_on_last_owner_leave">> => #option{type = boolean},
                  <<"config_schema">> => #list{items = config_schema_spec(),
                                               process = fun ?MODULE:process_config_schema/1}
                 },
@@ -256,6 +257,7 @@ config_spec() ->
                     <<"rooms_per_page">> => ?DEFAULT_ROOMS_PER_PAGE,
                     <<"rooms_in_rosters">> => ?DEFAULT_ROOMS_IN_ROSTERS,
                     <<"allow_multiple_owners">> => ?DEFAULT_ALLOW_MULTIPLE_OWNERS,
+                    <<"promote_on_last_owner_leave">> => ?DEFAULT_PROMOTE_ON_LAST_OWNER_LEAVE,
                     <<"config_schema">> => default_schema()}
       }.
 
@@ -266,6 +268,7 @@ config_schema_spec() ->
                  <<"string_value">> => #option{type = binary},
                  <<"integer_value">> => #option{type = integer},
                  <<"float_value">> => #option{type = float},
+                 <<"boolean_value">> => #option{type = boolean},
                  <<"internal_key">> => #option{type = atom,
                                                validate = non_empty}
                 },
@@ -281,13 +284,28 @@ process_config_schema_item(#{field := FieldName} = FieldSpec) ->
     FieldTypes = schema_field_types(),
     case [K || K <- maps:keys(FieldTypes), maps:is_key(K, FieldSpec)] of
         [Key] ->
-            {FieldName, maps:get(Key, FieldSpec), InternalKey, maps:get(Key, FieldTypes)};
+            FieldType = maps:get(Key, FieldTypes),
+            Value = maps:get(Key, FieldSpec),
+            validate_reserved_field(InternalKey, FieldType),
+            {FieldName, Value, InternalKey, FieldType};
         _ ->
             error(#{what => invalid_schema_field_specification, field_spec => FieldSpec})
     end.
 
 schema_field_types() ->
-    #{string_value => binary, integer_value => integer, float_value => float}.
+    #{string_value => binary, integer_value => integer, float_value => float, boolean_value => boolean}.
+
+validate_reserved_field(Field, Type) ->
+    case maps:get(Field, reserved_fields(), Type) of
+        Type ->
+            ok;
+        Expected ->
+            error(#{what => invalid_reserved_field_type,
+                    field => Field, type => Type, expected => Expected})
+    end.
+
+reserved_fields() ->
+    #{roomname => binary, all_can_configure => boolean, all_can_invite => boolean}.
 
 -spec hooks(mongooseim:host_type()) -> gen_hook:hook_list().
 hooks(HostType) ->