Skip to content

Commit 031c9bf

Browse files
mbhavembhave
committed
Validate authorities in MvcSecurityInterceptor
If Spring Security is on the classpath and `isUserInRole` returns false, check if user has the authority to access the actuator endpoints. Fixes gh-8255
1 parent e5e1f24 commit 031c9bf

File tree

3 files changed

+179
-1
lines changed

3 files changed

+179
-1
lines changed

‎spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/MvcEndpointSecurityInterceptor.java

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,10 @@
2727

2828
import org.springframework.http.HttpMethod;
2929
import org.springframework.http.HttpStatus;
30+
import org.springframework.security.core.Authentication;
31+
import org.springframework.security.core.GrantedAuthority;
32+
import org.springframework.security.core.context.SecurityContextHolder;
33+
import org.springframework.util.ClassUtils;
3034
import org.springframework.util.StringUtils;
3135
import org.springframework.web.cors.CorsUtils;
3236
import org.springframework.web.method.HandlerMethod;
@@ -69,15 +73,35 @@ public boolean preHandle(HttpServletRequest request, HttpServletResponse respons
6973
if (!mvcEndpoint.isSensitive()) {
7074
return true;
7175
}
76+
if (isUserAllowedAccess(request)) {
77+
return true;
78+
}
79+
sendFailureResponse(request, response);
80+
return false;
81+
}
82+
83+
private boolean isUserAllowedAccess(HttpServletRequest request) {
84+
AuthoritiesValidator authoritiesValidator = null;
85+
if (isSpringSecurityAvailable()) {
86+
authoritiesValidator = new AuthoritiesValidator();
87+
}
7288
for (String role : this.roles) {
7389
if (request.isUserInRole(role)) {
7490
return true;
7591
}
92+
if (authoritiesValidator != null && authoritiesValidator.hasAuthority(role)) {
93+
return true;
94+
}
7695
}
77-
sendFailureResponse(request, response);
7896
return false;
7997
}
8098

99+
private boolean isSpringSecurityAvailable() {
100+
return ClassUtils.isPresent(
101+
"org.springframework.security.config.annotation.web.WebSecurityConfigurer",
102+
getClass().getClassLoader());
103+
}
104+
81105
private void sendFailureResponse(HttpServletRequest request,
82106
HttpServletResponse response) throws Exception {
83107
if (request.getUserPrincipal() != null) {
@@ -101,4 +125,19 @@ private void logUnauthorizedAttempt() {
101125
}
102126
}
103127

128+
private class AuthoritiesValidator {
129+
130+
private boolean hasAuthority(String role) {
131+
Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
132+
if (authentication != null) {
133+
for (GrantedAuthority authority : authentication.getAuthorities()) {
134+
if (authority.getAuthority().equals(role)) {
135+
return true;
136+
}
137+
}
138+
}
139+
return false;
140+
}
141+
}
142+
104143
}

‎spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/MvcEndpointSecurityInterceptorTests.java

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,9 @@
1818

1919
import java.security.Principal;
2020
import java.util.Arrays;
21+
import java.util.Collections;
2122
import java.util.List;
23+
import java.util.Set;
2224

2325
import javax.servlet.http.HttpServletResponse;
2426

@@ -31,9 +33,13 @@
3133
import org.springframework.http.HttpStatus;
3234
import org.springframework.mock.web.MockHttpServletRequest;
3335
import org.springframework.mock.web.MockServletContext;
36+
import org.springframework.security.core.Authentication;
37+
import org.springframework.security.core.authority.SimpleGrantedAuthority;
38+
import org.springframework.security.core.context.SecurityContextHolder;
3439
import org.springframework.web.method.HandlerMethod;
3540

3641
import static org.assertj.core.api.Assertions.assertThat;
42+
import static org.mockito.Mockito.doReturn;
3743
import static org.mockito.Mockito.mock;
3844
import static org.mockito.Mockito.verify;
3945

@@ -123,6 +129,30 @@ public void sensitiveEndpointIfRoleIsNotCorrectShouldNotAllowAccess()
123129
"Access is denied. User must have one of the these roles: SUPER_HERO");
124130
}
125131

