From ea715129e9bf073c90c26591b5820ebcded283cd Mon Sep 17 00:00:00 2001 From: Badlop Date: Wed, 4 Mar 2009 18:34:02 +0000 Subject: [PATCH] * src/ejabberd_auth.erl: If anonymous auth is enabled, when checking if the account already exists in other auth methods, take into account if the auth method failed (EJAB-882) * src/ejabberd_auth_anonymous.erl: Likewise * src/ejabberd_auth_external.erl: Likewise * src/ejabberd_auth_internal.erl: Likewise * src/ejabberd_auth_ldap.erl: Likewise * src/ejabberd_auth_odbc.erl: Likewise * src/ejabberd_auth_pam.erl: Likewise SVN Revision: 1966 --- ChangeLog | 12 ++++++ src/ejabberd_auth.erl | 70 +++++++++++++++++++-------------- src/ejabberd_auth_anonymous.erl | 3 ++ src/ejabberd_auth_external.erl | 7 +++- src/ejabberd_auth_internal.erl | 5 ++- src/ejabberd_auth_ldap.erl | 5 ++- src/ejabberd_auth_odbc.erl | 43 ++++++++++++++------ src/ejabberd_auth_pam.erl | 2 + 8 files changed, 102 insertions(+), 45 deletions(-) diff --git a/ChangeLog b/ChangeLog index d00f57b51..fcdbf34ea 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,15 @@ +2009-03-04 Badlop + + * src/ejabberd_auth.erl: If anonymous auth is enabled, when + checking if the account already exists in other auth methods, take + into account if the auth method failed (EJAB-882) + * src/ejabberd_auth_anonymous.erl: Likewise + * src/ejabberd_auth_external.erl: Likewise + * src/ejabberd_auth_internal.erl: Likewise + * src/ejabberd_auth_ldap.erl: Likewise + * src/ejabberd_auth_odbc.erl: Likewise + * src/ejabberd_auth_pam.erl: Likewise + 2009-03-04 Christophe Romain * src/mod_pubsub/mod_pubsub.erl: Allow node creation without configure diff --git a/src/ejabberd_auth.erl b/src/ejabberd_auth.erl index 4be455b06..35b434f63 100644 --- a/src/ejabberd_auth.erl +++ b/src/ejabberd_auth.erl @@ -78,20 +78,21 @@ plain_password_required(Server) -> %% @spec (User::string(), Server::string(), Password::string()) -> %% true | false check_password(User, Server, Password) -> - lists:any( - fun(M) -> - M:check_password(User, Server, Password) - end, auth_modules(Server)). + case check_password_with_authmodule(User, Server, Password) of + {true, _AuthModule} -> true; + false -> false + end. %% @doc Check if the user and password can login in server. %% @spec (User::string(), Server::string(), Password::string(), %% StreamID::string(), Digest::string()) -> %% true | false check_password(User, Server, Password, StreamID, Digest) -> - lists:any( - fun(M) -> - M:check_password(User, Server, Password, StreamID, Digest) - end, auth_modules(Server)). + case check_password_with_authmodule(User, Server, Password, + StreamID, Digest) of + {true, _AuthModule} -> true; + false -> false + end. %% @doc Check if the user and password can login in server. %% The user can login if at least an authentication method accepts the user @@ -104,27 +105,23 @@ check_password(User, Server, Password, StreamID, Digest) -> %% | ejabberd_auth_internal | ejabberd_auth_ldap %% | ejabberd_auth_odbc | ejabberd_auth_pam check_password_with_authmodule(User, Server, Password) -> - Res = lists:dropwhile( - fun(M) -> - not apply(M, check_password, - [User, Server, Password]) - end, auth_modules(Server)), - case Res of - [] -> false; - [AuthMod | _] -> {true, AuthMod} - end. + check_password_loop(auth_modules(Server), [User, Server, Password]). check_password_with_authmodule(User, Server, Password, StreamID, Digest) -> - Res = lists:dropwhile( - fun(M) -> - not apply(M, check_password, - [User, Server, Password, StreamID, Digest]) - end, auth_modules(Server)), - case Res of - [] -> false; - [AuthMod | _] -> {true, AuthMod} + check_password_loop(auth_modules(Server), [User, Server, Password, + StreamID, Digest]). + +check_password_loop([], _Args) -> + false; +check_password_loop([AuthModule | AuthModules], Args) -> + case apply(AuthModule, check_password, Args) of + true -> + {true, AuthModule}; + false -> + check_password_loop(AuthModules, Args) end. + %% @spec (User::string(), Server::string(), Password::string()) -> %% ok | {error, ErrorType} %% where ErrorType = empty_password | not_allowed | invalid_jid @@ -259,11 +256,26 @@ is_user_exists(User, Server) -> %% Check if the user exists in all authentications module except the module %% passed as parameter +%% @spec (Module::atom(), User, Server) -> true | false | maybe is_user_exists_in_other_modules(Module, User, Server) -> - lists:any( - fun(M) -> - M:is_user_exists(User, Server) - end, auth_modules(Server)--[Module]). + is_user_exists_in_other_modules_loop( + auth_modules(Server)--[Module], + User, Server). +is_user_exists_in_other_modules_loop([], _User, _Server) -> + false; +is_user_exists_in_other_modules_loop([AuthModule|AuthModules], User, Server) -> + case AuthModule:is_user_exists(User, Server) of + true -> + true; + false -> + is_user_exists_in_other_modules_loop(AuthModules, User, Server); + {error, Error} -> + ?DEBUG("The authentication module ~p returned an error~nwhen " + "checking user ~p in server ~p~nError message: ~p", + [AuthModule, User, Server, Error]), + maybe + end. + %% @spec (User, Server) -> ok | error | {error, not_allowed} %% @doc Remove user. diff --git a/src/ejabberd_auth_anonymous.erl b/src/ejabberd_auth_anonymous.erl index 47a75ac29..9540e7c8c 100644 --- a/src/ejabberd_auth_anonymous.erl +++ b/src/ejabberd_auth_anonymous.erl @@ -178,7 +178,10 @@ check_password(User, Server, _Password, _StreamID, _Digest) -> %% they however are "reserved") case ejabberd_auth:is_user_exists_in_other_modules(?MODULE, User, Server) of + %% If user exists in other module, reject anonnymous authentication true -> false; + %% If we are not sure whether the user exists in other module, reject anon auth + maybe -> false; false -> login(User, Server) end. diff --git a/src/ejabberd_auth_external.erl b/src/ejabberd_auth_external.erl index af7c5b048..d35d37f73 100644 --- a/src/ejabberd_auth_external.erl +++ b/src/ejabberd_auth_external.erl @@ -83,8 +83,13 @@ get_password(_User, _Server) -> get_password_s(_User, _Server) -> "". +%% @spec (User, Server) -> true | false | {error, Error} is_user_exists(User, Server) -> - extauth:is_user_exists(User, Server). + try extauth:is_user_exists(User, Server) of + Res -> Res + catch + _:Error -> {error, Error} + end. remove_user(_User, _Server) -> {error, not_allowed}. diff --git a/src/ejabberd_auth_internal.erl b/src/ejabberd_auth_internal.erl index 14459521c..2b8c62fd6 100644 --- a/src/ejabberd_auth_internal.erl +++ b/src/ejabberd_auth_internal.erl @@ -225,6 +225,7 @@ get_password_s(User, Server) -> [] end. +%% @spec (User, Server) -> true | false | {error, Error} is_user_exists(User, Server) -> LUser = jlib:nodeprep(User), LServer = jlib:nameprep(Server), @@ -234,8 +235,8 @@ is_user_exists(User, Server) -> false; [_] -> true; - _ -> - false + Other -> + {error, Other} end. %% @spec (User, Server) -> ok diff --git a/src/ejabberd_auth_ldap.erl b/src/ejabberd_auth_ldap.erl index 2063ee5fc..1e57e0216 100644 --- a/src/ejabberd_auth_ldap.erl +++ b/src/ejabberd_auth_ldap.erl @@ -179,10 +179,11 @@ get_password(_User, _Server) -> get_password_s(_User, _Server) -> "". +%% @spec (User, Server) -> true | false | {error, Error} is_user_exists(User, Server) -> case catch is_user_exists_ldap(User, Server) of - {'EXIT', _} -> - false; + {'EXIT', Error} -> + {error, Error}; Result -> Result end. diff --git a/src/ejabberd_auth_odbc.erl b/src/ejabberd_auth_odbc.erl index da2e6cb59..edd493581 100644 --- a/src/ejabberd_auth_odbc.erl +++ b/src/ejabberd_auth_odbc.erl @@ -57,6 +57,7 @@ start(_Host) -> plain_password_required() -> false. +%% @spec (User, Server, Password) -> true | false | {error, Error} check_password(User, Server, Password) -> case jlib:nodeprep(User) of error -> @@ -64,14 +65,22 @@ check_password(User, Server, Password) -> LUser -> Username = ejabberd_odbc:escape(LUser), LServer = jlib:nameprep(Server), - case catch odbc_queries:get_password(LServer, Username) of + try odbc_queries:get_password(LServer, Username) of {selected, ["password"], [{Password}]} -> - Password /= ""; - _ -> - false + Password /= ""; %% Password is correct, and not empty + {selected, ["password"], [{_Password2}]} -> + false; %% Password is not correct + {selected, ["password"], []} -> + false; %% Account does not exist + {error, _Error} -> + false %% Typical error is that table doesn't exist + catch + _:_ -> + false %% Typical error is database not accessible end end. +%% @spec (User, Server, Password, StreamID, Digest) -> true | false | {error, Error} check_password(User, Server, Password, StreamID, Digest) -> case jlib:nodeprep(User) of error -> @@ -79,7 +88,8 @@ check_password(User, Server, Password, StreamID, Digest) -> LUser -> Username = ejabberd_odbc:escape(LUser), LServer = jlib:nameprep(Server), - case catch odbc_queries:get_password(LServer, Username) of + try odbc_queries:get_password(LServer, Username) of + %% Account exists, check if password is valid {selected, ["password"], [{Passwd}]} -> DigRes = if Digest /= "" -> @@ -92,8 +102,13 @@ check_password(User, Server, Password, StreamID, Digest) -> true -> (Passwd == Password) and (Password /= "") end; - _ -> - false + {selected, ["password"], []} -> + false; %% Account does not exist + {error, _Error} -> + false %% Typical error is that table doesn't exist + catch + _:_ -> + false %% Typical error is database not accessible end end. @@ -204,6 +219,7 @@ get_password_s(User, Server) -> end end. +%% @spec (User, Server) -> true | false | {error, Error} is_user_exists(User, Server) -> case jlib:nodeprep(User) of error -> @@ -211,11 +227,16 @@ is_user_exists(User, Server) -> LUser -> Username = ejabberd_odbc:escape(LUser), LServer = jlib:nameprep(Server), - case catch odbc_queries:get_password(LServer, Username) of + try odbc_queries:get_password(LServer, Username) of {selected, ["password"], [{_Password}]} -> - true; - _ -> - false + true; %% Account exists + {selected, ["password"], []} -> + false; %% Account does not exist + {error, Error} -> + {error, Error} %% Typical error is that table doesn't exist + catch + _:B -> + {error, B} %% Typical error is database not accessible end end. diff --git a/src/ejabberd_auth_pam.erl b/src/ejabberd_auth_pam.erl index c8a8031c2..72237282e 100644 --- a/src/ejabberd_auth_pam.erl +++ b/src/ejabberd_auth_pam.erl @@ -80,6 +80,8 @@ get_password(_User, _Server) -> get_password_s(_User, _Server) -> "". +%% @spec (User, Server) -> true | false | {error, Error} +%% TODO: Improve this function to return an error instead of 'false' when connection to PAM failed is_user_exists(User, Host) -> Service = get_pam_service(Host), case catch epam:acct_mgmt(Service, User) of