diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/EndpointWebMvcManagementContextConfiguration.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/EndpointWebMvcManagementContextConfiguration.java index 0bc891edf7..c55e0ff640 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/EndpointWebMvcManagementContextConfiguration.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/EndpointWebMvcManagementContextConfiguration.java @@ -159,9 +159,11 @@ public class EndpointWebMvcManagementContextConfiguration { @Bean @ConditionalOnBean(HealthEndpoint.class) @ConditionalOnEnabledEndpoint("health") - public HealthMvcEndpoint healthMvcEndpoint(HealthEndpoint delegate) { + public HealthMvcEndpoint healthMvcEndpoint(HealthEndpoint delegate, + ManagementServerProperties managementServerProperties) { HealthMvcEndpoint healthMvcEndpoint = new HealthMvcEndpoint(delegate, - this.managementServerProperties.getSecurity().isEnabled()); + this.managementServerProperties.getSecurity().isEnabled(), + managementServerProperties.getSecurity().getRoles()); if (this.healthMvcEndpointProperties.getMapping() != null) { healthMvcEndpoint .addStatusMapping(this.healthMvcEndpointProperties.getMapping()); 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 ada10fec51..053a583230 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 @@ -16,7 +16,9 @@ package org.springframework.boot.actuate.endpoint.mvc; +import java.util.Arrays; import java.util.HashMap; +import java.util.List; import java.util.Map; import javax.servlet.http.HttpServletRequest; @@ -52,6 +54,8 @@ public class HealthMvcEndpoint extends AbstractEndpointMvcAdapter roles; + private Map statusMapping = new HashMap(); private RelaxedPropertyResolver securityPropertyResolver; @@ -65,9 +69,15 @@ public class HealthMvcEndpoint extends AbstractEndpointMvcAdapter roles) { super(delegate); this.secure = secure; setupDefaultStatusMapping(); + this.roles = roles; } private void setupDefaultStatusMapping() { @@ -173,10 +183,7 @@ public class HealthMvcEndpoint extends AbstractEndpointMvcAdapter getRoles() { + if (this.roles != null) { + return this.roles; + } + String[] roles = StringUtils.commaDelimitedListToStringArray( + this.securityPropertyResolver.getProperty("roles", "ROLE_ACTUATOR")); + roles = StringUtils.trimArrayElements(roles); + return Arrays.asList(roles); + } + } diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/MvcEndpointSecurityInterceptor.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/MvcEndpointSecurityInterceptor.java index aab244dcaa..4607ef2e4f 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/MvcEndpointSecurityInterceptor.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/MvcEndpointSecurityInterceptor.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2017 the original author or authors. + * Copyright 2012-2016 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -125,7 +125,10 @@ public class MvcEndpointSecurityInterceptor extends HandlerInterceptorAdapter { } } - private class AuthoritiesValidator { + /** + * Inner class to check authorities using Spring Security (when available). + */ + private static class AuthoritiesValidator { private boolean hasAuthority(String role) { Authentication authentication = SecurityContextHolder.getContext() diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/autoconfigure/HealthMvcEndpointAutoConfigurationTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/autoconfigure/HealthMvcEndpointAutoConfigurationTests.java index eee75c0176..1d9ceefbdf 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/autoconfigure/HealthMvcEndpointAutoConfigurationTests.java +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/autoconfigure/HealthMvcEndpointAutoConfigurationTests.java @@ -16,6 +16,8 @@ package org.springframework.boot.actuate.autoconfigure; +import java.util.Arrays; + import org.junit.After; import org.junit.Test; @@ -34,6 +36,7 @@ import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockServletContext; +import org.springframework.test.util.ReflectionTestUtils; import org.springframework.web.context.support.AnnotationConfigWebApplicationContext; import static org.assertj.core.api.Assertions.assertThat; @@ -83,6 +86,20 @@ public class HealthMvcEndpointAutoConfigurationTests { assertThat(map.getDetails().get("foo")).isEqualTo("bar"); } + @Test + public void testSetRoles() throws Exception { + // gh-8314 + this.context = new AnnotationConfigWebApplicationContext(); + this.context.setServletContext(new MockServletContext()); + this.context.register(TestConfiguration.class); + EnvironmentTestUtils.addEnvironment(this.context, + "management.security.roles[0]=super"); + this.context.refresh(); + HealthMvcEndpoint health = this.context.getBean(HealthMvcEndpoint.class); + assertThat(ReflectionTestUtils.getField(health, "roles")) + .isEqualTo(Arrays.asList("super")); + } + @Configuration @ImportAutoConfiguration({ SecurityAutoConfiguration.class, JacksonAutoConfiguration.class, WebMvcAutoConfiguration.class, 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 cca40927dc..7a49d15d81 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 @@ -16,6 +16,7 @@ package org.springframework.boot.actuate.endpoint.mvc; +import java.util.Arrays; import java.util.Collections; import javax.servlet.http.HttpServletRequest; @@ -182,6 +183,19 @@ public class HealthMvcEndpointTests { assertThat(((Health) result).getDetails().get("foo")).isNull(); } + @Test + public void customRoleFromListShouldNotExposeDetailsForDefaultRole() { + // gh-8314 + this.mvc = new HealthMvcEndpoint(this.endpoint, true, + Arrays.asList("HERO", "USER")); + given(this.endpoint.invoke()) + .willReturn(new Health.Builder().up().withDetail("foo", "bar").build()); + 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 healthIsCached() { given(this.endpoint.getTimeToLive()).willReturn(10000L); diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/orm/jpa/DataSourceInitializedPublisher.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/orm/jpa/DataSourceInitializedPublisher.java index 5db663d902..74810e47a2 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/orm/jpa/DataSourceInitializedPublisher.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/orm/jpa/DataSourceInitializedPublisher.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2016 the original author or authors. + * Copyright 2012-2017 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -64,20 +64,33 @@ class DataSourceInitializedPublisher implements BeanPostProcessor { if (bean instanceof JpaProperties) { this.properties = (JpaProperties) bean; } - if (bean instanceof EntityManagerFactory && this.dataSource != null - && isInitializingDatabase()) { - this.applicationContext - .publishEvent(new DataSourceInitializedEvent(this.dataSource)); + if (bean instanceof EntityManagerFactory) { + publishEventIfRequired((EntityManagerFactory) bean); } return bean; } - private boolean isInitializingDatabase() { + private void publishEventIfRequired(EntityManagerFactory entityManagerFactory) { + DataSource dataSource = findDataSource(entityManagerFactory); + if (dataSource != null && isInitializingDatabase(dataSource)) { + this.applicationContext + .publishEvent(new DataSourceInitializedEvent(dataSource)); + } + } + + private DataSource findDataSource(EntityManagerFactory entityManagerFactory) { + Object dataSource = entityManagerFactory.getProperties() + .get("javax.persistence.nonJtaDataSource"); + return (dataSource != null && dataSource instanceof DataSource + ? (DataSource) dataSource : this.dataSource); + } + + private boolean isInitializingDatabase(DataSource dataSource) { if (this.properties == null) { return true; // better safe than sorry } Map hibernate = this.properties - .getHibernateProperties(this.dataSource); + .getHibernateProperties(dataSource); if (hibernate.containsKey("hibernate.hbm2ddl.auto")) { return true; } diff --git a/spring-boot/src/main/java/org/springframework/boot/context/embedded/jetty/JettyEmbeddedServletContainer.java b/spring-boot/src/main/java/org/springframework/boot/context/embedded/jetty/JettyEmbeddedServletContainer.java index a52a789078..8941c3d614 100644 --- a/spring-boot/src/main/java/org/springframework/boot/context/embedded/jetty/JettyEmbeddedServletContainer.java +++ b/spring-boot/src/main/java/org/springframework/boot/context/embedded/jetty/JettyEmbeddedServletContainer.java @@ -205,9 +205,6 @@ public class JettyEmbeddedServletContainer implements EmbeddedServletContainer { @Override public void stop() { synchronized (this.monitor) { - if (!this.started) { - return; - } this.started = false; try { this.server.stop(); diff --git a/spring-boot/src/main/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainer.java b/spring-boot/src/main/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainer.java index 27ab3dc021..b9a59b4fc2 100644 --- a/spring-boot/src/main/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainer.java +++ b/spring-boot/src/main/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainer.java @@ -90,28 +90,34 @@ public class TomcatEmbeddedServletContainer implements EmbeddedServletContainer synchronized (this.monitor) { try { addInstanceIdToEngineName(); - - // Remove service connectors to that protocol binding doesn't happen yet - removeServiceConnectors(); - - // Start the server to trigger initialization listeners - this.tomcat.start(); - - // We can re-throw failure exception directly in the main thread - rethrowDeferredStartupExceptions(); - - Context context = findContext(); try { - ContextBindings.bindClassLoader(context, getNamingToken(context), - getClass().getClassLoader()); - } - catch (NamingException ex) { - // Naming is not enabled. Continue - } + // Remove service connectors to that protocol binding doesn't happen + // yet + removeServiceConnectors(); - // Unlike Jetty, all Tomcat threads are daemon threads. We create a - // blocking non-daemon to stop immediate shutdown - startDaemonAwaitThread(); + // Start the server to trigger initialization listeners + this.tomcat.start(); + + // We can re-throw failure exception directly in the main thread + rethrowDeferredStartupExceptions(); + + Context context = findContext(); + try { + ContextBindings.bindClassLoader(context, getNamingToken(context), + getClass().getClassLoader()); + } + catch (NamingException ex) { + // Naming is not enabled. Continue + } + + // Unlike Jetty, all Tomcat threads are daemon threads. We create a + // blocking non-daemon to stop immediate shutdown + startDaemonAwaitThread(); + } + catch (Exception ex) { + containerCounter.decrementAndGet(); + throw ex; + } } catch (Exception ex) { throw new EmbeddedServletContainerException( @@ -279,9 +285,7 @@ public class TomcatEmbeddedServletContainer implements EmbeddedServletContainer @Override public void stop() throws EmbeddedServletContainerException { synchronized (this.monitor) { - if (!this.started) { - return; - } + boolean wasStarted = this.started; try { this.started = false; try { @@ -297,7 +301,9 @@ public class TomcatEmbeddedServletContainer implements EmbeddedServletContainer "Unable to stop embedded Tomcat", ex); } finally { - containerCounter.decrementAndGet(); + if (wasStarted) { + containerCounter.decrementAndGet(); + } } } } diff --git a/spring-boot/src/test/java/org/springframework/boot/context/embedded/AbstractEmbeddedServletContainerFactoryTests.java b/spring-boot/src/test/java/org/springframework/boot/context/embedded/AbstractEmbeddedServletContainerFactoryTests.java index 9611d01e7b..14bce61593 100644 --- a/spring-boot/src/test/java/org/springframework/boot/context/embedded/AbstractEmbeddedServletContainerFactoryTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/context/embedded/AbstractEmbeddedServletContainerFactoryTests.java @@ -174,6 +174,16 @@ public abstract class AbstractEmbeddedServletContainerFactoryTests { assertThat(this.output.toString()).containsOnlyOnce("started on port"); } + @Test + public void stopCalledTwice() throws Exception { + AbstractEmbeddedServletContainerFactory factory = getFactory(); + this.container = factory + .getEmbeddedServletContainer(exampleServletRegistration()); + this.container.start(); + this.container.stop(); + this.container.stop(); + } + @Test public void emptyServerWhenPortIsMinusOne() throws Exception { AbstractEmbeddedServletContainerFactory factory = getFactory(); @@ -315,16 +325,6 @@ public abstract class AbstractEmbeddedServletContainerFactoryTests { getFactory().setContextPath("/"); } - @Test - public void doubleStop() throws Exception { - AbstractEmbeddedServletContainerFactory factory = getFactory(); - this.container = factory - .getEmbeddedServletContainer(exampleServletRegistration()); - this.container.start(); - this.container.stop(); - this.container.stop(); - } - @Test public void multipleConfigurations() throws Exception { AbstractEmbeddedServletContainerFactory factory = getFactory(); diff --git a/spring-boot/src/test/java/org/springframework/boot/context/embedded/jetty/JettyEmbeddedServletContainerFactoryTests.java b/spring-boot/src/test/java/org/springframework/boot/context/embedded/jetty/JettyEmbeddedServletContainerFactoryTests.java index 46edc917ef..28c23cfd92 100644 --- a/spring-boot/src/test/java/org/springframework/boot/context/embedded/jetty/JettyEmbeddedServletContainerFactoryTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/context/embedded/jetty/JettyEmbeddedServletContainerFactoryTests.java @@ -143,6 +143,16 @@ public class JettyEmbeddedServletContainerFactoryTests .isEmpty(); } + @Test + public void stopCalledWithoutStart() throws Exception { + JettyEmbeddedServletContainerFactory factory = getFactory(); + this.container = factory + .getEmbeddedServletContainer(exampleServletRegistration()); + this.container.stop(); + Server server = ((JettyEmbeddedServletContainer) this.container).getServer(); + assertThat(server.isStopped()).isTrue(); + } + @Override protected void addConnector(final int port, AbstractEmbeddedServletContainerFactory factory) { diff --git a/spring-boot/src/test/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainerFactoryTests.java b/spring-boot/src/test/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainerFactoryTests.java index 4bbbfdae18..ed5ea62394 100644 --- a/spring-boot/src/test/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainerFactoryTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainerFactoryTests.java @@ -353,6 +353,16 @@ public class TomcatEmbeddedServletContainerFactoryTests .doesNotContain("appears to have started a thread named [main]"); } + @Test + public void stopCalledWithoutStart() throws Exception { + TomcatEmbeddedServletContainerFactory factory = getFactory(); + this.container = factory + .getEmbeddedServletContainer(exampleServletRegistration()); + this.container.stop(); + Tomcat tomcat = ((TomcatEmbeddedServletContainer) this.container).getTomcat(); + assertThat(tomcat.getServer().getState()).isSameAs(LifecycleState.DESTROYED); + } + @Override protected void addConnector(int port, AbstractEmbeddedServletContainerFactory factory) {