]> granicus.if.org Git - ejabberd/commitdiff
* src/ejabberd_auth.erl: If anonymous auth is enabled, when
authorBadlop <badlop@process-one.net>
Wed, 4 Mar 2009 18:34:02 +0000 (18:34 +0000)
committerBadlop <badlop@process-one.net>
Wed, 4 Mar 2009 18:34:02 +0000 (18:34 +0000)
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
src/ejabberd_auth.erl
src/ejabberd_auth_anonymous.erl
src/ejabberd_auth_external.erl
src/ejabberd_auth_internal.erl
src/ejabberd_auth_ldap.erl
src/ejabberd_auth_odbc.erl
src/ejabberd_auth_pam.erl

index 44fe628cef94b6aaa81c47e89f6e6d9afeef99bc..ed5705e86ac0e438f4abe320771eeb3a42f1ad1e 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,15 @@
+2009-03-04  Badlop  <badlop@process-one.net>
+
+       * 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 <christophe.romain@process-one.net>
 
        * src/mod_pubsub/mod_pubsub.erl: Allow node creation without configure
index 8578142e08171b462af5aa20b2b539e18f2afb76..99a4eb8ff880a809906b2521df6e8f30b8981c7c 100644 (file)
@@ -80,20 +80,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
@@ -106,27 +107,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
@@ -261,11 +258,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.
index 47a75ac290447fdfb5be243ec8b77c064b3697f9..9540e7c8c5106e287db9f214a63fc25d27f3e543 100644 (file)
@@ -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.
 
index aa02ff0c8feae2e34ab16078cc9e511f5be96360..af064c9cf6890f1feb845003502db0e93e3e489c 100644 (file)
@@ -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}.
index b5920e7162aa5052bd11f4530d9fe18775092f35..f98fde19b4f4e9cf383b6369a2945cb1d00e2baa 100644 (file)
@@ -230,6 +230,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),
@@ -239,8 +240,8 @@ is_user_exists(User, Server) ->
            false;
        [_] ->
            true;
-       _ ->
-           false
+       Other ->
+           {error, Other}
     end.
 
 %% @spec (User, Server) -> ok
index 2f96e4ef960b280010a53d0bbe93359120db5283..3b8300961d52e831cb3bede1243f34b88ec321a7 100644 (file)
@@ -186,10 +186,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.
index 8b5cb6537689ccdaf172e8672d674a974449414c..af8207d4f95a46854ff9244e524d9d1e4554f02c 100644 (file)
@@ -61,6 +61,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 ->
@@ -68,14 +69,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}]} ->
-                   true;
-               _ ->
-                   false
+                   true; %% Password is correct
+               {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 ->
@@ -83,7 +92,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 /= "" ->
@@ -96,8 +106,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.
 
@@ -208,6 +223,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 ->
@@ -215,11 +231,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.
 
index c8a8031c2d4dcabe48f986b5599d9ea9d3f5b8ab..72237282e0cc5e1c97a57defca938ca3a18a8aaf 100644 (file)
@@ -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