Commit 8b13e8f9 authored by Robert Lyon's avatar Robert Lyon Committed by Gerrit Code Review
Browse files

Merge "Changing PluginAuth API to specifically indicate whether Auth requires remote username"

parents ef3d71aa 20512fdb
......@@ -157,7 +157,8 @@ if (count($authinstances) > 1) {
// that's the second part of the "if"
if ($USER->can_edit_institution($authinstance->name) || ($authinstance->id == 1 && $user->authinstance == 1)) {
$options[$authinstance->id] = $authinstance->displayname . ': ' . $authinstance->instancename;
if ($authinstance->authname != 'internal' && $authinstance->authname != 'none') {
$authobj = AuthFactory::create($authinstance->id);
if ($authobj->needs_remote_username()) {
$externalauthjs[] = $authinstance->id;
$external = true;
}
......@@ -178,9 +179,11 @@ if (count($authinstances) > 1) {
'type' => 'text',
'title' => get_string('remoteusername', 'admin'),
'description' => get_string('remoteusernamedescription1', 'admin', hsc(get_config('sitename'))),
'defaultvalue' => $un ? $un : $user->username,
'help' => true,
);
if ($un) {
$elements['remoteusername']['defaultvalue'] = $un;
}
}
$remoteusernames = json_encode(get_records_menu('auth_remote_user', 'localusr', $id));
$js = "<script type='text/javascript'>
......@@ -206,6 +209,9 @@ if (count($authinstances) > 1) {
// if value exists in auth_remote_user display it
jQuery('#edituser_site_remoteusername').val(remoteusernames[id]);
}
else {
jQuery('#edituser_site_remoteusername').val('');
}
}
else {
// is internal option so hide external auth field and help text rows
......@@ -382,9 +388,20 @@ function edituser_site_submit(Pieform $form, $values) {
// 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) || $user->authinstance == 1))) {
// But don't bother if the auth instance doesn't take a remote username
$authobj = AuthFactory::create($values['authinstance']);
if (
$authobj->needs_remote_username() && (
$USER->get('admin')
|| (
$USER->is_institutional_admin($authinst[$values['authinstance']]->institution)
&& (
$USER->is_institutional_admin($authinst[$user->authinstance]->institution)
|| $user->authinstance == 1
)
)
)
) {
// determine the current remoteuser
$current_remotename = get_field('auth_remote_user', 'remoteusername',
'authinstance', $user->authinstance, 'localusr', $user->id);
......@@ -395,35 +412,31 @@ function edituser_site_submit(Pieform $form, $values) {
if (strlen(trim($values['remoteusername'])) == 0) {
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' &&
$authinst[$values['authinstance']]->authname != 'none') {
// 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'];
}
}
// 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,
));
// 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'];
}
}
// 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,
));
}
// update the ai on the user master
$user->authinstance = $values['authinstance'];
......
......@@ -332,6 +332,35 @@ abstract class Auth {
public function logout() {
}
/**
* Indicates whether this auth instance is parent to another auth instance
* @return boolean (For backwards-compatibility reasons, it actually returns $this or null)
*/
public function is_parent_authority() {
if (count_records('auth_instance_config', 'field', 'parent', 'value', $this->instanceid)) {
return $this;
}
else {
return null;
}
}
/**
* Returns the ID of this instance's parent authority; or FALSE if it has no parent authority
* @return int|false
*/
public function get_parent_authority() {
return get_field('auth_instance_config', 'value', 'instance', $this->id, 'field', 'parent');
}
/**
* Indicates whether or not this auth instance uses the remote username. Most auth instances
* will only use it if they are the parent to another auth instance.
*/
public function needs_remote_username() {
return (boolean) $this->is_parent_authority();
}
}
......@@ -1353,7 +1382,7 @@ class AuthFactory {
* Take an instanceid and create an auth object for that instance.
*
* @param int $id The id of the auth instance
* @return mixed An intialised auth object or false, if the
* @return Auth An intialised auth object or false, if the
* instance doesn't exist (Should never happen)
*/
public static function create($id) {
......@@ -1491,7 +1520,7 @@ function login_submit(Pieform $form, $values) {
try {
// If this authinstance is a parent auth for some xmlrpc authinstance, pass it along to create_user
// so that this username also gets recorded as the username for sso from the remote sites.
$remoteauth = count_records('auth_instance_config', 'field', 'parent', 'value', $authinstance->id) ? $authinstance : null;
$remoteauth = $auth->is_parent_authority();
create_user($USER, $profilefields, $institution, $remoteauth);
$USER->reanimate($USER->id, $authinstance->id);
}
......
......@@ -268,6 +268,10 @@ class AuthSaml extends Auth {
public function after_auth_setup_page_hook() {
return;
}
public function needs_remote_username() {
return $this->config['remoteuser'] || parent::needs_remote_username();
}
}
/**
......
......@@ -576,6 +576,9 @@ class AuthXmlrpc extends Auth {
$SESSION->set('mnetuser', null);
}
public function needs_remote_username() {
return true;
}
}
/**
......
......@@ -2118,7 +2118,7 @@ function addfriend_submit(Pieform $form, $values) {
* @param object $user stdclass or User object for the usr table
* @param array $profile profile field/values to set
* @param string|object $institution Institution the user should joined to (name or Institution object)
* @param stdclass $remoteauth authinstance record for a remote authinstance
* @param bool $remoteauth authinstance record for a remote authinstance
* @param string $remotename username on the remote site
* @param array $accountprefs user account preferences to set
* @return integer id of the new user
......@@ -2177,7 +2177,10 @@ function create_user($user, $profile=array(), $institution=null, $remoteauth=nul
$accountprefs['licensedefault'] = LICENSE_INSTITUTION_DEFAULT;
}
$authobj = get_record('auth_instance', 'id', $user->authinstance);
if (!empty($remoteauth) && $authobj->authname != 'internal') {
$authinstance = AuthFactory::create($authobj->id);
// For legacy compatibility purposes, we'll also put the remote auth on there if it has been
// specifically requested.
if ($authinstance->needs_remote_username() || (!empty($remoteauth))) {
if (isset($remotename) && strlen($remotename) > 0) {
$un = $remotename;
}
......@@ -2186,7 +2189,7 @@ function create_user($user, $profile=array(), $institution=null, $remoteauth=nul
}
// remote username must not already exist
if (record_exists('auth_remote_user', 'remoteusername', $un, 'authinstance', $user->authinstance)) {
throw new InvalidArgumentException("user_create: remoteusername already exists: ".$un);
throw new InvalidArgumentException("user_create: remoteusername already exists: ({$un}, {$user->authinstance})");
}
insert_record('auth_remote_user', (object) array(
'authinstance' => $user->authinstance,
......
Supports Markdown
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