Skip to content

Commit 230dc85

Browse files
evgeniychebanevgeniycheban
committed
Exceptions for Authorized Objects should propagate when returned from a Controller
Closes gh-16058 Signed-off-by: Evgeniy Cheban <[email protected]>
1 parent ff8b77d commit 230dc85

File tree

2 files changed

+275
-1
lines changed

2 files changed

+275
-1
lines changed

‎config/src/main/java/org/springframework/security/config/annotation/method/configuration/AuthorizationProxyWebConfiguration.java

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,27 +16,49 @@
1616

1717
package org.springframework.security.config.annotation.method.configuration;
1818

19+
import java.util.List;
1920
import java.util.Map;
2021

22+
import jakarta.servlet.http.HttpServletRequest;
23+
import jakarta.servlet.http.HttpServletResponse;
24+
2125
import org.springframework.beans.factory.config.BeanDefinition;
2226
import org.springframework.context.annotation.Bean;
2327
import org.springframework.context.annotation.Configuration;
2428
import org.springframework.context.annotation.Role;
2529
import org.springframework.http.HttpEntity;
2630
import org.springframework.http.ResponseEntity;
31+
import org.springframework.http.converter.HttpMessageNotWritableException;
32+
import org.springframework.security.access.AccessDeniedException;
2733
import org.springframework.security.authorization.method.AuthorizationAdvisorProxyFactory;
34+
import org.springframework.security.web.util.ThrowableAnalyzer;
35+
import org.springframework.web.servlet.HandlerExceptionResolver;
2836
import org.springframework.web.servlet.ModelAndView;
2937
import org.springframework.web.servlet.View;
38+
import org.springframework.web.servlet.config.annotation.WebMvcConfigurer;
39+
import org.springframework.web.servlet.mvc.support.DefaultHandlerExceptionResolver;
3040

