Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Limit the number of users per domain #4059

Merged
merged 3 commits into from
Jul 21, 2023
Merged

Limit the number of users per domain #4059

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4c5892e
Add max_users_per_domain option to config
jacekwegr Jul 17, 2023
d14a055
Add test to graphql_account_SUITE.erl
jacekwegr Jul 19, 2023
3516724
Move max_users_per_domain option to auth section
jacekwegr Jul 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions big_tests/tests/graphql_account_SUITE.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ admin_account_tests() ->
admin_check_non_existing_user,
admin_register_user,
admin_register_random_user,
admin_register_user_non_existing_domain,
admin_register_user_limit_error,
admin_remove_non_existing_user,
admin_remove_existing_user,
admin_ban_user,
Expand Down Expand Up @@ -78,6 +80,7 @@ domain_admin_tests() ->
domain_admin_register_user_no_permission,
admin_register_random_user,
domain_admin_register_random_user_no_permission,
admin_register_user_limit_error,
admin_remove_existing_user,
domain_admin_remove_user_no_permission,
admin_ban_user,
Expand Down Expand Up @@ -144,6 +147,13 @@ init_per_testcase(admin_check_plain_password_hash = C, Config) ->
Config2 = escalus:create_users(Config1, escalus:get_users([carol])),
escalus:init_per_testcase(C, Config2)
end;
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},
Config1 = mongoose_helper:backup_and_set_config_option(Config, OptKey, 3),
Config2 = [{user1, <<"bob">>}, {user2, <<"kate">>}, {user3, <<"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),
case lists:member(ejabberd_auth_ldap, AuthMods) of
Expand All @@ -169,6 +179,12 @@ end_per_testcase(admin_register_user = C, Config) ->
end_per_testcase(admin_check_plain_password_hash, Config) ->
mongoose_helper:restore_config(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]),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be good to delete all users here to make sure that if the user creation succeeds, all accounts are deleted and the state is clean.

mongoose_helper:restore_config(Config),
escalus:end_per_testcase(C, Config);
end_per_testcase(domain_admin_check_plain_password_hash_no_permission, Config) ->
mongoose_helper:restore_config(Config),
escalus:delete_users(Config, escalus:get_users([carol, alice_bis]));
Expand Down Expand Up @@ -347,6 +363,25 @@ admin_register_random_user(Config) ->
?assertNotEqual(nomatch, binary:match(Msg, <<"successfully registered">>)),
{ok, _} = rpc(mim(), mongoose_account_api, unregister_user, [Username, Server]).

admin_register_user_non_existing_domain(Config) ->
% Try to register a user with a non-existing domain
Resp = register_user(<<"unknown">>, <<"alice">>, <<"test_password">>, Config),
?assertMatch({_, _}, binary:match(get_err_msg(Resp), <<"not_allowed">>)).

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),
?assertNotEqual(nomatch, binary:match(get_ok_value(Path, Resp1), <<"successfully registered">>)),
Resp2 = register_user(Domain, proplists:get_value(user2, 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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Three does not exceed the limit of three, to be precise.

Resp3 = register_user(Domain, proplists:get_value(user3, Config), Password, Config),
?assertMatch({_, _}, binary:match(get_err_msg(Resp3), <<"limit has been exceeded">>)).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a note: maybe make sure the third account is not created?


admin_remove_non_existing_user(Config) ->
% Non-existing user, non-existing domain
Resp = remove_user(?NOT_EXISTING_JID, Config),
Expand Down
7 changes: 7 additions & 0 deletions doc/configuration/general.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,13 @@ 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`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is in the wrong section - it is user account management, not session management.

Just a question: why not place it in the auth section which has all options related to user accounts?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’ve chosen to include it in the general section as I believed it was more connected with the domains themselves rather than user authentication. However, if it would be more appropriate, I can move it to the auth section.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually it only limits certain authentication methods, because some of them (external, http, jwt, pki, ...) don't create users the usual way, so I think it's best placed in the auth section - not to give the impression that it works for all methods.

* **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.
Expand Down
1 change: 1 addition & 0 deletions doc/configuration/host_config.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ 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

Expand Down
21 changes: 18 additions & 3 deletions src/auth/ejabberd_auth.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ set_password(#jid{luser = LUser, lserver = LServer}, Password) ->
end.

-spec try_register(jid:jid() | error, binary()) ->
ok | {error, exists | not_allowed | invalid_jid | null_password}.
ok | {error, exists | not_allowed | invalid_jid | null_password | limit_per_domain_exceeded}.
try_register(_, <<>>) ->
{error, null_password};
try_register(#jid{luser = <<>>}, _) ->
Expand All @@ -221,7 +221,7 @@ try_register(JID, Password) ->
do_try_register_if_does_not_exist(Exists, JID, Password).

-spec do_try_register_if_does_not_exist(boolean(), jid:jid(), binary()) ->
ok | {error, exists | not_allowed | invalid_jid | null_password}.
ok | {error, exists | not_allowed | invalid_jid | null_password | limit_per_domain_exceeded}.
do_try_register_if_does_not_exist(true, _, _) ->
{error, exists};
do_try_register_if_does_not_exist(_, JID, Password) ->
Expand All @@ -236,7 +236,12 @@ do_try_register_if_does_not_exist(_, JID, Password) ->
end
end,
Opts = #{default => {error, not_allowed}, metric => try_register},
call_auth_modules_for_domain(LServer, F, Opts).
case is_user_limit_per_domain_exceeded(LServer) of
true ->
{error, limit_per_domain_exceeded};
false ->
call_auth_modules_for_domain(LServer, F, Opts)
end.

%% @doc Registered users list do not include anonymous users logged
-spec get_vh_registered_users(Server :: jid:server()) -> [jid:simple_bare_jid()].
Expand Down Expand Up @@ -578,3 +583,13 @@ fold_auth_modules([AuthModule | AuthModules], F, CurAcc) ->
{stop, Value} ->
Value
end.

is_user_limit_per_domain_exceeded(Domain) ->
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function name is misleading. If there are exactly as many users as the limit, it is not exceeded, yet it returns true.

case mongoose_domain_api:get_domain_host_type(Domain) of
{ok, HostType} ->
Limit = mongoose_config:get_opt({max_users_per_domain, HostType}),
Current = get_vh_registered_users_number(Domain),
Current >= Limit;
{error, not_found} ->
false
end.
8 changes: 6 additions & 2 deletions src/config/mongoose_config_spec.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,10 @@ general() ->
wrap = global_config},
<<"domain_certfile">> => #list{items = domain_cert(),
format_items = map,
wrap = global_config}
wrap = global_config},
<<"max_users_per_domain">> => #option{type = int_or_infinity,
validate = positive,
wrap = host_config}
},
wrap = none,
format_items = list
Expand All @@ -212,7 +215,8 @@ general_defaults() ->
<<"mongooseimctl_access_commands">> => #{},
<<"routing_modules">> => mongoose_router:default_routing_modules(),
<<"replaced_wait_timeout">> => 2000,
<<"hide_service_name">> => false}.
<<"hide_service_name">> => false,
<<"max_users_per_domain">> => infinity}.

ctl_access_rule() ->
#section{
Expand Down
6 changes: 5 additions & 1 deletion src/mongoose_account_api.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@
check_password_hash/4,
import_users/1]).

-type register_result() :: {ok | exists | invalid_jid | cannot_register, iolist()}.
-type register_result() :: {ok | exists | invalid_jid | cannot_register |
limit_per_domain_exceeded, iolist()}.

-type unregister_result() :: {ok | not_allowed | invalid_jid | user_does_not_exist, string()}.

Expand Down Expand Up @@ -87,6 +88,9 @@ register_user(User, Host, Password) ->
{error, invalid_jid} ->
String = io_lib:format("Invalid JID ~s@~s", [User, Host]),
{invalid_jid, String};
{error, limit_per_domain_exceeded} ->
String = io_lib:format("User limit has been exceeded for domain ~s", [Host]),
{limit_per_domain_exceeded, String};
{error, Reason} ->
String =
io_lib:format("Can't register user ~s at node ~p: ~p",
Expand Down
3 changes: 2 additions & 1 deletion src/mongoose_import_users.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@
-define(REGISTER_WORKERS_NUM, 10).

-type summary() :: #{reason() => [jid:jid() | binary()]}.
-type reason() :: ok | exists | not_allowed | invalid_jid | null_password | bad_csv.
-type reason() :: ok | exists | not_allowed | invalid_jid | null_password |
limit_per_domain_exceeded | bad_csv.

-export_type([summary/0, reason/0]).

Expand Down
27 changes: 22 additions & 5 deletions test/common/config_parser_helper.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,12 @@ 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}];
{{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}];
options("miscellaneous") ->
[{all_metrics_are_global, false},
{http_server_name, "Apache"},
Expand Down Expand Up @@ -99,7 +104,9 @@ options("miscellaneous") ->
{{replaced_wait_timeout, <<"anonymous.localhost">>}, 2000},
{{replaced_wait_timeout, <<"localhost">>}, 2000},
{{route_subdomains, <<"anonymous.localhost">>}, s2s},
{{route_subdomains, <<"localhost">>}, s2s}];
{{route_subdomains, <<"localhost">>}, s2s},
{{max_users_per_domain, <<"anonymous.localhost">>}, infinity},
{{max_users_per_domain, <<"localhost">>}, infinity}];
options("modules") ->
[{all_metrics_are_global, false},
{default_server_domain, <<"localhost">>},
Expand All @@ -123,7 +130,9 @@ options("modules") ->
{{modules, <<"dummy_host">>}, all_modules()},
{{modules, <<"localhost">>}, all_modules()},
{{replaced_wait_timeout, <<"dummy_host">>}, 2000},
{{replaced_wait_timeout, <<"localhost">>}, 2000}];
{{replaced_wait_timeout, <<"localhost">>}, 2000},
{{max_users_per_domain, <<"dummy_host">>}, infinity},
{{max_users_per_domain, <<"localhost">>}, infinity}];
options("mongooseim-pgsql") ->
[{all_metrics_are_global, false},
{default_server_domain, <<"localhost">>},
Expand Down Expand Up @@ -280,6 +289,9 @@ 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,
Expand Down Expand Up @@ -353,7 +365,10 @@ options("outgoing_pools") ->
{{modules, <<"localhost.bis">>}, #{}},
{{replaced_wait_timeout, <<"anonymous.localhost">>}, 2000},
{{replaced_wait_timeout, <<"localhost">>}, 2000},
{{replaced_wait_timeout, <<"localhost.bis">>}, 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}];
options("s2s_only") ->
[{all_metrics_are_global, false},
{default_server_domain, <<"localhost">>},
Expand All @@ -377,7 +392,9 @@ options("s2s_only") ->
{{replaced_wait_timeout, <<"dummy_host">>}, 2000},
{{replaced_wait_timeout, <<"localhost">>}, 2000},
{{s2s, <<"dummy_host">>}, custom_s2s()},
{{s2s, <<"localhost">>}, custom_s2s()}].
{{s2s, <<"localhost">>}, custom_s2s()},
{{max_users_per_domain, <<"dummy_host">>}, infinity},
{{max_users_per_domain, <<"localhost">>}, infinity}].

all_modules() ->
#{mod_mam_rdbms_user => #{muc => true, pm => true},
Expand Down
11 changes: 9 additions & 2 deletions test/config_parser_SUITE.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,8 @@ groups() ->
routing_modules,
replaced_wait_timeout,
hide_service_name,
domain_certfile]},
domain_certfile,
max_users_per_domain]},
{listen, [parallel], [listen_duplicate,
listen_c2s,
listen_c2s_fast_tls,
Expand Down Expand Up @@ -443,6 +444,12 @@ 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) ->
Expand Down Expand Up @@ -3071,7 +3078,7 @@ create_files(Config) ->

ensure_copied(From, To) ->
case file:copy(From, To) of
{ok,_} ->
{ok, _} ->
ok;
Other ->
error(#{what => ensure_copied_failed, from => From, to => To,
Expand Down
3 changes: 2 additions & 1 deletion test/mongoose_config_SUITE.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,8 @@ minimal_config_opts() ->
{{auth, <<"localhost">>}, config_parser_helper:default_auth()},
{{modules, <<"localhost">>}, #{}},
{{replaced_wait_timeout, <<"localhost">>}, 2000},
{{s2s, <<"localhost">>}, config_parser_helper:default_s2s()}].
{{s2s, <<"localhost">>}, config_parser_helper:default_s2s()},
{{max_users_per_domain, <<"localhost">>}, infinity}].

start_slave_node(Config) ->
SlaveNode = do_start_slave_node(),
Expand Down