Move max_users_per_domain option to auth section

Moved max_users_per_domain to the auth section in the configuration file. Adjusted tests. Made the name of the function and comments more clear.
esl · Jul 20, 2023 · 3516724 · 3516724
1 parent d14a055
commit 3516724
Show file tree

Hide file tree

Showing 9 changed files with 54 additions and 64 deletions.
diff --git a/big_tests/tests/graphql_account_SUITE.erl b/big_tests/tests/graphql_account_SUITE.erl
@@ -150,9 +150,9 @@ init_per_testcase(admin_check_plain_password_hash = C, Config) ->
 init_per_testcase(admin_register_user_limit_error = C, Config) ->
     Domain = domain_helper:domain(),
     {ok, HostType} = rpc(mim(), mongoose_domain_api, get_domain_host_type, [Domain]),
-    OptKey = {max_users_per_domain, HostType},
+    OptKey = [{auth, HostType}, max_users_per_domain],
     Config1 = mongoose_helper:backup_and_set_config_option(Config, OptKey, 3),
-    Config2 = [{user1, <<"bob">>}, {user2, <<"kate">>}, {user3, <<"john">>} | Config1],
+    Config2 = [{bob, <<"bob">>}, {kate, <<"kate">>}, {john, <<"john">>} | Config1],
     escalus:init_per_testcase(C, Config2);
 init_per_testcase(domain_admin_check_plain_password_hash_no_permission = C, Config) ->
     {_, AuthMods} = lists:keyfind(ctl_auth_mods, 1, Config),
@@ -181,8 +181,9 @@ end_per_testcase(admin_check_plain_password_hash, Config) ->
     escalus:delete_users(Config, escalus:get_users([carol]));
 end_per_testcase(admin_register_user_limit_error = C, Config) ->
     Domain = domain_helper:domain(),
-    rpc(mim(), mongoose_account_api, unregister_user, [proplists:get_value(user1, Config), Domain]),
-    rpc(mim(), mongoose_account_api, unregister_user, [proplists:get_value(user2, Config), Domain]),
+    rpc(mim(), mongoose_account_api, unregister_user, [proplists:get_value(bob, Config), Domain]),
+    rpc(mim(), mongoose_account_api, unregister_user, [proplists:get_value(kate, Config), Domain]),
+    rpc(mim(), mongoose_account_api, unregister_user, [proplists:get_value(john, Config), Domain]),
     mongoose_helper:restore_config(Config),
     escalus:end_per_testcase(C, Config);
 end_per_testcase(domain_admin_check_plain_password_hash_no_permission, Config) ->
@@ -372,15 +373,20 @@ admin_register_user_limit_error(Config) ->
     Password = <<"password">>,
     Domain = domain_helper:domain(),
     Path = [data, account, registerUser, message],
-    list_users(Domain, Config),
-    Resp1 = register_user(Domain, proplists:get_value(user1, Config), Password, Config),
+    Resp1 = register_user(Domain, proplists:get_value(bob, Config), Password, Config),
     ?assertNotEqual(nomatch, binary:match(get_ok_value(Path, Resp1), <<"successfully registered">>)),
-    Resp2 = register_user(Domain, proplists:get_value(user2, Config), Password, Config),
+    Resp2 = register_user(Domain, proplists:get_value(kate, Config), Password, Config),
     ?assertNotEqual(nomatch, binary:match(get_ok_value(Path, Resp2), <<"successfully registered">>)),
     %% One user was registered in the init_per_group, and two more were registered in this test case
