Skip to content

Commit fb85470

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 fb85470

File tree

2 files changed

+236
-1
lines changed

2 files changed

+236
-1
lines changed

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

+39-1
Original file line numberDiff line numberDiff line change
@@ -16,27 +16,41 @@
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;
3039

3140
@Configuration
32-
class AuthorizationProxyWebConfiguration {
41+
class AuthorizationProxyWebConfiguration implements WebMvcConfigurer {
3342

3443
@Bean
3544
@Role(BeanDefinition.ROLE_INFRASTRUCTURE)
3645
AuthorizationAdvisorProxyFactory.TargetVisitor webTargetVisitor() {
3746
return new WebTargetVisitor();
3847
}
3948

49+
@Override
50+
public void extendHandlerExceptionResolvers(List<HandlerExceptionResolver> resolvers) {
51+
resolvers.add(0, new HttpMessageNotWritableAccessDeniedExceptionResolver());
52+
}
53+
4054
static class WebTargetVisitor implements AuthorizationAdvisorProxyFactory.TargetVisitor {
4155

4256
@Override
@@ -62,4 +76,28 @@ public Object visit(AuthorizationAdvisorProxyFactory proxyFactory, Object target
6276

6377
}
6478

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

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

+197
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,7 +64,9 @@
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;
6771
import org.springframework.security.access.AccessDeniedException;
6872
import org.springframework.security.access.PermissionEvaluator;
@@ -95,6 +99,7 @@
9599
import org.springframework.security.authorization.method.MethodInvocationResult;
96100
import org.springframework.security.authorization.method.PrePostTemplateDefaults;
97101
import org.springframework.security.config.annotation.SecurityContextChangedListenerConfig;
102+
import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
98103
import org.springframework.security.config.core.GrantedAuthorityDefaults;
99104
import org.springframework.security.config.observation.SecurityObservationSettings;
100105
import org.springframework.security.config.test.SpringTestContext;
@@ -107,12 +112,22 @@
107112
import org.springframework.security.test.context.support.WithMockUser;
108113
import org.springframework.security.test.context.support.WithSecurityContextTestExecutionListener;
109114
import org.springframework.stereotype.Component;
115+
import org.springframework.stereotype.Service;
110116
import org.springframework.test.context.ContextConfiguration;
111117
import org.springframework.test.context.TestExecutionListeners;
112118
import org.springframework.test.context.junit.jupiter.SpringExtension;
119+
import org.springframework.test.web.servlet.MockMvc;
120+
import org.springframework.test.web.servlet.MvcResult;
121+
import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder;
122+
import org.springframework.web.bind.annotation.ControllerAdvice;
123+
import org.springframework.web.bind.annotation.ExceptionHandler;
124+
import org.springframework.web.bind.annotation.GetMapping;
125+
import org.springframework.web.bind.annotation.RequestParam;
126+
import org.springframework.web.bind.annotation.RestController;
113127
import org.springframework.web.context.ConfigurableWebApplicationContext;
114128
import org.springframework.web.context.support.AnnotationConfigWebApplicationContext;
115129
import org.springframework.web.servlet.ModelAndView;
130+
import org.springframework.web.servlet.config.annotation.EnableWebMvc;
116131

117132
import static org.assertj.core.api.Assertions.assertThat;
118133
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
@@ -127,6 +142,9 @@
127142
import static org.mockito.Mockito.times;
128143
import static org.mockito.Mockito.verify;
129144
import static org.mockito.Mockito.verifyNoInteractions;
145+
import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.user;
146+
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
147+
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
130148

131149
/**
132150
* Tests for {@link PrePostMethodSecurityConfiguration}.
@@ -148,6 +166,9 @@ public class PrePostMethodSecurityConfigurationTests {
148166
@Autowired(required = false)
149167
BusinessService businessService;
150168

169+
@Autowired(required = false)
170+
MockMvc mvc;
171+
151172
@WithMockUser
152173
@Test
153174
public void customMethodSecurityPreAuthorizeAdminWhenRoleUserThenAccessDeniedException() {
@@ -1181,6 +1202,80 @@ void autowireWhenDefaultsThenAdvisorAnnotationsAreSorted() {
11811202
}
11821203
}
11831204

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

19202015
}
19212016

2017+
@EnableWebMvc
2018+
@EnableWebSecurity
2019+
@EnableMethodSecurity
2020+
static class WebMvcMethodSecurityConfig {
2021+
2022+
}
2023+
2024+
@EnableWebMvc
2025+
@EnableWebSecurity
2026+
@EnableMethodSecurity
2027+
static class WebMvcMethodSecurityCustomAdvisorConfig {
2028+
2029+
@Bean
2030+
AuthorizationAdvisor customAdvisor(SecurityContextHolderStrategy strategy) {
2031+
JdkRegexpMethodPointcut pointcut = new JdkRegexpMethodPointcut();
2032+
pointcut.setPattern(".*AuthorizedPerson.*getName");
2033+
return new AuthorizationAdvisor() {
2034+
@Override
2035+
public Object invoke(MethodInvocation mi) throws Throwable {
2036+
Authentication auth = strategy.getContext().getAuthentication();
2037+
Object result = mi.proceed();
2038+
if (auth.getName().equals(result)) {
2039+
return result;
2040+
}
2041+
throw new AccessDeniedException("Access Denied for User '" + auth.getName() + "'");
2042+
}
2043+
2044+
@Override
2045+
public Pointcut getPointcut() {
2046+
return pointcut;
2047+
}
2048+
2049+
@Override
2050+
public Advice getAdvice() {
2051+
return this;
2052+
}
2053+
2054+
@Override
2055+
public int getOrder() {
2056+
return AuthorizationInterceptorsOrder.POST_FILTER.getOrder() + 1;
2057+
}
2058+
};
2059+
}
2060+
2061+
}
2062+
2063+
@RestController
2064+
static class BasicController {
2065+
2066+
@Autowired(required = false)
2067+
BasicService service;
2068+
2069+
@GetMapping("/greetings/authorized-person")
2070+
String getAuthorizedPersonGreeting(@RequestParam String name) {
2071+
AuthorizedPerson authorizedPerson = this.service.getAuthorizedPerson(name);
2072+
return "Hello: " + authorizedPerson.getName();
2073+
}
2074+
2075+
@AuthorizeReturnObject
2076+
@GetMapping(value = "/authorized-person", produces = MediaType.APPLICATION_JSON_VALUE)
2077+
AuthorizedPerson getAuthorizedPerson(@RequestParam String name) {
2078+
return new AuthorizedPerson(name);
2079+
}
2080+
2081+
}
2082+
2083+
@ControllerAdvice
2084+
static class BasicControllerAdvice {
2085+
2086+
@ExceptionHandler(AccessDeniedException.class)
2087+
ResponseEntity<Map<String, String>> handleAccessDenied(AccessDeniedException ex) {
2088+
Map<String, String> responseBody = Map.of("message", ex.getMessage());
2089+
return ResponseEntity.status(HttpStatus.FORBIDDEN).body(responseBody);
2090+
}
2091+
2092+
}
2093+
2094+
@Service
2095+
static class BasicService {
2096+
2097+
@AuthorizeReturnObject
2098+
AuthorizedPerson getAuthorizedPerson(String name) {
2099+
return new AuthorizedPerson(name);
2100+
}
2101+
2102+
}
2103+
2104+
public static class AuthorizedPerson {
2105+
2106+
final String name;
2107+
2108+
AuthorizedPerson(String name) {
2109+
this.name = name;
2110+
}
2111+
2112+
@PostAuthorize("returnObject == authentication.name")
2113+
public String getName() {
2114+
return this.name;
2115+
}
2116+
2117+
}
2118+
19222119
}

0 commit comments

Comments
 (0)