Skip to content

Commit c06a977

Browse files
committed
Support list based role properties
Update `HealthMvcEndpoint` to respect `ManagementServerProperties` roles. Prior to this commit the `HealthMvcEndpoint` directly loaded roles rather than using bound properties. This meant that list values from yaml were not respected. Fixes gh-8314
1 parent 987b6c9 commit c06a977

File tree

4 files changed

+57
-11
lines changed

4 files changed

+57
-11
lines changed

spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/EndpointWebMvcManagementContextConfiguration.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2016 the original author or authors.
2+
* Copyright 2012-2017 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -144,9 +144,10 @@ public HeapdumpMvcEndpoint heapdumpMvcEndpoint() {
144144
@Bean
145145
@ConditionalOnBean(HealthEndpoint.class)
146146
@ConditionalOnEnabledEndpoint("health")
147-
public HealthMvcEndpoint healthMvcEndpoint(HealthEndpoint delegate) {
147+
public HealthMvcEndpoint healthMvcEndpoint(HealthEndpoint delegate,
148+
ManagementServerProperties managementServerProperties) {
148149
HealthMvcEndpoint healthMvcEndpoint = new HealthMvcEndpoint(delegate,
149-
isHealthSecure());
150+
isHealthSecure(), managementServerProperties.getSecurity().getRoles());
150151
if (this.healthMvcEndpointProperties.getMapping() != null) {
151152
healthMvcEndpoint
152153
.addStatusMapping(this.healthMvcEndpointProperties.getMapping());

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

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2016 the original author or authors.
2+
* Copyright 2012-2017 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -57,6 +57,8 @@ public class HealthMvcEndpoint extends AbstractEndpointMvcAdapter<HealthEndpoint
5757

5858
private final boolean secure;
5959

60+
private final List<String> roles;
61+
6062
private Map<String, HttpStatus> statusMapping = new HashMap<String, HttpStatus>();
6163

6264
private RelaxedPropertyResolver healthPropertyResolver;
@@ -70,13 +72,19 @@ public class HealthMvcEndpoint extends AbstractEndpointMvcAdapter<HealthEndpoint
7072
private Health cached;
7173

7274
public HealthMvcEndpoint(HealthEndpoint delegate) {
73-
this(delegate, true);
75+
this(delegate, true, null);
7476
}
7577

7678
public HealthMvcEndpoint(HealthEndpoint delegate, boolean secure) {
79+
this(delegate, secure, null);
80+
}
81+
82+
public HealthMvcEndpoint(HealthEndpoint delegate, boolean secure,
83+
List<String> roles) {
7784
super(delegate);
7885
this.secure = secure;
7986
setupDefaultStatusMapping();
87+
this.roles = roles;
8088
}
8189

8290
private void setupDefaultStatusMapping() {
@@ -192,12 +200,9 @@ private boolean isSecure(Principal principal) {
192200
}
193201
if (isSpringSecurityAuthentication(principal)) {
194202
Authentication authentication = (Authentication) principal;
195-
List<String> roles = Arrays.asList(StringUtils
196-
.trimArrayElements(StringUtils.commaDelimitedListToStringArray(
197-
this.roleResolver.getProperty("roles", "ROLE_ADMIN"))));
198203
for (GrantedAuthority authority : authentication.getAuthorities()) {
199204
String name = authority.getAuthority();
200-
for (String role : roles) {
205+
for (String role : getRoles()) {
201206
if (role.equals(name) || ("ROLE_" + role).equals(name)) {
202207
return true;
203208
}
@@ -207,6 +212,15 @@ private boolean isSecure(Principal principal) {
207212
return false;
208213
}
209214

215+
private List<String> getRoles() {
216+
if (this.roles != null) {
217+
return this.roles;
218+
}
219+
return Arrays.asList(
220+
StringUtils.trimArrayElements(StringUtils.commaDelimitedListToStringArray(
221+
this.roleResolver.getProperty("roles", "ROLE_ADMIN"))));
222+
}
223+
210224
private boolean isSpringSecurityAuthentication(Principal principal) {
211225
return ClassUtils.isPresent("org.springframework.security.core.Authentication",
212226
null) && (principal instanceof Authentication);

spring-boot-actuator/src/test/java/org/springframework/boot/actuate/autoconfigure/HealthMvcEndpointAutoConfigurationTests.java

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2016 the original author or authors.
2+
* Copyright 2012-2017 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -16,6 +16,8 @@
1616

1717
package org.springframework.boot.actuate.autoconfigure;
1818

19+
import java.util.Arrays;
20+
1921
import org.junit.After;
2022
import org.junit.Test;
2123

@@ -33,6 +35,7 @@
3335
import org.springframework.context.annotation.Bean;
3436
import org.springframework.context.annotation.Configuration;
3537
import org.springframework.mock.web.MockServletContext;
38+
import org.springframework.test.util.ReflectionTestUtils;
3639
import org.springframework.web.context.support.AnnotationConfigWebApplicationContext;
3740

3841
import static org.assertj.core.api.Assertions.assertThat;
@@ -81,6 +84,20 @@ public void testNotSecured() throws Exception {
8184
assertThat(map.getDetails().get("foo")).isEqualTo("bar");
8285
}
8386

87+
@Test
88+
public void testSetRoles() throws Exception {
89+
// gh-8314
90+
this.context = new AnnotationConfigWebApplicationContext();
91+
this.context.setServletContext(new MockServletContext());
92+
this.context.register(TestConfiguration.class);
93+
EnvironmentTestUtils.addEnvironment(this.context,
94+
"management.security.roles[0]=super");
95+
this.context.refresh();
96+
HealthMvcEndpoint health = this.context.getBean(HealthMvcEndpoint.class);
97+
assertThat(ReflectionTestUtils.getField(health, "roles"))
98+
.isEqualTo(Arrays.asList("super"));
99+
}
100+
84101
@Configuration
85102
@ImportAutoConfiguration({ SecurityAutoConfiguration.class,
86103
JacksonAutoConfiguration.class, WebMvcAutoConfiguration.class,

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

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2016 the original author or authors.
2+
* Copyright 2012-2017 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -16,6 +16,7 @@
1616

1717
package org.springframework.boot.actuate.endpoint.mvc;
1818

19+
import java.util.Arrays;
1920
import java.util.Collections;
2021

2122
import org.junit.Before;
@@ -164,6 +165,19 @@ public void secureCustomRole() {
164165
assertThat(((Health) result).getDetails().get("foo")).isEqualTo("bar");
165166
}
166167

168+
@Test
169+
public void secureCustomRoleList() {
170+
// gh-8314
171+
this.mvc = new HealthMvcEndpoint(this.endpoint, true,
172+
Arrays.asList("HERO", "USER"));
173+
given(this.endpoint.invoke())
174+
.willReturn(new Health.Builder().up().withDetail("foo", "bar").build());
175+
Object result = this.mvc.invoke(this.hero);
176+
assertThat(result instanceof Health).isTrue();
177+
assertThat(((Health) result).getStatus() == Status.UP).isTrue();
178+
assertThat(((Health) result).getDetails().get("foo")).isEqualTo("bar");
179+
}
180+
167181
@Test
168182
public void secureCustomRoleNoAccess() {
169183
this.environment.getPropertySources().addLast(SECURITY_ROLES);

0 commit comments

Comments
 (0)