moodle-tool_mfa icon indicating copy to clipboard operation
moodle-tool_mfa copied to clipboard

Totara 17.3+ patchless support.

Open LiamKearn opened this issue 2 years ago • 7 comments

Changes

This patch attempts to watch the newly introduced Totara hooks:

  • \core\hook\after_require_login
  • \core\hook\after_config

It will forward any calls to them along to the existing lib.php callbacks (tool_mfa_after_config & tool_mfa_after_require_login) that check for authentication.

These hooks were introduced as part of TL-35168. If you are not familar with Totara hooks documentation is available here: https://help.totaralearning.com/display/DEV/Hooks+developer+documentation

This will authenticate users on Totara (17.3+) without the need for core patches.

All changes in this patch are gated with a class_exists check for \totara_core\hook\base to ensure MOODLE compatibility.


Three things to consider

  1. It seems the plan in the future is for these hooks to automatically search for functions in lib.php as normal to preserve functionality with MOODLE plugins. That future plan will hopefully render this change obsolete and make this plugin work with Totara without any changes.

    The source for that plan is a code comment: TODO - PLATFORM-117 to create a watcher to enable those functions via this hook in one of the newly introduced hooks.

  2. I'm sure the \class_exists() in hooks.php is not needed but it can't hurt to be explicit.

  3. With this patch lib.php callbacks could potentially be called before the plugin is installed or enabled. This is different to the MOODLE hook style with get_plugins_with_function which does something like:

    if (!isset($installedplugins[$plugin])) {
        // Plugin code is still present on disk but it is not installed.
        unset($pluginfunctions[$plugintype][$plugin]);
        continue;
    }
    

    At the moment the tool_mfa\manager::require_auth invoked via the lib.php callbacks handles this in it's call to: \tool_mfa\manager::is_ready. However if anything new was introduced directly to lib.php callbacks that requires installed plugin functionality it may create issues.

    I think it's safe to ignore for now especially since Totara seemingly plans to make this obsolete in the future.

LiamKearn avatar Feb 26 '23 04:02 LiamKearn