catalyst / moodle-mod_scormremote

A Moodle activity for serving SCORM packages remotely to another LMS across domains
https://moodle.org/plugins/mod_scormremote
Other
9 stars 3 forks source link

Seat limit not checked if user already exists #65

Open leonstr opened 5 months ago

leonstr commented 5 months ago

If a user has already been created for one tier they can access other tiers even if the seat limit has been reached.

Steps to reproduce issue

  1. In Moodle set up two courses, for example, Scormremote1 and Scormremote2.
  2. Add a mod_scormremote instance to each course and upload a SCORM file.
  3. Add tiers for each course, Scormremote1 and Scormremote2, with one seat each.
  4. Add a client for the remote site (on mod/scormremote/clients.php) with course subscriptions to Scormremote1 and Scormremote2.
  5. Download the wrapper files for both mod_scormremote instances.
  6. On the remote site add two courses, Scorm1 and Scorm2.
  7. Add one SCORM activity to each course with the wrapper from the corresponding Moodle course.
  8. Add two students and enrol them on both courses.
  9. Log into the remote site as student1 and enter the SCORM in course Scorm1.
  10. Exit the SCORM and log out.
  11. Log in student2 and access the SCORM in course Scorm2.
  12. Exit the SCORM and log out.
  13. Log in student1 and access the SCORM in course Scorm2.

Expected results

Error 402 | Subscription limit reached because there is only one seat for tier Scormremote2 and that is in use by student2.

Actual results

The SCORM is displayed. The Moodle site admin sees ( 2 / 1) for the seats in use for tier Scormremote2 under Manage clients (mod/scormremote/clients.php)

leonstr commented 5 months ago

lib.php explicitly only checks the seat limit if the user doesn't already exist:

188         $user = utils::get_user($client, $username);
189         if (!$user) {
190             // If user doesn't exist create, only when seats are higher then participant count.
191             $tier = new tier($sub->get('tierid'));
192             if ( $sub->get_participant_count() >= (int) $tier->get('seats') ) {
193                 $errorurl = $CFG->wwwroot . '/mod/scormremote/error.php?error=sublimitreached';
194                 header('Content-Type: text/javascript');
195                 exit($OUTPUT->render_from_template('mod_scormremote/init', ['datasource' => $errorurl]));
196             } 
197             $user = utils::create_user($client->get('primarydomain'), $client, $username, $fullname);
198         }

But surely seats used > seat limit isn't the intended behaviour?

leonstr commented 5 months ago

Proposed fix in PR #66.