Skip to content

Commit 59d194a

Browse files
authored
fix(security): require administrator role for user and account management (#322)
* 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. * fix(security): hide admin-only links in the admin index page The user management links in the admin index page pointed at endpoints that now require the administrator role, so a researcher would follow them into a 403. Wrap the whole user management section in <sec:authorize> instead, which also drops the nested check around the logged-in user lists. Require the administrator role for /admin/project/updatesharedprojects as well. The page only drives the /api/admin/project/shared endpoints, which administrators alone may call. Drop the WebSecurityConfig comments; the rules speak for themselves. * fix(security): close trailing slash bypass on exact admin paths The admin-only rules for /admin/mergeProjectMetadata and /admin/run/replacebase64withpng.html were exact ant patterns. Spring MVC matches a trailing slash and routes /admin/mergeProjectMetadata/ to the controller, but the ant matcher does not, so the request fell through to the broader /admin/** rule and a researcher reached both endpoints. Match these paths the way Spring MVC matches them, and secure the two controllers directly so the rules are not the only guard.
1 parent 79d817e commit 59d194a

11 files changed

Lines changed: 65 additions & 38 deletions

src/main/java/org/wise/portal/presentation/web/controllers/admin/AdminUtilsController.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
package org.wise.portal.presentation.web.controllers.admin;
2525

2626
import org.springframework.beans.factory.annotation.Autowired;
27+
import org.springframework.security.access.annotation.Secured;
2728
import org.springframework.stereotype.Controller;
2829
import org.springframework.web.bind.annotation.RequestMapping;
2930
import org.wise.portal.domain.project.Project;
@@ -42,6 +43,7 @@
4243
* Admin utility functions like db migrations and batch scripts
4344
* @author Hiroki Terashima
4445
*/
46+
@Secured("ROLE_ADMINISTRATOR")
4547
@Controller
4648
public class AdminUtilsController {
4749

src/main/java/org/wise/portal/presentation/web/controllers/admin/BatchCreateUserAccountsController.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import java.util.Calendar;
3131

3232
import org.springframework.beans.factory.annotation.Autowired;
33+
import org.springframework.security.access.annotation.Secured;
3334
import org.springframework.stereotype.Controller;
3435
import org.springframework.ui.ModelMap;
3536
import org.springframework.validation.BindingResult;
@@ -55,6 +56,7 @@
5556
*
5657
* @author Hiroki Terashima
5758
*/
59+
@Secured("ROLE_ADMINISTRATOR")
5860
@Controller
5961
@RequestMapping("/admin/account/batchcreateuseraccounts.html")
6062
public class BatchCreateUserAccountsController {

src/main/java/org/wise/portal/presentation/web/controllers/admin/EnableDisableUserController.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import javax.servlet.http.HttpServletResponse;
3030

3131
import org.springframework.beans.factory.annotation.Autowired;
32+
import org.springframework.security.access.annotation.Secured;
3233
import org.springframework.stereotype.Controller;
3334
import org.springframework.ui.ModelMap;
3435
import org.springframework.web.bind.annotation.RequestMapping;
@@ -41,6 +42,7 @@
4142
* Only accessed by a WISE admin user.
4243
* @author Hiroki Terashima
4344
*/
45+
@Secured("ROLE_ADMINISTRATOR")
4446
@Controller
4547
@RequestMapping("/admin/account/enabledisableuser")
4648
public class EnableDisableUserController {

src/main/java/org/wise/portal/presentation/web/controllers/admin/LookupUserController.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import javax.servlet.http.HttpServletRequest;
3030

3131
import org.springframework.beans.factory.annotation.Autowired;
32+
import org.springframework.security.access.annotation.Secured;
3233
import org.springframework.stereotype.Controller;
3334
import org.springframework.ui.ModelMap;
3435
import org.springframework.validation.BindingResult;
@@ -50,6 +51,7 @@
5051
* @author Patrick Lawler
5152
* @author Hiroki Terashima
5253
*/
54+
@Secured("ROLE_ADMINISTRATOR")
5355
@Controller
5456
@RequestMapping("/admin/account/lookupuser")
5557
public class LookupUserController {

src/main/java/org/wise/portal/presentation/web/controllers/admin/ManageUserRolesController.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import javax.servlet.http.HttpServletResponse;
3030

3131
import org.springframework.beans.factory.annotation.Autowired;
32+
import org.springframework.security.access.annotation.Secured;
3233
import org.springframework.security.core.GrantedAuthority;
3334
import org.springframework.stereotype.Controller;
3435
import org.springframework.ui.ModelMap;
@@ -44,6 +45,7 @@
4445
*
4546
* @author Hiroki Terashima
4647
*/
48+
@Secured("ROLE_ADMINISTRATOR")
4749
@Controller
4850
@RequestMapping("/admin/account/manageuserroles.html")
4951
public class ManageUserRolesController {

src/main/java/org/wise/portal/presentation/web/controllers/admin/ShowAllUsersController.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
11
package org.wise.portal.presentation.web.controllers.admin;
22

33
import org.springframework.beans.factory.annotation.Autowired;
4+
import org.springframework.security.access.annotation.Secured;
45
import org.springframework.stereotype.Controller;
56
import org.springframework.ui.ModelMap;
67
import org.springframework.web.bind.annotation.GetMapping;
78
import org.springframework.web.bind.annotation.RequestMapping;
89
import org.springframework.web.bind.annotation.RequestParam;
910
import org.wise.portal.service.authentication.UserDetailsService;
1011

12+
@Secured("ROLE_ADMINISTRATOR")
1113
@Controller
1214
@RequestMapping("/admin/account/show-all-users")
1315
public class ShowAllUsersController {

src/main/java/org/wise/portal/presentation/web/controllers/admin/UpdateProjectSharedPermissionsAPIController.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package org.wise.portal.presentation.web.controllers.admin;
22

33
import org.springframework.beans.factory.annotation.Autowired;
4+
import org.springframework.security.access.annotation.Secured;
45
import org.springframework.security.acls.model.Permission;
56
import org.springframework.web.bind.annotation.*;
67
import org.wise.portal.dao.ObjectNotFoundException;
@@ -13,6 +14,7 @@
1314
import java.util.List;
1415
import java.util.Set;
1516

17+
@Secured("ROLE_ADMINISTRATOR")
1618
@RestController
1719
@RequestMapping("/api/admin/project/shared")
1820
public class UpdateProjectSharedPermissionsAPIController {

src/main/java/org/wise/portal/presentation/web/controllers/admin/UpdateSharedProjectsController.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
package org.wise.portal.presentation.web.controllers.admin;
22

3+
import org.springframework.security.access.annotation.Secured;
34
import org.springframework.stereotype.Controller;
45
import org.springframework.ui.ModelMap;
56
import org.springframework.web.bind.annotation.RequestMapping;
67
import org.springframework.web.bind.annotation.RequestMethod;
78
import org.springframework.web.servlet.ModelAndView;
89

10+
@Secured("ROLE_ADMINISTRATOR")
911
@Controller
1012
@RequestMapping("/admin/project/updatesharedprojects")
1113
public class UpdateSharedProjectsController {

src/main/java/org/wise/portal/presentation/web/controllers/run/ReplaceBase64WithPNGController.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import org.json.JSONObject;
44
import org.springframework.beans.factory.annotation.Autowired;
55
import org.springframework.core.env.Environment;
6+
import org.springframework.security.access.annotation.Secured;
67
import org.springframework.stereotype.Controller;
78
import org.springframework.ui.ModelMap;
89
import org.springframework.web.bind.annotation.GetMapping;
@@ -32,6 +33,7 @@
3233
* student data, it will replace the Base64 string with a reference to that png image in the
3334
* studentuploads folder.
3435
*/
36+
@Secured("ROLE_ADMINISTRATOR")
3537
@Controller
3638
@RequestMapping("/admin/run/replacebase64withpng.html")
3739
public class ReplaceBase64WithPNGController {

src/main/java/org/wise/portal/spring/impl/WebSecurityConfig.java

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -85,14 +85,23 @@ protected void configure(HttpSecurity http) throws Exception {
8585
.addFilterAfter(googleOpenIdConnectFilter(), OAuth2ClientContextFilter.class)
8686
.addFilterAfter(microsoftOpenIdConnectFilter(), OAuth2ClientContextFilter.class)
8787
.addFilterAfter(authenticationProcessingFilter(), GoogleOpenIdConnectFilter.class)
88-
.authorizeRequests().antMatchers("/api/login/impersonate")
89-
.hasAnyRole("ADMINISTRATOR", "RESEARCHER").antMatchers("/admin/**")
90-
.hasAnyRole("ADMINISTRATOR", "RESEARCHER").antMatchers("/author/**").hasAnyRole("TEACHER")
88+
.authorizeRequests()
89+
// Matched the way Spring MVC matches, so a trailing slash cannot slip an exact path
90+
// past these rules and into the broader "/admin/**" rule below.
91+
.mvcMatchers("/admin/account/**", "/admin/portal/**", "/admin/news/**",
92+
"/admin/mergeProjectMetadata", "/admin/project/updatesharedprojects",
93+
"/admin/run/replacebase64withpng.html", "/api/admin/**")
94+
.hasRole("ADMINISTRATOR")
95+
.antMatchers("/api/login/impersonate").hasAnyRole("ADMINISTRATOR", "RESEARCHER")
96+
.antMatchers("/admin/**").hasAnyRole("ADMINISTRATOR", "RESEARCHER")
97+
.antMatchers("/author/**").hasAnyRole("TEACHER")
9198
.antMatchers("/project/notifyAuthor*/**").hasAnyRole("TEACHER")
92-
.antMatchers("/student/account/info").hasAnyRole("TEACHER").antMatchers("/student/**")
93-
.hasAnyRole("STUDENT").antMatchers("/studentStatus").hasAnyRole("TEACHER", "STUDENT")
94-
.antMatchers("/teacher/**").hasAnyRole("TEACHER").antMatchers("/sso/discourse")
95-
.hasAnyRole("TEACHER", "STUDENT").antMatchers("/").permitAll();
99+
.antMatchers("/student/account/info").hasAnyRole("TEACHER")
100+
.antMatchers("/student/**").hasAnyRole("STUDENT")
101+
.antMatchers("/studentStatus").hasAnyRole("TEACHER", "STUDENT")
102+
.antMatchers("/teacher/**").hasAnyRole("TEACHER")
103+
.antMatchers("/sso/discourse").hasAnyRole("TEACHER", "STUDENT")
104+
.antMatchers("/").permitAll();
96105
http.formLogin().loginPage("/login").permitAll();
97106
http.logout().addLogoutHandler(wiseLogoutHandler())
98107
.logoutRequestMatcher(new AntPathRequestMatcher("/api/logout"));

0 commit comments

Comments
 (0)