]> gitweb.fluxo.info Git - lorea/elgg.git/commitdiff
Refs #1417 Elgg core now passes back useful messages to user when log in fails -...
authorcash <cash@36083f99-b078-4883-b0ff-0f9b5a30f544>
Sun, 14 Nov 2010 23:22:13 +0000 (23:22 +0000)
committercash <cash@36083f99-b078-4883-b0ff-0f9b5a30f544>
Sun, 14 Nov 2010 23:22:13 +0000 (23:22 +0000)
git-svn-id: http://code.elgg.org/elgg/trunk@7317 36083f99-b078-4883-b0ff-0f9b5a30f544

actions/login.php
engine/classes/ElggPAM.php [new file with mode: 0644]
engine/classes/LoginException.php [new file with mode: 0644]
engine/lib/api.php
engine/lib/pam.php
engine/lib/sessions.php
engine/tests/services/api.php
languages/en.php

index 936d0a7d99b58f8eac905b5558a1971b4e642a3e..1b4fbe1fdf6516f92c406943417f13eecdd85391 100644 (file)
@@ -12,53 +12,45 @@ $persistent = get_input("persistent", FALSE);
 $result = FALSE;
 
 if (empty($username) || empty($password)) {
-       register_error(elgg_echo('loginerror'));
+       register_error(elgg_echo('login:empty'));
        forward();
 }
 
-// check first if logging in with email address
+// check if logging in with email address
+// @todo Are usernames with @ not allowed?
 if (strpos($username, '@') !== FALSE && ($users = get_user_by_email($username))) {
        $username = $users[0]->username;
 }
 
