Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-69377

Group settings form does not perform case & accent sensitive search for existing enrolment keys (with patch proposal)

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Open
    • Priority: Minor
    • Resolution: Unresolved
    • Affects Version/s: 3.5.13, 3.7.7, 3.8.4, 3.9.1
    • Fix Version/s: None
    • Component/s: Groups
    • Labels:
      None
    • Affected Branches:
      MOODLE_35_STABLE, MOODLE_37_STABLE, MOODLE_38_STABLE, MOODLE_39_STABLE

      Description

      Hi! First time reporting a bug here, so please forgive me if I fumble any conventions. =) I discovered an issue with the group settings form, enrolment keys, and what I believe is undesirable behavior in group/group_form.php.

      FULL STEPS TO REPRODUCE

      1. First, make sure you are using a database type that does not natively do case sensitive string comparisons, such as MySQL or MariaDB.
      2. Then make sure you have groupenrolmentkeypolicy checked in Site administration > Security > Site security settings.
      3. Also make sure your site password policy requires at least one capital letter. For the sake of this test, make this the only requirement of your password policy. 
      4. Now, in a course where users can self-enrol with group enrolment keys, create a group and give it the enrolment key "TrustMe".
      5. Create another group and try to give it the enrolment key "Trustme".

      WHAT I EXPECTED

      • I expected that I would be allowed to have "Trustme" as an enrolment key because it's different than "TrustMe."
      • By different, I specifically mean that when enrol_self_plugin->enrol_self does a foreach loop through all of a course's groups to find an enrolmentkey that matches $data->enrolpassword, there is an if statement that does a strict comparison check (which is case and accent sensitive): if ($group->enrolmentkey === $data->enrolpassword).

      WHAT ACTUALLY HAPPENS

      • I get the enrolmentkeyalreadyinuse error ("This enrolment key is already used for another group.").

      REALITY CHECK

      • In practice, it's probably stressful and annoying to have "Trustme" and "TrustMe" as group enrolment codes in the same course. Imagine telling a learner: "Go enrol in my group by typing in TrustMe, but make sure you don't type Trustme... or you will be enrolled in the group of my nemesis!!!!"
      • Then again, people use groups so many different ways, and these comparisons should match, especially if they're somewhat "under the umbrella" of fussy password policies. =)

      PROPOSED PATCH

      $sql = "SELECT id FROM {groups} WHERE id <> :groupid AND courseid = :courseid AND enrolmentkey = :key";

      • In databases with case-insensitive string comparison, this makes for the unexpected behavior. I suggest using the Moodle $DB API function sql_equal here with case sensitivity and accent sensitivity to match the behavior of the === operator in PHP:

      $sql = "SELECT id FROM {groups} WHERE id <> :groupid AND courseid = :courseid AND " . $DB->sql_equal('enrolmentkey', ':key', true,true);

      THANK YOU! =)

        Attachments

          Activity

            People

            Assignee:
            Unassigned Unassigned
            Reporter:
            mikeayoung Mike Young
            Participants:
            Component watchers:
            Andrew Nicols, Jun Pataleta, Michael Hawkins, Shamim Rezaie, Simey Lameze
            Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

              Dates

              Created:
              Updated: