Skip to content

Commit

Permalink
MDL-34862 question preview: improve preview ownership check.
Browse files Browse the repository at this point in the history
Users should only be able to access their own quetion preview. In the
past, for reasons I can no longer remember, this was enforced
using the session. It is much better to set the question_usage to belong
to the user's context.
  • Loading branch information
timhunt committed Aug 13, 2012
1 parent c36de79 commit 1907f7e
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 10 deletions.
4 changes: 2 additions & 2 deletions question/engine/questionusage.php
Expand Up @@ -63,7 +63,7 @@ class question_usage_by_activity {
*/
protected $preferredbehaviour = null;

/** @var object the context this usage belongs to. */
/** @var context the context this usage belongs to. */
protected $context;

/** @var string plugin name of the plugin this usage belongs to. */
Expand Down Expand Up @@ -104,7 +104,7 @@ public function get_preferred_behaviour() {
return $this->preferredbehaviour;
}

/** @return object the context this usage belongs to. */
/** @return context the context this usage belongs to. */
public function get_owning_context() {
return $this->context;
}
Expand Down
14 changes: 6 additions & 8 deletions question/preview.php
Expand Up @@ -75,14 +75,10 @@
$PAGE->set_url(question_preview_url($id, $options->behaviour, $options->maxmark,
$options, $options->variant, $context));

// Get and validate exitsing preview, or start a new one.
// Get and validate existing preview, or start a new one.
$previewid = optional_param('previewid', 0, PARAM_INT);

if ($previewid) {
if (!isset($SESSION->question_previews[$previewid])) {
print_error('notyourpreview', 'question');
}

try {
$quba = question_engine::load_questions_usage_by_activity($previewid);

Expand All @@ -94,6 +90,10 @@
$options->maxmark, $options, $options->variant, $context), null, $e);
}

if ($quba->get_owning_context()->instanceid != $USER->id) {
print_error('notyourpreview', 'question');
}

$slot = $quba->get_first_question_number();
$usedquestion = $quba->get_question($slot);
if ($usedquestion->id != $question->id) {
Expand All @@ -104,7 +104,7 @@

} else {
$quba = question_engine::make_questions_usage_by_activity(
'core_question_preview', $context);
'core_question_preview', context_user::instance($USER->id));
$quba->set_preferred_behaviour($options->behaviour);
$slot = $quba->add_question($question, $options->maxmark);

Expand All @@ -119,8 +119,6 @@
$transaction = $DB->start_delegated_transaction();
question_engine::save_questions_usage_by_activity($quba);
$transaction->allow_commit();

$SESSION->question_previews[$quba->get_id()] = true;
}
$options->behaviour = $quba->get_preferred_behaviour();
$options->maxmark = $quba->get_question_max_mark($slot);
Expand Down

0 comments on commit 1907f7e

Please sign in to comment.