From ccb94debcf65cdb52dc9868964195b46036f043d Mon Sep 17 00:00:00 2001 From: Paul Holden Date: Fri, 9 May 2025 00:09:54 +0100 Subject: [PATCH] MDL-85421 cohort: limit requests for all cohorts to system context. --- cohort/externallib.php | 6 ++++++ cohort/tests/externallib_test.php | 22 ++++++++++++++++------ 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/cohort/externallib.php b/cohort/externallib.php index 08f62ee78f2..56a19b4c735 100644 --- a/cohort/externallib.php +++ b/cohort/externallib.php @@ -380,6 +380,12 @@ class core_cohort_external extends external_api { $results = array_merge($results, cohort_get_available_cohorts($context, COHORT_ALL, $limitfrom, $limitnum, $query)); } } else if ($includes == 'all') { + $contextsystem = context_system::instance(); + if (!$context instanceof context_system && + !has_any_capability(['moodle/cohort:view', 'moodle/cohort:manage'], $contextsystem)) { + + throw new required_capability_exception($contextsystem, 'moodle/cohort:view', 'nopermissions', ''); + } $results = cohort_get_all_cohorts($limitfrom, $limitnum, $query); $results = $results['cohorts']; } else { diff --git a/cohort/tests/externallib_test.php b/cohort/tests/externallib_test.php index c2ff24e3783..dba15e26d93 100644 --- a/cohort/tests/externallib_test.php +++ b/cohort/tests/externallib_test.php @@ -555,6 +555,7 @@ final class externallib_test extends externallib_advanced_testcase { role_assign($userrole, $catuser->id, $catcontext->id); // Enrol user in the course. + $this->getDataGenerator()->enrol_user($creator->id, $course->id); $this->getDataGenerator()->enrol_user($courseuser->id, $course->id, 'courserole'); $syscontext = array('contextid' => \context_system::instance()->id); @@ -612,18 +613,27 @@ final class externallib_test extends externallib_advanced_testcase { $this->assertEquals(3, count($result['cohorts'])); // A user in the course context with the system cohort:view capability. Check that all the system cohorts are returned. - $this->setUser($courseuser); $result = core_cohort_external::search_cohorts("Cohortsearch", $coursecontext, 'all'); - $this->assertEquals(1, count($result['cohorts'])); - $this->assertEquals('Cohortsearch 1', $result['cohorts'][$cohort1->id]->name); + $this->assertEquals(3, count($result['cohorts'])); + + // A user in the course context without the ability to view system cohorts. + $this->setUser($courseuser); + try { + $result = core_cohort_external::search_cohorts("Cohortsearch", $coursecontext, 'all'); + $this->fail('Exception expected'); + } catch (\Throwable $e) { + $this->assertInstanceOf(\required_capability_exception::class, $e); + $this->assertStringContainsString('(View site-wide cohorts)', $e->getMessage()); + } // Detect invalid parameter $includes. $this->setUser($creator); try { $result = core_cohort_external::search_cohorts("Cohortsearch", $syscontext, 'invalid'); - $this->fail('Invalid parameter includes'); - } catch (\coding_exception $e) { - // All good. + $this->fail('Exception expected'); + } catch (\Throwable $e) { + $this->assertInstanceOf(\coding_exception::class, $e); + $this->assertStringContainsString('Invalid parameter value for \'includes\'', $e->getMessage()); } } }