Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-21782 Major 2.0 enrolments rewrite META
  3. MDL-23232

enrol UI ajax must explicitly ask each enrol plugin instance if manual enrol allowed

    Details

    • Type: Sub-task
    • Status: Closed
    • Priority: Blocker
    • Resolution: Fixed
    • Affects Version/s: 2.0
    • Fix Version/s: 2.0
    • Component/s: Enrolments
    • Labels:
      None
    • Affected Branches:
      MOODLE_20_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE

      Description

      At present the enrol ajax behind the UI uses $plugin->enrol_user($instance, $user->id, $roleid, $timestart, $timeend);

      This is very wrong, you must explicitly ask the plugin if it allows manual enrolments. At minimum you would not be allowed to unenrol, but it could create severe other problems or break synchronisation.

        Gliffy Diagrams

          Activity

          Hide
          skodak Petr Skoda added a comment -

          OH! there is no access control check when enrolling users in enrol/ajax.php

          Show
          skodak Petr Skoda added a comment - OH! there is no access control check when enrolling users in enrol/ajax.php
          Hide
          skodak Petr Skoda added a comment -

          found more places with missing access control there, fixing......

          Show
          skodak Petr Skoda added a comment - found more places with missing access control there, fixing......
          Hide
          skodak Petr Skoda added a comment -

          hey! you have removed all role access control and validation even from enrol/users!
          you must validate that the role is actually assignable by that user in that context before doing anything with it

          Show
          skodak Petr Skoda added a comment - hey! you have removed all role access control and validation even from enrol/users! you must validate that the role is actually assignable by that user in that context before doing anything with it
          Hide
          skodak Petr Skoda added a comment -

          I have added a allow_enrol() method into the base class and patched enrol/manual.

          problems:
          1/ add roles validation and access control into both ajax.php and users.php to both assign and unassign
          2/ add enrol access control and plugin validation - again both
          3/ edit_enrolment uses incorrect capability, should be manage

          I would personally prefer to have the basic access control outside of manager class because it makes reviews many times easier. It would not matter if it was duplicated somewhere down deep such as the role:assign cap

          Show
          skodak Petr Skoda added a comment - I have added a allow_enrol() method into the base class and patched enrol/manual. problems: 1/ add roles validation and access control into both ajax.php and users.php to both assign and unassign 2/ add enrol access control and plugin validation - again both 3/ edit_enrolment uses incorrect capability, should be manage I would personally prefer to have the basic access control outside of manager class because it makes reviews many times easier. It would not matter if it was duplicated somewhere down deep such as the role:assign cap
          Hide
          samhemelryk Sam Hemelryk added a comment -

          Hi Petr,

          I've just commit a series of changes to locallib, users, and ajax that add the missing capability and plugin checks and also copy the checks back into users and ajax.
          Could you please have a look at the changes I've made and the files in general and let me know if I have missed anything.

          Cheers
          Sam

          Show
          samhemelryk Sam Hemelryk added a comment - Hi Petr, I've just commit a series of changes to locallib, users, and ajax that add the missing capability and plugin checks and also copy the checks back into users and ajax. Could you please have a look at the changes I've made and the files in general and let me know if I have missed anything. Cheers Sam

            People

            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:
                Fix Release Date:
                24/Nov/10