diff --git a/backup/controller/backup_controller.class.php b/backup/controller/backup_controller.class.php index 1235549c7bf..b342c13685e 100644 --- a/backup/controller/backup_controller.class.php +++ b/backup/controller/backup_controller.class.php @@ -506,6 +506,15 @@ class backup_controller extends base_controller { return $this->plan; } + /** + * For debug only. Get a simple test display of all the settings. + * + * @return string + */ + public function debug_display_all_settings_values(): string { + return $this->get_plan()->debug_display_all_settings_values(); + } + /** * Sets the user roles that should be kept in the destination course * for a course copy operation. diff --git a/backup/controller/restore_controller.class.php b/backup/controller/restore_controller.class.php index f73cb3f00da..1c79c860f0c 100644 --- a/backup/controller/restore_controller.class.php +++ b/backup/controller/restore_controller.class.php @@ -350,6 +350,15 @@ class restore_controller extends base_controller { } } + /** + * For debug only. Get a simple test display of all the settings. + * + * @return string + */ + public function debug_display_all_settings_values(): string { + return $this->get_plan()->debug_display_all_settings_values(); + } + public function get_info() { return $this->info; } diff --git a/backup/moodle2/restore_stepslib.php b/backup/moodle2/restore_stepslib.php index 9e570e18ca0..5b6d3cde29c 100644 --- a/backup/moodle2/restore_stepslib.php +++ b/backup/moodle2/restore_stepslib.php @@ -2144,7 +2144,6 @@ class restore_ras_and_caps_structure_step extends restore_structure_step { public function process_override($data) { $data = (object)$data; - // Check roleid is one of the mapped ones $newrole = $this->get_mapping('role', $data->roleid); $newroleid = $newrole->newitemid ?? false; @@ -2162,8 +2161,8 @@ class restore_ras_and_caps_structure_step extends restore_structure_step { // Check if the new role is an overrideable role AND if the user performing the restore has the // capability to assign the capability. if (in_array($newrole->info['shortname'], $overrideableroles) && - ($safecapability && has_capability('moodle/role:safeoverride', $context, $userid) || - !$safecapability && has_capability('moodle/role:override', $context, $userid)) + (has_capability('moodle/role:override', $context, $userid) || + ($safecapability && has_capability('moodle/role:safeoverride', $context, $userid))) ) { assign_capability($data->capability, $data->permission, $newroleid, $this->task->get_contextid()); } else { diff --git a/backup/tests/roles_backup_restore_test.php b/backup/tests/roles_backup_restore_test.php new file mode 100644 index 00000000000..5e2ac5b8c91 --- /dev/null +++ b/backup/tests/roles_backup_restore_test.php @@ -0,0 +1,180 @@ +. + +defined('MOODLE_INTERNAL') || die(); + +// Include all the needed stuff. +global $CFG; +require_once($CFG->dirroot . '/backup/util/includes/backup_includes.php'); +require_once($CFG->dirroot . '/backup/util/includes/restore_includes.php'); + + +/** + * Unit tests for how backup and restore handles role-related things. + * + * @package core_backup + * @copyright 2021 The Open University + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class roles_backup_restore_test extends advanced_testcase { + + /** + * Create a course where the (non-editing) Teacher role is overridden + * to have 'moodle/user:loginas' and 'moodle/site:accessallgroups'. + * + * @return stdClass the new course. + */ + protected function create_course_with_role_overrides(): stdClass { + $generator = $this->getDataGenerator(); + $course = $generator->create_course(); + $teacher = $generator->create_user(); + + $context = context_course::instance($course->id); + $generator->enrol_user($teacher->id, $course->id, 'teacher'); + + $editingteacherrole = $this->get_role('teacher'); + role_change_permission($editingteacherrole->id, $context, 'moodle/user:loginas', CAP_ALLOW); + role_change_permission($editingteacherrole->id, $context, 'moodle/site:accessallgroups', CAP_ALLOW); + + return $course; + } + + /** + * Get the role id from a shortname. + * + * @param string $shortname the role shortname. + * @return stdClass the role from the DB. + */ + protected function get_role(string $shortname): stdClass { + global $DB; + return $DB->get_record('role', ['shortname' => $shortname]); + } + + /** + * Get an array capability => CAP_... constant for all the orverrides set for a given role on a given context. + * + * @param string $shortname role shortname. + * @param context $context context. + * @return array the overrides set here. + */ + protected function get_overrides_for_role_on_context(string $shortname, context $context): array { + $overridedata = get_capabilities_from_role_on_context($this->get_role($shortname), $context); + $overrides = []; + foreach ($overridedata as $override) { + $overrides[$override->capability] = $override->permission; + } + return $overrides; + } + + /** + * Makes a backup of the course. + * + * @param stdClass $course The course object. + * @return string Unique identifier for this backup. + */ + protected function backup_course(\stdClass $course): string { + global $CFG, $USER; + + // Turn off file logging, otherwise it can't delete the file (Windows). + $CFG->backup_file_logger_level = backup::LOG_NONE; + + // Do backup with default settings. MODE_IMPORT means it will just + // create the directory and not zip it. + $bc = new \backup_controller(backup::TYPE_1COURSE, $course->id, + backup::FORMAT_MOODLE, backup::INTERACTIVE_NO, backup::MODE_IMPORT, + $USER->id); + $backupid = $bc->get_backupid(); + $bc->execute_plan(); + $bc->destroy(); + + return $backupid; + } + + /** + * Restores a backup that has been made earlier. + * + * @param string $backupid The unique identifier of the backup. + * @param string $asroleshortname Which role in the new cousre the restorer should have. + * @return int The new course id. + */ + protected function restore_adding_to_course(string $backupid, string $asroleshortname): int { + global $CFG, $USER; + + // Create course to restore into, and a user to do the restore. + $generator = $this->getDataGenerator(); + $course = $generator->create_course(); + $restorer = $generator->create_user(); + + $generator->enrol_user($restorer->id, $course->id, $asroleshortname); + $this->setUser($restorer); + + // Turn off file logging, otherwise it can't delete the file (Windows). + $CFG->backup_file_logger_level = backup::LOG_NONE; + + // Do restore to new course with default settings. + $rc = new \restore_controller($backupid, $course->id, + backup::INTERACTIVE_NO, backup::MODE_GENERAL, $USER->id, + backup::TARGET_CURRENT_ADDING); + + $precheck = $rc->execute_precheck(); + $this->assertTrue($precheck); + $rc->get_plan()->get_setting('role_assignments')->set_value(true); + $rc->get_plan()->get_setting('permissions')->set_value(true); + $rc->execute_plan(); + $rc->destroy(); + + return $course->id; + } + + public function test_restore_role_overrides_as_manager(): void { + $this->resetAfterTest(); + $this->setAdminUser(); + + // Create a course and back it up. + $course = $this->create_course_with_role_overrides(); + $backupid = $this->backup_course($course); + + // When manager restores, both role overrides should be restored. + $newcourseid = $this->restore_adding_to_course($backupid, 'manager'); + + // Verify. + $overrides = $this->get_overrides_for_role_on_context('teacher', + context_course::instance($newcourseid)); + $this->assertArrayHasKey('moodle/user:loginas', $overrides); + $this->assertEquals(CAP_ALLOW, $overrides['moodle/user:loginas']); + $this->assertArrayHasKey('moodle/site:accessallgroups', $overrides); + $this->assertEquals(CAP_ALLOW, $overrides['moodle/site:accessallgroups']); + } + + public function test_restore_role_overrides_as_teacher(): void { + $this->resetAfterTest(); + $this->setAdminUser(); + + // Create a course and back it up. + $course = $this->create_course_with_role_overrides(); + $backupid = $this->backup_course($course); + + // When teacher restores, only the safe override should be restored. + $newcourseid = $this->restore_adding_to_course($backupid, 'editingteacher'); + + // Verify. + $overrides = $this->get_overrides_for_role_on_context('teacher', + context_course::instance($newcourseid)); + $this->assertArrayNotHasKey('moodle/user:loginas', $overrides); + $this->assertArrayHasKey('moodle/site:accessallgroups', $overrides); + $this->assertEquals(CAP_ALLOW, $overrides['moodle/site:accessallgroups']); + } +} diff --git a/backup/util/plan/base_plan.class.php b/backup/util/plan/base_plan.class.php index 5113d5f818a..1421b349429 100644 --- a/backup/util/plan/base_plan.class.php +++ b/backup/util/plan/base_plan.class.php @@ -126,6 +126,19 @@ abstract class base_plan implements checksumable, executable { return $result; } + /** + * For debug only. Get a simple test display of all the settings. + * + * @return string + */ + public function debug_display_all_settings_values(): string { + $result = ''; + foreach ($this->settings as $name => $setting) { + $result .= $name . ': ' . $setting->get_value() . "\n"; + } + return $result; + } + /** * Wrapper over @get_setting() that returns if the requested setting exists or no */ diff --git a/lib/db/access.php b/lib/db/access.php index d1cdce21a9d..4062f923901 100644 --- a/lib/db/access.php +++ b/lib/db/access.php @@ -664,6 +664,7 @@ $capabilities = array( ) ), + // The ability to override the permissions for any capability. 'moodle/role:override' => array( 'riskbitmask' => RISK_SPAM | RISK_PERSONAL | RISK_XSS, @@ -675,6 +676,8 @@ $capabilities = array( ) ), + // The ability to override the permissions for 'safe' capabilities (those without risks). + // If a user has moodle/role:override then you should not check this capability. 'moodle/role:safeoverride' => array( 'riskbitmask' => RISK_SPAM,