Moodle

Cannot set auth type using IMS Enterprise plugin

Details

  • Type: Bug Bug
  • Status: Open Open
  • Priority: Major Major
  • Resolution: Unresolved
  • Affects Version/s: 1.9, 1.9.1, 1.9.2, 2.0
  • Fix Version/s: None
  • Component/s: Enrolments
  • Labels:
    None
  • Database:
    Any
  • Affected Branches:
    MOODLE_19_STABLE, MOODLE_20_STABLE

Description

The IMS Enterprise enrolment plugin allows you to create accounts, and set the user's auth type to $CFG->auth

File: /enrol/imsenterprise/enrol.php
Line: 647

$person->auth = $CFG->auth;

However, since the multiauth changes, $CFG->auth returns a comma-separated list of enabled auth plugins. The effect is, for example, to set mdl_user.auth to 'ldap,manual' rather than one or the other. This means the user cannot log in.

I'm really not sure how to address this. It's not possible to specify the authorisation method in the IMS document; or at least your IMS document wouldn't be compliant anymore.

I've marked this as "Major" because it's going to affect some large institutions come October.

Issue Links

Activity

Hide
Chris Fryer added a comment -

OK, I've given it some thought, and here's a patch that allows you to choose the default authentication method for newly-created users from a drop-down list. It appears on /admin/enrol_config.php?enrol=imsenterprise

Seems to work. I'd appreciate some feedback.

Show
Chris Fryer added a comment - OK, I've given it some thought, and here's a patch that allows you to choose the default authentication method for newly-created users from a drop-down list. It appears on /admin/enrol_config.php?enrol=imsenterprise Seems to work. I'd appreciate some feedback.
Hide
Iñaki Arenaza added a comment -

(Added Petr Skodak as a watcher, so he can give his opinion on this)

Chris, I think the bug appears in this commit: http://cvs.moodle.org/moodle/enrol/imsenterprise/enrol.php?r1=1.8&r2=1.8.6.1&pathrev=MOODLE_18_STABLE

Instead of chaning the authentication value, we changed the language value. So I'd say that commit should have read like this:

$person->lang = $CFG->lang;
$person->auth = 'manual';

Of course, that imposes 'manual' authentication to all users created by the IMS Enrolment plugin, which may or may not be a good idea. This is exactly where I'd like Petr to share his opinion on the issue. Should we hardcode 'manual' or should we have a configurable value like Chris' patch allows? I'd favor the latter, but I'd like to have a second opinion on this.

Saludos. Iñaki.

Show
Iñaki Arenaza added a comment - (Added Petr Skodak as a watcher, so he can give his opinion on this) Chris, I think the bug appears in this commit: http://cvs.moodle.org/moodle/enrol/imsenterprise/enrol.php?r1=1.8&r2=1.8.6.1&pathrev=MOODLE_18_STABLE Instead of chaning the authentication value, we changed the language value. So I'd say that commit should have read like this: $person->lang = $CFG->lang; $person->auth = 'manual'; Of course, that imposes 'manual' authentication to all users created by the IMS Enrolment plugin, which may or may not be a good idea. This is exactly where I'd like Petr to share his opinion on the issue. Should we hardcode 'manual' or should we have a configurable value like Chris' patch allows? I'd favor the latter, but I'd like to have a second opinion on this. Saludos. Iñaki.
Hide
Chris Fryer added a comment -

> Of course, that imposes 'manual' authentication to all users created
> by the IMS Enrolment plugin, which may or may not be a good idea.

I've linked this to a related issue, which may help you decide. It's possible to specify a password in a "person" object in IMS Enterprise. In that case, it would make sense to set the auth type to manual. I think it's better to allow administrators to choose an authentication method for users who don't have a password in the IMS document.

There is a conflict between the patches in each issue. MDL-15864 has

if (!isset($person->auth)) $person->auth = $CFG->auth

while MDL-15863 (this bug) has

if(!isset($person->auth)) $person->auth = $CFG->enrol_imse_defaultauth;

Not difficult to resolve, but I thought I'd let you know in case you decide to incorporate the patches into the core.

Show
Chris Fryer added a comment - > Of course, that imposes 'manual' authentication to all users created > by the IMS Enrolment plugin, which may or may not be a good idea. I've linked this to a related issue, which may help you decide. It's possible to specify a password in a "person" object in IMS Enterprise. In that case, it would make sense to set the auth type to manual. I think it's better to allow administrators to choose an authentication method for users who don't have a password in the IMS document. There is a conflict between the patches in each issue. MDL-15864 has if (!isset($person->auth)) $person->auth = $CFG->auth while MDL-15863 (this bug) has if(!isset($person->auth)) $person->auth = $CFG->enrol_imse_defaultauth; Not difficult to resolve, but I thought I'd let you know in case you decide to incorporate the patches into the core.
Hide
Chris Fryer added a comment -

Looks like the same problem exists in Moodle 2.0. The code is now in enrol/imsenterprise/lib.php

Show
Chris Fryer added a comment - Looks like the same problem exists in Moodle 2.0. The code is now in enrol/imsenterprise/lib.php
Hide
Petr Škoda (skodak) added a comment - - edited

I do not like the "$authplugins = get_list_of_plugins('auth');" because it would put there all auth plugins which is wrong because you can not manipulate users manually in all auth types - this could create ugly problems such as imsenrol adding users and other plugin deleting them immediately, I would personally find this setting very confusing.

I would vote for a new method "this_auth_plugin_supports_manual_user_creation" in auth_plugin_base or plugin_supports() + include only enabled auth plugins. I think this would be suitable for 2.0.1. We use this also in standard edit advanced UI.

Thanks for the report and patch!

Petr

Show
Petr Škoda (skodak) added a comment - - edited I do not like the "$authplugins = get_list_of_plugins('auth');" because it would put there all auth plugins which is wrong because you can not manipulate users manually in all auth types - this could create ugly problems such as imsenrol adding users and other plugin deleting them immediately, I would personally find this setting very confusing. I would vote for a new method "this_auth_plugin_supports_manual_user_creation" in auth_plugin_base or plugin_supports() + include only enabled auth plugins. I think this would be suitable for 2.0.1. We use this also in standard edit advanced UI. Thanks for the report and patch! Petr
Hide
Chris Fryer added a comment -

I'm not sure I fully understand your suggestions. The edit advanced UI has a drop down list of auth plugins that is generated in exactly the same way:

$auths = get_plugin_list('auth');
$auth_options = array();
foreach ($auths as $auth => $unused) {
    $auth_options[$auth] = get_string('pluginname', "auth_{$auth}");
}

Do you think it should only display enabled auth plugins here, too?

Show
Chris Fryer added a comment - I'm not sure I fully understand your suggestions. The edit advanced UI has a drop down list of auth plugins that is generated in exactly the same way:
$auths = get_plugin_list('auth');
$auth_options = array();
foreach ($auths as $auth => $unused) {
    $auth_options[$auth] = get_string('pluginname', "auth_{$auth}");
}
Do you think it should only display enabled auth plugins here, too?
Hide
Petr Škoda (skodak) added a comment -

We should not allow selection of auth plugins that do not make sense here - such as mnet, there is no way ims enrol could add users with mnet auth type.

Show
Petr Škoda (skodak) added a comment - We should not allow selection of auth plugins that do not make sense here - such as mnet, there is no way ims enrol could add users with mnet auth type.

People

Vote (5)
Watch (7)

Dates

  • Created:
    Updated: