Permalink
Browse files

MDL-19125 module restrictions: use a capability

1. This used to use a complex legacy system which was buggy.

2. It now relies on a new mod/...:addinstance capability for each module.

3. All the legacy code has been stripped out.

4. Old restriction data is upgraded by creating the necessary permission
overrides. Similarly, when old backups are restored, the old settings
are converted to be overrides.

5. The required addinstance capabilities will be added as a separate
commit.

6. There is a developer debug warning about modules that are missing the
addinstance capability, unless they are MOD_ARCHETYPE_SYSTEM mods.
  • Loading branch information...
1 parent a2b30aa commit 9665ecd27555e919555100f159f1879d52e31793 @timhunt timhunt committed Mar 7, 2012
@@ -86,19 +86,6 @@
$ADMIN->add('security', $temp);
- // "modulesecurity" settingpage
- $temp = new admin_settingpage('modulesecurity', new lang_string('modulesecurity', 'admin'));
- $temp->add(new admin_setting_configselect('restrictmodulesfor', new lang_string('restrictmodulesfor', 'admin'), new lang_string('configrestrictmodulesfor', 'admin'), 'none', array('none' => new lang_string('nocourses'),
- 'all' => new lang_string('fulllistofcourses'),
- 'requested' => new lang_string('requestedcourses'))));
- $temp->add(new admin_setting_configcheckbox('restrictbydefault', new lang_string('restrictbydefault', 'admin'), new lang_string('configrestrictbydefault', 'admin'), 0));
- $temp->add(new admin_setting_configmultiselect_modules('defaultallowedmodules',
- new lang_string('defaultallowedmodules', 'admin'),
- new lang_string('configdefaultallowedmodules', 'admin')));
- $ADMIN->add('security', $temp);
-
-
-
// "notifications" settingpage
$temp = new admin_settingpage('notifications', new lang_string('notifications', 'admin'));
$temp->add(new admin_setting_configselect('displayloginfailures', new lang_string('displayloginfailures', 'admin'), new lang_string('configdisplayloginfailures', 'admin'), '', array('' => new lang_string('nobody'),
@@ -403,7 +403,7 @@ protected function define_structure() {
'visible', 'hiddensections', 'groupmode', 'groupmodeforce',
'defaultgroupingid', 'lang', 'theme',
'timecreated', 'timemodified',
- 'requested', 'restrictmodules',
+ 'requested',
'enablecompletion', 'completionstartonenrol', 'completionnotify'));
$category = new backup_nested_element('category', array('id'), array(
@@ -414,10 +414,6 @@ protected function define_structure() {
$tag = new backup_nested_element('tag', array('id'), array(
'name', 'rawname'));
- $allowedmodules = new backup_nested_element('allowed_modules');
-
- $module = new backup_nested_element('module', array(), array('modulename'));
-
// attach format plugin structure to $course element, only one allowed
$this->add_plugin_structure('format', $course, false);
@@ -444,9 +440,6 @@ protected function define_structure() {
$course->add_child($tags);
$tags->add_child($tag);
- $course->add_child($allowedmodules);
- $allowedmodules->add_child($module);
-
// Set the sources
$courserec = $DB->get_record('course', array('id' => $this->task->get_courseid()));
@@ -466,11 +459,6 @@ protected function define_structure() {
backup_helper::is_sqlparam('course'),
backup::VAR_PARENTID));
- $module->set_source_sql('SELECT m.name AS modulename
- FROM {modules} m
- JOIN {course_allowed_modules} cam ON m.id = cam.module
- WHERE course = ?', array(backup::VAR_COURSEID));
-
// Some annotations
$course->annotate_ids('grouping', 'defaultgroupingid');
@@ -1069,6 +1069,21 @@ protected function after_execute() {
* the course record (never inserting)
*/
class restore_course_structure_step extends restore_structure_step {
+ /**
+ * @var bool this gets set to true by {@link process_course()} if we are
+ * restoring an old coures that used the legacy 'module security' feature.
+ * If so, we have to do more work in {@link after_execute()}.
+ */
+ protected $legacyrestrictmodules = false;
+
+ /**
+ * @var array Used when {@link $legacyrestrictmodules} is true. This is an
+ * array with array keys the module names ('forum', 'quiz', etc.). These are
+ * the modules that are allowed according to the data in the backup file.
+ * In {@link after_execute()} we then have to prevent adding of all the other
+ * types of activity.
+ */
+ protected $legacyallowedmodules = array();
protected function define_structure() {
@@ -1132,7 +1147,7 @@ public function process_course($data) {
}
// Only restrict modules if original course was and target site too for new courses
- $data->restrictmodules = $data->restrictmodules && !empty($CFG->restrictmodulesfor) && $CFG->restrictmodulesfor == 'all';
+ $this->legacyrestrictmodules = !empty($data->restrictmodules);
$data->startdate= $this->apply_date_offset($data->startdate);
if ($data->defaultgroupingid) {
@@ -1187,31 +1202,44 @@ public function process_tag($data) {
}
public function process_allowed_module($data) {
- global $CFG, $DB;
-
$data = (object)$data;
- // only if enabled by admin setting
- if (!empty($CFG->restrictmodulesfor) && $CFG->restrictmodulesfor == 'all') {
- $available = get_plugin_list('mod');
- $mname = $data->modulename;
- if (array_key_exists($mname, $available)) {
- if ($module = $DB->get_record('modules', array('name' => $mname, 'visible' => 1))) {
- $rec = new stdclass();
- $rec->course = $this->get_courseid();
- $rec->module = $module->id;
- if (!$DB->record_exists('course_allowed_modules', (array)$rec)) {
- $DB->insert_record('course_allowed_modules', $rec);
- }
- }
- }
+ // Backwards compatiblity support for the data that used to be in the
+ // course_allowed_modules table.
+ if ($this->legacyrestrictmodules) {
+ $this->legacyallowedmodules[$data->modulename] = 1;
}
}
protected function after_execute() {
+ global $DB;
+
// Add course related files, without itemid to match
$this->add_related_files('course', 'summary', null);
$this->add_related_files('course', 'legacy', null);
+
+ // Deal with legacy allowed modules.
+ if ($this->legacyrestrictmodules) {
+ $context = context_course::instance($this->get_courseid());
+
+ list($roleids) = get_roles_with_cap_in_context($context, 'moodle/course:manageactivities');
+ list($managerroleids) = get_roles_with_cap_in_context($context, 'moodle/site:config');
+ foreach ($managerroleids as $roleid) {
+ unset($roleids[$roleid]);
+ }
+
+ foreach (get_plugin_list('mod') as $modname => $notused) {
+ if (isset($this->legacyallowedmodules[$modname])) {
+ // Module is allowed, no worries.
+ continue;
+ }
+
+ $capability = 'mod/' . $modname . ':addinstance';
+ foreach ($roleids as $roleid) {
+ assign_capability($capability, CAP_PREVENT, $roleid, $context);
+ }
+ }
+ }
}
}
View
@@ -65,18 +65,6 @@
// Prepare course and the editor
$editoroptions = array('maxfiles' => EDITOR_UNLIMITED_FILES, 'maxbytes'=>$CFG->maxbytes, 'trusttext'=>false, 'noclean'=>true);
if (!empty($course)) {
- $allowedmods = array();
- if ($am = $DB->get_records('course_allowed_modules', array('course'=>$course->id))) {
- foreach ($am as $m) {
- $allowedmods[] = $m->module;
- }
- } else {
- // this happens in case we edit course created before enabling module restrictions or somebody disabled everything :-(
- if (empty($course->restrictmodules) and !empty($CFG->defaultallowedmodules)) {
- $allowedmods = explode(',', $CFG->defaultallowedmodules);
- }
- }
- $course->allowedmods = $allowedmods;
//add context for editor
$editoroptions['context'] = $coursecontext;
$course = file_prepare_standard_editor($course, 'summary', $editoroptions, $coursecontext, 'course', 'summary', 0);
View
@@ -256,48 +256,6 @@ function definition() {
$mform->setDefault('completionstartonenrol',0);
}
-//--------------------------------------------------------------------------------
- if (has_capability('moodle/site:config', $systemcontext)) {
- if (((!empty($course->requested) && $CFG->restrictmodulesfor == 'requested') || $CFG->restrictmodulesfor == 'all')) {
- $mform->addElement('header', '', get_string('restrictmodules'));
-
- $options = array();
- $options['0'] = get_string('no');
- $options['1'] = get_string('yes');
- $mform->addElement('select', 'restrictmodules', get_string('restrictmodules'), $options);
- if (!empty($CFG->restrictbydefault)) {
- $mform->setDefault('restrictmodules', 1);
- }
-
- $mods = array(0=>get_string('allownone'));
- $allmods = $DB->get_records_menu('modules', array('visible' => 1),
- 'name', 'id, name');
- foreach ($allmods as $key => $value) {
- // Add module to list unless it cannot be added by users anyway
- if (plugin_supports('mod', $value, FEATURE_MOD_ARCHETYPE, MOD_ARCHETYPE_OTHER) !==
- MOD_ARCHETYPE_SYSTEM) {
- $mods[$key] = get_string('pluginname', $value);
- }
- }
- $mform->addElement('select', 'allowedmods', get_string('to'), $mods, array('multiple'=>'multiple', 'size'=>'10'));
- $mform->disabledIf('allowedmods', 'restrictmodules', 'eq', 0);
- // defaults are already in $course
- } else {
- // remove any mod restriction
- $mform->addElement('hidden', 'restrictmodules', 0);
- $mform->setType('restrictmodules', PARAM_INT);
- }
- } else {
- $mform->addElement('hidden', 'restrictmodules');
- $mform->setType('restrictmodules', PARAM_INT);
- if (empty($course->id)) {
- $mform->setConstant('restrictmodules', (int)($CFG->restrictmodulesfor == 'all'));
- } else {
- // keep previous
- $mform->setConstant('restrictmodules', $course->restrictmodules);
- }
- }
-
/// customizable role names in this course
//--------------------------------------------------------------------------------
$mform->addElement('header','rolerenaming', get_string('rolerenaming'));
View
@@ -3391,58 +3391,37 @@ function course_format_name ($course,$max=100) {
}
}
-function update_restricted_mods($course, $mods) {
- global $DB;
-
-/// Delete all the current restricted list
- $DB->delete_records('course_allowed_modules', array('course'=>$course->id));
-
- if (empty($course->restrictmodules)) {
- return; // We're done
- }
-
-/// Insert the new list of restricted mods
- foreach ($mods as $mod) {
- if ($mod == 0) {
- continue; // this is the 'allow none' option
- }
- $am = new stdClass();
- $am->course = $course->id;
- $am->module = $mod;
- $DB->insert_record('course_allowed_modules',$am);
- }
-}
-
/**
- * This function will take an int (module id) or a string (module name)
- * and return true or false, whether it's allowed in the given course (object)
- * $mod is not allowed to be an object, as the field for the module id is inconsistent
- * depending on where in the code it's called from (sometimes $mod->id, sometimes $mod->module)
+ * Is the user allowed to add this type of module to this course?
+ * @param object $course the course settings. Only $course->id is used.
+ * @param string $modname the module name. E.g. 'forum' or 'quiz'.
+ * @return bool whether the current user is allowed to add this type of module to this course.
*/
-
-function course_allowed_module($course,$mod) {
+function course_allowed_module($course, $modname) {
global $DB;
- if (empty($course->restrictmodules)) {
- return true;
+ if (is_numeric($modname)) {
+ throw new coding_exception('Function course_allowed_module no longer
+ supports numeric module ids. Please update your code to pass the module name.');
}
- // Admins and admin-like people who can edit everything can also add anything.
- // Originally there was a course:update test only, but it did not match the test in course edit form
- if (has_capability('moodle/site:config', get_context_instance(CONTEXT_SYSTEM))) {
- return true;
- }
+ $capability = 'mod/' . $modname . ':addinstance';
+ if (!get_capability_info($capability)) {
+ // Debug warning that the capability does not exist, but no more than once per page.
+ static $warned = array();
+ $archetype = plugin_supports('mod', $modname, FEATURE_MOD_ARCHETYPE, MOD_ARCHETYPE_OTHER);
+ if (!isset($warned[$modname]) && $archetype !== MOD_ARCHETYPE_SYSTEM) {
+ debugging('The module ' . $modname . ' does not define the standard capability ' .
+ $capability , DEBUG_DEVELOPER);
+ $warned[$modname] = 1;
+ }
- if (is_numeric($mod)) {
- $modid = $mod;
- } else if (is_string($mod)) {
- $modid = $DB->get_field('modules', 'id', array('name'=>$mod));
- }
- if (empty($modid)) {
- return false;
+ // If the capability does not exist, the module can always be added.
+ return true;
}
- return $DB->record_exists('course_allowed_modules', array('course'=>$course->id, 'module'=>$modid));
+ $coursecontext = context_course::instance($course->id);
+ return has_capability($capability, $coursecontext);
}
/**
@@ -3910,17 +3889,6 @@ function create_course($data, $editoroptions = NULL) {
fix_course_sortorder();
- // update module restrictions
- if ($course->restrictmodules) {
- if (isset($data->allowedmods)) {
- update_restricted_mods($course, $data->allowedmods);
- } else {
- if (!empty($CFG->defaultallowedmodules)) {
- update_restricted_mods($course, explode(',', $CFG->defaultallowedmodules));
- }
- }
- }
-
// new context created - better mark it as dirty
mark_context_dirty($context->path);
@@ -3999,11 +3967,6 @@ function update_course($data, $editoroptions = NULL) {
// Test for and remove blocks which aren't appropriate anymore
blocks_remove_inappropriate($course);
- // update module restrictions
- if (isset($data->allowedmods)) {
- update_restricted_mods($course, $data->allowedmods);
- }
-
// Save any custom role names.
save_local_role_names($course->id, $data);
@@ -4316,9 +4279,6 @@ public function approve() {
// Set misc settings
$data->requested = 1;
- if (!empty($CFG->restrictmodulesfor) && $CFG->restrictmodulesfor != 'none' && !empty($CFG->restrictbydefault)) {
- $data->restrictmodules = 1;
- }
// Apply course default settings
$data->format = $courseconfig->format;
View
@@ -59,7 +59,7 @@
$cw = get_course_section($section, $course->id);
- if (!course_allowed_module($course, $module->id)) {
+ if (!course_allowed_module($course, $module->name)) {
print_error('moduledisable');
}
View
@@ -171,7 +171,6 @@
$string['configdebugpageinfo'] = 'Enable if you want page information printed in page footer.';
$string['configdebugsmtp'] = 'Enable verbose debug information during sending of email messages to SMTP server.';
$string['configdebugvalidators'] = 'Enable if you want to have links to external validator servers in page footer. You may need to create new user with username <em>w3cvalidator</em>, and enable guest access. These changes may allow unauthorized access to server, do not enable on production sites!';
-$string['configdefaultallowedmodules'] = 'For the courses which fall into the above category, which modules do you want to allow by default <b>when the course is created</b>?';
$string['configdefaulthomepage'] = 'This determines the home page for logged in users';
$string['configdefaultrequestcategory'] = 'Courses requested by users will be automatically placed in this category.';
$string['configdefaultrequestedcategory'] = 'Default category to put courses that were requested into, if they\'re approved.';
@@ -285,8 +284,6 @@
$string['configrequestedteachername'] = 'Word for teacher used in requested courses';
$string['configrequestedteachersname'] = 'Word for teachers used in requested courses';
$string['configrequiremodintro'] = 'Disable this option if you do not want to force users to enter description of each activity.';
-$string['configrestrictbydefault'] = 'Should new courses that are created that fall into the above category have their modules restricted by default?';
-$string['configrestrictmodulesfor'] = 'Which courses should have <b>the setting</b> for disabling some activity modules? Note that this setting only applies to teachers, administrators will still be able to add any activity to a course.';
$string['configrunclamavonupload'] = 'When enabled, clam AV will be used to scan all uploaded files.';
$string['configrunclamonupload'] = 'Run clam AV on file upload? You will need a correct path in pathtoclam for this to work. (Clam AV is a free virus scanner that you can get from http://www.clamav.net/)';
$string['configuserquota'] = 'The maximum number of bytes that a user can store in their own private file area. {$a->bytes} bytes == {$a->displaysize}';
@@ -407,7 +404,6 @@
$string['debugstringids'] = 'Show origin of languages strings';
$string['debugstringids_desc'] = 'This option is designed to help translators. When this option is enabled, if you add the parameter strings=1 to a request URL, it will show the language file and string id beside each string that is output.';
$string['debugvalidators'] = 'Show validator links';
-$string['defaultallowedmodules'] = 'Default allowed modules';
$string['defaultcity'] = 'Default city';
$string['defaultcity_help'] = 'A city entered here will be the default city when creating new user accounts.';
$string['defaulthomepage'] = 'Default home page for users';
@@ -861,8 +857,6 @@
$string['purgecachesfinished']= 'All caches were purged.';
$string['restorernewroleid'] = 'Restorers\' role in courses';
$string['restorernewroleid_help'] = 'If the user does not already have the permission to manage the newly restored course, the user is automatically assigned this role and enrolled if necessary. Select "None" if you do not want restorers to be able to manage every restored course.';
-$string['restrictbydefault'] = 'Restrict modules by default';
-$string['restrictmodulesfor'] = 'Restrict modules for';
$string['reverseproxy'] = 'Reverse proxy';
$string['riskconfig'] = 'Users could change site configuration and behaviour';
$string['riskconfigshort'] = 'Configuration risk';
View
@@ -1385,7 +1385,6 @@
$string['restoreusersprecheck'] = 'Checking user data';
$string['restoreusersprecheckerror'] = 'Some problems were detected when checking user data';
$string['restricted'] = 'Restricted';
-$string['restrictmodules'] = 'Restrict activity modules?';
$string['returningtosite'] = 'Returning to this web site?';
$string['returntooriginaluser'] = 'Return to {$a}';
$string['revert'] = 'Revert';
Oops, something went wrong.

0 comments on commit 9665ecd

Please sign in to comment.