mahara-contributors team mailing list archive
-
mahara-contributors team
-
Mailing list archive
-
Message #31792
[Bug 988254] Re: function get_file in api/xmlrpc/lib.php not respecting usersuniquebyusername
Hi Azrael & Yaju,
I've just had a look through the code, and I can confirm no progress has
been made on this bug. Unfortunately, because it only affects the small
subset of sites that are using "usersuniquebyusername" and MNet, and
it's non-trivial to fix and test, we're not likely to get to fixing this
one any time soon, unless someone wants to specifically fund it or
provide their own patch.
After looking at the code, I think your suggestion sounds about right,
Azrael. Perhaps you can try fixing it by patching the find_remote_user()
method in mahara/api/xmlrpc/lib.php.
I took a quick stab at it myself to see if I could solve it in a few
minutes, and this is what I came up with. Note, though, that if you mave
multiple XMLRPC auth instances with the same WWWROOT, then there's no
way for us know which one we should be using in these cases, when we
want to check whether the "we import content" flags, etc are turned on.
In the solution I whipped up (but haven't tested yet) I just logged a
debug message in these cases and arbitrarily used the first matching
auth instance. I think a comprehensive solution would require perhaps
syncing all the auth instances for a MNet peer together; or collating
the settings from all of them; etc.
function find_remote_user($username, $wwwroot) {
$authinstances = auth_get_auth_instances_for_wwwroot($wwwroot);
$candidates = array();
// TODO: The usersuniquebyusername logic is partially replicated in
// XMLRPC, SAML, and now here. Maybe we could abstract it to a shared
// location?
if (get_config('usersuniquebyusername')) {
// When turned on, this setting means that it doesn't matter
// which other application the user SSOs from, they will be
// given the same account in Mahara.
//
// This setting is one that has security implications unless
// only turned on by people who know what they're doing. In
// particular, every system linked to Mahara should be making
// sure that same username == same person. This happens for
// example if two Moodles are using the same LDAP server for
// authentication.
//
// If this setting is on, it must NOT be possible to self
// register on the site for ANY institution - otherwise users
// could simply pick usernames of people's accounts they wished
// to steal.
if ($institutions = get_column('institution', 'name', 'registerallowed', '1')) {
log_warn("usersuniquebyusername is turned on but registration is allowed for an institution. "
. "No institution can have registration allowed for it, for security reasons.\n"
. "The following institutions have registration enabled:\n " . join("\n ", $institutions));
throw new AccessDeniedException();
}
if (!get_config('usersallowedmultipleinstitutions')) {
log_warn("usersuniquebyusername is turned on but usersallowedmultipleinstitutions is off. "
. "This makes no sense, as users will then change institution every time they log in from "
. "somewhere else. Please turn this setting on in Site Options");
throw new AccessDeniedException();
}
$user = new User();
try {
$user->find_by_username($remoteuser->username);
}
catch (AuthUnknownUserException $e) {
return false;
}
if (count($authinstances > 1)) {
log_debug("More than one XMLRPC instance with wwwroot {$wwwroot}; arbitrarily picking the first one.");
foreach ($authinstances as $authinstance) {
if ($authinstance->authname='xmlrpc') {
$candidates[$authinstance->id] = $user;
break;
}
}
}
else {
$authinstance = reset($authinstances);
$candidates[$authinstance->id] = $user;
}
}
else {
foreach ($authinstances as $authinstance) {
if ($authinstance->authname != 'xmlrpc') {
continue;
}
try {
$user = new User;
$user->find_by_instanceid_username($authinstance->id, $username, true);
$candidates[$authinstance->id] = $user;
} catch (Exception $e) {
// we don't care
continue;
}
}
if (count($candidates) != 1) {
return false;
}
}
safe_require('auth', 'xmlrpc');
return array(end($candidates), new AuthXmlrpc(key($candidates)));
}
Try dropping that into api/xmlrpc/lib.php in place of the current
find_remote_user(), and see if it works.
Cheers,
Aaron
--
You received this bug notification because you are a member of Mahara
Contributors, which is subscribed to Mahara.
Matching subscriptions: Subscription for all Mahara Contributors -- please ask on #mahara-dev or mahara.org forum before editing or unsubscribing it!
https://bugs.launchpad.net/bugs/988254
Title:
function get_file in api/xmlrpc/lib.php not respecting
usersuniquebyusername
Status in Mahara:
Triaged
Bug description:
When I hook my 1 Mahara installation up to 2 (or more) Moodle
installations, and try to push content from Moodle to Mahara, I
encounter an error.
Failed to start communication with remote server: remote server error: code: , message: Could not find user BLAH for https://BLAH.BLAH.ac.ukERROR 4:
4: remote server error: code: , message: Could not find user BLAH for https://BLAH.BLAH.ac.uk
This error is from xmlrpc/lib.php get_file() (line 286) which calls
find_remote_user($username, $REMOTEWWWROOT).
I am fairly sure there is a bug in xmlrpc/lib.php find_remote_user()
because if I change the authentication of a user in Mahara to be from
a specific Moodle, I can export from that Moodle but not the other,
and vice versa.
This makes sense if each user from different systems (even if they
have the same username) is treated as a different user, which is
default behaviour. However I have the config option of
usersuniquebyusername set so that users from different systems with
the same username are treated as the same user.
$cfg->usersuniquebyusername = true;
In this case I would expect Mahara NOT to try and confirm that the
source of the user is the same one listed within Mahara as their
authentication method.
My gut reaction is that the find_remote_user function in
xmlrpc/lib.php (line 125) should be checking for the CFG value of
usersuniquebyusername and if set then specifically NOT discriminate by
authentication method.
So something along the lines of the following, changing line 134
from:
$authinstances =
auth_get_auth_instances_for_institution($institution);
to:
if (get_config('usersuniquebyusername')) {
$authinstances = auth_get_auth_instances();
}
else {
$authinstances = auth_get_auth_instances_for_institution($institution);
}
To manage notifications about this bug go to:
https://bugs.launchpad.net/mahara/+bug/988254/+subscriptions
References