From 4882544c843a0a04764b05747b627c063614c18f Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Sat, 13 Aug 2016 08:02:04 +0200 Subject: [PATCH] Polish contribution Closes gh-6540 --- .../endpoint/mvc/HealthMvcEndpoint.java | 6 +--- .../endpoint/mvc/HealthMvcEndpointTests.java | 31 +++++++++++++------ 2 files changed, 23 insertions(+), 14 deletions(-) diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/HealthMvcEndpoint.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/HealthMvcEndpoint.java index adb34e8c341..2e65671ed9c 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/HealthMvcEndpoint.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/HealthMvcEndpoint.java @@ -18,7 +18,6 @@ package org.springframework.boot.actuate.endpoint.mvc; import java.security.Principal; import java.util.Arrays; -import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -190,10 +189,7 @@ public class HealthMvcEndpoint extends AbstractEndpointMvcAdapter roles = Arrays.asList(StringUtils.trimArrayElements(StringUtils - .commaDelimitedListToStringArray(this.roleResolver.getProperty("roles")))); - if (roles.isEmpty()) { - roles = Collections.singletonList("ROLE_ADMIN"); - } + .commaDelimitedListToStringArray(this.roleResolver.getProperty("roles", "ROLE_ADMIN")))); for (GrantedAuthority authority : authentication.getAuthorities()) { String name = authority.getAuthority(); for (String role : roles) { diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/HealthMvcEndpointTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/HealthMvcEndpointTests.java index e4038923665..e03ab21ac5b 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/HealthMvcEndpointTests.java +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/HealthMvcEndpointTests.java @@ -60,13 +60,17 @@ public class HealthMvcEndpointTests { private MockEnvironment environment; - private UsernamePasswordAuthenticationToken user = new UsernamePasswordAuthenticationToken( - "user", "password", - AuthorityUtils.commaSeparatedStringToAuthorityList("ROLE_USER")); + private UsernamePasswordAuthenticationToken user = createAuthenticationToken("ROLE_USER"); - private UsernamePasswordAuthenticationToken admin = new UsernamePasswordAuthenticationToken( - "user", "password", - AuthorityUtils.commaSeparatedStringToAuthorityList("ROLE_ADMIN")); + private UsernamePasswordAuthenticationToken admin = createAuthenticationToken("ROLE_ADMIN"); + + private UsernamePasswordAuthenticationToken hero = createAuthenticationToken("ROLE_HERO"); + + private UsernamePasswordAuthenticationToken createAuthenticationToken(String authority) { + return new UsernamePasswordAuthenticationToken( + "user", "password", + AuthorityUtils.commaSeparatedStringToAuthorityList(authority)); + } @Before public void init() { @@ -147,17 +151,26 @@ public class HealthMvcEndpointTests { @Test public void secureCustomRole() { - this.mvc = new HealthMvcEndpoint(this.endpoint, false); - this.mvc.setEnvironment(this.environment); this.environment.getPropertySources().addLast(SECURITY_ROLES); given(this.endpoint.invoke()) .willReturn(new Health.Builder().up().withDetail("foo", "bar").build()); - Object result = this.mvc.invoke(this.user); + Object result = this.mvc.invoke(this.hero); assertThat(result instanceof Health).isTrue(); assertThat(((Health) result).getStatus() == Status.UP).isTrue(); assertThat(((Health) result).getDetails().get("foo")).isEqualTo("bar"); } + @Test + public void secureCustomRoleNoAccess() { + this.environment.getPropertySources().addLast(SECURITY_ROLES); + given(this.endpoint.invoke()) + .willReturn(new Health.Builder().up().withDetail("foo", "bar").build()); + Object result = this.mvc.invoke(this.admin); + assertThat(result instanceof Health).isTrue(); + assertThat(((Health) result).getStatus() == Status.UP).isTrue(); + assertThat(((Health) result).getDetails().get("foo")).isNull(); + } + @Test public void healthIsCached() { given(this.endpoint.getTimeToLive()).willReturn(10000L);