From 1344b0ca7f505cf00b018e71aa228e5da764b64d Mon Sep 17 00:00:00 2001 From: Petr Skoda Date: Sat, 29 Oct 2011 09:45:54 +0200 Subject: [PATCH 1/2] MDL-29982 improve can_access_course() Hopefully faster and more accurate. --- lib/accesslib.php | 146 +++++++++++++++++++++++++++++------------- lib/navigationlib.php | 13 ++-- mod/forum/lib.php | 4 +- 3 files changed, 110 insertions(+), 53 deletions(-) diff --git a/lib/accesslib.php b/lib/accesslib.php index 6ef78db739fee..e4dae05aedbbe 100644 --- a/lib/accesslib.php +++ b/lib/accesslib.php @@ -1969,31 +1969,68 @@ function is_enrolled(context $context, $user = null, $withcapability = '', $only * of this function can_access_course and require_login. Doing so WILL break future * versions. * - * @param context $context - * @param stdClass|null $user + * @param stdClass $course record + * @param stdClass|int|null $user user record or id, current user if null * @param string $withcapability Check for this capability as well. * @param bool $onlyactive consider only active enrolments in enabled plugins and time restrictions - * @param boolean $trustcache If set to false guest access will always be checked - * against the enrolment plugins from the course, rather - * than the cache generated by require_login. * @return boolean Returns true if the user is able to access the course */ -function can_access_course(context $context, $user = null, $withcapability = '', $onlyactive = false, $trustcache = true) { +function can_access_course(stdClass $course, $user = null, $withcapability = '', $onlyactive = false) { global $DB, $USER; - $coursecontext = $context->get_course_context(); - $courseid = $coursecontext->instanceid; + // this function originally accepted $coursecontext parameter + if ($course instanceof context) { + if ($course instanceof context_course) { + debugging('deprecated context parameter, please use $course record'); + $coursecontext = $course; + $course = $DB->get_record('course', array('id'=>$coursecontext->instanceid)); + } else { + debugging('Invalid context parameter, please use $course record'); + return false; + } + } else { + $coursecontext = context_course::instance($course->id); + } + + if (!isset($USER->id)) { + // should never happen + $USER->id = 0; + } + + // make sure there is a user specified + if ($user === null) { + $userid = $USER->id; + } else { + $userid = is_object($user) ? $user->id : $user; + } + unset($user); + + if ($withcapability and !has_capability($withcapability, $coursecontext, $userid)) { + return false; + } + + if ($userid == $USER->id) { + if (!empty($USER->access['rsw'][$coursecontext->path])) { + // the fact that somebody switched role means they can access the course no matter to what role they switched + return true; + } + } + + if (!$course->visible and !has_capability('moodle/course:viewhiddencourses', $coursecontext)) { + return false; + } - // First check the obvious, is the user viewing or is the user enrolled. - if (is_viewing($coursecontext, $user, $withcapability) || is_enrolled($coursecontext, $user, $withcapability, $onlyactive)) { - // How easy was that! + if (is_viewing($coursecontext, $userid)) { return true; } - $access = false; + if ($userid != $USER->id) { + // for performance reasons we do not verify temporary guest access for other users, sorry... + return is_enrolled($coursecontext, $userid, '', $onlyactive); + } + + // verify our caches if (!isset($USER->enrol)) { - // Cache hasn't been generated yet so we can't trust it - $trustcache = false; /** * These flags within the $USER object should NEVER be used outside of this * function can_access_course and the function require_login. @@ -2003,41 +2040,64 @@ function can_access_course(context $context, $user = null, $withcapability = '', $USER->enrol['enrolled'] = array(); $USER->enrol['tempguest'] = array(); } + if (isset($USER->enrol['enrolled'][$course->id])) { + if ($USER->enrol['enrolled'][$course->id] == 0) { + return true; + } else if ($USER->enrol['enrolled'][$course->id] > time()) { + return true; + } + } + if (isset($USER->enrol['tempguest'][$course->id])) { + if ($USER->enrol['tempguest'][$course->id] == 0) { + return true; + } else if ($USER->enrol['tempguest'][$course->id] > time()) { + return true; + } + } - // If we don't trust the cache we need to check with the courses enrolment - // plugin instances to see if the user can access the course as a guest. - if (!$trustcache) { - // Ok, off to the database we go! - $instances = $DB->get_records('enrol', array('courseid'=>$courseid, 'status'=>ENROL_INSTANCE_ENABLED), 'sortorder, id ASC'); - $enrols = enrol_get_plugins(true); - foreach($instances as $instance) { - if (!isset($enrols[$instance->enrol])) { - continue; - } - $until = $enrols[$instance->enrol]->try_guestaccess($instance); - if ($until !== false) { - // Never use me anywhere but here and require_login - $USER->enrol['tempguest'][$courseid] = $until; - $access = true; - break; - } + if (is_enrolled($coursecontext, $USER, '', true)) { + // active participants may always access + // TODO: refactor this into some new function + $now = time(); + $sql = "SELECT MAX(ue.timeend) + FROM {user_enrolments} ue + JOIN {enrol} e ON (e.id = ue.enrolid AND e.courseid = :courseid) + JOIN {user} u ON u.id = ue.userid + WHERE ue.userid = :userid AND ue.status = :active AND e.status = :enabled AND u.deleted = 0 + AND ue.timestart < :now1 AND (ue.timeend = 0 OR ue.timeend > :now2)"; + $params = array('enabled'=>ENROL_INSTANCE_ENABLED, 'active'=>ENROL_USER_ACTIVE, + 'userid'=>$USER->id, 'courseid'=>$coursecontext->instanceid, 'now1'=>$now, 'now2'=>$now); + $until = $DB->get_field_sql($sql, $params); + if (!$until or $until > time() + ENROL_REQUIRE_LOGIN_CACHE_PERIOD) { + $until = time() + ENROL_REQUIRE_LOGIN_CACHE_PERIOD; } + + $USER->enrol['enrolled'][$course->id] = $until; + + // remove traces of previous temp guest access + remove_temp_course_roles($coursecontext); + + return true; } + unset($USER->enrol['enrolled'][$course->id]); - // If we don't already have access (from above) check the cache and see whether - // there is record of it in there. - if (!$access && isset($USER->enrol['tempguest'][$courseid])) { - // Never use me anywhere but here and require_login - if ($USER->enrol['tempguest'][$courseid] == 0) { - $access = true; - } else if ($USER->enrol['tempguest'][$courseid] > time()) { - $access = true; - } else { - //expired - unset($USER->enrol['tempguest'][$courseid]); + // if not enrolled try to gain temporary guest access + $instances = $DB->get_records('enrol', array('courseid'=>$course->id, 'status'=>ENROL_INSTANCE_ENABLED), 'sortorder, id ASC'); + $enrols = enrol_get_plugins(true); + foreach($instances as $instance) { + if (!isset($enrols[$instance->enrol])) { + continue; + } + // Get a duration for the guestaccess, a timestamp in the future or false. + $until = $enrols[$instance->enrol]->try_guestaccess($instance); + if ($until !== false) { + $USER->enrol['tempguest'][$course->id] = $until; + return true; } } - return $access; + unset($USER->enrol['tempguest'][$course->id]); + + return false; } /** diff --git a/lib/navigationlib.php b/lib/navigationlib.php index e5463fd1e20f0..94c42fd7ae3b4 100644 --- a/lib/navigationlib.php +++ b/lib/navigationlib.php @@ -1112,10 +1112,9 @@ public function initialise() { // If the user is not enrolled then we only want to show the // course node and not populate it. - $coursecontext = get_context_instance(CONTEXT_COURSE, $course->id); // Not enrolled, can't view, and hasn't switched roles - if (!can_access_course($coursecontext)) { + if (!can_access_course($course)) { // TODO: very ugly hack - do not force "parents" to enrol into course their child is enrolled in, // this hack has been propagated from user/view.php to display the navigation node. (MDL-25805) $isparent = false; @@ -1164,8 +1163,7 @@ public function initialise() { // If the user is not enrolled then we only want to show the // course node and not populate it. - $coursecontext = get_context_instance(CONTEXT_COURSE, $course->id); - if (!can_access_course($coursecontext)) { + if (!can_access_course($course)) { $coursenode->make_active(); $canviewcourseprofile = false; break; @@ -1232,8 +1230,7 @@ public function initialise() { // If the user is not enrolled then we only want to show the // course node and not populate it. - $coursecontext = get_context_instance(CONTEXT_COURSE, $course->id); - if (!can_access_course($coursecontext)) { + if (!can_access_course($course)) { $coursenode->make_active(); $canviewcourseprofile = false; break; @@ -2089,7 +2086,7 @@ protected function load_for_user($user=null, $forceforcontext=false) { $usercoursenode->add(get_string('notes', 'notes'), $url, self::TYPE_SETTING); } - if (can_access_course(get_context_instance(CONTEXT_COURSE, $usercourse->id), $user->id)) { + if (can_access_course($usercourse, $user->id)) { $usercoursenode->add(get_string('entercourse'), new moodle_url('/course/view.php', array('id'=>$usercourse->id)), self::TYPE_SETTING, null, null, new pix_icon('i/course', '')); } @@ -3614,7 +3611,7 @@ protected function generate_user_settings($courseid, $userid, $gstitle='usercurr } else { $canviewusercourse = has_capability('moodle/user:viewdetails', $coursecontext); $canaccessallgroups = has_capability('moodle/site:accessallgroups', $coursecontext); - if ((!$canviewusercourse && !$canviewuser) || !can_access_course($coursecontext, $user->id)) { + if ((!$canviewusercourse && !$canviewuser) || !can_access_course($course, $user->id)) { return false; } if (!$canaccessallgroups && groups_get_course_groupmode($course) == SEPARATEGROUPS) { diff --git a/mod/forum/lib.php b/mod/forum/lib.php index 80c4edf9557d9..cc5ef170ef702 100644 --- a/mod/forum/lib.php +++ b/mod/forum/lib.php @@ -8148,7 +8148,7 @@ function forum_get_posts_by_user($user, array $courses, $musthaveaccess = false, } else { // Check whether the current user is enrolled or has access to view the course // if they don't we immediately have a problem. - if (!can_access_course($coursecontext)) { + if (!can_access_course($course)) { if ($musthaveaccess) { print_error('errorenrolmentrequired', 'forum'); } @@ -8157,7 +8157,7 @@ function forum_get_posts_by_user($user, array $courses, $musthaveaccess = false, // Check whether the requested user is enrolled or has access to view the course // if they don't we immediately have a problem. - if (!can_access_course($coursecontext, $user)) { + if (!can_access_course($course, $user)) { if ($musthaveaccess) { print_error('notenrolled', 'forum'); } From 0f14c8273e9927154cee7dd41252e738172ff409 Mon Sep 17 00:00:00 2001 From: Petr Skoda Date: Sun, 30 Oct 2011 10:53:42 +0100 Subject: [PATCH 2/2] MDL-29982 fixed missing $userid Thanks Eloy! --- lib/accesslib.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/accesslib.php b/lib/accesslib.php index e4dae05aedbbe..44793792c72b0 100644 --- a/lib/accesslib.php +++ b/lib/accesslib.php @@ -2016,7 +2016,7 @@ function can_access_course(stdClass $course, $user = null, $withcapability = '', } } - if (!$course->visible and !has_capability('moodle/course:viewhiddencourses', $coursecontext)) { + if (!$course->visible and !has_capability('moodle/course:viewhiddencourses', $coursecontext, $userid)) { return false; } @@ -2029,6 +2029,8 @@ function can_access_course(stdClass $course, $user = null, $withcapability = '', return is_enrolled($coursecontext, $userid, '', $onlyactive); } + // === from here we deal only with $USER === + // verify our caches if (!isset($USER->enrol)) { /**