diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/CrshAutoConfiguration.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/CrshAutoConfiguration.java index adfdd22618c..33172d69819 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/CrshAutoConfiguration.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/CrshAutoConfiguration.java @@ -45,7 +45,7 @@ import org.crsh.vfs.spi.FSDriver; import org.springframework.beans.factory.ListableBeanFactory; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.actuate.properties.ShellProperties; -import org.springframework.boot.actuate.properties.ShellProperties.AuthenticationProperties; +import org.springframework.boot.actuate.properties.ShellProperties.CrshShellProperties; import org.springframework.boot.actuate.properties.ShellProperties.JaasAuthenticationProperties; import org.springframework.boot.actuate.properties.ShellProperties.KeyAuthenticationProperties; import org.springframework.boot.actuate.properties.ShellProperties.SimpleAuthenticationProperties; @@ -110,29 +110,25 @@ public class CrshAutoConfiguration { @Bean @ConditionalOnExpression("'${shell.auth:simple}' == 'jaas'") - @ConditionalOnMissingBean({ AuthenticationProperties.class }) - public AuthenticationProperties jaasAuthenticationProperties() { + public CrshShellProperties jaasAuthenticationProperties() { return new JaasAuthenticationProperties(); } @Bean @ConditionalOnExpression("'${shell.auth:simple}' == 'key'") - @ConditionalOnMissingBean({ AuthenticationProperties.class }) - public AuthenticationProperties keyAuthenticationProperties() { + public CrshShellProperties keyAuthenticationProperties() { return new KeyAuthenticationProperties(); } @Bean @ConditionalOnExpression("'${shell.auth:simple}' == 'simple'") - @ConditionalOnMissingBean({ AuthenticationProperties.class }) - public AuthenticationProperties simpleAuthenticationProperties() { + public CrshShellProperties simpleAuthenticationProperties() { return new SimpleAuthenticationProperties(); } @Bean @ConditionalOnExpression("'${shell.auth:simple}' == 'spring'") - @ConditionalOnMissingBean({ AuthenticationProperties.class }) - public AuthenticationProperties SpringAuthenticationProperties() { + public CrshShellProperties SpringAuthenticationProperties() { return new SpringAuthenticationProperties(); } @@ -140,7 +136,7 @@ public class CrshAutoConfiguration { @ConditionalOnMissingBean({ PluginLifeCycle.class }) public PluginLifeCycle shellBootstrap() { CrshBootstrapBean bootstrapBean = new CrshBootstrapBean(); - bootstrapBean.setConfig(this.properties.asCrashShellConfig()); + bootstrapBean.setConfig(this.properties.asCrshShellConfig()); return bootstrapBean; } @@ -244,7 +240,7 @@ public class CrshAutoConfiguration { Authentication token = new UsernamePasswordAuthenticationToken(username, password); try { - // Authenticate first to make credentials are valid + // Authenticate first to make sure credentials are valid token = this.authenticationManager.authenticate(token); } catch (AuthenticationException ex) { @@ -321,7 +317,7 @@ public class CrshAutoConfiguration { List> plugins = new ArrayList>(); for (CRaSHPlugin p : super.getPlugins()) { - if (!shouldFilter(p)) { + if (isEnabled(p)) { plugins.add(p); } } @@ -329,7 +325,7 @@ public class CrshAutoConfiguration { Collection pluginBeans = this.beanFactory.getBeansOfType( CRaSHPlugin.class).values(); for (CRaSHPlugin pluginBean : pluginBeans) { - if (!shouldFilter(pluginBean)) { + if (isEnabled(pluginBean)) { plugins.add(pluginBean); } } @@ -338,33 +334,33 @@ public class CrshAutoConfiguration { } @SuppressWarnings("rawtypes") - protected boolean shouldFilter(CRaSHPlugin plugin) { - Assert.notNull(plugin); + protected boolean isEnabled(CRaSHPlugin plugin) { + Assert.notNull(plugin, "Plugin must not be null"); if (ObjectUtils.isEmpty(this.disabledPlugins)) { - return false; + return true; } Set pluginClasses = ClassUtils.getAllInterfacesAsSet(plugin); pluginClasses.add(plugin.getClass()); for (Class pluginClass : pluginClasses) { - if (isDisabled(pluginClass)) { + if (isEnabled(pluginClass)) { return true; } } return false; } - private boolean isDisabled(Class pluginClass) { + private boolean isEnabled(Class pluginClass) { for (String disabledPlugin : this.disabledPlugins) { if (ClassUtils.getShortName(pluginClass).equalsIgnoreCase(disabledPlugin) || ClassUtils.getQualifiedName(pluginClass).equalsIgnoreCase( disabledPlugin)) { - return true; + return false; } } - return false; + return true; } } diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/properties/ShellProperties.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/properties/ShellProperties.java index 59a94b1ebf4..10e8e3b59a6 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/properties/ShellProperties.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/properties/ShellProperties.java @@ -41,7 +41,7 @@ public class ShellProperties { private String auth = "simple"; @Autowired(required = false) - private AuthenticationProperties authenticationProperties = new SimpleAuthenticationProperties(); + private CrshShellProperties[] additionalProperties = new CrshShellProperties[] { new SimpleAuthenticationProperties() }; private int commandRefreshInterval = -1; @@ -65,15 +65,13 @@ public class ShellProperties { return this.auth; } - public void setAuthenticationProperties( - AuthenticationProperties authenticationProperties) { - Assert.notNull(authenticationProperties, - "AuthenticationProperties must not be null"); - this.authenticationProperties = authenticationProperties; + public void setAdditionalProperties(CrshShellProperties[] additionalProperties) { + Assert.notNull(additionalProperties, "additionalProperties must not be null"); + this.additionalProperties = additionalProperties; } - public AuthenticationProperties getAuthenticationProperties() { - return this.authenticationProperties; + public CrshShellProperties[] getAdditionalProperties() { + return this.additionalProperties; } public void setCommandRefreshInterval(int commandRefreshInterval) { @@ -123,16 +121,15 @@ public class ShellProperties { * Return a properties file configured from these settings that can be applied to a * CRaSH shell instance. */ - public Properties asCrashShellConfig() { + public Properties asCrshShellConfig() { Properties properties = new Properties(); - this.ssh.applyToCrashShellConfig(properties); - this.telnet.applyToCrashShellConfig(properties); + this.ssh.applyToCrshShellConfig(properties); + this.telnet.applyToCrshShellConfig(properties); properties.put("crash.auth", this.auth); - if (this.authenticationProperties != null) { - this.authenticationProperties.applyToCrashShellConfig(properties); + for (CrshShellProperties shellProperties : this.additionalProperties) { + shellProperties.applyToCrshShellConfig(properties); } - if (this.commandRefreshInterval > 0) { properties.put("crash.vfs.refresh_period", String.valueOf(this.commandRefreshInterval)); @@ -151,10 +148,22 @@ public class ShellProperties { return properties; } + /** + * Base class for Auth specific properties. + */ + public static abstract class CrshShellProperties { + + /** + * Apply the properties to a CRaSH configuration. + */ + protected abstract void applyToCrshShellConfig(Properties config); + + } + /** * SSH properties */ - public static class Ssh { + public static class Ssh extends CrshShellProperties { private boolean enabled = true; @@ -162,7 +171,8 @@ public class ShellProperties { private String port = "2000"; - protected void applyToCrashShellConfig(Properties config) { + @Override + protected void applyToCrshShellConfig(Properties config) { if (this.enabled) { config.put("crash.ssh.port", this.port); if (this.keyPath != null) { @@ -180,12 +190,12 @@ public class ShellProperties { } public void setKeyPath(String keyPath) { - Assert.hasText(keyPath); + Assert.hasText(keyPath, "keyPath must have text"); this.keyPath = keyPath; } public void setPort(Integer port) { - Assert.notNull(port); + Assert.notNull(port, "port must not be null"); this.port = port.toString(); } @@ -194,13 +204,14 @@ public class ShellProperties { /** * Telnet properties */ - public static class Telnet { + public static class Telnet extends CrshShellProperties { private boolean enabled = false; private String port = "5000"; - protected void applyToCrashShellConfig(Properties config) { + @Override + protected void applyToCrshShellConfig(Properties config) { if (this.enabled) { config.put("crash.telnet.port", this.port); } @@ -215,39 +226,27 @@ public class ShellProperties { } public void setPort(Integer port) { - Assert.notNull(port); + Assert.notNull(port, "port must not be null"); this.port = port.toString(); } } - /** - * Base class for Auth specific properties. - */ - public static abstract class AuthenticationProperties { - - /** - * Apply the settings to a CRaSH configuration. - */ - protected abstract void applyToCrashShellConfig(Properties config); - - } - /** * Auth specific properties for JAAS authentication */ @ConfigurationProperties(name = "shell.auth.jaas", ignoreUnknownFields = false) - public static class JaasAuthenticationProperties extends AuthenticationProperties { + public static class JaasAuthenticationProperties extends CrshShellProperties { private String domain = "my-domain"; @Override - protected void applyToCrashShellConfig(Properties config) { + protected void applyToCrshShellConfig(Properties config) { config.put("crash.auth.jaas.domain", this.domain); } public void setDomain(String domain) { - Assert.hasText(domain); + Assert.hasText(domain, "domain must have text"); this.domain = domain; } @@ -257,19 +256,19 @@ public class ShellProperties { * Auth specific properties for key authentication */ @ConfigurationProperties(name = "shell.auth.key", ignoreUnknownFields = false) - public static class KeyAuthenticationProperties extends AuthenticationProperties { + public static class KeyAuthenticationProperties extends CrshShellProperties { private String path; @Override - protected void applyToCrashShellConfig(Properties config) { + protected void applyToCrshShellConfig(Properties config) { if (this.path != null) { config.put("crash.auth.key.path", this.path); } } public void setPath(String path) { - Assert.hasText(path); + Assert.hasText(path, "path must have text"); this.path = path; } @@ -279,7 +278,7 @@ public class ShellProperties { * Auth specific properties for simple authentication */ @ConfigurationProperties(name = "shell.auth.simple", ignoreUnknownFields = false) - public static class SimpleAuthenticationProperties extends AuthenticationProperties { + public static class SimpleAuthenticationProperties extends CrshShellProperties { private static Log logger = LogFactory .getLog(SimpleAuthenticationProperties.class); @@ -291,7 +290,7 @@ public class ShellProperties { private boolean defaultPassword = true; @Override - protected void applyToCrashShellConfig(Properties config) { + protected void applyToCrshShellConfig(Properties config) { config.put("crash.auth.simple.username", this.username); config.put("crash.auth.simple.password", this.password); if (this.defaultPassword) { @@ -305,7 +304,7 @@ public class ShellProperties { } public void setUsername(String username) { - Assert.hasLength(username); + Assert.hasLength(username, "username must have text"); this.username = username; } @@ -324,12 +323,12 @@ public class ShellProperties { * Auth specific properties for Spring authentication */ @ConfigurationProperties(name = "shell.auth.spring", ignoreUnknownFields = false) - public static class SpringAuthenticationProperties extends AuthenticationProperties { + public static class SpringAuthenticationProperties extends CrshShellProperties { private String[] roles = new String[] { "ROLE_ADMIN" }; @Override - protected void applyToCrashShellConfig(Properties config) { + protected void applyToCrshShellConfig(Properties config) { if (this.roles != null) { config.put("crash.auth.spring.roles", StringUtils.arrayToCommaDelimitedString(this.roles)); @@ -337,7 +336,9 @@ public class ShellProperties { } public void setRoles(String[] roles) { - Assert.notNull(roles); + // 'roles' can be empty. This means no special to access right to connect to + // shell is required. + Assert.notNull(roles, "roles must not be null"); this.roles = roles; } diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/properties/ShellPropertiesTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/properties/ShellPropertiesTests.java index 41f30571b05..04349e0d843 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/properties/ShellPropertiesTests.java +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/properties/ShellPropertiesTests.java @@ -20,16 +20,25 @@ import java.util.Collections; import java.util.HashMap; import java.util.Map; import java.util.Properties; +import java.util.UUID; +import org.crsh.plugin.PluginLifeCycle; import org.junit.Assert; import org.junit.Test; import org.springframework.beans.MutablePropertyValues; +import org.springframework.boot.actuate.autoconfigure.CrshAutoConfiguration; +import org.springframework.boot.actuate.properties.ShellProperties.CrshShellProperties; import org.springframework.boot.actuate.properties.ShellProperties.JaasAuthenticationProperties; import org.springframework.boot.actuate.properties.ShellProperties.KeyAuthenticationProperties; import org.springframework.boot.actuate.properties.ShellProperties.SimpleAuthenticationProperties; import org.springframework.boot.actuate.properties.ShellProperties.SpringAuthenticationProperties; import org.springframework.boot.bind.RelaxedDataBinder; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; import org.springframework.core.convert.support.DefaultConversionService; +import org.springframework.mock.env.MockEnvironment; +import org.springframework.mock.web.MockServletContext; +import org.springframework.web.context.support.AnnotationConfigWebApplicationContext; import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; @@ -125,7 +134,7 @@ public class ShellPropertiesTests { binder.bind(new MutablePropertyValues(map)); assertFalse(binder.getBindingResult().hasErrors()); - Properties p = props.asCrashShellConfig(); + Properties p = props.asCrshShellConfig(); assertEquals("2222", p.get("crash.ssh.port")); assertEquals("~/.ssh/test.pem", p.get("crash.ssh.keypath")); @@ -143,7 +152,7 @@ public class ShellPropertiesTests { binder.bind(new MutablePropertyValues(map)); assertFalse(binder.getBindingResult().hasErrors()); - Properties p = props.asCrashShellConfig(); + Properties p = props.asCrshShellConfig(); assertNull(p.get("crash.ssh.port")); assertNull(p.get("crash.ssh.keypath")); @@ -160,7 +169,7 @@ public class ShellPropertiesTests { binder.bind(new MutablePropertyValues(map)); assertFalse(binder.getBindingResult().hasErrors()); - Properties p = props.asCrashShellConfig(); + Properties p = props.asCrshShellConfig(); assertEquals("2222", p.get("crash.telnet.port")); } @@ -176,7 +185,7 @@ public class ShellPropertiesTests { binder.bind(new MutablePropertyValues(map)); assertFalse(binder.getBindingResult().hasErrors()); - Properties p = props.asCrashShellConfig(); + Properties p = props.asCrshShellConfig(); assertNull(p.get("crash.telnet.port")); } @@ -192,7 +201,7 @@ public class ShellPropertiesTests { assertFalse(binder.getBindingResult().hasErrors()); Properties p = new Properties(); - props.applyToCrashShellConfig(p); + props.applyToCrshShellConfig(p); assertEquals("my-test-domain", p.get("crash.auth.jaas.domain")); } @@ -208,7 +217,7 @@ public class ShellPropertiesTests { assertFalse(binder.getBindingResult().hasErrors()); Properties p = new Properties(); - props.applyToCrashShellConfig(p); + props.applyToCrshShellConfig(p); assertEquals("~/.ssh/test.pem", p.get("crash.auth.key.path")); } @@ -223,7 +232,7 @@ public class ShellPropertiesTests { assertFalse(binder.getBindingResult().hasErrors()); Properties p = new Properties(); - props.applyToCrashShellConfig(p); + props.applyToCrshShellConfig(p); assertNull(p.get("crash.auth.key.path")); } @@ -240,7 +249,7 @@ public class ShellPropertiesTests { assertFalse(binder.getBindingResult().hasErrors()); Properties p = new Properties(); - props.applyToCrashShellConfig(p); + props.applyToCrshShellConfig(p); assertEquals("username123", p.get("crash.auth.simple.username")); assertEquals("password123", p.get("crash.auth.simple.password")); @@ -275,9 +284,41 @@ public class ShellPropertiesTests { assertFalse(binder.getBindingResult().hasErrors()); Properties p = new Properties(); - props.applyToCrashShellConfig(p); + props.applyToCrshShellConfig(p); assertEquals("role1, role2", p.get("crash.auth.spring.roles")); } + @Test + public void testCustomShellProperties() throws Exception { + MockEnvironment env = new MockEnvironment(); + env.setProperty("shell.auth", "simple"); + AnnotationConfigWebApplicationContext context = new AnnotationConfigWebApplicationContext(); + context.setEnvironment(env); + context.setServletContext(new MockServletContext()); + context.register(TestShellConfiguration.class); + context.register(CrshAutoConfiguration.class); + context.refresh(); + + PluginLifeCycle lifeCycle = context.getBean(PluginLifeCycle.class); + String uuid = lifeCycle.getConfig().getProperty("test.uuid"); + assertEquals(TestShellConfiguration.uuid, uuid); + } + + @Configuration + public static class TestShellConfiguration { + + public static String uuid = UUID.randomUUID().toString(); + + @Bean + public CrshShellProperties testProperties() { + return new CrshShellProperties() { + + @Override + protected void applyToCrshShellConfig(Properties config) { + config.put("test.uuid", uuid); + } + }; + } + } }