Skip to content

Commit

Permalink
Improved access control for grades and reports
Browse files Browse the repository at this point in the history
New capability for viewing all user results
Get all results if viewallresults capability
Get own results if viewresults capability and requesting own results
HFP-1075
  • Loading branch information
thomasmars committed May 14, 2017
1 parent 405b250 commit 806267c
Show file tree
Hide file tree
Showing 11 changed files with 74 additions and 13 deletions.
25 changes: 19 additions & 6 deletions classes/results.php
Original file line number Diff line number Diff line change
Expand Up @@ -81,18 +81,22 @@ protected function filter_input() {
* Print results data
*/
public function print_results() {
global $DB;
global $DB, $USER;

// Check permission
$course = $DB->get_field('hvp', 'course', array('id' => $this->content_id));
$context = \context_course::instance($course);
if (!has_capability('mod/hvp:viewresults', $context)) {
$view_own_results = has_capability('mod/hvp:viewresults', $context);
$view_all_results = has_capability('mod/hvp:viewallresults', $context);
if (!$view_own_results && !$view_all_results) {
\H5PCore::ajaxError(get_string('nopermissiontoviewresult', 'hvp'));
http_response_code(403);
exit;
}

$results = $this->get_results();
// Only get own results if can't view all.
$uid = $view_all_results ? NULL : (int)$USER->id;
$results = $this->get_results($uid);

// Make data readable for humans
$rows = array();
Expand Down Expand Up @@ -143,13 +147,15 @@ public function print_results() {
* Builds the SQL query required to retrieve results for the given
* interactive content.
*
* @param int $uid Only get results for uid
*
* @throws \coding_exception
* @return array
*/
protected function get_results() {
protected function get_results($uid=NULL) {
// Add extra fields, joins and where for the different result lists
if ($this->content_id !== 0) {
list($fields, $join, $where, $order, $args) = $this->get_content_sql();
list($fields, $join, $where, $order, $args) = $this->get_content_sql($uid);
}
else {
throw new \coding_exception('missing content_id');
Expand Down Expand Up @@ -288,9 +294,10 @@ public static function get_ordered_user_name_fields($prefix = 'u.') {
* (An alternative to this could be getting all the results for a
* specified user.)
*
* @param int $uid Only get users with this id
* @return array $fields, $join, $where, $order, $args
*/
protected function get_content_sql() {
protected function get_content_sql($uid=NULL) {
global $DB;

$usernamefields = implode(', ', self::get_ordered_user_name_fields());
Expand All @@ -299,6 +306,12 @@ protected function get_content_sql() {
$where = array("i.iteminstance = ?");
$args = array($this->content_id);

// Only get entries with own user id
if (isset($uid)) {
array_push($where, "u.id = ?");
array_push($args, $uid);
}

if (isset($this->filters[0])) {
$keywordswhere = array();

Expand Down
10 changes: 10 additions & 0 deletions db/access.php
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,16 @@
)
),

'mod/hvp:viewallresults' => array(
'captype' => 'read',
'contextlevel' => CONTEXT_COURSE,
'archetypes' => array(
'manager' => CAP_ALLOW,
'editingteacher' => CAP_ALLOW,
'teacher' => CAP_ALLOW
)
),

'mod/hvp:getcachedassets' => array(
'captype' => 'read',
'contextlevel' => CONTEXT_SYSTEM,
Expand Down
18 changes: 16 additions & 2 deletions grade.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,11 @@
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/

global $DB, $PAGE, $USER, $COURSE;
require_once(dirname(__FILE__) . '/../../config.php');

$id = required_param('id', PARAM_INT);
$userid = optional_param('userid', 0, PARAM_INT);
$userid = optional_param('userid', (int)$USER->id, PARAM_INT);

if (! $cm = get_coursemodule_from_id('hvp', $id)) {
print_error('invalidcoursemodule');
Expand All @@ -34,9 +35,22 @@
}
require_course_login($course, false, $cm);

// Check permission
$course_context = context_course::instance($COURSE->id);
$can_view_own = has_capability('mod/hvp:viewresults', $course_context);
$can_view_all = has_capability('mod/hvp:viewallresults', $course_context);

// Check if user has permission to view own content
if ($userid === (int)$USER->id) {
// If it's the same user, redirect to content.
// Require either capability to view own or all content.
if (!$can_view_own && !$can_view_all) {
// Not allowed to see any results, redirect.
redirect(new moodle_url('/mod/hvp/view.php', array('id' => $cm->id)));
}
}
else {
// Other user's content, require view all user results capability
require_capability('mod/hvp:viewallresults', $course_context);
}

// Load H5P Content.
Expand Down
1 change: 1 addition & 0 deletions lang/de/hvp.php
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@
$string['hvp:savecontentuserdata'] = 'H5P-Nutzerinhalt speichern';
$string['hvp:saveresults'] = 'Ergebnis des H5P-Inhalts speichern';
$string['hvp:viewresults'] = 'Ergebnis des H5P-Inhalts ansehen';
$string['hvp:viewallresults'] = 'View result for all users in course';
$string['hvp:getcachedassets'] = 'Zwischengespeicherte H5P-Inhaltswerte erhalten';
$string['hvp:getcontent'] = 'H5P-Dateiinhalt im Kurs verwenden/ansehen';
$string['hvp:getexport'] = 'Exportierte H5P Datei im Kurs verwenden';
Expand Down
3 changes: 2 additions & 1 deletion lang/en/hvp.php
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,8 @@
$string['hvp:userestrictedlibraries'] = 'Use restricted H5P libraries';
$string['hvp:savecontentuserdata'] = 'Save H5P content user data';
$string['hvp:saveresults'] = 'Save result for H5P content';
$string['hvp:viewresults'] = 'View result for H5P content';
$string['hvp:viewresults'] = 'View result for own questions in course';
$string['hvp:viewallresults'] = 'View result for all users in course';
$string['hvp:getcachedassets'] = 'Get cached H5P content assets';
$string['hvp:getcontent'] = 'Get/view content of H5P file in course';
$string['hvp:getexport'] = 'Get export file from H5P in course';
Expand Down
1 change: 1 addition & 0 deletions lang/fr/hvp.php
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@
$string['hvp:savecontentuserdata'] = 'Sauvegarder les données utilisateur H5P';
$string['hvp:saveresults'] = 'Sauvegarder les résultats';
$string['hvp:viewresults'] = 'Visualiser les résultats';
$string['hvp:viewallresults'] = 'View result for all users in course';
$string['hvp:getcachedassets'] = 'Récupérer les assets mis en cache';
$string['hvp:getcontent'] = 'Visualiser le contenu d\'un fichier H5P dans un cours';
$string['hvp:getexport'] = 'Récupérer un fichier H5P dans un cours';
Expand Down
1 change: 1 addition & 0 deletions lang/he/hvp.php
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@
$string['hvp:savecontentuserdata'] = 'שמירת נתוני משתמש מתוך פעילות H5P';
$string['hvp:saveresults'] = 'שמירת תוצאות שימוש ברכיב H5P';
$string['hvp:viewresults'] = 'צפיה בתוצאות שימוש ברכיב H5P';
$string['hvp:viewallresults'] = 'View result for all users in course';
$string['hvp:getcachedassets'] = 'אחזור משאבי מטמון של רכיב H5P';
$string['hvp:getcontent'] = 'צפיה בתוכן פעילות H5P מתוך הקורס';
$string['hvp:getexport'] = 'יצוא תוכן פעילות H5P מתוך הקורס';
Expand Down
3 changes: 2 additions & 1 deletion lang/no/hvp.php
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,8 @@
$string['hvp:userestrictedlibraries'] = 'Bruke begrenset H5P-bibliotek';
$string['hvp:savecontentuserdata'] = 'Lagre H5P-brukerdata';
$string['hvp:saveresults'] = 'Lagre resultater for H5P-innhold';
$string['hvp:viewresults'] = 'Vise resultater for H5P-innhold';
$string['hvp:viewresults'] = 'Vis resultater for eget H5P-innhold';
$string['hvp:viewallresults'] = 'Vis resultater for alle brukeres H5P-innhold';
$string['hvp:getcachedassets'] = 'Tilgang til bufret H5P-innholdsressurser';
$string['hvp:getcontent'] = 'Tilgang til innholdet til H5P-fil i kurs';
$string['hvp:getexport'] = 'Tilgang til eksportfil fra H5P i kurs';
Expand Down
1 change: 1 addition & 0 deletions lang/tr/hvp.php
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@
$string['hvp:savecontentuserdata'] = 'H5P içerik kullanıcısı verisini kaydet';
$string['hvp:saveresults'] = 'H5P içeriği için sonucu kaydet';
$string['hvp:viewresults'] = 'H5P içeriği için sonucu gör';
$string['hvp:viewallresults'] = 'View result for all users in course';
$string['hvp:getcachedassets'] = 'Ön belleğe alınmış H5P içerik değerlerini al';
$string['hvp:getcontent'] = 'Kurs içìndeki H5P dosyası içeriğini al/gör';
$string['hvp:getexport'] = 'Kurs içindeki H5P içeriğinden dışa aktarma dosyası al';
Expand Down
22 changes: 20 additions & 2 deletions review.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/

global $PAGE, $DB, $CFG, $OUTPUT, $COURSE;
global $USER, $PAGE, $DB, $CFG, $OUTPUT, $COURSE;
require_once(dirname(__FILE__) . '/../../config.php');

$id = required_param('id', PARAM_INT);
$userid = optional_param('user', 0, PARAM_INT);
$userid = optional_param('user', (int)$USER->id, PARAM_INT);

if (!$cm = get_coursemodule_from_instance('hvp', $id)) {
print_error('invalidcoursemodule');
Expand All @@ -35,6 +35,24 @@
}
require_login($course, FALSE, $cm);

// Check permission
$course_context = context_course::instance($COURSE->id);
$can_view_own = has_capability('mod/hvp:viewresults', $course_context);
$can_view_all = has_capability('mod/hvp:viewallresults', $course_context);

// Check if user has permission to view own content
if ($userid === (int)$USER->id) {
// Require either capability to view own or all content.
if (!$can_view_own && !$can_view_all) {
// Not allowed to see any results, redirect.
redirect(new moodle_url('/mod/hvp/view.php', array('id' => $cm->id)));
}
}
else {
// Other user's content, require view all user results capability
require_capability('mod/hvp:viewallresults', $course_context);
}

// Load H5P Content.
$hvp = $DB->get_record_sql(
"SELECT h.id,
Expand Down
2 changes: 1 addition & 1 deletion version.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@

defined('MOODLE_INTERNAL') || die();

$plugin->version = 2017051001;
$plugin->version = 2017051400;
$plugin->requires = 2013051403;
$plugin->cron = 0;
$plugin->component = 'mod_hvp';
Expand Down

0 comments on commit 806267c

Please sign in to comment.