Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

escape variables in question bank columns sql #277

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

escape variables in question bank columns sql #277

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
39f3856
fix #275, allow a user to see his own question, even when hidden
dionysius Oct 13, 2020
e39e6c2
reduce custom assignments in extra joins by using a joined current se…
dionysius Oct 14, 2020
7f5b9b7
Merge branch 'master' into escape_variables_in_question_bank_columns
dionysius Oct 14, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 3 additions & 15 deletions classes/question/bank/attempts_column.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,19 +47,7 @@ class attempts_column extends \core_question\bank\column_base {
* Initialise Parameters for join
*/
protected function init() {

global $DB, $USER, $PAGE;
$this->currentuserid = $USER->id;
// Build context, categoryid and cmid here for use later.
$context = $this->qbank->get_most_specific_context();
$this->categoryid = question_get_default_category($context->id)->id;
$cmid = $context->instanceid;
// TODO: Get StudentQuiz id from infrastructure instead of DB!
// TODO: Exception handling lookup fails somehow.
$sq = $DB->get_record('studentquiz', array('coursemodule' => $cmid));
$this->studentquizid = $sq->id;
$this->studentquiz = $sq;
// TODO: Sanitize!
global $PAGE;
$this->renderer = $PAGE->get_renderer('mod_studentquiz');
}

Expand Down Expand Up @@ -95,8 +83,8 @@ protected function display_content($question, $rowclasses) {
*/
public function get_extra_joins() {
return array('sp' => "LEFT JOIN {studentquiz_progress} sp ON sp.questionid = q.id
AND sp.userid = " . $this->currentuserid . "
AND sp.studentquizid = " . $this->studentquizid);
AND sp.userid = current.userid
AND sp.studentquizid = current.studentquizid");
}

/**
Expand Down
18 changes: 3 additions & 15 deletions classes/question/bank/difficulty_level_column.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,22 +41,11 @@ class difficulty_level_column extends \core_question\bank\column_base {
*/
protected $renderer;

/** @var \stdClass */
protected $studentquiz;

/**
* Initialise Parameters for join
*/
protected function init() {
global $DB, $USER, $PAGE;
$this->currentuserid = $USER->id;
$cmid = $this->qbank->get_most_specific_context()->instanceid;
// TODO: Get StudentQuiz id from infrastructure instead of DB!
// TODO: Exception handling lookup fails somehow.
$sq = $DB->get_record('studentquiz', array('coursemodule' => $cmid));
$this->studentquizid = $sq->id;
$this->studentquiz = $sq;
// TODO: Sanitize!
global $PAGE;
$this->renderer = $PAGE->get_renderer('mod_studentquiz');
}

Expand Down Expand Up @@ -84,11 +73,10 @@ public function get_extra_joins() {
return array('dl' => "LEFT JOIN (
SELECT ROUND(1 - AVG(CAST(correctattempts AS DECIMAL) /
CAST(attempts AS DECIMAL)), 2) AS difficultylevel,
questionid
questionid, studentquizid
FROM {studentquiz_progress}
WHERE studentquizid = " . $this->studentquizid . "
GROUP BY questionid
) dl ON dl.questionid = q.id");
) dl ON dl.questionid = q.id AND dl.studentquizid = current.studentquizid");
}

/**
Expand Down
14 changes: 3 additions & 11 deletions classes/question/bank/rate_column.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,7 @@ class rate_column extends \core_question\bank\column_base {
* Initialise Parameters for join
*/
protected function init() {
global $DB, $USER, $PAGE;
$this->currentuserid = $USER->id;
$cmid = $this->qbank->get_most_specific_context()->instanceid;
// TODO: Get StudentQuiz id from infrastructure instead of DB!
// TODO: Exception handling lookup fails somehow.
$sq = $DB->get_record('studentquiz', array('coursemodule' => $cmid));
$this->studentquizid = $sq->id;
// TODO: Sanitize!
global $PAGE;
$this->renderer = $PAGE->get_renderer('mod_studentquiz');
}

Expand Down Expand Up @@ -93,11 +86,10 @@ public function get_extra_joins() {
GROUP BY questionid
) vo ON vo.questionid = q.id",
'myrate' => "LEFT JOIN (
SELECT rate AS myrate, q.id AS questionid
SELECT rate AS myrate, q.id AS questionid, rate.userid
FROM {question} q
LEFT JOIN {studentquiz_rate} rate ON q.id = rate.questionid
AND rate.userid = " . $this->currentuserid . "
) myrate ON myrate.questionid = q.id");
) myrate ON myrate.questionid = q.id AND myrate.userid = current.userid");
}

/**
Expand Down
2 changes: 1 addition & 1 deletion classes/question/bank/sq_hidden_column.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ protected function display_content($question, $rowclasses) {
*/
public function get_extra_joins() {
$hidden = "sqh.hidden = 0";
$mine = "q.createdby = $this->currentuserid";
$mine = "q.createdby = current.userid";

// Without permission, a user can only see non-hidden question or its their own.
$sqlextra = "AND ($hidden OR $mine)";
Expand Down
7 changes: 7 additions & 0 deletions classes/question/bank/studentquiz_bank_view.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -450,6 +450,13 @@ protected function build_query() {
$joins = array();
$fields = array('q.hidden', 'q.category', 'q.timecreated', 'q.createdby');
$tests = array('q.parent = 0', 'q.hidden = 0');

// Additional fields for inside comparisons, so columns don't introduce the same variables again. This variant
// is chosen since we can't use sql variables multiple times in queries, but this way we can compare against
// these variables in any wished way multiple times. Usage example: q.createdby = current.userid, if you want
// to know the questions created by the current user
$joins["current"] = "JOIN (SELECT {$this->userid} as userid, {$this->studentquiz->id} as studentquizid) current";

foreach ($this->requiredcolumns as $column) {
$extrajoins = $column->get_extra_joins();
foreach ($extrajoins as $prefix => $join) {
Expand Down
4 changes: 1 addition & 3 deletions classes/question/bank/tag_column.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,7 @@ class tag_column extends \core_question\bank\column_base {
* Initialise Parameters for join
*/
protected function init() {

global $DB, $PAGE;

global $PAGE;
// Build context and categoryid here for use later.
$context = $this->qbank->get_most_specific_context();
$this->categoryid = question_get_default_category($context->id)->id;
Expand Down