Skip to content

Commit

Permalink
Fix metrics for auth backend not being reported
Browse files Browse the repository at this point in the history
  • Loading branch information
NelsonVides committed Mar 11, 2022
1 parent ef8b27c commit 7e08200
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 27 deletions.
57 changes: 40 additions & 17 deletions big_tests/tests/auth_methods_for_c2s_SUITE.erl
Original file line number Diff line number Diff line change
@@ -1,38 +1,50 @@
-module(auth_methods_for_c2s_SUITE).

-compile([export_all, nowarn_export_all]).

-include_lib("common_test/include/ct.hrl").
-include_lib("eunit/include/eunit.hrl").
-include_lib("exml/include/exml.hrl").

-import(distributed_helper, [mim/0, rpc/4]).

all() ->
[{group, two_methods_enabled}].
[
{group, two_methods_enabled},
{group, metrics}
].

groups() ->
[{two_methods_enabled, [parallel],
[can_login_with_allowed_method,
[
{two_methods_enabled, [parallel],
[
can_login_with_allowed_method,
cannot_login_with_not_allowed_method,
can_login_to_another_listener]}].
can_login_to_another_listener
]},
{metrics, [],
[
metrics_incremented_on_user_connect
]}
].

init_per_suite(Config) ->
Config0 = escalus:init_per_suite(Config),
Config1 = ejabberd_node_utils:init(Config0),
ejabberd_node_utils:backup_config_file(Config1),
Config1.
escalus:init_per_suite(Config).

end_per_suite(Config) ->
ejabberd_node_utils:restore_config_file(Config),
ejabberd_node_utils:restart_application(mongooseim),
escalus:end_per_suite(Config).

init_per_group(_, Config) ->
modify_config_and_restart(Config),
escalus_cleaner:start(Config).
init_per_group(metrics, Config) ->
Config;
init_per_group(_, Config0) ->
Config1 = ejabberd_node_utils:init(Config0),
ejabberd_node_utils:backup_config_file(Config1),
modify_config_and_restart(Config1),
escalus_cleaner:start(Config1).

end_per_group(_, _Config) ->
end_per_group(metrics, _Config) ->
escalus_fresh:clean();
end_per_group(_, Config) ->
ejabberd_node_utils:restore_config_file(Config),
ejabberd_node_utils:restart_application(mongooseim),
escalus_fresh:clean().

init_per_testcase(TC, Config) ->
Expand Down Expand Up @@ -66,8 +78,19 @@ can_login_to_another_listener(Config) ->
{password, <<"wrong">>}|Spec],
{ok, _, _} = escalus_connection:start(Spec2).

%% Helpers
metrics_incremented_on_user_connect(ConfigIn) ->
F = fun(Alice, Bob) ->
Body = <<"Hello Bob">>,
escalus:send(Alice, escalus_stanza:chat_to(Bob, Body)),
escalus:assert(is_chat_message, [Body], escalus:wait_for_stanza(Bob))
end,
HostType = domain_helper:host_type(),
HostTypePrefix = domain_helper:make_metrics_prefix(HostType),
MongooseMetrics = [{[HostTypePrefix, backends, auth, authorize], changed}],
Config = [{mongoose_metrics, MongooseMetrics} | ConfigIn],
escalus_fresh:story(Config, [{alice, 1}, {bob, 1}], F).

%% Helpers
%% If dummy backend is enabled, it is not possible to create new users
%% (we check if an user does exist before registering the user).
register_internal_user(Spec) ->
Expand Down
12 changes: 11 additions & 1 deletion big_tests/tests/mongooseimctl_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -439,11 +439,21 @@ check_password_hash(Config) ->

check_password(Config) ->
{User, Domain, Pass} = get_user_data(alice, Config),

