MDL-29602 accesslib improvements

Refactoring and improvements of the accesslib.php library including prevention of access for not-logged-in users when forcelogin enabled, improved context caching, OOP refactoring of contexts, fixed context loading, deduplication of role definitions in user sessions, installation improvements, decoupling of enrolment checking from capability loading, added detection of deleted and non-existent users in has_capability(), new function accesslib test, auth and enrol upgrade notes.

More details are available in tracker subtasks.

admin/report/questioninstances/index.php

@@ -65,13 +65,15 @@
 
     // Get the question counts, and all the context information, for each
     // context. That is, rows of these results can be used as $context objects.
+    $ctxpreload = context_helper::get_preload_record_columns_sql('con');
+    $ctxgroupby = implode(',', array_keys(context_helper::get_preload_record_columns('con')));
     $counts = $DB->get_records_sql("
-            SELECT qc.contextid, count(1) as numquestions, sum(hidden) as numhidden, con.id, con.contextlevel, con.instanceid, con.path, con.depth
+            SELECT qc.contextid, count(1) as numquestions, sum(hidden) as numhidden, $ctxpreload
             FROM {question} q
             JOIN {question_categories} qc ON q.category = qc.id
             JOIN {context} con ON con.id = qc.contextid
             $sqlqtypetest
-            GROUP BY contextid, con.id, con.contextlevel, con.instanceid, con.path, con.depth
+            GROUP BY qc.contextid, $ctxgroupby
             ORDER BY numquestions DESC, numhidden ASC, con.contextlevel ASC, con.id ASC", $params);
 
     // Print the report heading.
@@ -94,8 +96,10 @@
     $totalhidden = 0;
     foreach ($counts as $count) {
         // Work out a link for editing questions in this context.
-        $contextname = print_context_name($count);
-        $url = question_edit_url($count);
+        context_helper::preload_from_record($count);
+        $context = context::instance_by_id($count->contextid);
+        $contextname = $context->get_context_name();
+        $url = question_edit_url($context);
         if ($url) {
             $contextname = '<a href="' . $url . '" title="' .
                     get_string('editquestionshere', 'report_questioninstances') .

admin/roles/usersroles.php

@@ -96,7 +96,8 @@
 
 /// Put the contexts into a tree structure.
 foreach ($contexts as $conid => $con) {
-    $parentcontextid = get_parent_contextid($con);
+    $context = context::instance_by_id($conid);
+    $parentcontextid = get_parent_contextid($context);
     if ($parentcontextid) {
         $contexts[$parentcontextid]->children[] = $conid;
     }
@@ -156,13 +157,13 @@ function print_report_tree($contextid, $contexts, $systemcontext, $fullname) {
     }
 
     // Pull the current context into an array for convinience.
-    $context = $contexts[$contextid];
+    $context = context::instance_by_id($contextid);
 
     // Print the context name.
-    echo $OUTPUT->heading(print_context_name($contexts[$contextid]), 4, 'contextname');
+    echo $OUTPUT->heading($context->get_context_name(), 4, 'contextname');
 
     // If there are any role assignments here, print them.
-    foreach ($context->roleassignments as $ra) {
+    foreach ($contexts[$contextid]->roleassignments as $ra) {
         $value = $ra->contextid . ',' . $ra->roleid;
         $inputid = 'unassign' . $value;
 

admin/tool/capability/index.php

@@ -138,7 +138,8 @@
 
     // Put the contexts into a tree structure.
     foreach ($contexts as $conid => $con) {
-        $parentcontextid = get_parent_contextid($con);
+        $context = context::instance_by_id($conid);
+        $parentcontextid = get_parent_contextid($context);
         if ($parentcontextid) {
             $contexts[$parentcontextid]->children[] = $conid;
         }
@@ -196,7 +197,8 @@ function print_report_tree($contextid, $contexts, $allroles) {
         $url = "$CFG->wwwroot/$CFG->admin/roles/override.php?contextid=$contextid";
         $title = get_string('changeoverrides', 'tool_capability');
     }
-    echo '<h3><a href="' . $url . '" title="' . $title . '">', print_context_name($contexts[$contextid]), '</a></h3>';
+    $context = context::instance_by_id($contextid);
+    echo '<h3><a href="' . $url . '" title="' . $title . '">', $context->get_context_name(), '</a></h3>';
 
     // If there are any role overrides here, print them.
     if (!empty($contexts[$contextid]->rolecapabilities)) {

auth/shibboleth/index.php

@@ -39,7 +39,9 @@
 
         if ($shibbolethauth->user_login($frm->username, $frm->password)) {
 
-            $USER = authenticate_user_login($frm->username, $frm->password);
+            $user = authenticate_user_login($frm->username, $frm->password);
+            enrol_check_plugins($user);
+            session_set_user($user);
 
             $USER->loggedin = true;
             $USER->site     = $CFG->wwwroot; // for added security, store the site in the
@@ -75,9 +77,6 @@
                 }
             }
 
-            enrol_check_plugins($USER);
-            load_all_capabilities();     /// This is what lets the user do anything on the site  :-)
-
             redirect($urltogo);
 
             exit;

auth/upgrade.txt

@@ -0,0 +1,10 @@
+This files describes API changes in /auth/* - plugins,
+information provided here is intended especially for developers.
+
+=== 2.2 ===
+
+required changes in code:
+* the correct sequence to set up global $USER is:
+    $user = get_complete_user_data('username', $username); // or $user = authenticate_user_login()
+    enrol_check_plugins($user);
+    session_set_user($user);

blog/edit.php

@@ -143,9 +143,9 @@
     if ($CFG->useblogassociations && ($blogassociations = $DB->get_records('blog_association', array('blogid' => $entry->id)))) {
 
         foreach ($blogassociations as $assocrec) {
-            $contextrec = $DB->get_record('context', array('id' => $assocrec->contextid));
+            $context = get_context_instance_by_id($assocrec->contextid);
 
-            switch ($contextrec->contextlevel) {
+            switch ($context->contextlevel) {
                 case CONTEXT_COURSE:
                     $entry->courseassoc = $assocrec->contextid;
                     break;

blog/edit_form.php

@@ -95,7 +95,7 @@ function definition() {
                     $a->modname = $mod->name;
                     $context = get_context_instance(CONTEXT_MODULE, $modid);
                 } else {
-                    $context = $DB->get_record('context', array('id' => $entry->modassoc));
+                    $context = get_context_instance_by_id($entry->modassoc);
                     $cm = $DB->get_record('course_modules', array('id' => $context->instanceid));
                     $a = new stdClass();
                     $a->modtype = $DB->get_field('modules', 'name', array('id' => $cm->module));
@@ -134,7 +134,7 @@ function validation($data, $files) {
 
         // validate course association
         if (!empty($data['courseassoc']) && has_capability('moodle/blog:associatecourse', $sitecontext)) {
-            $coursecontext = $DB->get_record('context', array('id' => $data['courseassoc'], 'contextlevel' => CONTEXT_COURSE));
+            $coursecontext = get_context_instance(CONTEXT_COURSE, $data['courseassoc']);
 
             if ($coursecontext)  {
                 if (!is_enrolled($coursecontext) and !is_viewing($coursecontext)) {
@@ -149,12 +149,12 @@ function validation($data, $files) {
         if (!empty($data['modassoc'])) {
             $modcontextid = $data['modassoc'];
 
-            $modcontext = $DB->get_record('context', array('id' => $modcontextid, 'contextlevel' => CONTEXT_MODULE));
+            $modcontext = get_context_instance(CONTEXT_MODULE, $modcontextid);
 
             if ($modcontext) {
                 // get context of the mod's course
                 $path = explode('/', $modcontext->path);
-                $coursecontext = $DB->get_record('context', array('id' => $path[(count($path) - 2)]));
+                $coursecontext = get_context_instance_by_id($path[(count($path) - 2)]);
 
                 // ensure only one course is associated
                 if (!empty($data['courseassoc'])) {

blog/locallib.php

@@ -247,10 +247,10 @@ public function print_html($return=false) {
 
             // First find and show the associated course
             foreach ($blogassociations as $assocrec) {
-                $contextrec = $DB->get_record('context', array('id' => $assocrec->contextid));
-                if ($contextrec->contextlevel ==  CONTEXT_COURSE) {
-                    $assocurl = new moodle_url('/course/view.php', array('id' => $contextrec->instanceid));
-                    $text = $DB->get_field('course', 'shortname', array('id' => $contextrec->instanceid)); //TODO: performance!!!!
+                $context = get_context_instance_by_id($assocrec->contextid);
+                if ($context->contextlevel ==  CONTEXT_COURSE) {
+                    $assocurl = new moodle_url('/course/view.php', array('id' => $context->instanceid));
+                    $text = $DB->get_field('course', 'shortname', array('id' => $context->instanceid)); //TODO: performance!!!!
                     $assocstr .= $OUTPUT->action_icon($assocurl, new pix_icon('i/course', $text), null, array(), true);
                     $hascourseassocs = true;
                     $assoctype = get_string('course');
@@ -259,15 +259,15 @@ public function print_html($return=false) {
 
             // Now show mod association
             foreach ($blogassociations as $assocrec) {
-                $contextrec = $DB->get_record('context', array('id' => $assocrec->contextid));
+                $context = get_context_instance_by_id($assocrec->contextid);
 
-                if ($contextrec->contextlevel ==  CONTEXT_MODULE) {
+                if ($context->contextlevel ==  CONTEXT_MODULE) {
                     if ($hascourseassocs) {
                         $assocstr .= ', ';
                         $hascourseassocs = false;
                     }
 
-                    $modinfo = $DB->get_record('course_modules', array('id' => $contextrec->instanceid));
+                    $modinfo = $DB->get_record('course_modules', array('id' => $context->instanceid));
                     $modname = $DB->get_field('modules', 'name', array('id' => $modinfo->module));
 
                     $assocurl = new moodle_url('/mod/'.$modname.'/view.php', array('id' => $modinfo->id));

enrol/guest/lib.php

@@ -77,7 +77,7 @@ public function try_guestaccess(stdClass $instance) {
         if (empty($instance->password)) {
             // Temporarily assign them some guest role for this context
             $context = get_context_instance(CONTEXT_COURSE, $instance->courseid);
-            $USER->access = load_temp_role($context, $CFG->guestroleid, $USER->access);
+            load_temp_course_role($context, $CFG->guestroleid);
             return ENROL_REQUIRE_LOGIN_CACHE_PERIOD + time();
         }
 
@@ -131,7 +131,7 @@ public function enrol_page_hook(stdClass $instance) {
 
                 // add guest role
                 $context = get_context_instance(CONTEXT_COURSE, $instance->courseid);
-                $USER->access = load_temp_role($context, $CFG->guestroleid, $USER->access);
+                load_temp_course_role($context, $CFG->guestroleid);
 
                 // go to the originally requested page
                 if (!empty($SESSION->wantsurl)) {

enrol/paypal/return.php

@@ -38,9 +38,6 @@
 
 require_login();
 
-// Refreshing enrolment data in the USER session
-load_all_capabilities();
-
 if ($SESSION->wantsurl) {
     $destination = $SESSION->wantsurl;
     unset($SESSION->wantsurl);

enrol/upgrade.txt

@@ -0,0 +1,8 @@
+This files describes API changes in /enrol/* - plugins,
+information provided here is intended especially for developers.
+
+=== 2.2 ===
+
+required changes in code:
+* load_temp_role() is deprecated, use load_temp_course_role() instead, temp role not loaded
+* remove_temp_role() is deprecated, use remove_temp_course_roles() instead