Commit 34ebbfe4 authored by Piers Harding's avatar Piers Harding

Enable multiple auth_remote_user connections

Bug #810302

Enable links to multiple auth instances to be maintained so that
users can have dual login eg: internal + auth/saml etc.

Improve validation around switching auth_instance and
changing the remoteuser at the same time.

Add checks to ensure remoteuser does not get clobbered by update
for another user.  Allow override for the CVS upload case (file == unit
of update).

Change-Id: I5321c0270aeaa93bd193e8e759b08ab7f8b50ded
Signed-off-by: default avatarPiers Harding <piers@catalyst.net.nz>
parent a95292b4
......@@ -244,15 +244,42 @@ function edituser_site_validate(Pieform $form, $values) {
}
}
// Check that the external username isn't already in use
if (isset($values['remoteusername']) &&
$usedby = get_record_select('auth_remote_user',
'authinstance = ? AND remoteusername = ? AND localusr != ?',
array($values['authinstance'], $values['remoteusername'], $values['id']))
) {
$usedbyuser = get_field('usr', 'username', 'id', $usedby->localusr);
$SESSION->add_error_msg(get_string('duplicateremoteusername', 'auth', $usedbyuser));
$form->set_error('remoteusername', get_string('duplicateremoteusernameformerror', 'auth'));
// Check that the external username isn't already in use by someone else
if (isset($values['authinstance']) && isset($values['remoteusername'])) {
// there are 4 cases for changes on the page
// 1) ai and remoteuser have changed
// 2) just ai has changed
// 3) just remoteuser has changed
// 4) the ai changes and the remoteuser is wiped - this is a delete of the old ai-remoteuser
// determine the current remoteuser
$current_remotename = get_field('auth_remote_user', 'remoteusername',
'authinstance', $user->authinstance, 'localusr', $user->id);
if (!$current_remotename) {
$current_remotename = $user->username;
}
// what should the new remoteuser be
$new_remoteuser = get_field('auth_remote_user', 'remoteusername',
'authinstance', $values['authinstance'], 'localusr', $user->id);
if (!$new_remoteuser) {
$new_remoteuser = $user->username;
}
if (strlen(trim($values['remoteusername'])) > 0) {
// value changed on page - use it
if ($values['remoteusername'] != $current_remotename) {
$new_remoteuser = $values['remoteusername'];
}
}
// what really counts is who owns the target remoteuser slot
$target_owner = get_field('auth_remote_user', 'localusr',
'authinstance', $values['authinstance'], 'remoteusername', $new_remoteuser);
// target remoteuser is owned by someone else
if ($target_owner && $target_owner != $user->id) {
$usedbyuser = get_field('usr', 'username', 'id', $target_owner);
$SESSION->add_error_msg(get_string('duplicateremoteusername', 'auth', $usedbyuser));
$form->set_error('remoteusername', get_string('duplicateremoteusernameformerror', 'auth'));
}
}
}
......@@ -292,34 +319,55 @@ function edituser_site_submit(Pieform $form, $values) {
}
set_account_preference($user->id, 'maildisabled', $values['maildisabled']);
// Authinstance can be changed by institutional admins if both the
// old and new authinstances belong to the admin's institutions
$remotename = get_field('auth_remote_user', 'remoteusername', 'authinstance', $user->authinstance, 'localusr', $user->id);
if (!$remotename) {
$remotename = $user->username;
}
if (isset($values['authinstance'])
&& ($values['authinstance'] != $user->authinstance
|| (isset($values['remoteusername']) && $values['remoteusername'] != $remotename))) {
$authinst = get_records_select_assoc('auth_instance', 'id = ? OR id = ?',
// process the change of the authinstance and or the remoteuser
if (isset($values['authinstance']) && isset($values['remoteusername'])) {
// Authinstance can be changed by institutional admins if both the
// old and new authinstances belong to the admin's institutions
$authinst = get_records_select_assoc('auth_instance', 'id = ? OR id = ?',
array($values['authinstance'], $user->authinstance));
if ($USER->get('admin') ||
($USER->is_institutional_admin($authinst[$values['authinstance']]->institution) &&
$USER->is_institutional_admin($authinst[$user->authinstance]->institution))) {
delete_records('auth_remote_user', 'localusr', $user->id);
// determine the current remoteuser
$current_remotename = get_field('auth_remote_user', 'remoteusername',
'authinstance', $user->authinstance, 'localusr', $user->id);
if (!$current_remotename) {
$current_remotename = $user->username;
}
// if the remoteuser is empty and the ai has changed - delete the old remoteuser record
if (strlen(trim($values['remoteusername'])) == 0 &&
$values['authinstance'] != $user->authinstance) {
delete_records('auth_remote_user', 'authinstance', $user->authinstance, 'localusr', $user->id);
}
// we do not create a remoteuser record for internal ai's
if ($authinst[$values['authinstance']]->authname != 'internal') {
if (isset($values['remoteusername']) && strlen($values['remoteusername']) > 0) {
$un = $values['remoteusername'];
// what should the new remoteuser be
$new_remoteuser = get_field('auth_remote_user', 'remoteusername',
'authinstance', $values['authinstance'], 'localusr', $user->id);
// save the remotename for the target existence check
$target_remotename = $new_remoteuser;
if (!$new_remoteuser) {
$new_remoteuser = $user->username;
}
if (strlen(trim($values['remoteusername'])) > 0) {
// value changed on page - use it
if ($values['remoteusername'] != $current_remotename) {
$new_remoteuser = $values['remoteusername'];
}
}
else {
$un = $remotename;
// only update remote name if the input actually changed on the page or it doesn't yet exist
if ($current_remotename != $new_remoteuser || !$target_remotename) {
// only remove the ones related to this traget authinstance as we now allow multiple
// for dual login mechanisms
delete_records('auth_remote_user', 'authinstance', $values['authinstance'], 'localusr', $user->id);
insert_record('auth_remote_user', (object) array(
'authinstance' => $values['authinstance'],
'remoteusername' => $new_remoteuser,
'localusr' => $user->id,
));
}
insert_record('auth_remote_user', (object) array(
'authinstance' => $values['authinstance'],
'remoteusername' => $un,
'localusr' => $user->id,
));
}
// update the ai on the user master
$user->authinstance = $values['authinstance'];
// update the global $authobj to match the new authinstance
......@@ -331,7 +379,7 @@ function edituser_site_submit(Pieform $form, $values) {
// Only change the pw if the new auth instance allows for it
if (method_exists($authobj, 'change_password')) {
$user->passwordchange = (int) ($values['passwordchange'] == 'on');
$user->passwordchange = (int) (isset($values['passwordchange']) && $values['passwordchange'] == 'on' ? 1 : 0);
if (isset($values['password']) && $values['password'] !== '') {
$userobj = new User();
......
......@@ -522,7 +522,7 @@ function uploadcsv_submit(Pieform $form, $values) {
log_debug('added user ' . $user->username);
}
else if (isset($UPDATES[$user->username])) {
$updated = update_user($user, $profilefields, $remoteuser, $values);
$updated = update_user($user, $profilefields, $remoteuser, $values, true);
if (empty($updated)) {
// Nothing changed for this user
......
......@@ -1865,7 +1865,10 @@ function create_user($user, $profile=array(), $institution=null, $remoteauth=nul
else {
$un = $user->username;
}
delete_records('auth_remote_user', 'authinstance', $user->authinstance, 'remoteusername', $un);
// remote username must not allready exist
if (record_exists('auth_remote_user', 'remoteusername', $un, 'authinstance', $user->authinstance)) {
throw new InvalidArgumentException("user_create: remoteusername allready exists: ".$un);
}
insert_record('auth_remote_user', (object) array(
'authinstance' => $user->authinstance,
'remoteusername' => $un,
......@@ -1903,9 +1906,10 @@ function create_user($user, $profile=array(), $institution=null, $remoteauth=nul
* @param object $profile profile field/values to set
* @param string $remotename username on the remote site
* @param array $accountprefs user account preferences to set
* @param bool $forceupdateremote force delete of remotename before update attempted
* @return array list of updated fields
*/
function update_user($user, $profile, $remotename=null, $accountprefs=array()) {
function update_user($user, $profile, $remotename=null, $accountprefs=array(), $forceupdateremote=false) {
require_once(get_config('docroot') . 'auth/session.php');
if (!empty($user->id)) {
......@@ -1952,7 +1956,16 @@ function update_user($user, $profile, $remotename=null, $accountprefs=array()) {
$updated['remoteuser'] = $remotename;
}
delete_records('auth_remote_user', 'authinstance', $user->authinstance, 'localusr', $userid);
delete_records('auth_remote_user', 'authinstance', $user->authinstance, 'remoteusername', $remotename);
// force the update of the remoteuser - for the case of a series of user updates swapping the remoteuser name
if ($forceupdateremote) {
delete_records('auth_remote_user', 'authinstance', $user->authinstance, 'remoteusername', $remotename);
}
else {
// remote username must not allready exist
if (record_exists('auth_remote_user', 'remoteusername', $remotename, 'authinstance', $user->authinstance)) {
throw new InvalidArgumentException("user_update: remoteusername allready in use: ".$remotename);
}
}
insert_record('auth_remote_user', (object) array(
'authinstance' => $user->authinstance,
'remoteusername' => $remotename,
......
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