-    %% The next registration should exceed the limit of three
-    Resp3 = register_user(Domain, proplists:get_value(user3, Config), Password, Config),
-    ?assertMatch({_, _}, binary:match(get_err_msg(Resp3), <<"limit has been exceeded">>)).
+    %% There are three registered users at this moment
+    %% The next (fourth) registration should exceed the limit of three
+    JohnNick = proplists:get_value(john, Config),
+    Resp3 = register_user(Domain, JohnNick, Password, Config),
+    ?assertMatch({_, _}, binary:match(get_err_msg(Resp3), <<"limit has been exceeded">>)),
+    %% Make sure the fourth account wasn't created
+    CheckUserPath = [data, account, checkUser],
+    Resp4 = check_user(<<JohnNick/binary, "@", Domain/binary>>, Config),
+    ?assertMatch(#{<<"exist">> := false, <<"message">> := _}, get_ok_value(CheckUserPath, Resp4)).
 
 admin_remove_non_existing_user(Config) ->
     % Non-existing user, non-existing domain

diff --git a/doc/configuration/auth.md b/doc/configuration/auth.md
@@ -81,6 +81,16 @@ There are three possible ways of using the `SASL EXTERNAL` mechanism:
 
 This option allows you to list the enabled ones in the order of preference (they are tried until one succeeds or the list is exhausted).
 
+### `auth.max_users_per_domain`
+* **Syntax:** positive integer or string `"infinity"`, representing maximum amount of users that can be registered in a domain
+* **Default:** `"infinity"`
+* **Example:** `max_users_per_domain = 10000`
+
+Limits the number of users that can be registered for each domain. If the option is configured to the value `"infinity"`, no limit is present.
+
+!!! Warning
+    The limit only works for the following authentication methods: `internal`, `rdbms` and `ldap`.
+
 ## Password-related options
 
 These options are common to the `http`, `rdbms` and `internal` methods.

diff --git a/doc/configuration/general.md b/doc/configuration/general.md
@@ -157,13 +157,6 @@ See the section about [redis connection setup](./outgoing-connections.md#redis-s
 
 When a user's session is replaced (due to a full JID conflict) by a new one, this parameter specifies the time MongooseIM waits for the old sessions to close. The default value is sufficient in most cases. If you observe `replaced_wait_timeout` warning in logs, then most probably the old sessions are frozen for some reason and it should be investigated.
 
-### `general.max_users_per_domain`
-* **Syntax:** positive integer or string `"infinity"`, representing maximum amount of users that can be registered in a domain
-* **Default:** `"infinity"`
-* **Example:** `max_users_per_domain = 10000`
-
-Limits the number of users that can be registered for each domain. If the option is configured to the value `"infinity"`, no limit is present.
-
 ## Message routing
 
 The following options influence the way MongooseIM routes incoming messages to their recipients.

diff --git a/doc/configuration/host_config.md b/doc/configuration/host_config.md
@@ -38,7 +38,6 @@ The following options are allowed:
 
 * [`route_subdomains`](general.md#generalroute_subdomains)
 * [`replaced_wait_timeout`](general.md#generalreplaced_wait_timeout)
-* [`max_users_per_domain`](general.md#generalmax_users_per_domain)
 
 #### Example
 

diff --git a/src/auth/ejabberd_auth.erl b/src/auth/ejabberd_auth.erl
@@ -236,11 +236,11 @@ do_try_register_if_does_not_exist(_, JID, Password) ->
                 end
         end,
     Opts = #{default => {error, not_allowed}, metric => try_register},
-    case is_user_limit_per_domain_exceeded(LServer) of
+    case is_user_number_below_limit(LServer) of
         true ->
-            {error, limit_per_domain_exceeded};
+            call_auth_modules_for_domain(LServer, F, Opts);
         false ->
-            call_auth_modules_for_domain(LServer, F, Opts)
+            {error, limit_per_domain_exceeded}
     end.
 
 %% @doc Registered users list do not include anonymous users logged
@@ -584,12 +584,12 @@ fold_auth_modules([AuthModule | AuthModules], F, CurAcc) ->
             Value
     end.
 
-is_user_limit_per_domain_exceeded(Domain) ->
+is_user_number_below_limit(Domain) ->
     case mongoose_domain_api:get_domain_host_type(Domain) of
         {ok, HostType} ->
-            Limit = mongoose_config:get_opt({max_users_per_domain, HostType}),
+            Limit = mongoose_config:get_opt([{auth, HostType}, max_users_per_domain]),
             Current = get_vh_registered_users_number(Domain),
-            Current >= Limit;
+            Current < Limit;
         {error, not_found} ->
-            false
+            true
     end.
diff --git a/src/config/mongoose_config_spec.erl b/src/config/mongoose_config_spec.erl
@@ -194,10 +194,7 @@ general() ->
                                                     wrap = global_config},
                  <<"domain_certfile">> => #list{items = domain_cert(),
                                                 format_items = map,
-                                                wrap = global_config},
-                 <<"max_users_per_domain">> => #option{type = int_or_infinity,
-                                                          validate = positive,
-                                                          wrap = host_config}
+                                                wrap = global_config}
                 },
        wrap = none,
        format_items = list
