From 0c0c6f52a76c3d750c0f9595fda798887dfcdc97 Mon Sep 17 00:00:00 2001 From: Adrian Sarmas Date: Sun, 11 Oct 2026 21:22:35 +0300 Subject: [PATCH 1/2] Check group access before showing entries and serving entry files Follow-up to #163. In separate groups mode, report.php showed all entries to a grader who is in no group and cannot access all groups, and journal_pluginfile served entry and feedback files of any student to any grader. Both now respect group visibility. --- lib.php | 5 +++++ report.php | 7 +++++++ tests/behat/grade_entries.feature | 23 +++++++++++++++++++-- tests/lib_test.php | 34 +++++++++++++++++++++++++++++++ 4 files changed, 67 insertions(+), 2 deletions(-) diff --git a/lib.php b/lib.php index 102883f..2857071 100644 --- a/lib.php +++ b/lib.php @@ -1256,6 +1256,11 @@ function journal_pluginfile($course, $cm, $context, $filearea, $args, $forcedown return false; } + // Graders can only see entries of users in groups they have access to. + if ($canmanage && $USER->id != $entry->userid && !groups_user_groups_visible($course, $entry->userid, $cm)) { + return false; + } + if ($filearea !== 'entry' && $filearea !== 'feedback') { return false; } diff --git a/report.php b/report.php index a208e8d..e216c79 100644 --- a/report.php +++ b/report.php @@ -82,6 +82,13 @@ $groupmode = groups_get_activity_groupmode($cm); $currentgroup = groups_get_activity_group($cm, true); +// In separate groups mode, a grader who is in no group and cannot access all groups must not see any entries. +if ($groupmode == SEPARATEGROUPS && !$currentgroup && !has_capability('moodle/site:accessallgroups', $context)) { + echo $OUTPUT->notification(get_string('notingroup'), \core\output\notification::NOTIFY_INFO); + echo $OUTPUT->footer(); + exit; +} + // Fetch users who can ADD entries (Students, but also Teachers/Admins). $groups = $currentgroup ? $currentgroup : ''; $users = get_users_by_capability($context, 'mod/journal:addentries', '', '', '', '', $groups); diff --git a/tests/behat/grade_entries.feature b/tests/behat/grade_entries.feature index c5a2e3d..a3e7f19 100644 --- a/tests/behat/grade_entries.feature +++ b/tests/behat/grade_entries.feature @@ -21,8 +21,8 @@ Feature: Teacher can view, comment and grade students entries | student2 | C1 | student | | student3 | C1 | student | And the following "activities" exist: - | activity | name | intro | course | idnumber | - | journal | Test journal name | Journal question | C1 | journal1 | + | activity | name | intro | course | idnumber | groupmode | + | journal | Test journal name | Journal question | C1 | journal1 | 1 | And I log in as "student1" And I am on "Course1" course homepage And I follow "Test journal name" @@ -106,3 +106,22 @@ Feature: Teacher can view, comment and grade students entries And I should see "Entry has changed since last feedback was saved" And I should see "Student 1 edited first reply" And I should see "Student 2 first reply" + + Scenario: Non-editing teacher in no group cannot see entries in separate groups mode + Given the following "users" exist: + | username | firstname | lastname | email | + | teacher2 | Teacher | 2 | teacher2@asd.com | + And the following "course enrolments" exist: + | user | course | role | + | teacher2 | C1 | teacher | + And the following "groups" exist: + | name | course | idnumber | + | Group 1 | C1 | G1 | + And the following "group members" exist: + | user | group | + | student1 | G1 | + When I am on the "Test journal name" "journal activity" page logged in as "teacher2" + And I follow "View 0 journal entries" + Then I should see "Sorry, but you need to be part of a group to see this page." + And I should not see "Student 1 first reply" + And I should not see "Student 2 first reply" diff --git a/tests/lib_test.php b/tests/lib_test.php index 8d2c800..fff8f22 100644 --- a/tests/lib_test.php +++ b/tests/lib_test.php @@ -50,4 +50,38 @@ public function test_reset(): void { $this->assertCount(1, $status); $this->assertFalse($status[0]['error']); } + + /** + * A grader must not be served files of an entry of a student in another separate group. + * + * @covers ::journal_pluginfile + */ + public function test_pluginfile_rejects_entry_outside_graders_groups(): void { + global $CFG; + require_once($CFG->dirroot . '/mod/journal/lib.php'); + $this->resetAfterTest(); + + $generator = $this->getDataGenerator(); + $journalgenerator = $generator->get_plugin_generator('mod_journal'); + + $course = $generator->create_course(['groupmode' => SEPARATEGROUPS, 'groupmodeforce' => 1]); + $journal = $journalgenerator->create_instance(['course' => $course->id]); + $groupa = $generator->create_group(['courseid' => $course->id]); + $groupb = $generator->create_group(['courseid' => $course->id]); + + $teacher = $generator->create_and_enrol($course, 'teacher'); + $student = $generator->create_and_enrol($course, 'student'); + $generator->create_group_member(['groupid' => $groupa->id, 'userid' => $teacher->id]); + $generator->create_group_member(['groupid' => $groupb->id, 'userid' => $student->id]); + + $entryid = $journalgenerator->create_entry((object)['journal' => $journal->id, 'userid' => $student->id]); + + $cm = get_coursemodule_from_instance('journal', $journal->id, $course->id, false, MUST_EXIST); + $context = \context_module::instance($cm->id); + + $this->setUser($teacher); + foreach (['entry', 'feedback'] as $filearea) { + $this->assertFalse(journal_pluginfile($course, $cm, $context, $filearea, [$entryid, 'file.txt'], false)); + } + } } From 15b98e3281d265d0ad341cfad3f78057632e958a Mon Sep 17 00:00:00 2001 From: Adrian Sarmas Date: Sun, 11 Oct 2026 22:32:02 +0300 Subject: [PATCH 2/2] Move separate groups behat scenario to its own feature Setting separate groups on the shared background journal changed the entry count shown to the teacher on the journals list page and broke an existing scenario. --- tests/behat/grade_entries.feature | 23 ++------------- tests/behat/separate_groups.feature | 43 +++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 21 deletions(-) create mode 100644 tests/behat/separate_groups.feature diff --git a/tests/behat/grade_entries.feature b/tests/behat/grade_entries.feature index a3e7f19..c5a2e3d 100644 --- a/tests/behat/grade_entries.feature +++ b/tests/behat/grade_entries.feature @@ -21,8 +21,8 @@ Feature: Teacher can view, comment and grade students entries | student2 | C1 | student | | student3 | C1 | student | And the following "activities" exist: - | activity | name | intro | course | idnumber | groupmode | - | journal | Test journal name | Journal question | C1 | journal1 | 1 | + | activity | name | intro | course | idnumber | + | journal | Test journal name | Journal question | C1 | journal1 | And I log in as "student1" And I am on "Course1" course homepage And I follow "Test journal name" @@ -106,22 +106,3 @@ Feature: Teacher can view, comment and grade students entries And I should see "Entry has changed since last feedback was saved" And I should see "Student 1 edited first reply" And I should see "Student 2 first reply" - - Scenario: Non-editing teacher in no group cannot see entries in separate groups mode - Given the following "users" exist: - | username | firstname | lastname | email | - | teacher2 | Teacher | 2 | teacher2@asd.com | - And the following "course enrolments" exist: - | user | course | role | - | teacher2 | C1 | teacher | - And the following "groups" exist: - | name | course | idnumber | - | Group 1 | C1 | G1 | - And the following "group members" exist: - | user | group | - | student1 | G1 | - When I am on the "Test journal name" "journal activity" page logged in as "teacher2" - And I follow "View 0 journal entries" - Then I should see "Sorry, but you need to be part of a group to see this page." - And I should not see "Student 1 first reply" - And I should not see "Student 2 first reply" diff --git a/tests/behat/separate_groups.feature b/tests/behat/separate_groups.feature new file mode 100644 index 0000000..38fe2f3 --- /dev/null +++ b/tests/behat/separate_groups.feature @@ -0,0 +1,43 @@ +@mod @mod_journal +Feature: Journal entries respect separate groups + In order to keep entries private between groups + As a teacher without access to all groups + I must not see entries of students whose groups I am not in + + Background: + Given the following "courses" exist: + | fullname | shortname | category | groupmode | + | Course1 | C1 | 0 | 1 | + And the following "users" exist: + | username | firstname | lastname | email | + | teacher1 | Teacher | 1 | teacher1@asd.com | + | student1 | Student | 1 | student1@asd.com | + And the following "course enrolments" exist: + | user | course | role | + | teacher1 | C1 | teacher | + | student1 | C1 | student | + And the following "groups" exist: + | name | course | idnumber | + | Group 1 | C1 | G1 | + And the following "group members" exist: + | user | group | + | student1 | G1 | + And the following "activities" exist: + | activity | name | intro | course | idnumber | groupmode | + | journal | Test journal name | Journal question | C1 | journal1 | 1 | + And I log in as "student1" + And I am on "Course1" course homepage + And I follow "Test journal name" + And I press "Start or edit my journal entry" + And I set the following fields to these values: + | Entry | Student 1 first reply | + And I press "Save changes" + And I log out + + Scenario: Non-editing teacher in no group cannot see entries in separate groups mode + When I log in as "teacher1" + And I am on "Course1" course homepage + And I follow "Test journal name" + And I follow "View 0 journal entries" + Then I should see "Sorry, but you need to be part of a group to see this page." + And I should not see "Student 1 first reply"