search

adlnet / Moodle-mod_cmi5launch

A Moodle plugin which allows teachers to upload cmi5 packaged lessons within a Moodle Course Activity and then assign the activity to students
Apache License 2.0
5 stars 5 forks source link

Url work mb #1

Closed ADLMeganBohland closed 1 year ago

ADLMeganBohland commented 1 year ago

Work done on enabled Moodle to communicate with CMI5 player. CMI5 player now generates and sends re ID. Moodle uses this instead of randomly creating it's own.

rajkowski commented 1 year ago

Thanks @ADLMeganBohland I'm starting to review the changes.

Can you combine the hardcoded http, the cmi5launchplayerurl value, and the cmi5launchplayerport value?

When the Admin configures the 'cmi5 player url', it should be a single setting containing the URL scheme, name, and optional port. So one setting which an admin will populate with a full and valid URL: "https://mycmi5player.example.com"

In the code there is an assumption that http will be used (it could be a remote service requiring https) and it's possible that a port would not be specified:

//Build URL to import course to $url= "http://" . $settings['cmi5launchplayerurl'] . ":" . $settings['cmi5launchplayerport'] . "/api/v1/course" ;

ADLMeganBohland commented 1 year ago

Hey Matt,

I think I can make these changes. When the plugin is installed it asks for the addy/port separate. You want me to change this correct? So the admin only enters one thing?

rajkowski commented 1 year ago

Yes, exactly that! "So the admin only enters one thing"

rajkowski commented 1 year ago

Hi @ADLMeganBohland, I noticed another instance of the Moodle URL itself is being referenced. There might be a global function (I looked but didn't find one), so I have a recommendation...

Replace:

 //We need to parse and cutoff from the ///
     //Build url to pass as returnUrl
    $returnUrl = ('http://' . $_SERVER['HTTP_HOST'] . $_SERVER['PHP_SELF'] .'/mod/cmi5launch/view.php'. '?id=' .$cm->id);

with:

$returnUrl = $CFG->wwwroot . $_SERVER['SCRIPT_NAME'] . '/mod/cmi5launch/view.php'. '?id=' .$cm->id;

Use CFG->wwwroot to get the Admin's intended Moodle URL, with scheme (the url could be behind a proxy, use SSL, etc.), and use SCRIPT_NAME to prevent spoofing

ADLMeganBohland commented 1 year ago

@rajkowski , By doing this: $returnUrl = $CFG->wwwroot .'/mod/cmi5launch/view.php'. '?id=' .$cm->id; We are able to build the return URL correctly. This should prevent issues with proxy servers, but have not found a way to also use 'SCRIPT_NAME'.