Commit 20512fdb authored by Aaron Wells's avatar Aaron Wells Committed by Gerrit Code Review

Changing PluginAuth API to specifically indicate whether Auth requires remote username

Bug 1160093: This adds a few new methods to the Auth class, which represents an auth instance:

 - is_parent_authority(): Indicates whether this auth instance is a parent authority or not
 - get_parent_authority(): Gets the ID of this auth instance's parent authority
 - needs_remote_username(): Indicates whether this auth instance needs the user to have a
      remote username setting (in auth_remote_user table)

I've also updated the SAML and XMLRPC auth types, which are the only ones that use remote username.
And I've updated create_user() to automatically populate auth_remote_user() for auth
instances that use it.

Note that an auth instance of ANY type will need a remote username if it's the parent to another
authority (the parent feature allows a user to log in via the parent or the child auth instance;
so it's quite possible for the user to have different usernames in the two of them. Currently
only XMLRPC uses the parent auth feature.)

Lastly, also updated the documentation of LiveUser->create_user() to indicate that it only
uses the $remoteauth parameter as a boolean (which was true even before my code changes).

Change-Id: I39b1b74e68cdbc9c2632b886655caaaece1bd312
parent 37e23533
......@@ -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,
......
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