Commit b57da3b2 authored by Piers Harding's avatar Piers Harding

Allow the trial of an individual auth instance to fail gracefully

so that others in the chain can still be tried (bug #536959)
Signed-off-by: default avatarPiers Harding <piers@catalyst.net.nz>
parent 6c60fe02
......@@ -188,9 +188,17 @@ function accountprefs_validate(Pieform $form, $values) {
if (isset($values['oldpassword'])) {
if ($values['oldpassword'] !== '') {
global $USER, $authtype, $authclass;
if (!$authobj->authenticate_user_account($USER, $values['oldpassword'])) {
$form->set_error('oldpassword', get_string('oldpasswordincorrect', 'account'));
return;
try {
if (!$authobj->authenticate_user_account($USER, $values['oldpassword'])) {
$form->set_error('oldpassword', get_string('oldpasswordincorrect', 'account'));
return;
}
}
// propagate error correctly for User validation issues - this should
// be catching AuthUnknownUserException and AuthInstanceException
catch (UserException $e) {
$form->set_error('oldpassword', $e->getMessage());
return;
}
password_validate($form, $values, $USER);
}
......
......@@ -44,5 +44,5 @@ $string['userattribute'] = 'User attribute';
$string['usertype'] = 'User type';
$string['weautocreateusers'] = 'We auto-create users';
$string['updateuserinfoonlogin'] = 'Update user info on login';
$string['cannotconnect'] = 'Cannot connect to any LDAP hosts';
?>
......@@ -95,7 +95,6 @@ class AuthLdap extends Auth {
if (empty($username) or empty($password)) {
return false;
}
// For update user info on login
$update = false;
......@@ -137,13 +136,14 @@ class AuthLdap extends Auth {
}
}
}
return true;
}
}
else {
@ldap_close($ldapconnection);
throw new AuthUnknownUserException('Cannot connect to any LDAP hosts');
// let's do some logging too
log_warn("LDAP connection failed: ".$this->config['host_url'].'/'.$this->config['contexts']);
throw new AuthInstanceException(get_string('cannotconnect', 'auth.ldap'));
}
return false; // No match
......
......@@ -35,6 +35,16 @@ require(get_config('docroot') . 'auth/user.php');
*/
class AuthUnknownUserException extends UserException {}
/**
* An instance of an auth plugin failed during execution
* e.g. LDAP auth failed to connect to a directory
* Developers can use this to fail an individual auth
* instance, but not kill all from being tried.
* If appropriate - the 'message' of the exception will be used
* as the display message, so don't forget to language translate it
*/
class AuthInstanceException extends UserException {}
/**
* We tried to call a method on an auth plugin that hasn't been init'ed
* successfully
......@@ -854,10 +864,17 @@ function requiredfields_validate(Pieform $form, $values) {
password_validate($form, $values, $USER);
// The password cannot be the same as the old one
if (!$form->get_error('password1')
&& $authobj->authenticate_user_account($USER, $values['password1'])) {
$form->set_error('password1', get_string('passwordnotchanged'));
try {
if (!$form->get_error('password1')
&& $authobj->authenticate_user_account($USER, $values['password1'])) {
$form->set_error('password1', get_string('passwordnotchanged'));
}
}
// propagate error up as the collective error AuthUnknownUserException
catch (AuthInstanceException $e) {
$form->set_error('password1', $e->getMessage());
}
if ($authobj->authname == 'internal' && isset($values['username']) && $values['username'] != $USER->get('username')) {
if (!AuthInternal::is_username_valid($values['username'])) {
......@@ -1158,10 +1175,16 @@ function login_submit(Pieform $form, $values) {
if (!$auth->can_auto_create_users()) {
continue;
}
if ($auth->authenticate_user_account($USER, $password)) {
$authenticated = true;
}
else {
// catch semi-fatal auth errors, but allow next auth instance to be
// tried
try {
if ($auth->authenticate_user_account($USER, $password)) {
$authenticated = true;
}
else {
continue;
}
} catch (AuthInstanceException $e) {
continue;
}
......
......@@ -858,23 +858,28 @@ class LiveUser extends User {
$instanceid = $parentid;
}
$auth = AuthFactory::create($instanceid);
if ($auth->authenticate_user_account($user, $password)) {
$this->authenticate($user, $auth->instanceid);
// Check for a suspended institution
$authinstance = get_record_sql('
SELECT i.suspended, i.displayname
FROM {institution} i JOIN {auth_instance} a ON a.institution = i.name
WHERE a.id = ?', array($instanceid));
if ($authinstance->suspended) {
$sitename = get_config('sitename');
throw new AccessTotallyDeniedException(get_string('accesstotallydenied_institutionsuspended', 'mahara', $authinstance->displayname, $sitename));
return false;
// catch the AuthInstanceException that allows authentication plugins to
// fail but pass onto the next possible plugin
try {
if ($auth->authenticate_user_account($user, $password)) {
$this->authenticate($user, $auth->instanceid);
// Check for a suspended institution
$authinstance = get_record_sql('
SELECT i.suspended, i.displayname
FROM {institution} i JOIN {auth_instance} a ON a.institution = i.name
WHERE a.id = ?', array($instanceid));
if ($authinstance->suspended) {
$sitename = get_config('sitename');
throw new AccessTotallyDeniedException(get_string('accesstotallydenied_institutionsuspended', 'mahara', $authinstance->displayname, $sitename));
return false;
}
return true;
}
return true;
} catch (AuthInstanceException $e) {
return false;
}
// Display a message to users who are only allowed to login via their
// external application.
if ($auth->authloginmsg != '') {
......
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