From 8b6b6f8315b5ad6ba42c32f14fd2efc8d40cdeec Mon Sep 17 00:00:00 2001 From: Isaries Date: Mon, 6 Jul 2026 21:41:03 +0800 Subject: [PATCH] fix(security): require administrator role for user and account management The admin surface was gated by a single URL rule allowing both administrators and researchers, so a researcher could reach the user, role, portal and news management endpoints and grant themselves the administrator role, disable accounts, or read other users' details. Restrict those endpoints to administrators at the URL layer and add method-level enforcement on the account management controllers so the restriction does not depend on the URL configuration alone. --- .../BatchCreateUserAccountsController.java | 2 ++ .../admin/EnableDisableUserController.java | 2 ++ .../admin/LookupUserController.java | 2 ++ .../admin/ManageUserRolesController.java | 2 ++ .../admin/ShowAllUsersController.java | 2 ++ ...ProjectSharedPermissionsAPIController.java | 2 ++ .../portal/spring/impl/WebSecurityConfig.java | 25 +++++++++++++------ 7 files changed, 30 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/wise/portal/presentation/web/controllers/admin/BatchCreateUserAccountsController.java b/src/main/java/org/wise/portal/presentation/web/controllers/admin/BatchCreateUserAccountsController.java index 1426aa3307..7fb9918775 100644 --- a/src/main/java/org/wise/portal/presentation/web/controllers/admin/BatchCreateUserAccountsController.java +++ b/src/main/java/org/wise/portal/presentation/web/controllers/admin/BatchCreateUserAccountsController.java @@ -30,6 +30,7 @@ import java.util.Calendar; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.security.access.annotation.Secured; import org.springframework.stereotype.Controller; import org.springframework.ui.ModelMap; import org.springframework.validation.BindingResult; @@ -55,6 +56,7 @@ * * @author Hiroki Terashima */ +@Secured("ROLE_ADMINISTRATOR") @Controller @RequestMapping("/admin/account/batchcreateuseraccounts.html") public class BatchCreateUserAccountsController { diff --git a/src/main/java/org/wise/portal/presentation/web/controllers/admin/EnableDisableUserController.java b/src/main/java/org/wise/portal/presentation/web/controllers/admin/EnableDisableUserController.java index e40c3c53e1..683e52ba28 100644 --- a/src/main/java/org/wise/portal/presentation/web/controllers/admin/EnableDisableUserController.java +++ b/src/main/java/org/wise/portal/presentation/web/controllers/admin/EnableDisableUserController.java @@ -29,6 +29,7 @@ import javax.servlet.http.HttpServletResponse; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.security.access.annotation.Secured; import org.springframework.stereotype.Controller; import org.springframework.ui.ModelMap; import org.springframework.web.bind.annotation.RequestMapping; @@ -41,6 +42,7 @@ * Only accessed by a WISE admin user. * @author Hiroki Terashima */ +@Secured("ROLE_ADMINISTRATOR") @Controller @RequestMapping("/admin/account/enabledisableuser") public class EnableDisableUserController { diff --git a/src/main/java/org/wise/portal/presentation/web/controllers/admin/LookupUserController.java b/src/main/java/org/wise/portal/presentation/web/controllers/admin/LookupUserController.java index 0f9b93adc5..009b7bc36b 100644 --- a/src/main/java/org/wise/portal/presentation/web/controllers/admin/LookupUserController.java +++ b/src/main/java/org/wise/portal/presentation/web/controllers/admin/LookupUserController.java @@ -29,6 +29,7 @@ import javax.servlet.http.HttpServletRequest; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.security.access.annotation.Secured; import org.springframework.stereotype.Controller; import org.springframework.ui.ModelMap; import org.springframework.validation.BindingResult; @@ -50,6 +51,7 @@ * @author Patrick Lawler * @author Hiroki Terashima */ +@Secured("ROLE_ADMINISTRATOR") @Controller @RequestMapping("/admin/account/lookupuser") public class LookupUserController { diff --git a/src/main/java/org/wise/portal/presentation/web/controllers/admin/ManageUserRolesController.java b/src/main/java/org/wise/portal/presentation/web/controllers/admin/ManageUserRolesController.java index 5462caab08..a669be6277 100644 --- a/src/main/java/org/wise/portal/presentation/web/controllers/admin/ManageUserRolesController.java +++ b/src/main/java/org/wise/portal/presentation/web/controllers/admin/ManageUserRolesController.java @@ -29,6 +29,7 @@ import javax.servlet.http.HttpServletResponse; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.security.access.annotation.Secured; import org.springframework.security.core.GrantedAuthority; import org.springframework.stereotype.Controller; import org.springframework.ui.ModelMap; @@ -44,6 +45,7 @@ * * @author Hiroki Terashima */ +@Secured("ROLE_ADMINISTRATOR") @Controller @RequestMapping("/admin/account/manageuserroles.html") public class ManageUserRolesController { diff --git a/src/main/java/org/wise/portal/presentation/web/controllers/admin/ShowAllUsersController.java b/src/main/java/org/wise/portal/presentation/web/controllers/admin/ShowAllUsersController.java index ea7aae0b5e..38cb480bf8 100644 --- a/src/main/java/org/wise/portal/presentation/web/controllers/admin/ShowAllUsersController.java +++ b/src/main/java/org/wise/portal/presentation/web/controllers/admin/ShowAllUsersController.java @@ -1,6 +1,7 @@ package org.wise.portal.presentation.web.controllers.admin; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.security.access.annotation.Secured; import org.springframework.stereotype.Controller; import org.springframework.ui.ModelMap; import org.springframework.web.bind.annotation.GetMapping; @@ -8,6 +9,7 @@ import org.springframework.web.bind.annotation.RequestParam; import org.wise.portal.service.authentication.UserDetailsService; +@Secured("ROLE_ADMINISTRATOR") @Controller @RequestMapping("/admin/account/show-all-users") public class ShowAllUsersController { diff --git a/src/main/java/org/wise/portal/presentation/web/controllers/admin/UpdateProjectSharedPermissionsAPIController.java b/src/main/java/org/wise/portal/presentation/web/controllers/admin/UpdateProjectSharedPermissionsAPIController.java index 74a352317b..cbff00917d 100644 --- a/src/main/java/org/wise/portal/presentation/web/controllers/admin/UpdateProjectSharedPermissionsAPIController.java +++ b/src/main/java/org/wise/portal/presentation/web/controllers/admin/UpdateProjectSharedPermissionsAPIController.java @@ -1,6 +1,7 @@ package org.wise.portal.presentation.web.controllers.admin; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.security.access.annotation.Secured; import org.springframework.security.acls.model.Permission; import org.springframework.web.bind.annotation.*; import org.wise.portal.dao.ObjectNotFoundException; @@ -13,6 +14,7 @@ import java.util.List; import java.util.Set; +@Secured("ROLE_ADMINISTRATOR") @RestController @RequestMapping("/api/admin/project/shared") public class UpdateProjectSharedPermissionsAPIController { diff --git a/src/main/java/org/wise/portal/spring/impl/WebSecurityConfig.java b/src/main/java/org/wise/portal/spring/impl/WebSecurityConfig.java index b608d1272e..0107e87e38 100644 --- a/src/main/java/org/wise/portal/spring/impl/WebSecurityConfig.java +++ b/src/main/java/org/wise/portal/spring/impl/WebSecurityConfig.java @@ -85,14 +85,25 @@ protected void configure(HttpSecurity http) throws Exception { .addFilterAfter(googleOpenIdConnectFilter(), OAuth2ClientContextFilter.class) .addFilterAfter(microsoftOpenIdConnectFilter(), OAuth2ClientContextFilter.class) .addFilterAfter(authenticationProcessingFilter(), GoogleOpenIdConnectFilter.class) - .authorizeRequests().antMatchers("/api/login/impersonate") - .hasAnyRole("ADMINISTRATOR", "RESEARCHER").antMatchers("/admin/**") - .hasAnyRole("ADMINISTRATOR", "RESEARCHER").antMatchers("/author/**").hasAnyRole("TEACHER") + .authorizeRequests() + // User, role, portal, news and destructive maintenance endpoints must be + // restricted to administrators. They were previously reachable by researchers + // through the broad "/admin/**" rule, which let a researcher grant themselves + // the administrator role or manage other users' accounts. These more specific + // rules are listed first so they take precedence over the broader ones below. + .antMatchers("/admin/account/**", "/admin/portal/**", "/admin/news/**", + "/admin/mergeProjectMetadata", "/admin/run/replacebase64withpng.html", "/api/admin/**") + .hasRole("ADMINISTRATOR") + .antMatchers("/api/login/impersonate").hasAnyRole("ADMINISTRATOR", "RESEARCHER") + .antMatchers("/admin/**").hasAnyRole("ADMINISTRATOR", "RESEARCHER") + .antMatchers("/author/**").hasAnyRole("TEACHER") .antMatchers("/project/notifyAuthor*/**").hasAnyRole("TEACHER") - .antMatchers("/student/account/info").hasAnyRole("TEACHER").antMatchers("/student/**") - .hasAnyRole("STUDENT").antMatchers("/studentStatus").hasAnyRole("TEACHER", "STUDENT") - .antMatchers("/teacher/**").hasAnyRole("TEACHER").antMatchers("/sso/discourse") - .hasAnyRole("TEACHER", "STUDENT").antMatchers("/").permitAll(); + .antMatchers("/student/account/info").hasAnyRole("TEACHER") + .antMatchers("/student/**").hasAnyRole("STUDENT") + .antMatchers("/studentStatus").hasAnyRole("TEACHER", "STUDENT") + .antMatchers("/teacher/**").hasAnyRole("TEACHER") + .antMatchers("/sso/discourse").hasAnyRole("TEACHER", "STUDENT") + .antMatchers("/").permitAll(); http.formLogin().loginPage("/login").permitAll(); http.logout().addLogoutHandler(wiseLogoutHandler()) .logoutRequestMatcher(new AntPathRequestMatcher("/api/logout"));