This commit is contained in:
Jun Pataleta 2024-08-20 11:04:11 +08:00
commit 99f7cb0a8a
No known key found for this signature in database
GPG Key ID: F83510526D99E2C7
6 changed files with 241 additions and 9 deletions

View File

@ -497,6 +497,7 @@ $string['user:viewalldetails'] = 'View user full information';
$string['user:viewdetails'] = 'View user profiles';
$string['user:viewhiddendetails'] = 'View hidden details of users';
$string['user:viewlastip'] = 'View user last ip address';
$string['user:viewprofilepictures'] = 'View user profile pictures (if force login enabled)';
$string['user:viewuseractivitiesreport'] = 'See user activity reports';
$string['user:viewusergrades'] = 'View user grades';
$string['roleresetdefaults'] = 'Defaults';

View File

@ -212,6 +212,47 @@ class user_picture implements renderable {
return $return;
}
/**
* Checks if the current user is permitted to view user profile images.
*
* This is based on the forcelogin and forceloginforprofileimage config settings, and the
* moodle/user:viewprofilepictures capability.
*
* Logged-in users are allowed to view their own profile image regardless of capability.
*
* @param int $imageuserid User id of profile image being viewed
* @return bool True if current user can view profile images
*/
public static function allow_view(int $imageuserid): bool {
global $CFG, $USER;
// Not allowed to view profile images if forcelogin is enabled and not logged in (guest
// allowed), or forceloginforprofileimage is enabled and not logged in or guest.
if (
(!empty($CFG->forcelogin) && !isloggedin()) ||
(!empty($CFG->forceloginforprofileimage) && (!isloggedin() || isguestuser()))
) {
return false;
}
// Unless one of the forcelogin options is enabled, users can download profile pics
// without login, so the capability should not be checked as it might lead to a
// false sense of security (i.e. you log in as a test user, the HTML page doesn't
// show the picture, but they can still access it if they just log out).
// When the capability is checked, use system context for performance (if we check at
// user level, pages that show a lot of user pictures will individually load a lot of
// user contexts).
if (
(!empty($CFG->forcelogin) || !empty($CFG->forceloginforprofileimage)) &&
$USER->id != $imageuserid &&
!has_capability('moodle/user:viewprofilepictures', \context_system::instance())
) {
return false;
}
return true;
}
/**
* Works out the URL for the users picture.
*
@ -253,13 +294,7 @@ class user_picture implements renderable {
$defaulturl = $renderer->image_url('u/' . $filename); // Default image.
if (
(!empty($CFG->forcelogin) && !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.
if (!self::allow_view($this->user->id)) {
return $defaulturl;
}

View File

@ -529,6 +529,15 @@ $capabilities = array(
)
),
'moodle/user:viewprofilepictures' => [
'captype' => 'read',
'contextlevel' => CONTEXT_SYSTEM,
'archetypes' => [
'guest' => CAP_ALLOW,
'user' => CAP_ALLOW,
],
],
'moodle/user:viewalldetails' => array(
'riskbitmask' => RISK_PERSONAL,
'captype' => 'read',

View File

@ -4747,8 +4747,7 @@ function file_pluginfile($relativepath, $forcedownload, $preview = null, $offlin
$filename = 'f1';
}
if ((!empty($CFG->forcelogin) and !isloggedin()) ||
(!empty($CFG->forceloginforprofileimage) && (!isloggedin() || isguestuser()))) {
if (!\core\output\user_picture::allow_view($context->instanceid)) {
// 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

View File

@ -0,0 +1,72 @@
@core @_file_upload
Feature: Profile picture access
In order to enable precise security control and meet legal requirements
As site administrators
We should be able to prevent certain users from viewing profile pictures
Background:
Given the following "users" exist:
| username | firstname | lastname |
| student1 | Alice | in Wonderland |
| student2 | Bob | a Job Week |
And the following "courses" exist:
| shortname |
| C1 |
And the following "course enrolments" exist:
| user | course | role |
| student1 | C1 | student |
| student2 | C1 | student |
And the following "activity" exists:
| course | C1 |
| activity | forum |
| name | TestForum |
| idnumber | forum1 |
And the following "mod_forum > discussions" exist:
| user | forum | name | message | timemodified |
| student1 | forum1 | Post1 | This is the first post | ##now -1 second## |
And the following "roles" exist:
| shortname |
| dangerous |
And the following "role capability" exists:
| role | dangerous |
| moodle/user:viewprofilepictures | prohibit |
And I am on the "Profile editing" page logged in as "student1"
And I upload "/course/tests/fixtures/image.jpg" file to "New picture" filemanager
And I set the field "Picture description" to "MyPic"
And I press "Update profile"
@javascript
Scenario: Users can view pictures on forum page when permitted
When I am on the "forum1" "forum activity" page logged in as "student2"
# By default you can see user pics.
And ".discussion-list img.userpicture[src*='user/icon']" "css_element" should be visible
# Even if you don't have the capability, you can still see them...
And the following "system role assigns" exist:
| user | role | contextlevel |
| student2 | dangerous | System |
And I reload the page
And ".discussion-list img.userpicture[src*='user/icon']" "css_element" should be visible
# ...unless forcelogin is on, when the system kicks in and hides it.
And the following config values are set as admin:
| forcelogin | 1 |
And I reload the page
Then ".discussion-list img.userpicture[src*='user/icon']" "css_element" should not exist
@javascript
Scenario: Users can view pictures on profile page when permitted
When I am on the "forum1" "forum activity" page logged in as "student2"
And I follow "Post1"
And I follow "Alice in Wonderland"
# By default you can see user pics.
And ".page-header-image img.userpicture[src*='user/icon']" "css_element" should be visible
# Even if you don't have the capability, you can still see them...
And the following "system role assigns" exist:
| user | role | contextlevel |
| student2 | dangerous | System |
And I reload the page
And ".page-header-image img.userpicture[src*='user/icon']" "css_element" should be visible
# ...unless forcelogin is on, when the system kicks in and hides it.
And the following config values are set as admin:
| forcelogin | 1 |
And I reload the page
Then ".page-header-image img.userpicture[src*='user/icon']" "css_element" should not exist

View File

@ -0,0 +1,116 @@
<?php
// This file is part of Moodle - http://moodle.org/
//
// Moodle is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// Moodle is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
//
// You should have received a copy of the GNU General Public License
// along with Moodle. If not, see <http://www.gnu.org/licenses/>.
namespace core\output;
/**
* Unit tests for the user_picture class.
*
* @package core
* @copyright 2024 The Open University
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
* @covers \core\output\user_picture
*/
final class user_picture_test extends \advanced_testcase {
/**
* Tests {@see user_picture::allow_view()} for a not-logged-in request.
*/
public function test_allow_view_not_logged_in(): void {
global $DB;
$this->resetAfterTest();
$adminid = $DB->get_field('user', 'id', ['username' => 'admin'], MUST_EXIST);
// Default config allows user pictures when not logged in.
$this->assertTrue(user_picture::allow_view($adminid));
// Not allowed with either or both forcelogin options.
set_config('forcelogin', 1);
$this->assertFalse(user_picture::allow_view($adminid));
set_config('forcelogin', 0);
set_config('forceloginforprofileimage', 1);
$this->assertFalse(user_picture::allow_view($adminid));
set_config('forcelogin', 1);
$this->assertFalse(user_picture::allow_view($adminid));
}
/**
* Tests {@see user_picture::allow_view()} for a guest user request.
*/
public function test_allow_view_guest(): void {
global $DB;
$this->resetAfterTest();
$this->setGuestUser();
$adminid = $DB->get_field('user', 'id', ['username' => 'admin'], MUST_EXIST);
// Default config allows user pictures for guests.
$this->assertTrue(user_picture::allow_view($adminid));
// Not allowed with forceloginforprofileimage.
set_config('forceloginforprofileimage', 1);
$this->assertFalse(user_picture::allow_view($adminid));
// Allowed by default with just forcelogin.
set_config('forceloginforprofileimage', 0);
set_config('forcelogin', 1);
$this->assertTrue(user_picture::allow_view($adminid));
// But would not be allowed if we change guest role to remove capability.
$guestroleid = $DB->get_field('role', 'id', ['shortname' => 'guest'], MUST_EXIST);
assign_capability('moodle/user:viewprofilepictures', CAP_INHERIT, $guestroleid,
\context_system::instance()->id, true);
$this->assertFalse(user_picture::allow_view($adminid));
}
/**
* Tests {@see user_picture::allow_view()} for a logged in user.
*/
public function test_allow_view_user(): void {
global $DB;
$this->resetAfterTest();
$adminid = $DB->get_field('user', 'id', ['username' => 'admin'], MUST_EXIST);
$generator = self::getDataGenerator();
$user = $generator->create_user();
$this->setUser($user);
// Default config allows user pictures.
$this->assertTrue(user_picture::allow_view($adminid));
// Also allowed with either or both forcelogin option, because they are logged in.
set_config('forcelogin', 1);
$this->assertTrue(user_picture::allow_view($adminid));
set_config('forcelogin', 0);
set_config('forceloginforprofileimage', 1);
$this->assertTrue(user_picture::allow_view($adminid));
set_config('forcelogin', 1);
$this->assertTrue(user_picture::allow_view($adminid));
// But would not be allowed if we change user role to remove capability.
$userroleid = $DB->get_field('role', 'id', ['shortname' => 'user'], MUST_EXIST);
assign_capability('moodle/user:viewprofilepictures', CAP_INHERIT, $userroleid,
\context_system::instance()->id, true);
$this->assertFalse(user_picture::allow_view($adminid));
// Except you are still allowed to view your own user picture.
$this->assertTrue(user_picture::allow_view($user->id));
}
}