3141
@Configuration
32-
class AuthorizationProxyWebConfiguration {
42+
class AuthorizationProxyWebConfiguration implements WebMvcConfigurer {
3343

3444
@Bean
3545
@Role(BeanDefinition.ROLE_INFRASTRUCTURE)
3646
AuthorizationAdvisorProxyFactory.TargetVisitor webTargetVisitor() {
3747
return new WebTargetVisitor();
3848
}
3949

50+
@Override
51+
public void extendHandlerExceptionResolvers(List<HandlerExceptionResolver> resolvers) {
52+
int index;
53+
for (index = 0; index < resolvers.size(); index++) {
54+
HandlerExceptionResolver resolver = resolvers.get(index);
55+
if (resolver instanceof DefaultHandlerExceptionResolver) {
56+
break;
57+
}
58+
}
59+
resolvers.add(index, new HttpMessageNotWritableAccessDeniedExceptionResolver());
60+
}
61+
4062
static class WebTargetVisitor implements AuthorizationAdvisorProxyFactory.TargetVisitor {
4163

4264
@Override
@@ -62,4 +84,28 @@ public Object visit(AuthorizationAdvisorProxyFactory proxyFactory, Object target
6284

6385
}
6486

87+
static class HttpMessageNotWritableAccessDeniedExceptionResolver implements HandlerExceptionResolver {
88+
89+
final ThrowableAnalyzer throwableAnalyzer = new ThrowableAnalyzer();
90+
91+
@Override
92+
public ModelAndView resolveException(HttpServletRequest request, HttpServletResponse response, Object handler,
93+
Exception ex) {
94+
// Only resolves AccessDeniedException if it occurred during serialization,
95+
// otherwise lets the user-defined handler deal with it.
96+
if (ex instanceof HttpMessageNotWritableException) {
97+
Throwable[] causeChain = this.throwableAnalyzer.determineCauseChain(ex);
98+
Throwable accessDeniedException = this.throwableAnalyzer
99+
.getFirstThrowableOfType(AccessDeniedException.class, causeChain);
100+
if (accessDeniedException != null) {
101+
return new ModelAndView((model, req, res) -> {
102+
throw ex;
103+
});
104+
}
105+
}
106+
return null;
107+
}
108+
109+
}
110+
65111
}

‎config/src/test/java/org/springframework/security/config/annotation/method/configuration/PrePostMethodSecurityConfigurationTests.java

Lines changed: 228 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import io.micrometer.observation.ObservationRegistry;
3434
import io.micrometer.observation.ObservationTextPublisher;
3535
import jakarta.annotation.security.DenyAll;
36+
import org.aopalliance.aop.Advice;
3637
import org.aopalliance.intercept.MethodInterceptor;
3738
import org.aopalliance.intercept.MethodInvocation;
3839
import org.junit.jupiter.api.Test;
@@ -42,6 +43,7 @@
4243
import org.mockito.Mockito;
4344

4445
import org.springframework.aop.Advisor;
46+
import org.springframework.aop.Pointcut;
4547
import org.springframework.aop.config.AopConfigUtils;
4648
import org.springframework.aop.support.DefaultPointcutAdvisor;
4749
import org.springframework.aop.support.JdkRegexpMethodPointcut;
@@ -62,8 +64,11 @@
6264
import org.springframework.core.annotation.AnnotationAwareOrderComparator;
6365
import org.springframework.core.annotation.AnnotationConfigurationException;
6466
import org.springframework.core.annotation.Order;
67+
import org.springframework.http.HttpStatus;
6568
import org.springframework.http.HttpStatusCode;
69+
import org.springframework.http.MediaType;
6670
import org.springframework.http.ResponseEntity;
71+
import org.springframework.http.converter.HttpMessageNotWritableException;
6772
import org.springframework.security.access.AccessDeniedException;
6873
import org.springframework.security.access.PermissionEvaluator;
6974
import org.springframework.security.access.annotation.BusinessService;
@@ -95,6 +100,7 @@
95100
import org.springframework.security.authorization.method.MethodInvocationResult;
96101
import org.springframework.security.authorization.method.PrePostTemplateDefaults;
97102
import org.springframework.security.config.annotation.SecurityContextChangedListenerConfig;
103+
import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
98104
import org.springframework.security.config.core.GrantedAuthorityDefaults;
99105
import org.springframework.security.config.observation.SecurityObservationSettings;
100106
import org.springframework.security.config.test.SpringTestContext;
@@ -106,13 +112,24 @@
106112
import org.springframework.security.test.context.support.WithAnonymousUser;
107113
import org.springframework.security.test.context.support.WithMockUser;
108114
import org.springframework.security.test.context.support.WithSecurityContextTestExecutionListener;
115+
import org.springframework.security.web.util.ThrowableAnalyzer;
109116
import org.springframework.stereotype.Component;
117+
import org.springframework.stereotype.Service;
110118
import org.springframework.test.context.ContextConfiguration;
111119
import org.springframework.test.context.TestExecutionListeners;
112120
import org.springframework.test.context.junit.jupiter.SpringExtension;
121+
import org.springframework.test.web.servlet.MockMvc;
122+
import org.springframework.test.web.servlet.MvcResult;
123+
import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder;
124+
import org.springframework.web.bind.annotation.ControllerAdvice;
125+
import org.springframework.web.bind.annotation.ExceptionHandler;
126+
import org.springframework.web.bind.annotation.GetMapping;
127+
import org.springframework.web.bind.annotation.RequestParam;
128+
import org.springframework.web.bind.annotation.RestController;
113129
import org.springframework.web.context.ConfigurableWebApplicationContext;
114130
import org.springframework.web.context.support.AnnotationConfigWebApplicationContext;
115131
import org.springframework.web.servlet.ModelAndView;
132+
import org.springframework.web.servlet.config.annotation.EnableWebMvc;
116133

117134
import static org.assertj.core.api.Assertions.assertThat;
118135
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
@@ -127,6 +144,9 @@
127144
import static org.mockito.Mockito.times;
128145
import static org.mockito.Mockito.verify;
129146
import static org.mockito.Mockito.verifyNoInteractions;
147+
import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.user;
148+
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
149+
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
130150

131151
/**
132152
* Tests for {@link PrePostMethodSecurityConfiguration}.
@@ -148,6 +168,9 @@ public class PrePostMethodSecurityConfigurationTests {
148168
@Autowired(required = false)
149169
BusinessService businessService;
150170

171+
@Autowired(required = false)
172+
MockMvc mvc;
173+
151174
@WithMockUser
152175
@Test
153176
public void customMethodSecurityPreAuthorizeAdminWhenRoleUserThenAccessDeniedException() {
@@ -1181,6 +1204,97 @@ void autowireWhenDefaultsThenAdvisorAnnotationsAreSorted() {
11811204
}
11821205
}
11831206

1207+
@Test
1208+
void getWhenPostAuthorizeAuthenticationNameMatchesThenRespondsWithOk() throws Exception {
1209+
this.spring.register(WebMvcMethodSecurityConfig.class, BasicController.class).autowire();
1210+
// @formatter:off
1211+
MockHttpServletRequestBuilder requestWithUser = get("/authorized-person")
1212+
.param("name", "rob")
1213+
.with(user("rob"));
1214+
// @formatter:on
1215+
this.mvc.perform(requestWithUser).andExpect(status().isOk());
1216+
}
1217+
1218+
@Test
1219+
void getWhenPostAuthorizeAuthenticationNameNotMatchThenRespondsWithForbidden() throws Exception {
1220+
this.spring.register(WebMvcMethodSecurityConfig.class, BasicController.class).autowire();
1221+
// @formatter:off
1222+
MockHttpServletRequestBuilder requestWithUser = get("/authorized-person")
1223+
.param("name", "john")
1224+
.with(user("rob"));
1225+
// @formatter:on
1226+
this.mvc.perform(requestWithUser).andExpect(status().isForbidden());
1227+
}
1228+
1229+
@Test
1230+
void getWhenPostAuthorizeWithinServiceAuthenticationNameMatchesThenRespondsWithOk() throws Exception {
1231+
this.spring.register(WebMvcMethodSecurityConfig.class, BasicController.class, BasicService.class).autowire();
1232+
// @formatter:off
1233+
MockHttpServletRequestBuilder requestWithUser = get("/greetings/authorized-person")
1234+
.param("name", "rob")
1235+
.with(user("rob"));
1236+
// @formatter:on
1237+
MvcResult mvcResult = this.mvc.perform(requestWithUser).andExpect(status().isOk()).andReturn();
1238+
assertThat(mvcResult.getResponse().getContentAsString()).isEqualTo("Hello: rob");
1239+
}
1240+
1241+
@Test
1242+
void getWhenPostAuthorizeWithinServiceAuthenticationNameNotMatchThenCustomHandlerRespondsWithForbidden()
1243+
throws Exception {
1244+
this.spring
1245+
.register(WebMvcMethodSecurityConfig.class, BasicController.class, BasicService.class,
1246+
BasicControllerAdvice.class)
1247+
.autowire();
1248+
// @formatter:off
1249+
MockHttpServletRequestBuilder requestWithUser = get("/greetings/authorized-person")
1250+
.param("name", "john")
1251+
.with(user("rob"));
1252+
// @formatter:on
1253+
MvcResult mvcResult = this.mvc.perform(requestWithUser).andExpect(status().isForbidden()).andReturn();
1254+
assertThat(mvcResult.getResponse().getContentAsString()).isEqualTo("""
1255+
{"message":"Access Denied"}\
1256+
""");
1257+
}
1258+
1259+
@Test
1260+
void getWhenPostAuthorizeAuthenticationNameNotMatchThenCustomHandlerRespondsWithForbidden() throws Exception {
1261+
this.spring
1262+
.register(WebMvcMethodSecurityConfig.class, BasicController.class, BasicService.class,
1263+
BasicControllerAdvice.class)
1264+
.autowire();
1265+
// @formatter:off
1266+
MockHttpServletRequestBuilder requestWithUser = get("/authorized-person")
1267+
.param("name", "john")
1268+
.with(user("rob"));
1269+
// @formatter:on
1270+
MvcResult mvcResult = this.mvc.perform(requestWithUser).andExpect(status().isForbidden()).andReturn();
1271+
assertThat(mvcResult.getResponse().getContentAsString()).isEqualTo("""
1272+
{"message":"Could not write JSON: Access Denied"}\
1273+
""");
1274+
}
1275+
1276+
@Test
1277+
void getWhenCustomAdvisorAuthenticationNameMatchesThenRespondsWithOk() throws Exception {
1278+
this.spring.register(WebMvcMethodSecurityCustomAdvisorConfig.class, BasicController.class).autowire();
1279+
// @formatter:off
1280+
MockHttpServletRequestBuilder requestWithUser = get("/authorized-person")
1281+
.param("name", "rob")
1282+
.with(user("rob"));
1283+
// @formatter:on
1284+
this.mvc.perform(requestWithUser).andExpect(status().isOk());
1285+
}
1286+
1287+
@Test
1288+
void getWhenCustomAdvisorAuthenticationNameNotMatchThenRespondsWithForbidden() throws Exception {
1289+
this.spring.register(WebMvcMethodSecurityCustomAdvisorConfig.class, BasicController.class).autowire();
1290+
// @formatter:off
1291+
MockHttpServletRequestBuilder requestWithUser = get("/authorized-person")
1292+
.param("name", "john")
1293+
.with(user("rob"));
1294+
// @formatter:on
1295+
this.mvc.perform(requestWithUser).andExpect(status().isForbidden());
1296+
}
1297+
11841298
private static Consumer<ConfigurableWebApplicationContext> disallowBeanOverriding() {
11851299
return (context) -> ((AnnotationConfigWebApplicationContext) context).setAllowBeanDefinitionOverriding(false);
11861300
}
@@ -1919,4 +2033,118 @@ void onRequestDenied(AuthorizationDeniedEvent<? extends MethodInvocation> denied
19192033

19202034
}
19212035

2036+
@EnableWebMvc
2037+
@EnableWebSecurity
2038+
@EnableMethodSecurity
2039+
static class WebMvcMethodSecurityConfig {
2040+
2041+
}
2042+
2043+
@EnableWebMvc
2044+
@EnableWebSecurity
2045+
@EnableMethodSecurity
2046+
static class WebMvcMethodSecurityCustomAdvisorConfig {
2047+
2048+
@Bean
2049+
AuthorizationAdvisor customAdvisor(SecurityContextHolderStrategy strategy) {
2050+
JdkRegexpMethodPointcut pointcut = new JdkRegexpMethodPointcut();
2051+
pointcut.setPattern(".*AuthorizedPerson.*getName");
2052+
return new AuthorizationAdvisor() {
2053+
@Override
2054+
public Object invoke(MethodInvocation mi) throws Throwable {
2055+
Authentication auth = strategy.getContext().getAuthentication();
2056+
Object result = mi.proceed();
2057+
if (auth.getName().equals(result)) {
2058+
return result;
2059+
}
2060+
throw new AccessDeniedException("Access Denied for User '" + auth.getName() + "'");
2061+
}
2062+
2063+
@Override
2064+
public Pointcut getPointcut() {
2065+
return pointcut;
2066+
}
2067+
2068+
@Override
2069+
public Advice getAdvice() {
2070+
return this;
2071+
}
2072+
2073+
@Override
2074+
public int getOrder() {
2075+
return AuthorizationInterceptorsOrder.POST_FILTER.getOrder() + 1;
2076+
}
2077+
};
2078+
}
2079+
2080+
}
2081+
2082+
@RestController
2083+
static class BasicController {
2084+
2085+
@Autowired(required = false)
2086+
BasicService service;
2087+
2088+
@GetMapping("/greetings/authorized-person")
2089+
String getAuthorizedPersonGreeting(@RequestParam String name) {
2090+
AuthorizedPerson authorizedPerson = this.service.getAuthorizedPerson(name);
2091+
return "Hello: " + authorizedPerson.getName();
2092+
}
2093+
2094+
@AuthorizeReturnObject
2095+
@GetMapping(value = "/authorized-person", produces = MediaType.APPLICATION_JSON_VALUE)
2096+
AuthorizedPerson getAuthorizedPerson(@RequestParam String name) {
2097+
return new AuthorizedPerson(name);
2098+
}
2099+
2100+
}
2101+
2102+
@ControllerAdvice
2103+
static class BasicControllerAdvice {
2104+
2105+
@ExceptionHandler(AccessDeniedException.class)
2106+
ResponseEntity<Map<String, String>> handleAccessDenied(AccessDeniedException ex) {
2107+
Map<String, String> responseBody = Map.of("message", ex.getMessage());
2108+
return ResponseEntity.status(HttpStatus.FORBIDDEN).body(responseBody);
2109+
}
2110+
2111+
@ExceptionHandler(HttpMessageNotWritableException.class)
2112+
ResponseEntity<Map<String, String>> handleHttpMessageNotWritable(HttpMessageNotWritableException ex) {
2113+
ThrowableAnalyzer throwableAnalyzer = new ThrowableAnalyzer();
2114+
Throwable[] causeChain = throwableAnalyzer.determineCauseChain(ex);
2115+
Throwable t = throwableAnalyzer.getFirstThrowableOfType(AccessDeniedException.class, causeChain);
2116+
if (t != null) {
2117+
Map<String, String> responseBody = Map.of("message", ex.getMessage());
2118+
return ResponseEntity.status(HttpStatus.FORBIDDEN).body(responseBody);
2119+
}
2120+
throw ex;
2121+
}
2122+
2123+
}
2124+
2125+
@Service
2126+
static class BasicService {
2127+
2128+
@AuthorizeReturnObject
2129+
AuthorizedPerson getAuthorizedPerson(String name) {
2130+
return new AuthorizedPerson(name);
2131+
}
2132+
2133+
}
2134+
2135+
public static class AuthorizedPerson {
2136+
2137+
final String name;
2138+
2139+
AuthorizedPerson(String name) {
2140+
this.name = name;
2141+
}
2142+
2143+
@PostAuthorize("returnObject == authentication.name")
2144+
public String getName() {
2145+
return this.name;
2146+
}
2147+
2148+
}
2149+
19222150
}

0 commit comments

Comments
 (0)