From fb1857e8938e50282394701dd623add3be94da6e Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Fri, 31 Jul 2015 09:08:30 +0800 Subject: [PATCH] MDL-48245 core: Improve checks for forceloginforprofileimage --- admin/settings/security.php | 3 ++- lang/en/admin.php | 3 ++- lib/filelib.php | 36 ++++++++++++++++++++++++++++-------- lib/outputcomponents.php | 22 +++++++++++++++++----- 4 files changed, 49 insertions(+), 15 deletions(-) diff --git a/admin/settings/security.php b/admin/settings/security.php index 66cae6b..9da2f76 100644 --- a/admin/settings/security.php +++ b/admin/settings/security.php @@ -16,7 +16,8 @@ if ($hassiteconfig) { // speedup for non-admins, add all caps used on this page $temp->add(new admin_setting_configcheckbox('protectusernames', new lang_string('protectusernames', 'admin'), new lang_string('configprotectusernames', 'admin'), 1)); $temp->add(new admin_setting_configcheckbox('forcelogin', new lang_string('forcelogin', 'admin'), new lang_string('configforcelogin', 'admin'), 0)); $temp->add(new admin_setting_configcheckbox('forceloginforprofiles', new lang_string('forceloginforprofiles', 'admin'), new lang_string('configforceloginforprofiles', 'admin'), 1)); - $temp->add(new admin_setting_configcheckbox('forceloginforprofileimage', new lang_string('forceloginforprofileimage', 'admin'), new lang_string('forceloginforprofileimage_help', 'admin'), 0)); + $options = array(-1 => new lang_string('followforcelogin', 'admin'), 0 => get_string('no'), 1 => 'yes'); + $temp->add(new admin_setting_configselect('forceloginforprofileimage', new lang_string('forceloginforprofileimage', 'admin'), new lang_string('forceloginforprofileimage_help', 'admin'), -1, $options)); $temp->add(new admin_setting_configcheckbox('opentogoogle', new lang_string('opentogoogle', 'admin'), new lang_string('configopentogoogle', 'admin'), 0)); $temp->add(new admin_setting_pickroles('profileroles', new lang_string('profileroles','admin'), diff --git a/lang/en/admin.php b/lang/en/admin.php index 11ab138..b6d287b 100644 --- a/lang/en/admin.php +++ b/lang/en/admin.php @@ -522,9 +522,10 @@ $string['filters'] = 'Filters'; $string['filtersettings'] = 'Manage filters'; $string['filtersettingsgeneral'] = 'General filter settings'; $string['filteruploadedfiles'] = 'Filter uploaded files'; +$string['followforcelogin'] = 'Respect forcelogin'; $string['forcelogin'] = 'Force users to log in'; $string['forceloginforprofileimage'] = 'Force users to log in to view user pictures'; -$string['forceloginforprofileimage_help'] = 'If enabled, users must log in in order to view user profile pictures and the default user picture will be used in all notification emails.'; +$string['forceloginforprofileimage_help'] = 'If enabled, users must log in in order to view user profile pictures and the default user picture will be used in all notification emails. Note: Guests are prevented from viewing profile images in this case too.'; $string['forceloginforprofiles'] = 'Force users to log in for profiles'; $string['forcetimezone'] = 'Force timezone'; $string['formatuninstallwithcourses'] = 'There are {$a->count} courses using {$a->format}. Their format will be changed to {$a->defaultformat} (default format for this site). Some format-specific data may be lost. Are you sure you want to proceed?'; diff --git a/lib/filelib.php b/lib/filelib.php index 6375faf..e7f6436 100644 --- a/lib/filelib.php +++ b/lib/filelib.php @@ -3981,13 +3981,26 @@ function file_pluginfile($relativepath, $forcedownload, $preview = null) { $filename = 'f1'; } - if ((!empty($CFG->forcelogin) and !isloggedin()) || - (!empty($CFG->forceloginforprofileimage) && (!isloggedin() || isguestuser()))) { - // protect images if login required and not logged in; - // also if login is required for profile images and is not logged in or guest - // do not use require_login() because it is expensive and not suitable here anyway + // Protect images based on the forceloginforprofileimage and forcelogin settings. + // Note: forceloginforprofileimage does not allow guest access, while forcelogin does. + + // Always allow access if we have a loggedin user who is not the guest. + $allowaccess = isloggedin() && !isguestuser(); + + // Allow access if we actively do not force login for the profile image. + $allowaccess = $allowaccess || empty($CFG->forceloginforprofileimage); + + // Allow access if we respect the value of forcelogin, and do not actively force login. + $allowaccess = $allowaccess || ($CFG->forceloginforprofileimage === -1 && empty($CFG->forcelogin)); + + // Allow access if we respect the value of forcelogin, and actively force login, and the user is a guest. + $allowaccess = $allowaccess || ($CFG->forceloginforprofileimage === -1 && $CFG->forcelogin) && isguestuser(); + + if (!$allowaccess) { + // Do not use require_login() because it is expensive and not suitable here anyway. $theme = theme_config::load($themename); - redirect($theme->pix_url('u/'.$filename, 'moodle')); // intentionally not cached + // Note: This is intentionally not cached. + redirect($theme->pix_url('u/'.$filename, 'moodle')); } if (!$file = $fs->get_file($context->id, 'user', 'icon', 0, '/', $filename.'.png')) { @@ -4014,8 +4027,15 @@ function file_pluginfile($relativepath, $forcedownload, $preview = null) { } $options = array('preview' => $preview); - if (empty($CFG->forcelogin) && empty($CFG->forceloginforprofileimage)) { - // Profile images should be cache-able by both browsers and proxies according + + // Allow caching if we do not actively force login for the profile image. + $allowcache = empty($CFG->forceloginforprofileimage); + + // Allow caching if we respect the value of forcelogin, and do not actively force login. + $allowcache = $allowcache || ($CFG->forceloginforprofileimage === -1 && empty($CFG->forcelogin)); + + if ($allowcache) { + // Profile images should be cacheable by both browsers and proxies according // to $CFG->forcelogin and $CFG->forceloginforprofileimage. $options['cacheability'] = 'public'; } diff --git a/lib/outputcomponents.php b/lib/outputcomponents.php index 4efe670..4b88f99 100644 --- a/lib/outputcomponents.php +++ b/lib/outputcomponents.php @@ -361,11 +361,23 @@ class user_picture implements renderable { $defaulturl = $renderer->pix_url('u/'.$filename); // default image - if ((!empty($CFG->forcelogin) and !isloggedin()) || - (!empty($CFG->forceloginforprofileimage) && (!isloggedin() || isguestuser()))) { - // Protect images if login required and not logged in; - // also if login is required for profile images and is not logged in or guest - // do not use require_login() because it is expensive and not suitable here anyway. + // Protect images based on the forceloginforprofileimage and forcelogin settings. + // Note: forceloginforprofileimage does not allow guest access, while forcelogin does. + + // Always allow access if we have a loggedin user who is not the guest. + $allowaccess = isloggedin() && !isguestuser(); + + // Allow access if we actively do not force login for the profile image. + $allowaccess = $allowaccess || empty($CFG->forceloginforprofileimage); + + // Allow access if we respect the value of forcelogin, and do not actively force login. + $allowaccess = $allowaccess || ($CFG->forceloginforprofileimage === -1 && empty($CFG->forcelogin)); + + // Allow access if we respect the value of forcelogin, and actively force login, and the user is a guest. + $allowaccess = $allowaccess || ($CFG->forceloginforprofileimage === -1 && $CFG->forcelogin) && isguestuser(); + + if (!$allowaccess) { + // Do not use require_login() because it is expensive and not suitable here anyway. return $defaulturl; } -- 2.4.0