@@ -215,8 +212,7 @@ general_defaults() ->
       <<"mongooseimctl_access_commands">> => #{},
       <<"routing_modules">> => mongoose_router:default_routing_modules(),
       <<"replaced_wait_timeout">> => 2000,
-      <<"hide_service_name">> => false,
-      <<"max_users_per_domain">> => infinity}.
+      <<"hide_service_name">> => false}.
 
 ctl_access_rule() ->
     #section{
@@ -402,10 +398,13 @@ auth() ->
                       <<"sasl_mechanisms">> =>
                           #list{items = #option{type = atom,
                                                 validate = {module, cyrsasl},
-                                                process = fun ?MODULE:process_sasl_mechanism/1}}
+                                                process = fun ?MODULE:process_sasl_mechanism/1}},
+                      <<"max_users_per_domain">> => #option{type = int_or_infinity,
+                                                            validate = positive}
                      },
        defaults = #{<<"sasl_external">> => [standard],
-                    <<"sasl_mechanisms">> => cyrsasl:default_modules()},
+                    <<"sasl_mechanisms">> => cyrsasl:default_modules(),
+                    <<"max_users_per_domain">> => infinity},
        process = fun ?MODULE:process_auth/1,
        wrap = host_config
       }.

diff --git a/test/common/config_parser_helper.erl b/test/common/config_parser_helper.erl
@@ -52,12 +52,7 @@ options("host_types") ->
      {{replaced_wait_timeout, <<"localhost">>}, 2000},
      {{replaced_wait_timeout, <<"some host type">>}, 2000},
      {{replaced_wait_timeout, <<"this is host type">>}, 2000},
-     {{replaced_wait_timeout, <<"yet another host type">>}, 2000},
-     {{max_users_per_domain, <<"another host type">>}, infinity},
-     {{max_users_per_domain, <<"localhost">>}, infinity},
-     {{max_users_per_domain, <<"some host type">>}, infinity},
-     {{max_users_per_domain, <<"this is host type">>}, infinity},
-     {{max_users_per_domain, <<"yet another host type">>}, infinity}];
+     {{replaced_wait_timeout, <<"yet another host type">>}, 2000}];
 options("miscellaneous") ->
     [{all_metrics_are_global, false},
      {http_server_name, "Apache"},
@@ -104,9 +99,7 @@ options("miscellaneous") ->
      {{replaced_wait_timeout, <<"anonymous.localhost">>}, 2000},
      {{replaced_wait_timeout, <<"localhost">>}, 2000},
      {{route_subdomains, <<"anonymous.localhost">>}, s2s},
-     {{route_subdomains, <<"localhost">>}, s2s},
-     {{max_users_per_domain, <<"anonymous.localhost">>}, infinity},
-     {{max_users_per_domain, <<"localhost">>}, infinity}];
+     {{route_subdomains, <<"localhost">>}, s2s}];
 options("modules") ->
     [{all_metrics_are_global, false},
      {default_server_domain, <<"localhost">>},
@@ -130,9 +123,7 @@ options("modules") ->
      {{modules, <<"dummy_host">>}, all_modules()},
      {{modules, <<"localhost">>}, all_modules()},
      {{replaced_wait_timeout, <<"dummy_host">>}, 2000},
-     {{replaced_wait_timeout, <<"localhost">>}, 2000},
-     {{max_users_per_domain, <<"dummy_host">>}, infinity},
-     {{max_users_per_domain, <<"localhost">>}, infinity}];
+     {{replaced_wait_timeout, <<"localhost">>}, 2000}];
 options("mongooseim-pgsql") ->
     [{all_metrics_are_global, false},
      {default_server_domain, <<"localhost">>},
@@ -289,9 +280,6 @@ options("mongooseim-pgsql") ->
      {{access, <<"anonymous.localhost">>}, pgsql_access()},
      {{access, <<"localhost">>}, pgsql_access()},
      {{access, <<"localhost.bis">>}, pgsql_access()},
-     {{max_users_per_domain, <<"anonymous.localhost">>}, infinity},
-     {{max_users_per_domain, <<"localhost">>}, infinity},
-     {{max_users_per_domain, <<"localhost.bis">>}, infinity},
      {{acl, global}, #{local => [#{match => current_domain,
                                    user_regexp => <<>>}]}},
      {{acl, <<"anonymous.localhost">>}, #{local => [#{match => current_domain,
@@ -365,10 +353,7 @@ options("outgoing_pools") ->
      {{modules, <<"localhost.bis">>}, #{}},
      {{replaced_wait_timeout, <<"anonymous.localhost">>}, 2000},
      {{replaced_wait_timeout, <<"localhost">>}, 2000},
-     {{replaced_wait_timeout, <<"localhost.bis">>}, 2000},
-     {{max_users_per_domain, <<"anonymous.localhost">>}, infinity},
-     {{max_users_per_domain, <<"localhost">>}, infinity},
-     {{max_users_per_domain, <<"localhost.bis">>}, infinity}];
+     {{replaced_wait_timeout, <<"localhost.bis">>}, 2000}];
 options("s2s_only") ->
     [{all_metrics_are_global, false},
      {default_server_domain, <<"localhost">>},
@@ -392,9 +377,7 @@ options("s2s_only") ->
      {{replaced_wait_timeout, <<"dummy_host">>}, 2000},
      {{replaced_wait_timeout, <<"localhost">>}, 2000},
      {{s2s, <<"dummy_host">>}, custom_s2s()},
-     {{s2s, <<"localhost">>}, custom_s2s()},
-     {{max_users_per_domain, <<"dummy_host">>}, infinity},
-     {{max_users_per_domain, <<"localhost">>}, infinity}].
+     {{s2s, <<"localhost">>}, custom_s2s()}].
 
 all_modules() ->
     #{mod_mam_rdbms_user => #{muc => true, pm => true},
