Permalink
Browse files

MDL-18258 fixed legacy type and improved risky default course role hi…

…nts; backported from HEAD
  • Loading branch information...
skodak
skodak committed Feb 15, 2009
1 parent 0ea8ab7 commit e613a1aacb972e9095baf46d3a5de3b54f9fca95
Showing with 48 additions and 35 deletions.
  1. +46 −33 admin/report/security/lib.php
  2. +2 −2 lang/en_utf8/report_security.php
@@ -647,6 +647,16 @@ function report_security_check_courserole($detailed=false) {
$roleids = array_keys($student_roles);
+ $sql = "SELECT DISTINCT rc.roleid
+ FROM {$CFG->prefix}role_capabilities rc
+ WHERE (rc.capability = 'moodle/legacy:coursecreator' OR rc.capability = 'moodle/legacy:admin'
+ OR rc.capability = 'moodle/legacy:teacher' OR rc.capability = 'moodle/legacy:editingteacher')
+ AND rc.permission = ".CAP_ALLOW."";
+
+ $riskyroleids = get_records_sql($sql);
+ $riskyroleids = array_keys($riskyroleids);
+
+
// first test if do anything enabled - that would be really crazy!!!!!!
$inroles = implode(',', $roleids);
$sql = "SELECT rc.roleid, rc.contextid
@@ -670,41 +680,44 @@ function report_security_check_courserole($detailed=false) {
}
rs_close($rs);
- // risky caps in any level - usually very dangerous!!
- $inroles = implode(',', $roleids);
- $sql = "SELECT rc.roleid, rc.contextid
- FROM {$CFG->prefix}role_capabilities rc
- JOIN {$CFG->prefix}capabilities cap ON cap.name = rc.capability
- WHERE ".sql_bitand('cap.riskbitmask', (RISK_XSS | RISK_CONFIG))." <> 0
- AND rc.permission = ".CAP_ALLOW."
- AND rc.roleid IN ($inroles)
- GROUP BY rc.roleid, rc.contextid
- ORDER BY rc.roleid, rc.contextid";
- $rs = get_recordset_sql($sql);
- while ($res = rs_fetch_next_record($rs)) {
- $roleid = $res->roleid;
- $contextid = $res->contextid;
- if ($contextid == SYSCONTEXTID) {
- $a = "$CFG->wwwroot/$CFG->admin/roles/manage.php?action=view&amp;roleid=$roleid";
- } else {
- $a = "$CFG->wwwroot/$CFG->admin/roles/override.php?contextid=$contextid&amp;roleid=$roleid";
+ // any XSS legacy cap does not make any sense here!
+ $inroles = implode(',', $riskyroleids);
+ $sql = "SELECT DISTINCT c.id, c.shortname
+ FROM {$CFG->prefix}course c
+ WHERE c.defaultrole IN ($inroles)
+ ORDER BY c.sortorder";
+ if ($courses = get_records_sql($sql)) {
+ foreach ($courses as $course) {
+ $a = (object)array('url'=>"$CFG->wwwroot/course/edit.php?id=$course->id", 'shortname'=>$course->shortname);
+ $problems[] = get_string('check_courserole_riskylegacy', 'report_security', $a);
}
- $problems[] = get_string('check_courserole_risky', 'report_security', $a);
- }
- rs_close($rs);
-
- // course creator or administrator does not make any sense here!
- $inroles = implode(',', $roleids);
- $sql = "SELECT DISTINCT rc.roleid
- FROM {$CFG->prefix}role_capabilities rc
- WHERE (rc.capability = 'moodle/legacy:coursecreator' OR rc.capability = 'moodle/legacy:admin')
- AND rc.permission = ".CAP_ALLOW."
- AND rc.roleid IN ($inroles)";
- if ($legacys = get_records_sql($sql)) {
- foreach ($legacys as $roleid=>$unused) {
- $a = "$CFG->wwwroot/$CFG->admin/roles/manage.php?action=view&amp;roleid=$roleid";
- $problems[] = get_string('check_defaultcourserole_legacy', 'report_security', $a);
+ } else {
+ $course = array();
+ }
+
+ // risky caps in any level for roles not marked as risky yet - usually very dangerous!!
+ if ($checkroles = array_diff($roleids, $riskyroleids)) {
+ $inroles = implode(',', $checkroles);
+ $sql = "SELECT rc.roleid, rc.contextid
+ FROM {$CFG->prefix}role_capabilities rc
+ JOIN {$CFG->prefix}capabilities cap ON cap.name = rc.capability
+ WHERE ".sql_bitand('cap.riskbitmask', (RISK_XSS | RISK_CONFIG))." <> 0
+ AND rc.permission = ".CAP_ALLOW."
+ AND rc.roleid IN ($inroles)
+ GROUP BY rc.roleid, rc.contextid
+ ORDER BY rc.roleid, rc.contextid";
+ $rs = get_recordset_sql($sql);
+ while ($res = rs_fetch_next_record($rs)) {
+ $roleid = $res->roleid;
+ $contextid = $res->contextid;
+ if ($contextid == SYSCONTEXTID) {
+ $a = "$CFG->wwwroot/$CFG->admin/roles/manage.php?action=view&amp;roleid=$roleid";
+ } else {
+ $a = "$CFG->wwwroot/$CFG->admin/roles/override.php?contextid=$contextid&amp;roleid=$roleid";
+ }
+ $problems[] = get_string('check_courserole_risky', 'report_security', $a);
}
+ rs_close($rs);
}
@@ -32,7 +32,7 @@
$string['check_courserole_details'] = '<p>Each course has one default enrolment role specified. Please make sure no risky capabilities are allowed for this role.</p>
<p>The only supported legacy type for the default course role is <em>Student</em>.</p>';
$string['check_courserole_error'] = 'Incorrectly defined default course roles detected!';
-$string['check_courserole_legacy'] = 'Unsupported legacy type detected in the <a href=\"$a\">role</a>.';
+$string['check_courserole_riskylegacy'] = 'Risky legacy type detected in <a href=\"$a->url\">$a->shortname</a>.';
$string['check_courserole_name'] = 'Default roles (courses)';
$string['check_courserole_notyet'] = 'Used only default course role.';
$string['check_courserole_ok'] = 'Default course role definitions is OK.';
@@ -42,7 +42,7 @@
$string['check_defaultcourserole_details'] = '<p>The default student role for course enrolment specifies the default role for courses. Please make sure no risky capabilities are allowed in this role.</p>
<p>The only supported legacy type for default role is <em>Student</em>.</p>';
$string['check_defaultcourserole_error'] = 'Incorrectly defined default course role \"$a\" detected!';
-$string['check_defaultcourserole_legacy'] = 'Unsupported legacy type detected.';
+$string['check_defaultcourserole_legacy'] = 'Risky legacy type detected.';
$string['check_defaultcourserole_name'] = 'Default course role (global)';
$string['check_defaultcourserole_notset'] = 'Default role is not set.';
$string['check_defaultcourserole_ok'] = 'Site default role definition is OK.';

0 comments on commit e613a1a

Please sign in to comment.