-if ($user = authenticate($username, $password)) {
-       $result = login($user, $persistent);
+$result = elgg_authenticate($username, $password);
+if ($result !== true) {
+       register_error($result);
+       forward(REFERER);
 }
 
-// forward to correct page
-if ($result) {
-       system_message(elgg_echo('loginok'));
+$user = get_user_by_username($username);
+if (!$user) {
+       register_error(elgg_echo('login:baduser'));
+       forward(REFERER);
+}
 
-       if (isset($_SESSION['last_forward_from']) && $_SESSION['last_forward_from']) {
-               $forward_url = $_SESSION['last_forward_from'];
-               unset($_SESSION['last_forward_from']);
+try {
+       login($user, $persistent);
+} catch (LoginException $e) {
+       register_error($e->getMessage());
+       forward(REFERER);
+}
 
-               forward($forward_url);
-       } else {
-               if (get_input('returntoreferer')) {
-                       forward(REFERER);
-               } else {
-                       // forward to index for front page overrides.
-                       // index will forward to dashboard if appropriate.
-                       forward('index.php');
-               }
-       }
+// forward to correct page
+if (isset($_SESSION['last_forward_from']) && $_SESSION['last_forward_from']) {
+       $forward_url = $_SESSION['last_forward_from'];
+       unset($_SESSION['last_forward_from']);
+} elseif (get_input('returntoreferer')) {
+       $forward_url = REFERER;
 } else {
-       register_error(elgg_echo('loginerror'));
-       //      // let a plugin hook say why login failed or react to it.
-       //      $params = array(
-       //              'username' => $username,
-       //              'password' => $password,
-       //              'persistent' => $persistent,
-       //              'user' => $user
-       //      );
-       //
-       //      // Returning FALSE to this function will generate a standard
-       //      // "Could not log you in" message.
-       //      // Plugins should use this hook to provide details, and then return TRUE.
-       //      if (!elgg_trigger_plugin_hook('failed_login', 'user', $params, FALSE)) {
-       //              register_error(elgg_echo('loginerror'));
-       //      }
+       // forward to main index page
+       $forward_url = '';
 }
 
-forward(REFERRER);
+system_message(elgg_echo('loginok'));
+forward($forward_url);
diff --git a/engine/classes/ElggPAM.php b/engine/classes/ElggPAM.php
new file mode 100644 (file)
index 0000000..a3e4f9a
--- /dev/null
@@ -0,0 +1,93 @@
+<?php
+/**
+ * ElggPAM Pluggable Authentication Module
+ *
+ * @package    Elgg.Core
+ * @subpackage Authentication
+ */
+class ElggPAM {
+       /**
+        * @var string PAM policy type: user, api or plugin-defined policies
+        */
+       protected $policy;
+
+       /**
+        * @var array Failure mesages
+        */
+       protected $messages;
+
+       /**
+        * ElggPAM constructor
+        * 
+        * @param string $policy PAM policy type: user, api, or plugin-defined policies
+        */
+       public function __construct($policy) {
+               $this->policy = $policy;
+               $this->messages = array('sufficient' => array(), 'required' => array());
+       }
+
+       /**
+        * Authenticate a set of credentials against a policy
+        * This function will process all registered PAM handlers or stop when the first
+        * handler fails. A handler fails by either returning false or throwing an
+        * exception. The advantage of throwing an exception is that it returns a message
+        * that can be passed to the user. The processing order of the handlers is
+        * determined by the order that they were registered.
+        *
+        * If $credentials are provided, the PAM handler should authenticate using the
+        * provided credentials. If not, then credentials should be prompted for or
+        * otherwise retrieved (eg from the HTTP header or $_SESSION).
+        *
+        * @param array $credentials Credentials array dependant on policy type
+        * @return bool
+        */
+       public function authenticate($credentials) {
+               global $_PAM_HANDLERS;
+
+               $authenticated = false;
+
+               foreach ($_PAM_HANDLERS[$this->policy] as $k => $v) {
+                       $handler = $v->handler;
+                       $importance = $v->importance;
+
+                       try {
+                               // Execute the handler
+                               if ($handler($credentials)) {
+                                       $authenticated = true;
+                               } else {
+                                       if ($importance == 'required') {
+                                               $this->messages['required'][] = "$handler:failed";
+                                               return false;
+                                       } else {
+                                               $this->messages['sufficient'][] = "$handler:failed";
+                                       }
+                               }
+                       } catch (Exception $e) {
+                               if ($importance == 'required') {
+                                       $this->messages['required'][] = $e->getMessage();
+                                       return false;
+                               } else {
+                                       $this->messages['sufficient'][] = $e->getMessage();
+                               }
+                       }
+               }
+
+               return $authenticated;
+       }
+
+       /**
+        * Get a failure message to display to user
+        * 
+        * @return string
+        */
+       public function getFailureMessage() {
+               $message = elgg_echo('auth:nopams');
+               if (!empty($this->messages['required'])) {
+                       $message = $this->messages['required'][0];
+               } elseif (!empty($this->messages['sufficient'])) {
+                       $message = $this->messages['sufficient'][0];
+               }
+
+               return elgg_trigger_plugin_hook('fail', 'auth', $this->messages, $message);
+       }
+}
diff --git a/engine/classes/LoginException.php b/engine/classes/LoginException.php
new file mode 100644 (file)
index 0000000..9189a6a
--- /dev/null
@@ -0,0 +1,10 @@
+<?php
+/**
+ * Login Exception Stub
+ *
+ * Generic parent class for login exceptions.
+ *
+ * @package    Elgg.Core
+ * @subpackage Exceptions.Stub
+ */
+class LoginException extends Exception {}
\ No newline at end of file
index 7313fb5e96f1df20371bd2acc63269a5a23e2fbf..2c566b4791cd2da5ba4d638f64a8b4cb52c4faba 100644 (file)
@@ -170,17 +170,19 @@ function authenticate_method($method) {
 
        // check API authentication if required
        if ($API_METHODS[$method]["require_api_auth"] == true) {
-               if (pam_authenticate(null, "api") == false) {
+               $api_pam = new ElggPAM('api');
+               if ($api_pam->authenticate() !== true) {
                        throw new APIException(elgg_echo('APIException:APIAuthenticationFailed'));
                }
        }
 
-       $user_auth_result = pam_authenticate();
+       $user_pam = new ElggPAM('user');
+       $user_auth_result = $user_pam->authenticate();
 
        // check if user authentication is required
        if ($API_METHODS[$method]["require_user_auth"] == true) {
                if ($user_auth_result == false) {
-                       throw new APIException(elgg_echo('APIException:UserAuthenticationFailed'));
+                       throw new APIException($user_pam->getFailureMessage());
                }
        }
 
index 21cfdbbb918225f2d34441dcbdc7a255346c614d..f1df3febafffbdd0e27996ce13da7d8c177c33e5 100644 (file)
  * For more information on PAMs see:
  * http://www.freebsd.org/doc/en/articles/pam/index.html
  *
+ * @see ElggPAM
+ *
  * @package Elgg.Core
  * @subpackage Authentication.PAM
  */
 
 $_PAM_HANDLERS = array();
-$_PAM_HANDLERS_MSG = array();
 
 /**
  * Register a PAM handler.
@@ -66,25 +67,8 @@ function unregister_pam_handler($handler, $policy = "user") {
        unset($_PAM_HANDLERS[$policy][$handler]);
 }
 
-/**
- * Attempt to authenticate.
- * This function will process all registered PAM handlers or stop when the first
- * handler fails. A handler fails by either returning false or throwing an
- * exception. The advantage of throwing an exception is that it returns a message
- * through the global $_PAM_HANDLERS_MSG which can be used in communication with
- * a user. The order that handlers are processed is determined by the order that
- * they were registered.
- *
- * If $credentials are provided the PAM handler should authenticate using the
- * provided credentials, if not then credentials should be prompted for or
- * otherwise retrieved (eg from the HTTP header or $_SESSION).
- *
- * @param mixed  $credentials Mixed PAM handler specific credentials (e.g. username, password)
- * @param string $policy      The policy type, default is "user"
- *
- * @return bool true if authenticated, false if not.
- */
 function pam_authenticate($credentials = NULL, $policy = "user") {
+       elgg_deprecated_notice('pam_authenticate has been deprecated for ElggPAM', 1.8);
        global $_PAM_HANDLERS, $_PAM_HANDLERS_MSG;
 
        $_PAM_HANDLERS_MSG = array();
index b4722d38c5a5588badd653157754102ed7e5a7cf..c42af2ed33e08317084d0923af3e2de0f42c295e 100644 (file)
@@ -127,6 +127,26 @@ function elgg_is_admin_user($user_guid) {
        return FALSE;
 }
 
+/**
+ * Perform user authentication with a given username and password.
+ *
+ * @see login
+ *
+ * @param string $username The username
+ * @param string $password The password
+ *
+ * @return true|string True or an error message on failure
+ */
+function elgg_authenticate($username, $password) {
+       $pam = new ElggPAM('user');
+       $credentials = array('username' => $username, 'password' => $password);
+       $result = $pam->authenticate($credentials);
+       if (!$result) {
+               return $pam->getFailureMessage();
+       }
+       return true;
+}
+
 /**
  * Perform standard authentication with a given username and password.
  * Returns an ElggUser object for use with login.
@@ -138,12 +158,14 @@ function elgg_is_admin_user($user_guid) {
  *
  * @return ElggUser|false The authenticated user object, or false on failure.
  */
-
 function authenticate($username, $password) {
-       if (pam_authenticate(array('username' => $username, 'password' => $password))) {
+       elgg_deprecated_notice('authenticate() has been deprecated for elgg_authenticate()', 1.8);
+       $pam = new ElggPAM('user');
+       $credentials = array('username' => $username, 'password' => $password);
+       $result = $pam->authenticate($credentials);
+       if ($result) {
                return get_user_by_username($username);
        }
-
        return false;
 }
 
@@ -152,31 +174,33 @@ function authenticate($username, $password) {
  * it against a known user.
  *
  * @param array $credentials Associated array of credentials passed to
- *                           pam_authenticate. This function expects
+ *                           Elgg's PAM system. This function expects
  *                           'username' and 'password' (cleartext).
  *
  * @return bool
+ * @throws LoginException
  */
 function pam_auth_userpass($credentials = NULL) {
 
-       if (is_array($credentials) && ($credentials['username']) && ($credentials['password'])) {
-               if ($user = get_user_by_username($credentials['username'])) {
-                       // User has been banned, so prevent from logging in
-                       if ($user->isBanned()) {
-                               return FALSE;
-                       }
+       if (!is_array($credentials) && (!$credentials['username']) && (!$credentials['password'])) {
+               return false;
+       }
 
-                       if ($user->password == generate_user_password($user, $credentials['password'])) {
-                               return TRUE;
-                       } else {
-                               // Password failed, log.
-                               log_login_failure($user->guid);
-                       }
+       $user = get_user_by_username($credentials['username']);
+       if (!$user) {
+               throw new LoginException(elgg_echo('LoginException:UsernameFailure'));
+       }
 
-               }
+       if (check_rate_limit_exceeded($user->guid)) {
+               throw new LoginException(elgg_echo('LoginException:AccountLocked'));
        }
 
-       return FALSE;
+       if ($user->password !== generate_user_password($user, $credentials['password'])) {
+               log_login_failure($user->guid);
+               throw new LoginException(elgg_echo('LoginException:PasswordFailure'));
+       } 
+
+       return true;
 }
 
 /**
@@ -207,7 +231,7 @@ function log_login_failure($user_guid) {
  *
  * @param int $user_guid User GUID
  *
- * @return bool on success (success = user has no logged failed attempts)
+ * @return bool true on success (success = user has no logged failed attempts)
  */
 function reset_login_failure_count($user_guid) {
        $user_guid = (int)$user_guid;
@@ -270,26 +294,22 @@ function check_rate_limit_exceeded($user_guid) {
 
 /**
  * Logs in a specified ElggUser. For standard registration, use in conjunction
- * with authenticate.
+ * with elgg_authenticate.
  *
- * @see authenticate
+ * @see elgg_authenticate
  *
  * @param ElggUser $user       A valid Elgg user object
  * @param boolean  $persistent Should this be a persistent login?
  *
- * @return bool Whether login was successful
+ * @return true or throws exception
+ * @throws LoginException
  */
 function login(ElggUser $user, $persistent = false) {
        global $CONFIG;
 
        // User is banned, return false.
        if ($user->isBanned()) {
-               return false;
-       }
-
-       // Check rate limit
-       if (check_rate_limit_exceeded($user->guid)) {
-               return false;
+               throw new LoginException(elgg_echo('LoginException:BannedUser'));
        }
 
        $_SESSION['user'] = $user;
@@ -314,7 +334,7 @@ function login(ElggUser $user, $persistent = false) {
                unset($_SESSION['id']);
                unset($_SESSION['user']);
                setcookie("elggperm", "", (time() - (86400 * 30)), "/");
-               return false;
+               throw new LoginException(elgg_echo('LoginException:Unknown'));
        }
 
        // Users privilege has been elevated, so change the session id (prevents session fixation)
index 57396c3a085db6de4e08f2775e58ec515229f156..39951da1c81d42780f93a174a7bed7dcc813fae1 100644 (file)
@@ -128,7 +128,6 @@ class ElggCoreServicesApiTest extends ElggCoreUnitTest {
                        $this->assertTrue(FALSE);
                } catch (Exception $e) {
                        $this->assertIsA($e, 'APIException');
-                       $this->assertIdentical($e->getMessage(), elgg_echo('APIException:UserAuthenticationFailed'));
                }                               
        }
        
@@ -284,9 +283,9 @@ class ElggCoreServicesApiTest extends ElggCoreUnitTest {
        }
        
 // api key methods
-       public function testApiAuthenticate() {
-               $this->assertFalse(pam_authenticate(null, "api"));
-       }
+       //public function testApiAuthenticate() {
+       //      $this->assertFalse(pam_authenticate(null, "api"));
+       //}
        
        public function testApiAuthKeyNoKey() {
                try {
index 522f9e364c11024f3abbd0dd7a31bb7cbcb9a188..118ad888332d872635b76bf62b9628a336e7f868 100644 (file)
@@ -21,6 +21,9 @@ $english = array(
        'login' => "Log in",
        'loginok' => "You have been logged in.",
        'loginerror' => "We couldn't log you in. Please check your credentials and try again.",
+       'login:empty' => "Username and password are required.",
+       'login:baduser' => "Unable to load your user account.",
+       'auth:nopams' => "Internal error. No user authentication method installed.",
 
        'logout' => "Log out",
        'logoutok' => "You have been logged out.",
@@ -174,6 +177,10 @@ $english = array(
 
        'RegistrationException:EmptyPassword' => 'The password fields cannot be empty',
        'RegistrationException:PasswordMismatch' => 'Passwords must match',
+       'LoginException:BannedUser' => 'You have been banned from this site and cannot log in',
+       'LoginException:UsernameFailure' => 'We could not log you in. Please check your username and password.',
+       'LoginException:PasswordFailure' => 'We could not log you in. Please check your username and password.',
+       'LoginException:AccountLocked' => 'Your account has been locked for too many log in failures.',
 
        'memcache:notinstalled' => 'PHP memcache module not installed, you must install php5-memcache',
        'memcache:noservers' => 'No memcache servers defined, please populate the $CONFIG->memcache_servers variable',