Commit b868e5ff authored by Nigel McNie's avatar Nigel McNie Committed by Nigel McNie
Browse files

- added an inactivity TODO

  - changed auth_check_password_change to just source $USER directly
  - implemented auth_get_authtype_for_institution
  - changed auth_check_password_change to allow any authentication method
    to change the password, if they specify a config url or if they
    implement a method
  - changed change_password_validate to allow any authentication method to
    validate a password
  - changed change_password_submit to allow any authentication method to
    update a password
  - added a register link to the login box
  - inlined the expire/suspended checks, since they only need to be checked
    in one place. Allowed any method to say who is suspended
parent 3eb610f3
...@@ -36,6 +36,12 @@ class AuthUnknownUserException extends Exception {} ...@@ -36,6 +36,12 @@ class AuthUnknownUserException extends Exception {}
/** /**
* Base authentication class. Provides a common interface with which * Base authentication class. Provides a common interface with which
* authentication can be carried out for system users. * authentication can be carried out for system users.
*
* @todo for authentication:
* - inactivity: each institution has inactivity timeout times, this needs
* to be supported
* - this means the lastlogin field needs to be updated on the usr table
* - warnings are handled by cron
*/ */
abstract class Auth { abstract class Auth {
...@@ -216,7 +222,8 @@ function auth_setup () { ...@@ -216,7 +222,8 @@ function auth_setup () {
// The session is still active, so continue it. // The session is still active, so continue it.
log_debug('session still active from previous time'); log_debug('session still active from previous time');
$USER = $SESSION->renew(); $USER = $SESSION->renew();
auth_check_password_change($USER); log_debug($USER);
auth_check_password_change();
return $USER; return $USER;
} }
else if ($sessionlogouttime > 0) { else if ($sessionlogouttime > 0) {
...@@ -257,11 +264,13 @@ function auth_setup () { ...@@ -257,11 +264,13 @@ function auth_setup () {
* Given an institution, returns the authentication method used by it. * Given an institution, returns the authentication method used by it.
* *
* @return string * @return string
* @todo<nigel>: Currently, the system doesn't have a concept of institution
* at the database level, so the internal authentication method is assumed.
*/ */
function auth_get_authtype_for_institution($institution) { function auth_get_authtype_for_institution($institution) {
return 'internal'; static $cache = array();
if (isset($cache[$institution])) {
return $cache[$institution];
}
return $cache[$institution] = get_field('institution', 'authplugin', 'name', $institution);
} }
/** /**
...@@ -272,10 +281,28 @@ function auth_get_authtype_for_institution($institution) { ...@@ -272,10 +281,28 @@ function auth_get_authtype_for_institution($institution) {
* will, in theory, have different data stores, making changing the password * will, in theory, have different data stores, making changing the password
* via the internal form difficult. * via the internal form difficult.
*/ */
function auth_check_password_change($user) { function auth_check_password_change() {
global $USER;
log_debug('checking if the user needs to change their password'); log_debug('checking if the user needs to change their password');
if (auth_get_authtype_for_institution($user->institution) == 'internal' && $user->passwordchange) { if (!$USER->passwordchange) {
return;
}
$authtype = auth_get_authtype_for_institution($USER->institution);
$authclass = 'Auth' . ucfirst($authtype);
$url = '';
safe_require('auth', $authtype, 'lib.php', 'require_once');
// @todo auth preference for a password change screen for all auth methods other than internal
if (
($url = get_config_plugin('auth', $authtype, 'changepasswordurl'))
|| (method_exists($authclass, 'change_password'))) {
log_debug('user DOES need to change their password'); log_debug('user DOES need to change their password');
if ($url) {
redirect($url);
exit;
}
require_once('form.php'); require_once('form.php');
$form = array( $form = array(
'name' => 'change_password', 'name' => 'change_password',
...@@ -311,41 +338,6 @@ function auth_check_password_change($user) { ...@@ -311,41 +338,6 @@ function auth_check_password_change($user) {
} }
} }
/**
* Check if the given user's account has expired
*
* @param object $user The user to check for the expired password.
* @todo maybe later, just use $USER because that's all we are actually checking...
* @private
*/
function auth_check_user_expired($user) {
log_debug('Checking to see if the user has expired');
if ($user->expiry > 0 && time() > $user->expiry) {
// Trash the $USER object, used for checking if the user is logged in.
// Smarty uses it otherwise...
global $USER;
$USER = null;
die_info(get_string('accountexpired'));
}
}
/**
* Check if the given user's account has been suspended
*
* @param object $user The user to check for the suspended account.
* @private
*/
function auth_check_user_suspended($user) {
global $USER;
log_debug('Checking to see if the user is suspended');
$suspend = get_record('usr_suspension', 'usr', $user->id);
if ($suspend) {
global $USER;
$USER = null;
die_info(get_string('accountsuspended', 'mahara', $suspend->ctime, $suspend->reason));
}
}
/** /**
* Validates the form for changing the password for a user. * Validates the form for changing the password for a user.
* *
...@@ -353,14 +345,19 @@ function auth_check_user_suspended($user) { ...@@ -353,14 +345,19 @@ function auth_check_user_suspended($user) {
* *
* @todo As far as I can tell, the change password and registration forms will * @todo As far as I can tell, the change password and registration forms will
* only be used for internal authentication. And so, by proxy, will the * only be used for internal authentication. And so, by proxy, will the
* username/password valid methods for the Auth class. I think this means they * username/password valid methods for the Auth class. [THIS IS TRUE]
* can be removed from the Auth class, and instead just be part of AuthInternal *
* since they don't need to be specified for other types. *
* I think this means they can be removed from the Auth class, and instead just
* be part of AuthInternal since they don't need to be specified for other types.
* [THIS IS ALSO TRUE]
* *
* Furthermore, I think that the change_password stuff (as well as suspended * Furthermore, I think that the change_password stuff (as well as suspended
* and expired) are also quite possibly related to internal only. This will * and expired) are also quite possibly related to internal only. This will
* require a lot of thought about how to best structure it. * require a lot of thought about how to best structure it.
* *
* Change password will only be if a URL for it exists, or a function exists
*
* @param Form $form The form to check * @param Form $form The form to check
* @param array $values The values to check * @param array $values The values to check
*/ */
...@@ -369,74 +366,64 @@ function change_password_validate(Form $form, $values) { ...@@ -369,74 +366,64 @@ function change_password_validate(Form $form, $values) {
// Get the authentication type for the user (based on the institution), and // Get the authentication type for the user (based on the institution), and
// use the information to validate the password // use the information to validate the password
$authtype = auth_get_authtype_for_institution($SESSION->get('institution')); $authtype = auth_get_authtype_for_institution($SESSION->get('institution'));
if ($authtype == 'internal') { $authclass = 'Auth' . ucfirst($authtype);
safe_require('auth', $authtype, 'lib.php', 'require_once'); $authlang = 'auth.' . $authtype;
safe_require('auth', $authtype, 'lib.php', 'require_once');
// Check that the password is in valid form
if (!$form->get_error('password1')
&& !call_static_method('AuthInternal', 'is_password_valid', $values['password1'])) {
$form->set_error('password1', get_string('passwordinvalidform', 'auth.internal'));
}
// The password must not be too easy :) // Check that the password is in valid form
$suckypasswords = array( if (!$form->get_error('password1')
'mahara', 'password', $SESSION->get('username') && !call_static_method($authclass, 'is_password_valid', $values['password1'])) {
); $form->set_error('password1', get_string('passwordinvalidform', $authlang));
if (!$form->get_error('password1') && in_array($values['password1'], $suckypasswords)) { }
$form->set_error('password1', get_string('passwordtooeasy', 'auth.internal'));
}
// The password cannot be the same as the old one // The password must not be too easy :)
if (!$form->get_error('password1') && $values['password1'] == $USER->password) { $suckypasswords = array(
$form->set_error('password1', get_string('passwordnotchanged', 'auth.internal')); 'mahara', 'password', $SESSION->get('username')
} );
if (!$form->get_error('password1') && in_array($values['password1'], $suckypasswords)) {
$form->set_error('password1', get_string('passwordtooeasy'));
}
// The passwords must match // The password cannot be the same as the old one
if (!$form->get_error('password1') && !$form->get_error('password2') && $values['password1'] != $values['password2']) { if (!$form->get_error('password1') && $values['password1'] == $SESSION->get('password')) {
$form->set_error('password2', get_string('passwordsdonotmatch', 'auth.internal')); $form->set_error('password1', get_string('passwordnotchanged'));
}
} }
else {
throw new Exception('The user "' . $USER->username . '" is trying to' // The passwords must match
. ' change their password, but they do not use the internal' if (!$form->get_error('password1') && !$form->get_error('password2') && $values['password1'] != $values['password2']) {
. ' authentication method'); $form->set_error('password2', get_string('passwordsdonotmatch'));
} }
} }
/** /**
* Changes the password for a user, given that it is valid. * Changes the password for a user, given that it is valid.
* *
* This only applies to the internal authentication plugin.
*
* @param array $values The submitted form values * @param array $values The submitted form values
*/ */
function change_password_submit($values) { function change_password_submit($values) {
global $SESSION; global $SESSION, $USER;
log_debug('changing password to ' . $values['password1']); log_debug('changing password to ' . $values['password1']);
$authtype = auth_get_authtype_for_institution($SESSION->get('institution')); $authtype = auth_get_authtype_for_institution($SESSION->get('institution'));
if ($authtype == 'internal') { $authclass = 'Auth' . ucfirst($authtype);
// Create a salted password and set it for the user
safe_require('auth', $authtype, 'lib.php', 'require_once'); // This method should exists, because if it did not then the change
// password form would not have been shown.
if ($password = call_static_method($authclass, 'change_password', $USER, $values['password1'])) {
$user = new StdClass; $user = new StdClass;
$user->salt = substr(md5(rand(1000000, 9999999)), 2, 8); $user->password = $password;
$user->password = call_static_method('AuthInternal', 'encrypt_password', $values['password1'], $user->salt);
$user->passwordchange = 0; $user->passwordchange = 0;
$where = new StdClass; $where = new StdClass;
$where->username = $SESSION->get('username'); $where->username = $SESSION->get('username');
update_record('usr', $user, $where); update_record('usr', $user, $where);
$SESSION->set('password', $password);
$SESSION->set('passwordchange', 0); $SESSION->set('passwordchange', 0);
$SESSION->add_ok_msg(get_string('passwordsaved', 'auth.internal')); $SESSION->add_ok_msg(get_string('passwordsaved'));
redirect(get_config('wwwroot')); redirect(get_config('wwwroot'));
exit; exit;
} }
else {
throw new Exception('The user "' . $USER->username . '" is trying to' throw new Exception('You are trying to change your password, but the attempt failed');
. ' change their password, but they do not use the internal'
. ' authentication method');
}
} }
/** /**
...@@ -519,6 +506,9 @@ function auth_get_login_form() { ...@@ -519,6 +506,9 @@ function auth_get_login_form() {
'submit' => array( 'submit' => array(
'type' => 'submit', 'type' => 'submit',
'value' => get_string('login') 'value' => get_string('login')
),
'register' => array(
'value' => '<tr><td colspan="2"><a href="' . get_config('wwwroot') . 'register.php">' . get_string('register') . '</a></td></tr>'
) )
); );
...@@ -579,7 +569,7 @@ function login_submit($values) { ...@@ -579,7 +569,7 @@ function login_submit($values) {
log_debug('auth details supplied, attempting to log user in'); log_debug('auth details supplied, attempting to log user in');
$username = $values['login_username']; $username = $values['login_username'];
$password = $values['login_password']; $password = $values['login_password'];
$institution = (isset($values['login_institution'])) ? $values['login_institution'] : 0; $institution = (isset($values['login_institution'])) ? $values['login_institution'] : 'mahara';
$authtype = auth_get_authtype_for_institution($institution); $authtype = auth_get_authtype_for_institution($institution);
safe_require('auth', $authtype, 'lib.php', 'require_once'); safe_require('auth', $authtype, 'lib.php', 'require_once');
...@@ -588,12 +578,57 @@ function login_submit($values) { ...@@ -588,12 +578,57 @@ function login_submit($values) {
try { try {
if (call_static_method($authclass, 'authenticate_user_account', $username, $password, $institution)) { if (call_static_method($authclass, 'authenticate_user_account', $username, $password, $institution)) {
log_debug('user ' . $username . ' logged in OK'); log_debug('user ' . $username . ' logged in OK');
$USER = call_static_method($authclass, 'get_user_info', $username);
auth_check_user_expired($USER); if (!record_exists('usr', 'username', $username)) {
auth_check_user_suspended($USER); // We don't know about this user. But if the authentication
// method says they're fine, then we must insert data for them
// into the usr table.
log_debug('this user authenticated but not in the usr table, adding them');
// @todo document what needs to be returned by get_user_info
$USER = call_static_method($authclass, 'get_user_info', $username);
log_debug($USER);
insert_record('usr', $USER);
}
// @todo config form option for this for each external plugin. NOT for internal
else if (get_config_plugin('auth', $authtype, 'updateuserinfoonlogin')) {
log_debug('updating user info from auth method');
$USER = call_static_method($authclass, 'get_user_info', $username);
log_debug($USER);
$where = new StdClass;
$where->username = $username;
$where->institution = $institution;
// @todo as per the above todo about what needs to be returned by get_user_info,
// that needs to be validated somewhere. Because here we do an insert into the
// usr table, that needs to work. and provide enough information for future
// authentication attempts
update_record('usr', $USER, $where);
}
else {
log_debug('getting user info from database');
$USER = get_record('usr', 'username', $username, null, null, null, null, '*, ' . db_format_tsfield('expiry'));
log_debug($USER);
}
// Check if the user's account has expired
log_debug('Checking to see if the user has expired');
if ($USER->expiry > 0 && time() > $USER->expiry) {
// Trash the $USER object, used for checking if the user is logged in.
// Smarty uses it and puts login-only stuff in the output otherwise...
$USER = null;
die_info(get_string('accountexpired'));
}
// Check if the user's account has been suspended
log_debug('Checking to see if the user is suspended');
if ($suspend = call_static_method($authclass, 'is_user_suspended', $USER)) {
$USER = null;
die_info(get_string('accountsuspended', 'mahara', $suspend->ctime, $suspend->reason));
}
// User is allowed to log in
$SESSION->login($USER); $SESSION->login($USER);
$USER->logout_time = $SESSION->get('logout_time'); $USER->logout_time = $SESSION->get('logout_time');
auth_check_password_change($USER); auth_check_password_change();
} }
else { else {
// Login attempt failed // Login attempt failed
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment