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

function cohort_get_invisible_contexts incorrectly checks system level cohorts

XMLWordPrintable

    • MOODLE_33_STABLE, MOODLE_34_STABLE, MOODLE_35_STABLE
    • MOODLE_33_STABLE, MOODLE_34_STABLE
    • MDL-61005-master
    • 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"

      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.

       

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

            danmarsden Dan Marsden
            danmarsden Dan Marsden
            Sara Arjona (@sarjona) Sara Arjona (@sarjona)
            Eloy Lafuente (stronk7) Eloy Lafuente (stronk7)
            Janelle Barcega Janelle Barcega
            Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

              Created:
              Updated:
              Resolved:

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

                  Error rendering 'clockify-timesheets-time-tracking-reports:timer-sidebar'. Please contact your Jira administrators.