From 12d47455bac10b57cff44d1af37744d4ffaf0feb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chmielowski?= Date: Wed, 11 Oct 2023 14:16:51 +0200 Subject: [PATCH] Add `auth_external_user_exists_check` option This makes `user_check` hook work better with authentication methods that don't have a way to determine if user exists (like is the case for jwt and cert based authentication), and as result will improve mod_offline and mod_mam handling of offline messages to those users. This reuses information stored by `mod_last` for this purpose. Should fix issue #3377. --- src/ejabberd_auth.erl | 54 +++++++++++++++++++++++------------- src/ejabberd_option.erl | 8 ++++++ src/ejabberd_options.erl | 3 ++ src/ejabberd_options_doc.erl | 7 +++++ 4 files changed, 52 insertions(+), 20 deletions(-) diff --git a/src/ejabberd_auth.erl b/src/ejabberd_auth.erl index 027983e61..321527eda 100644 --- a/src/ejabberd_auth.erl +++ b/src/ejabberd_auth.erl @@ -398,15 +398,29 @@ user_exists(_User, <<"">>) -> user_exists(User, Server) -> case validate_credentials(User, Server) of {ok, LUser, LServer} -> - lists:any( - fun(M) -> - case db_user_exists(LUser, LServer, M) of - {error, _} -> - false; - Else -> - Else - end - end, auth_modules(LServer)); + {Exists, PerformExternalUserCheck} = + lists:foldl( + fun(M, {Exists0, PerformExternalUserCheck0}) -> + case db_user_exists(LUser, LServer, M) of + {{error, _}, Check} -> + {Exists0, PerformExternalUserCheck0 orelse Check}; + {Else, Check2} -> + {Exists0 orelse Else, PerformExternalUserCheck0 orelse Check2} + end + end, {false, false}, auth_modules(LServer)), + case (not Exists) andalso PerformExternalUserCheck andalso + ejabberd_option:auth_external_user_exists_check(Server) andalso + gen_mod:is_loaded(Server, mod_last) of + true -> + case mod_last:get_last_info(User, Server) of + not_found -> + false; + _ -> + true + end; + _ -> + Exists + end; _ -> false end. @@ -420,11 +434,11 @@ user_exists_in_other_modules_loop([], _User, _Server) -> false; user_exists_in_other_modules_loop([AuthModule | AuthModules], User, Server) -> case db_user_exists(User, Server, AuthModule) of - true -> + {true, _} -> true; - false -> + {false, _} -> user_exists_in_other_modules_loop(AuthModules, User, Server); - {error, _} -> + {{error, _}, _} -> maybe end. @@ -628,9 +642,9 @@ db_get_password(User, Server, Mod) -> db_user_exists(User, Server, Mod) -> case db_get_password(User, Server, Mod) of {ok, _} -> - true; + {true, false}; not_found -> - false; + {false, false}; error -> case {Mod:store_type(Server), use_cache(Mod, Server)} of {external, true} -> @@ -649,18 +663,18 @@ db_user_exists(User, Server, Mod) -> end, case Val of {ok, _} -> - true; + {true, Mod /= ejabberd_auth_anonymous} ; not_found -> - false; + {false, Mod /= ejabberd_auth_anonymous}; error -> - false; + {false, Mod /= ejabberd_auth_anonymous}; {error, _} = Err -> - Err + {Err, Mod /= ejabberd_auth_anonymous} end; {external, false} -> - ets_cache:untag(Mod:user_exists(User, Server)); + {ets_cache:untag(Mod:user_exists(User, Server)), Mod /= ejabberd_auth_anonymous}; _ -> - false + {false, false} end end. diff --git a/src/ejabberd_option.erl b/src/ejabberd_option.erl index 6ea63e561..173e412af 100644 --- a/src/ejabberd_option.erl +++ b/src/ejabberd_option.erl @@ -14,6 +14,7 @@ -export([auth_cache_life_time/0]). -export([auth_cache_missed/0]). -export([auth_cache_size/0]). +-export([auth_external_user_exists_check/0, auth_external_user_exists_check/1]). -export([auth_method/0, auth_method/1]). -export([auth_opts/0, auth_opts/1]). -export([auth_password_format/0, auth_password_format/1]). @@ -224,6 +225,13 @@ auth_cache_missed() -> auth_cache_size() -> ejabberd_config:get_option({auth_cache_size, global}). +-spec auth_external_user_exists_check() -> boolean(). +auth_external_user_exists_check() -> + auth_external_user_exists_check(global). +-spec auth_external_user_exists_check(global | binary()) -> boolean(). +auth_external_user_exists_check(Host) -> + ejabberd_config:get_option({auth_external_user_exists_check, Host}). + -spec auth_method() -> [atom()]. auth_method() -> auth_method(global). diff --git a/src/ejabberd_options.erl b/src/ejabberd_options.erl index 9f48839bb..95b3d92c3 100644 --- a/src/ejabberd_options.erl +++ b/src/ejabberd_options.erl @@ -81,6 +81,8 @@ opt_type(auth_password_format) -> econf:enum([plain, scram]); opt_type(auth_scram_hash) -> econf:enum([sha, sha256, sha512]); +opt_type(auth_external_user_exists_check) -> + econf:bool(); opt_type(auth_use_cache) -> econf:bool(); opt_type(c2s_cafile) -> @@ -542,6 +544,7 @@ options() -> {auth_opts, []}, {auth_password_format, plain}, {auth_scram_hash, sha}, + {auth_external_user_exists_check, true}, {auth_use_cache, fun(Host) -> ejabberd_config:get_option({use_cache, Host}) end}, {c2s_cafile, undefined}, diff --git a/src/ejabberd_options_doc.erl b/src/ejabberd_options_doc.erl index 9d80e721b..5d6e20db5 100644 --- a/src/ejabberd_options_doc.erl +++ b/src/ejabberd_options_doc.erl @@ -395,6 +395,13 @@ doc() -> "You shouldn't change this if you already have passwords generated with " "a different algorithm - users that have such passwords will not be able " "to authenticate. The default value is 'sha'.")}}, + {auth_external_user_exists_check, + #{value => "true | false", + desc => + ?T("Supplement check for user existence based on 'mod_last' data, for authentication " + "methods that don't have a way to reliable tell if user exists (like is the case for " + "'jwt' and certificate based authentication). This helps with processing offline message " + "for those users. The default value is 'true'.")}}, {auth_use_cache, #{value => "true | false", desc =>