MetricName = [backends, auth, check_password],
OldValue = get_metric(MetricName),
{_, 0} = mongooseimctl("check_password", [User, Domain, Pass], Config),
{_, ErrCode} = mongooseimctl("check_password", [User, Domain, <<Pass/binary, "Bad">>], Config),
mongoose_helper:wait_until(
fun() -> get_metric(MetricName) end, true,
#{validator => fun(NewValue) -> OldValue =/= NewValue end, name => ?FUNCTION_NAME}),
true = (ErrCode =/= 0). %% Must return code other than 0

get_metric(MetricName) ->
HostType = domain_helper:host_type(mim),
HostTypePrefix = domain_helper:make_metrics_prefix(HostType),
{ok, Value} = rpc(mim(), mongoose_metrics, get_metric_value, [[HostTypePrefix | MetricName]]),
Value.

check_account(Config) ->
{User, Domain, _Pass} = get_user_data(alice, Config),

Expand Down
32 changes: 23 additions & 9 deletions src/auth/ejabberd_auth.erl
Original file line number Diff line number Diff line change
Expand Up @@ -295,8 +295,7 @@ get_passterm_with_authmodule(HostType, #jid{luser = LUser, lserver = LServer}) -
-spec does_user_exist(JID :: jid:jid() | error) -> boolean().
does_user_exist(#jid{luser = LUser, lserver = LServer}) ->
F = fun(HostType, Mod) -> does_user_exist_in_module(HostType, LUser, LServer, Mod) end,
case call_auth_modules_for_domain(LServer, F,
#{default => false, metric => does_user_exist}) of
case call_auth_modules_for_domain(LServer, F, #{default => false, metric => does_user_exist}) of
{error, _Error} -> false;
Result -> Result
end;
Expand All @@ -314,10 +313,8 @@ does_user_exist(HostType, Jid, RequestType) ->
does_user_exist(false, HostType, Jid, stored) ->
true =:= does_stored_user_exist(HostType, Jid);
does_user_exist(false, HostType, #jid{luser = LUser, lserver = LServer}, with_anonymous) ->
F = fun(Mod) ->
does_user_exist_in_module(HostType, LUser, LServer, Mod)
end,
call_auth_modules_for_host_type(HostType, F, #{default => false});
F = fun(Mod) -> does_user_exist_in_module(HostType, LUser, LServer, Mod) end,
call_auth_modules_for_host_type(HostType, F, #{default => false, metric => does_user_exist});
does_user_exist(Status, _, _, _) ->
Status.

Expand All @@ -329,7 +326,7 @@ does_stored_user_exist(HostType, #jid{luser = LUser, lserver = LServer}) ->
F = fun(Mod) when Mod =/= ejabberd_auth_anonymous ->
does_user_exist_in_module(HostType, LUser, LServer, Mod)
end,
call_auth_modules_for_host_type(HostType, F, #{default => false});
call_auth_modules_for_host_type(HostType, F, #{default => false, metric => does_user_exist});
does_stored_user_exist(_HostType, error) ->
false.

Expand Down Expand Up @@ -511,14 +508,31 @@ bind_host_type(HostType, F) when is_function(F, 2) ->
mod_res() | [mod_res()].
call_auth_modules_for_host_type(HostType, F, Opts) ->
Modules = auth_modules_for_host_type(HostType),
call_auth_modules(Modules, F, Opts).
case maps:take(metric, Opts) of
{Metric, NewOpts} ->
{Time, Result} = timer:tc(fun call_auth_modules/3, [Modules, F, NewOpts]),
mongoose_metrics:update(HostType, ?METRIC(Metric), Time),
Result;
error ->
call_auth_modules(Modules, F, Opts)
end.

-spec call_auth_modules_with_creds(mongoose_credentials:t(),
mod_fun() | mod_fold_fun(), call_opts()) ->
mod_res() | [mod_res()].
call_auth_modules_with_creds(Creds, F, Opts) ->
Modules = mongoose_credentials:auth_modules(Creds),
call_auth_modules(Modules, F, Opts).
case maps:take(metric, Opts) of
{Metric, NewOpts} ->
HostType = mongoose_credentials:host_type(Creds),
{Time, Result} = timer:tc(fun call_auth_modules/3,
[Modules, F, NewOpts]),
mongoose_metrics:update(HostType, ?METRIC(Metric), Time),
Result;
error ->
call_auth_modules(Modules, F, Opts)
end.


%% @doc Perform a map or a fold operation with function F over the provided Modules
-spec call_auth_modules([authmodule()], mod_fun() | mod_fold_fun(), call_opts()) ->
Expand Down

0 comments on commit 7e08200

Please sign in to comment.