Skip to content

Commit dced154

Browse files
eddumelendezsnicoll
authored andcommitted
Fix health endpoint security
Commit b02aba4 has renamed `management.security.role` to `management.security.roles`. Unfortunately, the `HealthMvcEndpoint` was still looking at the old property. This commit makes sure that the proper key is used and any custom role is applied rather than an unconditional `ADMIN` role. See gh-6540
1 parent 4229165 commit dced154

File tree

2 files changed

+32
-3
lines changed

2 files changed

+32
-3
lines changed

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

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,10 @@
1717
package org.springframework.boot.actuate.endpoint.mvc;
1818

1919
import java.security.Principal;
20+
import java.util.Arrays;
21+
import java.util.Collections;
2022
import java.util.HashMap;
23+
import java.util.List;
2124
import java.util.Map;
2225

2326
import org.springframework.boot.actuate.endpoint.HealthEndpoint;
@@ -35,6 +38,7 @@
3538
import org.springframework.security.core.GrantedAuthority;
3639
import org.springframework.util.Assert;
3740
import org.springframework.util.ClassUtils;
41+
import org.springframework.util.StringUtils;
3842
import org.springframework.web.bind.annotation.RequestMapping;
3943
import org.springframework.web.bind.annotation.ResponseBody;
4044

@@ -45,6 +49,7 @@
4549
* @author Dave Syer
4650
* @author Andy Wilkinson
4751
* @author Phillip Webb
52+
* @author Eddú Meléndez
4853
* @since 1.1.0
4954
*/
5055
@ConfigurationProperties(prefix = "endpoints.health")
@@ -184,11 +189,17 @@ private boolean isSecure(Principal principal) {
184189
}
185190
if (isSpringSecurityAuthentication(principal)) {
186191
Authentication authentication = (Authentication) principal;
187-
String role = this.roleResolver.getProperty("role", "ROLE_ADMIN");
192+
List<String> roles = Arrays.asList(StringUtils.trimArrayElements(StringUtils
193+
.commaDelimitedListToStringArray(this.roleResolver.getProperty("roles"))));
194+
if (roles.isEmpty()) {
195+
roles = Collections.singletonList("ROLE_ADMIN");
196+
}
188197
for (GrantedAuthority authority : authentication.getAuthorities()) {
189198
String name = authority.getAuthority();
190-
if (role.equals(name) || ("ROLE_" + role).equals(name)) {
191-
return true;
199+
for (String role : roles) {
200+
if (role.equals(name) || ("ROLE_" + role).equals(name)) {
201+
return true;
202+
}
192203
}
193204
}
194205
}

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,13 +42,18 @@
4242
* @author Christian Dupuis
4343
* @author Dave Syer
4444
* @author Andy Wilkinson
45+
* @author Eddú Meléndez
4546
*/
4647
public class HealthMvcEndpointTests {
4748

4849
private static final PropertySource<?> NON_SENSITIVE = new MapPropertySource("test",
4950
Collections.<String, Object>singletonMap("endpoints.health.sensitive",
5051
"false"));
5152

53+
private static final PropertySource<?> SECURITY_ROLES = new MapPropertySource("test",
54+
Collections.<String, Object>singletonMap("management.security.roles",
55+
"HERO, USER"));
56+
5257
private HealthEndpoint endpoint = null;
5358

5459
private HealthMvcEndpoint mvc = null;
@@ -140,6 +145,19 @@ public void secureNonAdmin() {
140145
assertThat(((Health) result).getDetails().get("foo")).isNull();
141146
}
142147

148+
@Test
149+
public void secureCustomRole() {
150+
this.mvc = new HealthMvcEndpoint(this.endpoint, false);
151+
this.mvc.setEnvironment(this.environment);
152+
this.environment.getPropertySources().addLast(SECURITY_ROLES);
153+
given(this.endpoint.invoke())
154+
.willReturn(new Health.Builder().up().withDetail("foo", "bar").build());
155+
Object result = this.mvc.invoke(this.user);
156+
assertThat(result instanceof Health).isTrue();
157+
assertThat(((Health) result).getStatus() == Status.UP).isTrue();
158+
assertThat(((Health) result).getDetails().get("foo")).isEqualTo("bar");
159+
}
160+
143161
@Test
144162
public void healthIsCached() {
145163
given(this.endpoint.getTimeToLive()).willReturn(10000L);

0 commit comments

Comments
 (0)