132+
@Test
133+
public void sensitiveEndpointIfRoleNotCorrectShouldCheckAuthorities() throws Exception {
134+
Principal principal = mock(Principal.class);
135+
this.request.setUserPrincipal(principal);
136+
Authentication authentication = mock(Authentication.class);
137+
Set<SimpleGrantedAuthority> authorities = Collections.singleton(new SimpleGrantedAuthority("SUPER_HERO"));
138+
doReturn(authorities).when(authentication).getAuthorities();
139+
SecurityContextHolder.getContext().setAuthentication(authentication);
140+
assertThat(this.securityInterceptor.preHandle(this.request, this.response,
141+
this.handlerMethod)).isTrue();
142+
}
143+
144+
@Test
145+
public void sensitiveEndpointIfRoleAndAuthoritiesNotCorrectShouldNotAllowAccess() throws Exception {
146+
Principal principal = mock(Principal.class);
147+
this.request.setUserPrincipal(principal);
148+
Authentication authentication = mock(Authentication.class);
149+
Set<SimpleGrantedAuthority> authorities = Collections.singleton(new SimpleGrantedAuthority("HERO"));
150+
doReturn(authorities).when(authentication).getAuthorities();
151+
SecurityContextHolder.getContext().setAuthentication(authentication);
152+
assertThat(this.securityInterceptor.preHandle(this.request, this.response,
153+
this.handlerMethod)).isFalse();
154+
}
155+
126156
private static class TestEndpoint extends AbstractEndpoint<Object> {
127157

128158
TestEndpoint(String id) {

‎spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/NoSpringSecurityMvcEndpointSecurityInterceptorTests.java

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
/*
2+
* Copyright 2012-2016 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.boot.actuate.endpoint.mvc;
18+
19+
import java.security.Principal;
20+
import java.util.Arrays;
21+
import java.util.List;
22+
23+
import javax.servlet.http.HttpServletResponse;
24+
25+
import org.junit.Before;
26+
import org.junit.Rule;
27+
import org.junit.Test;
28+
import org.junit.runner.RunWith;
29+
30+
import org.springframework.boot.actuate.endpoint.AbstractEndpoint;
31+
import org.springframework.boot.junit.runner.classpath.ClassPathExclusions;
32+
import org.springframework.boot.junit.runner.classpath.ModifiedClassPathRunner;
33+
import org.springframework.boot.test.rule.OutputCapture;
34+
import org.springframework.mock.web.MockHttpServletRequest;
35+
import org.springframework.mock.web.MockServletContext;
36+
import org.springframework.web.method.HandlerMethod;
37+
38+
import static org.assertj.core.api.Assertions.assertThat;
39+
import static org.mockito.Mockito.mock;
40+
41+
/**
42+
* @author Madhura Bhave
43+
*/
44+
@RunWith(ModifiedClassPathRunner.class)
45+
@ClassPathExclusions("spring-security-*.jar")
46+
public class NoSpringSecurityMvcEndpointSecurityInterceptorTests {
47+
48+
@Rule
49+
public OutputCapture output = new OutputCapture();
50+
51+
private MvcEndpointSecurityInterceptor securityInterceptor;
52+
53+
private TestMvcEndpoint mvcEndpoint;
54+
55+
private TestEndpoint endpoint;
56+
57+
private HandlerMethod handlerMethod;
58+
59+
private MockHttpServletRequest request;
60+
61+
private HttpServletResponse response;
62+
63+
private MockServletContext servletContext;
64+
65+
private List<String> roles;
66+
67+
@Before
68+
public void setup() throws Exception {
69+
this.roles = Arrays.asList("SUPER_HERO");
70+
this.securityInterceptor = new MvcEndpointSecurityInterceptor(true, this.roles);
71+
this.endpoint = new TestEndpoint("a");
72+
this.mvcEndpoint = new TestMvcEndpoint(this.endpoint);
73+
this.handlerMethod = new HandlerMethod(this.mvcEndpoint, "invoke");
74+
this.servletContext = new MockServletContext();
75+
this.request = new MockHttpServletRequest(this.servletContext);
76+
this.response = mock(HttpServletResponse.class);
77+
}
78+
79+
@Test
80+
public void sensitiveEndpointIfRoleNotPresentShouldNotValidateAuthorities() throws Exception {
81+
Principal principal = mock(Principal.class);
82+
this.request.setUserPrincipal(principal);
83+
this.servletContext.declareRoles("HERO");
84+
assertThat(this.securityInterceptor.preHandle(this.request, this.response,
85+
this.handlerMethod)).isFalse();
86+
}
87+
88+
private static class TestEndpoint extends AbstractEndpoint<Object> {
89+
90+
TestEndpoint(String id) {
91+
super(id);
92+
}
93+
94+
@Override
95+
public Object invoke() {
96+
return null;
97+
}
98+
99+
}
100+
101+
private static class TestMvcEndpoint extends EndpointMvcAdapter {
102+
103+
TestMvcEndpoint(TestEndpoint delegate) {
104+
super(delegate);
105+
}
106+
107+
}
108+
}
109+

0 commit comments

Comments
 (0)