[Demo Section] Prevent teachers from modifying demo students.

[lfryemason]

• Introduces Policies::DemoSections — a policy class that maintains a
  config-driven list of demo student IDs (grouped by demo type: aif, csd) and
  provides demo_student? to check membership
• Prevents teachers from managing (editing, transferring, resetting) demo
  students by adding demo-student checks to the CanCanCan ability.rb — can
  :manage, User now returns false for demo students, which cascades to UserLevel
  and UserLevelEvaluation management
  • Updates controllers to respect the new permission boundary. In controllers, it adds checks on can?(:manage, student). Outside controllers it checks demo_section student IDs directly.
• Demo Section Student IDs are defined in config.yml.erb per environment

Concerns: All future code that modifies students or returns shared resources about students will also need to be careful to add these checks.

Links

• Jira: https://codedotorg.atlassian.net/browse/TEACHING-147

Testing story

Tested manually in bug bash.
Added Unit tests

[lfryemason]

This method is not used anywhere

[lfryemason]

Also nover used

[Copilot]

Pull request overview

Adds a centralized, config-driven notion of “demo students” and uses it to prevent teachers from modifying shared demo student accounts and related data, while also scoping certain feedback/evaluation reads to avoid cross-teacher leakage.

Changes:

• Introduces Policies::DemoSections backed by demo_student_ids config, plus unit tests.
• Updates CanCan abilities and multiple controllers/models to block or skip teacher actions on demo students (manage/edit/transfer/reset/progress writes).
• Scopes certain reads (teacher feedback / rubric evaluations) by teacher for demo students to prevent cross-teacher visibility.

Reviewed changes

Copilot reviewed 21 out of 21 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
dashboard/test/models/sections/section_test.rb	Removes tests for update_student_sharing after method removal.
dashboard/test/models/ability_test.rb	Adds coverage asserting teachers cannot manage demo students (and related UserLevel).
dashboard/test/lib/policies/demo_sections_test.rb	Adds unit tests for demo student ID parsing/lookup behavior.
dashboard/test/controllers/api/v1/sections_controller_test.rb	Removes tests for update_sharing_disabled endpoint.
dashboard/lib/policies/demo_sections.rb	New policy for demo-student ID lookup from config.
dashboard/app/models/teacher_score.rb	Skips scoring that would create/update progress for demo students.
dashboard/app/models/sections/section.rb	Avoids modifying sharing settings for demo students during roster/sharing workflows.
dashboard/app/models/follower.rb	Avoids assigning scripts / mutating names for demo students on follower lifecycle hooks.
dashboard/app/models/ability.rb	Blocks :manage (and code review) for demo students, cascading to related models.
dashboard/app/helpers/users_helper.rb	Scopes teacher feedback lookup by teacher for demo students to avoid cross-teacher leakage.
dashboard/app/controllers/transfers_controller.rb	Tightens authorization and skips transferring students the teacher can’t manage (e.g., demo).
dashboard/app/controllers/rubrics_controller.rb	Scopes rubric evaluation reads by teacher for demo students.
dashboard/app/controllers/pairings_controller.rb	Excludes demo students from pairing candidate lists.
dashboard/app/controllers/lesson_feedbacks_controller.rb	Scopes lesson feedback lookup by teacher for demo students.
dashboard/app/controllers/api/v1/users_controller.rb	Uses can?(:manage, target_user) to prevent modifying demo student records.
dashboard/app/controllers/api/v1/teacher_feedbacks_controller.rb	Scopes feedback reads for demo students; blocks keepWorking reset if student not manageable.
dashboard/app/controllers/api/v1/sections_students_controller.rb	Switches update authorization to can?(:manage, @student).
dashboard/app/controllers/api/v1/sections_controller.rb	Skips student assignment updates for unmanageable students; removes update_sharing_disabled action.
dashboard/app/controllers/aichat_events_controller.rb	Requires can?(:manage, student) before submitting teacher feedback.
config/adhoc.yml.erb	Adds demo_student_ids config key placeholder.
config.yml.erb	Adds demo_student_ids config key placeholder with comment.

Comments suppressed due to low confidence (1)

dashboard/app/controllers/api/v1/sections_students_controller.rb:40

• Replacing the explicit return head :forbidden if @student.teacher? check with can?(:manage, @student) appears to re-allow updates to teacher accounts that are enrolled as students in a section. In Ability, can :manage, User for teachers checks membership in user.students but does not check u.student?, and this endpoint permits sensitive fields (including :password and reset_secrets). Consider restoring the teacher-user guard here or tightening the :manage, User rule to only allow managing student accounts.

  # PATCH /sections/<section_id>/students/<id>
  def update
    return head :forbidden unless can?(:manage, @student)

    @student.reset_secrets if params[:secrets] == User::RESET_SECRETS

    if @student.update(student_params)

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[kobryan0619]

Hmm... I am assuming that this is used to add demo_student_ids? Do you want to leave a comment here explaining the role of this?

[lfryemason]

We can actually just remove this since it is in the base config.yml.erb. Good catch

[kobryan0619]

I wonder about deleting the comment here. I am probably more pro-comment than others (or maybe than is best practice), but I appreciate the context of this comment.

[kobryan0619]

Nice find here.

[kobryan0619]

The more I read these comments (which, as I mentioned, I love!), it makes me think this "Demo Student" / "Demo Section" project should really be highlighted at an engineering meeting! A. Because it is cool and B. Because it is a unique data situation that it might be good to have publicized a bit more.

[lfryemason]

Agree here, I think I want to get a bit further, but then I will announce in eng-weekly since it does have consequences

[kobryan0619]

Looks good - some small notes, but nothing blocking.

A note that I wrote in the comments, but wanted to highlight here, in case it hasn't come up with Bethany yet. This "Demo Student" / "Demo Section" project would be a great candidate to be highlighted at an engineering meeting! A. Because it is cool and B. Because it is a unique data situation that it might be good to have publicized a bit more.