MDL-85431 questions: Fix access checks on Question banks page

Users who could not manage activities on a course, but did have
permission to access particular question banks, could not access the
Question banks page.

This modifies the permission checks on that page to allow access if the
user can either manage activities, or has access to at least one
activity that publishes questions. Some additional changes to the output
were required to prevent non-editing users adding banks.

It also modifies the navigation hook to add the Question banks
navigation node if the user can access any activitiy that publishes
questions on the course. This is all based on cached modinfo and
permissions data so should remain performant.
This commit is contained in:
Mark Johnson
2026-01-05 15:20:51 +00:00
parent 6e82b46a48
commit fbf2963387
5 changed files with 68 additions and 17 deletions
+1
View File
@@ -252,6 +252,7 @@ $string['movingquestions'] = 'Moving questions and any files';
$string['movingquestionsandfiles'] = 'Are you sure you want to move question(s) {$a->questions} to context for <strong>"{$a->tocontext}"</strong>?<br /> We have detected <strong>{$a->urlcount} files</strong> linked from these question(s) in {$a->fromareaname}, would you like to copy or move these to {$a->toareaname}?';
$string['movingquestionsnofiles'] = 'Are you sure you want to move question(s) {$a->questions} to context for <strong>"{$a->tocontext}"</strong>?<br /> There are <strong>no files</strong> linked from these question(s) in {$a->fromareaname}.';
$string['needtochoosecat'] = 'You need to choose a category to move this question to or press \'cancel\'.';
$string['nobankpermissions'] = 'You do not have permission to access any question banks on this course.';
$string['nobanks'] = 'This course doesn\'t have any question banks yet.';
$string['nocate'] = 'No such category {$a}!';
$string['nopermissionadd'] = 'You don\'t have permission to add questions here.';
+27 -8
View File
@@ -1416,14 +1416,33 @@ function question_extend_settings_navigation(navigation_node $navigationnode, $c
$iscourse = $context->contextlevel === CONTEXT_COURSE;
if ($iscourse && has_capability('moodle/course:manageactivities', $context)) {
return $navigationnode->add(
get_string('questionbank_plural', 'question'),
new moodle_url($baseurl, ['courseid' => $context->instanceid]),
navigation_node::TYPE_CONTAINER,
null,
'questionbank'
);
if ($iscourse) {
$viewquestionbanks = has_capability('moodle/course:manageactivities', $context);
if (!$viewquestionbanks) {
// If the user can view any activities with shared questions, display the Question banks node.
// If they can access activities with private questions (such as quiz) they can be accessed elsewhere on the course.
$modtypes = \core_question\local\bank\question_bank_helper::get_activity_types_with_shareable_questions();
$modinfo = get_fast_modinfo($context->instanceid);
foreach ($modtypes as $modtype) {
foreach ($modinfo->get_instances_of($modtype) as $mod) {
if (has_capability("mod/{$modtype}:view", $mod->context)) {
$viewquestionbanks = true;
break 2;
}
}
}
}
if ($viewquestionbanks) {
return $navigationnode->add(
get_string('questionbank_plural', 'question'),
new moodle_url($baseurl, ['courseid' => $context->instanceid]),
navigation_node::TYPE_CONTAINER,
null,
'questionbank',
);
} else {
return;
}
} else if ($context->contextlevel == CONTEXT_MODULE) {
$params = ['cmid' => $context->instanceid];
} else {
+8 -2
View File
@@ -38,7 +38,7 @@ $course = get_course($courseid);
$coursecontext = context_course::instance($course->id);
require_login($course, false);
require_capability('moodle/course:manageactivities', \context_course::instance($course->id));
$canmanage = has_capability('moodle/course:manageactivities', \context_course::instance($course->id));
if (empty(question_bank_helper::get_activity_types_with_shareable_questions())) {
throw new moodle_exception('disabledbanks', 'question');
@@ -48,13 +48,19 @@ $allcaps = array_merge(question_edit_contexts::$caps['editq'], question_edit_con
$sharedbanks = question_bank_helper::get_activity_instances_with_shareable_questions([$course->id], [], $allcaps);
$privatebanks = question_bank_helper::get_activity_instances_with_private_questions([$course->id], [], $allcaps);
if (!$canmanage && empty($sharedbanks)) {
// Only allow non-managing users access to this page if they can access at least 1 activity with shared questions.
// If they can access activities with private questions (such as quiz) they can be accessed elsewhere on the course.
throw new \core\exception\moodle_exception('nobankpermissions', 'question');
}
$pageurl = question_bank_helper::get_url_for_qbank_list($course->id);
$PAGE->set_url($pageurl);
$PAGE->add_body_class('limitedwidth');
$PAGE->set_heading(format_string($course->fullname, true, ['context' => $coursecontext]));
$PAGE->set_title(get_string('questionbank_plural', 'question'));
if ($createdefault) {
if ($canmanage && $createdefault) {
require_sesskey();
question_bank_helper::create_default_open_instance(
$course,
+4 -4
View File
@@ -84,16 +84,16 @@ class view_banks implements \templatable, \renderable {
get_string('createdefault', 'question')
);
$cancreatedefault = has_capability('moodle/course:manageactivities', context_course::instance($this->course->id));
$cancreate = has_capability('moodle/course:manageactivities', context_course::instance($this->course->id));
return [
'addqbank' => $addqbanklink->export_for_template($output),
'addqbank' => $cancreate ? $addqbanklink->export_for_template($output) : false,
'hassharedbanks' => !empty($sharedbankscontext),
'sharedbanks' => $sharedbankscontext,
'hasprivatebanks' => !empty($privatebankscontext),
'privatebanks' => $privatebankscontext,
'addcustombanks' => $addcustombanksrenderable->export_for_template($output),
'createdefault' => $cancreatedefault ? $createdefaultrenderable->export_for_template($output) : false,
'addcustombanks' => $cancreate ? $addcustombanksrenderable->export_for_template($output) : false,
'createdefault' => $cancreate ? $createdefaultrenderable->export_for_template($output) : false,
];
}
}
+28 -3
View File
@@ -17,9 +17,9 @@ Feature: Manage question banks
| teacher1 | C1 | editingteacher |
| student1 | C1 | student |
And the following "activities" exist:
| activity | name | course | section | intro | showdescription |
| qbank | bank1 | C1 | 0 | Bank 1 intro | 0 |
| qbank | bank2 | C1 | 0 | Bank 2 intro | 0 |
| activity | name | course | section | intro | showdescription | idnumber |
| qbank | bank1 | C1 | 0 | Bank 1 intro | 0 | qbank1 |
| qbank | bank2 | C1 | 0 | Bank 2 intro | 0 | qbank2 |
@javascript
Scenario: Show description when show description checkbox ticked
@@ -56,3 +56,28 @@ Feature: Manage question banks
And I click on "Delete" "button"
Then I should not see "bank1"
But I should see "bank2"
Scenario: A student without permissions to access a bank cannot access the question banks page
Given I am on the "C1" "Course" page logged in as "student1"
Then "Question banks" "link" should not exist
Scenario: A student with permissions to access a bank can access the question banks page
Given the following "role assigns" exist:
| user | role | contextlevel | reference |
| student1 | editingteacher | Activity module | qbank2 |
And I am on the "C1" "Course" page logged in as "student1"
When I navigate to "Question banks" in current page administration
Then I should see "bank2"
And I should not see "bank1"
And "Add" "button" should not exist in the "region-main" "region"
Scenario: A teacher can access the question banks page when there are no question banks yet
Given the following "courses" exist:
| fullname | shortname | category |
| Course 2 | C2 | 0 |
And the following "course enrolments" exist:
| user | course | role |
| teacher1 | C2 | editingteacher |
When I am on the "C2" "Course" page logged in as "teacher1"
And I navigate to "Question banks" in current page administration
Then "Create default question bank" "button" should exist