@@ -707,7 +690,8 @@ default_auth() ->
       password => #{format => scram,
                     scram_iterations => 10000},
       sasl_external => [standard],
-      sasl_mechanisms => cyrsasl:default_modules()}.
+      sasl_mechanisms => cyrsasl:default_modules(),
+      max_users_per_domain => infinity}.
 
 pgsql_s2s() ->
     Outgoing = (default_s2s_outgoing())#{port => 5299},

diff --git a/test/config_parser_SUITE.erl b/test/config_parser_SUITE.erl
@@ -444,12 +444,6 @@ domain_certfile(_Config) ->
      || K <- maps:keys(DomCert)],
     ?err(#{<<"general">> => #{<<"domain_certfile">> => [DomCert, DomCert]}}).
 
-max_users_per_domain(_Config) ->
-    ?cfg({max_users_per_domain, ?HOST}, infinity, #{}), % global default
-    ?cfgh(max_users_per_domain, 1000, #{<<"general">> =>
-                                           #{<<"max_users_per_domain">> => 1000}}),
-    ?errh(#{<<"general">> => #{<<"max_users_per_domain">> => 0}}).
-
 %% tests: listen
 
 listen_duplicate(_Config) ->
@@ -734,6 +728,12 @@ auth_sasl_mechanisms(_Config) ->
           #{<<"auth">> => #{<<"sasl_mechanisms">> => [<<"external">>, <<"scram">>]}}),
     ?errh(#{<<"auth">> => #{<<"sasl_mechanisms">> => [<<"none">>]}}).
 
+max_users_per_domain(_Config) ->
+    ?cfg([{auth, ?HOST}, max_users_per_domain], infinity, #{}), % global default
+    ?cfgh([auth, max_users_per_domain], 1000, #{<<"auth">> =>
+                                                #{<<"max_users_per_domain">> => 1000}}),
+    ?errh(#{<<"auth">> => #{<<"max_users_per_domain">> => 0}}).
+
 auth_allow_multiple_connections(_Config) ->
     ?cfgh([auth, anonymous, allow_multiple_connections], true,
           auth_raw(<<"anonymous">>, #{<<"allow_multiple_connections">> => true})),

diff --git a/test/mongoose_config_SUITE.erl b/test/mongoose_config_SUITE.erl
@@ -190,8 +190,7 @@ minimal_config_opts() ->
      {{auth, <<"localhost">>}, config_parser_helper:default_auth()},
      {{modules, <<"localhost">>}, #{}},
      {{replaced_wait_timeout, <<"localhost">>}, 2000},
-     {{s2s, <<"localhost">>}, config_parser_helper:default_s2s()},
-     {{max_users_per_domain, <<"localhost">>}, infinity}].
+     {{s2s, <<"localhost">>}, config_parser_helper:default_s2s()}].
 
 start_slave_node(Config) ->
     SlaveNode = do_start_slave_node(),