Security: enforce teacher-student enrollment check on student tracking page#8602
Merged
Merged
Conversation
AngelFQC
force-pushed
the
security/auto-fix-GHSA-rp64-899j-x9f6
branch
from
c8c5235 to
4081740
Compare
myStudents.php granted access to a student's full tracking profile to any user holding a tracking-capable role (notably any teacher, via api_is_teacher()) without checking that the requester actually teaches the target student, so a teacher could read the learning records of any student on the platform by changing the `student` query parameter. Replace the three overlapping role checks (the broad $allowedToTrackUser OR, the current-course relationship block, and the extra per-student guard) with a single two-level rule: 1. Wide scope (platform/session admins, HR managers, student bosses) may view any student's tracking. 2. Everyone else needs a real relationship with the target student: teaches them (UserManager::isTeacherOfStudent, shared course), coaches them (Tracking::is_allowed_to_coach_student, shared session), or tutors a course the student is enrolled in (get_tutor_in_course_status + enrollment check). Course tutors keep access, but only to students enrolled in the course they tutor, instead of to any student as the previous check allowed. Refs GHSA-rp64-899j-x9f6 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
AngelFQC
force-pushed
the
security/auto-fix-GHSA-rp64-899j-x9f6
branch
from
4081740 to
73b01e3
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
my_space/myStudents.phpauthorized access to a student's full tracking profile (quiz scores, LP progress, time-on-task, last login, personal details) based only on the requester holding a tracking-capable role. Becauseapi_is_teacher()(and other course-scoped roles) short-circuit the access flag totrue, and the target student id comes straight from thestudentquery parameter, any teacher on the platform could read the learning records of any student — regardless of whether they share a course — by enumerating thestudentparameter.Fix
Add a per-student authorization layer right after the target user is resolved, in addition to the existing role gate:
UserManager::isTeacherOfStudent(), which intersects the teacher's taught courses with the student's enrolled courses) or being allowed to coach them in a session (Tracking::is_allowed_to_coach_student()).This mirrors the access rule already used in
public/main/lp/lp_tracking.php.Invariant now enforced
A teacher can only open the tracking profile of a student they actually teach or coach; requests for unrelated student ids are rejected with
api_not_allowed().Notes for the reviewer
UserManager::isTeacherOfStudent()compares base-course enrollments. A student enrolled in the teacher's course only through a session is covered by theTracking::is_allowed_to_coach_student()branch (session coach relationship). If a deployment relies on base-course teachers tracking session-only students without a coach relationship, that edge case should be reviewed — the same limitation already exists inlp_tracking.php, whose pattern is replicated here for consistency.$allowedToTrackUserrole logic is left untouched; this change only adds a second, stricter gate.OWASP control
A01:2021 – Broken Access Control (CWE-639, Authorization Bypass Through User-Controlled Key / IDOR).
Refs GHSA-rp64-899j-x9f6
🤖 Generated with Claude Code