From ff03c5b6b54250b1654ed2995662279d2906a023 Mon Sep 17 00:00:00 2001 From: Petr Skoda Date: Sun, 30 Jan 2011 21:49:22 +0100 Subject: [PATCH 1/2] MDL-26198 make user_has_role_assignment() check parent contexts too --- lib/accesslib.php | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/lib/accesslib.php b/lib/accesslib.php index 87a82873b88..3a2764a76bb 100755 --- a/lib/accesslib.php +++ b/lib/accesslib.php @@ -5526,18 +5526,34 @@ function get_users_from_role_on_context($role, $context) { } /** - * Simple function returning a boolean true if roles exist, otherwise false + * Simple function returning a boolean true if user has roles + * in context or parent contexts, otherwise false. * * @param int $userid * @param int $roleid - * @param int $contextid + * @param int $contextid empty means any context * @return bool */ function user_has_role_assignment($userid, $roleid, $contextid = 0) { global $DB; if ($contextid) { - return $DB->record_exists('role_assignments', array('userid'=>$userid, 'roleid'=>$roleid, 'contextid'=>$contextid)); + if (!$context = get_context_instance_by_id($contextid)) { + return false; + } + $parents = get_parent_contexts($context, true); + list($contexts, $params) = $DB->get_in_or_equal($parents, SQL_PARAMS_NAMED, 'r0000'); + $params['userid'] = $userid; + $params['roleid'] = $roleid; + + $sql = "SELECT COUNT(ra.id) + FROM {role_assignments} ra + WHERE ra.userid = :userid AND ra.roleid = :roleid AND ra.contextid $contexts"; + + $count = $DB->get_field_sql($sql, $params); + var_dump($count); + return ($count > 0); + } else { return $DB->record_exists('role_assignments', array('userid'=>$userid, 'roleid'=>$roleid)); } From 9cedb80c5d6318aa17cd66912d37e6ef3dca9455 Mon Sep 17 00:00:00 2001 From: Petr Skoda Date: Sun, 30 Jan 2011 21:50:04 +0100 Subject: [PATCH 2/2] MDL-26198 fix CSRF and missing access control + fix xhtml strict --- course/report/completion/index.php | 2 +- course/togglecompletion.php | 24 +++++++++++++++--------- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/course/report/completion/index.php b/course/report/completion/index.php index 4a630b2d0dd..9f3df05ec03 100644 --- a/course/report/completion/index.php +++ b/course/report/completion/index.php @@ -632,7 +632,7 @@ foreach ($progress as $user) { $describe = get_string('completion-alt-auto-'.$completiontype,'completion'); print ''. - ''. + ''. ''.$describe.''; //TODO: localize } else { diff --git a/course/togglecompletion.php b/course/togglecompletion.php index 9f5e82c4245..d7201443c89 100644 --- a/course/togglecompletion.php +++ b/course/togglecompletion.php @@ -38,9 +38,10 @@ if (!$cmid && !$courseid) { // Process self completion if ($courseid) { $PAGE->set_url(new moodle_url('/course/togglecompletion.php', array('course'=>$courseid))); - + // Check user is logged in $course = $DB->get_record('course', array('id' => $courseid), '*', MUST_EXIST); + $context = get_context_instance(CONTEXT_COURSE, $course->id); require_login($course); $completion = new completion_info($course); @@ -50,14 +51,19 @@ if ($courseid) { $rolec = optional_param('rolec', 0, PARAM_INT); if ($user && $rolec) { + require_sesskey(); - $criteria = completion_criteria::factory((object) array('id'=>$rolec, 'criteriatype'=>COMPLETION_CRITERIA_TYPE_ROLE)); - $criteria_completions = $completion->get_completions($user, COMPLETION_CRITERIA_TYPE_ROLE); + completion_criteria::factory((object) array('id'=>$rolec, 'criteriatype'=>COMPLETION_CRITERIA_TYPE_ROLE)); //TODO: this is dumb, because it does not fetch the data?!?! + $criteria = completion_criteria_role::fetch(array('id'=>$rolec)); - foreach ($criteria_completions as $criteria_completion) { - if ($criteria_completion->criteriaid == $rolec) { - $criteria->complete($criteria_completion); - break; + if ($criteria and user_has_role_assignment($USER->id, $criteria->role, $context->id)) { + $criteria_completions = $completion->get_completions($user, COMPLETION_CRITERIA_TYPE_ROLE); + + foreach ($criteria_completions as $criteria_completion) { + if ($criteria_completion->criteriaid == $rolec) { + $criteria->complete($criteria_completion); + break; + } } } @@ -71,7 +77,7 @@ if ($courseid) { } else { // Confirm with user - if ($confirm) { + if ($confirm and confirm_sesskey()) { $completion = $completion->get_completion($USER->id, COMPLETION_CRITERIA_TYPE_SELF); if (!$completion) { @@ -94,7 +100,7 @@ if ($courseid) { $PAGE->set_heading($course->fullname); $PAGE->navbar->add($strconfirm); echo $OUTPUT->header(); - $buttoncontinue = new single_button(new moodle_url('/course/togglecompletion.php', array('course'=>$courseid, 'confirm'=>1)), get_string('yes'), 'post'); + $buttoncontinue = new single_button(new moodle_url('/course/togglecompletion.php', array('course'=>$courseid, 'confirm'=>1, 'sesskey'=>sesskey())), get_string('yes'), 'post'); $buttoncancel = new single_button(new moodle_url('/course/view.php', array('id'=>$courseid)), get_string('no'), 'get'); echo $OUTPUT->confirm($strconfirm, $buttoncontinue, $buttoncancel); echo $OUTPUT->footer();