MDL-34862 question preview: improve preview ownership check.

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.

diff --git a/question/engine/questionusage.php b/question/engine/questionusage.php
index f9470c0..e4de197 100644 (file)
--- a/question/engine/questionusage.php
+++ b/question/engine/questionusage.php
@@ -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. */
@@ -104,7 +104,7 @@ class question_usage_by_activity {
         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;
     }

diff --git a/question/preview.php b/question/preview.php
index 56513e7..7e24ba3 100644 (file)
--- a/question/preview.php
+++ b/question/preview.php
@@ -75,14 +75,10 @@ $options->set_from_request();
 $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);
 
@@ -94,6 +90,10 @@ if ($previewid) {
                 $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) {
@@ -104,7 +104,7 @@ if ($previewid) {
 
 } 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);
 
@@ -119,8 +119,6 @@ if ($previewid) {
     $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);