From 322325c6884869c6c689fb129031342db8a9aae2 Mon Sep 17 00:00:00 2001
From: Isaries <leolove3very@gmail.com>
Date: Mon, 6 Jul 2026 21:58:56 +0800
Subject: [PATCH] fix(security): re-check run ownership when batch resetting
 group passwords

The batch student password reset only verified that the teacher owned the
run when loading the form. The submit handler read the group id from the
session-bound form object, which request parameters can override, and
reset every member's password without re-checking ownership, letting a
teacher reset another teacher's students' passwords via a different group
id.

Carry the run id through the form and, before resetting any passwords,
verify the teacher owns the run and that the group is one of that run's
periods.
---
 .../BatchStudentChangePasswordParameters.java |  4 +++
 .../BatchStudentChangePasswordController.java | 30 +++++++++++++++++--
 2 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/src/main/java/org/wise/portal/domain/authentication/impl/BatchStudentChangePasswordParameters.java b/src/main/java/org/wise/portal/domain/authentication/impl/BatchStudentChangePasswordParameters.java
index 11742000a1..f6c99c06d0 100644
--- a/src/main/java/org/wise/portal/domain/authentication/impl/BatchStudentChangePasswordParameters.java
+++ b/src/main/java/org/wise/portal/domain/authentication/impl/BatchStudentChangePasswordParameters.java
@@ -36,4 +36,8 @@ public class BatchStudentChangePasswordParameters extends ChangePasswordParamete
   @Getter
   @Setter
   private Long groupId;
+
+  @Getter
+  @Setter
+  private Long runId;
 }
diff --git a/src/main/java/org/wise/portal/presentation/web/controllers/teacher/management/BatchStudentChangePasswordController.java b/src/main/java/org/wise/portal/presentation/web/controllers/teacher/management/BatchStudentChangePasswordController.java
index 82fa15bbba..603bae4d29 100644
--- a/src/main/java/org/wise/portal/presentation/web/controllers/teacher/management/BatchStudentChangePasswordController.java
+++ b/src/main/java/org/wise/portal/presentation/web/controllers/teacher/management/BatchStudentChangePasswordController.java
@@ -89,12 +89,14 @@ public class BatchStudentChangePasswordController {
   @RequestMapping(method = RequestMethod.GET)
   public String initializeForm(ModelMap model, HttpServletRequest request) throws Exception {
     User user = ControllerUtil.getSignedInUser();
-    Run run = runService.retrieveById(Long.parseLong(request.getParameter("runId")));
+    Long runId = Long.parseLong(request.getParameter("runId"));
+    Run run = runService.retrieveById(runId);
 
     if (user.isAdmin() ||
         aclService.hasPermission(run, BasePermission.ADMINISTRATION, user) ||
         aclService.hasPermission(run, BasePermission.WRITE, user)) {
       BatchStudentChangePasswordParameters params = new BatchStudentChangePasswordParameters();
+      params.setRunId(runId);
       params.setGroupId(Long.parseLong(request.getParameter(GROUPID_PARAM_NAME)));
       params.setTeacherUser(user);
       model.addAttribute("batchStudentChangePasswordParameters", params);
@@ -116,7 +118,7 @@ public String initializeForm(ModelMap model, HttpServletRequest request) throws
   protected String onSubmit(
       @ModelAttribute("batchStudentChangePasswordParameters") BatchStudentChangePasswordParameters batchStudentChangePasswordParameters,
       BindingResult bindingResult,
-      SessionStatus sessionStatus) {
+      SessionStatus sessionStatus) throws NotAuthorizedException {
     String view = "";
     try {
       changePasswordParametersValidator.validate(batchStudentChangePasswordParameters, bindingResult);
@@ -125,6 +127,7 @@ protected String onSubmit(
         view = formView;
       } else {
         Long groupId = batchStudentChangePasswordParameters.getGroupId();
+        assertCanChangeGroupPasswords(batchStudentChangePasswordParameters.getRunId(), groupId);
         Group group = groupService.retrieveById(groupId);
         Iterator<User> membersIter = group.getMembers().iterator();
         User member;
@@ -143,4 +146,27 @@ protected String onSubmit(
     }
     return view;
   }
+
+  /**
+   * Verify that the signed-in teacher may change the passwords of the students in the
+   * given group. The teacher must own the run and the group must be one of that run's
+   * periods, so a teacher cannot reset the passwords of another teacher's students by
+   * submitting a group id that belongs to a run they do not own.
+   */
+  private void assertCanChangeGroupPasswords(Long runId, Long groupId)
+      throws ObjectNotFoundException, NotAuthorizedException {
+    if (runId == null || groupId == null) {
+      throw new NotAuthorizedException("You are not authorized to change these passwords.");
+    }
+    User teacher = ControllerUtil.getSignedInUser();
+    Run run = runService.retrieveById(runId);
+    boolean canManageRun = teacher.isAdmin()
+        || aclService.hasPermission(run, BasePermission.ADMINISTRATION, teacher)
+        || aclService.hasPermission(run, BasePermission.WRITE, teacher);
+    boolean groupBelongsToRun = run.getPeriods().stream()
+        .anyMatch(period -> groupId.equals(period.getId()));
+    if (!canManageRun || !groupBelongsToRun) {
+      throw new NotAuthorizedException("You are not authorized to change these passwords.");
+    }
+  }
 }