moodle/moodle
3
0
Fork 0
mirror of https://github.com/moodle/moodle.git synced 2025-08-04 16:36:37 +02:00
Code Issues Projects Releases Packages Wiki Activity Actions

MDL-32239 question bank: wrong cap checks editing/viewing quesitions

Browse source 
None of these problems affect the default roles. They only occur when
the permissions have been edited to allow restricted subsets of the
question capabilities.

In some cases, things that the restriced subset of capabilities should
have allowed were blocked by errors. In other cases, users were allowed
to do more than they should bave been due to failures to check the
right capabilities in the right places.

For full details, see the bug report.

Two of the bugs were previously reported separately as MDL-26395 and
MDL-27232.
This commit is contained in:
Tim Hunt 2012-03-28 12:09:12 +01:00
parent 58db57ad53
commit 29e247e44e
3 changed files with 26 additions and 18 deletions
Download patch file Download diff file Expand all files Collapse all files

8
question/editlib.php
View file

@ -605,7 +605,8 @@ abstract class question_bank_action_column_base extends question_bank_column_bas
}
public function get_required_fields() {
return array('q.id');
// createdby is required for permission checks.
return array('q.id, q.createdby');
}
}
@ -631,10 +632,9 @@ class question_bank_edit_action_column extends question_bank_action_column_base
}
protected function display_content($question, $rowclasses) {
if (question_has_capability_on($question, 'edit') ||
question_has_capability_on($question, 'move')) {
if (question_has_capability_on($question, 'edit')) {
$this->print_icon('t/edit', $this->stredit, $this->qbank->edit_question_url($question->id));
} else {
} else if (question_has_capability_on($question, 'view')) {
$this->print_icon('i/info', $this->strview, $this->qbank->edit_question_url($question->id));
}
}

28
question/question.php
View file

@ -126,6 +126,7 @@ if ($id) {
$question = new stdClass();
$question->category = $categoryid;
$question->qtype = $qtype;
$question->createdby = $USER->id;
// Check that users are allowed to create this question type at the moment.
if (!question_bank::qtype_enabled($qtype)) {
@ -157,7 +158,6 @@ $categorycontext = get_context_instance_by_id($category->contextid);
$addpermission = has_capability('moodle/question:add', $categorycontext);
if ($id) {
$canview = question_has_capability_on($question, 'view');
if ($movecontext){
$question->formoptions->canedit = false;
$question->formoptions->canmove = (question_has_capability_on($question, 'move') && $contexts->have_cap('moodle/question:add'));
@ -165,26 +165,28 @@ if ($id) {
$question->formoptions->repeatelements = false;
$question->formoptions->movecontext = true;
$formeditable = true;
question_require_capability_on($question, 'view');
question_require_capability_on($question, 'move');
} else {
$question->formoptions->canedit = question_has_capability_on($question, 'edit');
$question->formoptions->canmove = (question_has_capability_on($question, 'move') && $addpermission);
$question->formoptions->cansaveasnew = (($canview ||question_has_capability_on($question, 'edit')) && $addpermission);
$question->formoptions->repeatelements = ($question->formoptions->canedit || $question->formoptions->cansaveasnew);
$question->formoptions->canmove = $addpermission && question_has_capability_on($question, 'move');
$question->formoptions->cansaveasnew = $addpermission &&
(question_has_capability_on($question, 'view') || $question->formoptions->canedit);
$question->formoptions->repeatelements = $question->formoptions->canedit || $question->formoptions->cansaveasnew;
$formeditable = $question->formoptions->canedit || $question->formoptions->cansaveasnew || $question->formoptions->canmove;
$question->formoptions->movecontext = false;
if (!$formeditable){
if (!$formeditable) {
question_require_capability_on($question, 'view');
}
}
} else { // creating a new question
require_capability('moodle/question:add', $categorycontext);
$formeditable = true;
$question->formoptions->canedit = question_has_capability_on($question, 'edit');
$question->formoptions->canmove = (question_has_capability_on($question, 'move') && $addpermission);
$question->formoptions->cansaveasnew = false;
$question->formoptions->repeatelements = true;
$question->formoptions->movecontext = false;
$formeditable = true;
require_capability('moodle/question:add', $categorycontext);
}
// Validate the question type.
@ -245,7 +247,7 @@ if ($mform->is_cancelled()) {
/// If we are moving a question, check we have permission to move it from
/// whence it came. (Where we are moving to is validated by the form.)
list($newcatid) = explode(',', $fromform->category);
list($newcatid, $newcontextid) = explode(',', $fromform->category);
if (!empty($question->id) && $newcatid != $question->category) {
question_require_capability_on($question, 'move');
}
@ -261,6 +263,14 @@ if ($mform->is_cancelled()) {
} else {
// We are acutally saving the question.
if (!empty($question->id)) {
question_require_capability_on($question, 'edit');
} else {
require_capability('moodle/question:add', get_context_instance_by_id($newcontextid));
if (!empty($fromform->makecopy) && !$question->formoptions->cansaveasnew) {
print_error('nopermissions', '', '', 'edit');
}
}
$question = $qtypeobj->save_question($question, $fromform);
if (!empty($CFG->usetags) && isset($fromform->tags)) {
// A wizardpage from multipe pages questiontype like calculated may not

8
question/type/edit_question_form.php
View file

@ -237,9 +237,7 @@ abstract class question_edit_form extends question_wizard_form {
if ($this->question->formoptions->movecontext) {
$buttonarray[] = $mform->createElement('submit', 'submitbutton',
get_string('moveq', 'question'));
} else if ($this->question->formoptions->canedit ||
$this->question->formoptions->canmove ||
$this->question->formoptions->movecontext) {
} else if ($this->question->formoptions->canedit) {
$buttonarray[] = $mform->createElement('submit', 'submitbutton',
get_string('savechanges'));
}
@ -639,10 +637,10 @@ abstract class question_edit_form extends question_wizard_form {
public function validation($fromform, $files) {
$errors = parent::validation($fromform, $files);
if (empty($fromform->makecopy) && isset($this->question->id)
if (empty($fromform['makecopy']) && isset($this->question->id)
&& ($this->question->formoptions->canedit ||
$this->question->formoptions->cansaveasnew)
&& empty($fromform->usecurrentcat) && !$this->question->formoptions->canmove) {
&& empty($fromform['usecurrentcat']) && !$this->question->formoptions->canmove) {
$errors['currentgrp'] = get_string('nopermissionmove', 'question');
}
return $errors;