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

function cohort_get_invisible_contexts incorrectly checks system level cohorts

    XMLWordPrintable

    Details

    • Testing Instructions:
      Hide

      Setup

      1. Login as an admin.
      2. Create some system level cohorts.
      3. Access to "Site administration | Users | Permissions | Define roles" and edit the "Teacher" role:
        1. Add "System" to the "Context types where this role may be assigned".
        2. Check the site-level capability "moodle/cohort:view" is checked.
      4. Access to "Site administration | Users | Permissions | Assign system roles" and assign role "Teacher" in System to a user.

       Testing scenario 1

      1. Login as admin.
      2. Go to Site administration > users > accounts > cohorts > "All Cohorts" tab
      3. Search for some cohorts
        • Make sure the search works as expected.

      Testing scenario 2

      1. Login as the user you've assigned the "Teacher" role in system.
      2. Go to Site administration > users > accounts > cohorts > "All Cohorts" tab (you'll need to write the URL http://yourmoodleserver/cohort/index.php?contextid=1&showall=0).
      3. Search for some cohorts
        • Make sure the search works and all site-level cohorts created show as available.

      Testing scenario 3

      (Nothing to do, CiBoT will tell us): Execute the phpunit for the testcases defined in "cohort/tests/externallib_test.php"

      Show
      Setup Login as an admin. Create some system level cohorts. Access to "Site administration | Users | Permissions | Define roles" and edit the "Teacher" role: Add "System" to the "Context types where this role may be assigned". Check the site-level capability "moodle/cohort:view" is checked. Access to "Site administration | Users | Permissions | Assign system roles" and assign role "Teacher" in System to a user.  Testing scenario 1 Login as admin. Go to Site administration > users > accounts > cohorts > "All Cohorts" tab Search for some cohorts Make sure the search works as expected. Testing scenario 2 Login as the user you've assigned the "Teacher" role in system. Go to Site administration > users > accounts > cohorts > "All Cohorts" tab (you'll need to write the URL http://yourmoodleserver/cohort/index.php?contextid=1&showall=0 ). Search for some cohorts Make sure the search works and all site-level cohorts created show as available. Testing scenario 3 (Nothing to do, CiBoT will tell us): Execute the phpunit for the testcases defined in "cohort/tests/externallib_test.php"
    • Affected Branches:
      MOODLE_33_STABLE, MOODLE_34_STABLE, MOODLE_35_STABLE
    • Fixed Branches:
      MOODLE_33_STABLE, MOODLE_34_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-61005-master

      Description

      cohort/lib.php cohort_get_all_cohorts() states:

      The function assumes that user capability to view/manage cohorts on system level

      • has already been verified. This function only checks if such capabilities have been
      • revoked in child (categories) contexts.

      then it calls: 
      $excludedcontexts = cohort_get_invisible_contexts())

      which finds the contexts that all cohorts might be in and then checks:
      if (!has_any_capability(array('moodle/cohort:manage', 'moodle/cohort:view'), context::instance_by_id($ctx->id)))

      using the context of the cohorts. 

       

      Problem is that it also includes the system level context in the loop and then checks to see if the user has cohort:manage or cohort:view at the system context and if not, excludes the system level cohorts from the list.

      system level cohorts are "special" and permission can be provided at the course-level using the capability "moodle/cohort:view" so it should not be checking the system level context in that loop inside cohort_get_invisible_cohorts.

      hard to reproduce in core, but I'm looking at a separate improvement - re-using the admin tool_lp/cohort searchable cohort selector in the enrolment cohort sync plugin which discovered this particular issue.

      I'll try to provide a pull request to deal with this at some point.

       

        Attachments

        1. admin.PNG
          admin.PNG
          113 kB
        2. teacher.PNG
          teacher.PNG
          111 kB

          Issue Links

            Activity

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  19/Mar/18

                  Time Tracking

                  Estimated:
                  Original Estimate - 0 minutes
                  0m
                  Remaining:
                  Remaining Estimate - 0 minutes
                  0m
                  Logged:
                  Time Spent - 34 minutes
                  34m