From 4c16e191e16941cf241a6f746fd27cbe6575bf5b Mon Sep 17 00:00:00 2001
From: Tim Hunt <T.J.Hunt@open.ac.uk>
Date: Mon, 13 Aug 2012 16:53:15 +0100
Subject: [PATCH] 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.
---
 question/engine/questionusage.php |  4 ++--
 question/preview.php              | 14 ++++++--------
 2 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/question/engine/questionusage.php b/question/engine/questionusage.php
index f9470c0060e..e4de1979502 100644
--- 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 57dc777fb45..293876260a4 100644
--- 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);