JAMES-1958 Improve WebAdminConfiguration and TlsConfiguration
Project: http://git-wip-us.apache.org/repos/asf/james-project/repo Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/ca04fd6e Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/ca04fd6e Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/ca04fd6e Branch: refs/heads/master Commit: ca04fd6e3d314c042975263874245bed68bb893f Parents: ef119a4 Author: benwa <btell...@linagora.com> Authored: Tue Mar 14 15:53:29 2017 +0700 Committer: benwa <btell...@linagora.com> Committed: Wed Mar 15 09:02:32 2017 +0700 ---------------------------------------------------------------------- .../modules/server/WebAdminServerModule.java | 12 +++---- .../apache/james/webadmin/TlsConfiguration.java | 38 +++----------------- .../james/webadmin/WebAdminConfiguration.java | 26 ++++++++------ .../apache/james/webadmin/WebAdminServer.java | 4 +-- .../james/webadmin/TlsConfigurationTest.java | 19 ++-------- .../webadmin/WebAdminConfigurationTest.java | 31 ++++++++++++++-- 6 files changed, 60 insertions(+), 70 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/james-project/blob/ca04fd6e/server/container/guice/protocols/webadmin/src/main/java/org/apache/james/modules/server/WebAdminServerModule.java ---------------------------------------------------------------------- diff --git a/server/container/guice/protocols/webadmin/src/main/java/org/apache/james/modules/server/WebAdminServerModule.java b/server/container/guice/protocols/webadmin/src/main/java/org/apache/james/modules/server/WebAdminServerModule.java index 4695bdd..9c449a3 100644 --- a/server/container/guice/protocols/webadmin/src/main/java/org/apache/james/modules/server/WebAdminServerModule.java +++ b/server/container/guice/protocols/webadmin/src/main/java/org/apache/james/modules/server/WebAdminServerModule.java @@ -24,6 +24,7 @@ import static org.apache.james.webadmin.WebAdminServer.NO_CONFIGURATION; import java.io.FileNotFoundException; import java.util.List; +import java.util.Optional; import org.apache.commons.configuration.ConfigurationException; import org.apache.commons.configuration.PropertiesConfiguration; @@ -91,7 +92,7 @@ public class WebAdminServerModule extends AbstractModule { return WebAdminConfiguration.builder() .enable(configurationFile.getBoolean("enabled", DEFAULT_DISABLED)) .port(new FixedPort(configurationFile.getInt("port", WebAdminServer.DEFAULT_PORT))) - .https(readHttpsConfiguration(configurationFile)) + .tls(readHttpsConfiguration(configurationFile)) .enableCORS(configurationFile.getBoolean("cors.enable", DEFAULT_CORS_DISABLED)) .urlCORSOrigin(configurationFile.getString("cors.origin", DEFAULT_NO_CORS_ORIGIN)) .build(); @@ -115,18 +116,17 @@ public class WebAdminServerModule extends AbstractModule { } } - private TlsConfiguration readHttpsConfiguration(PropertiesConfiguration configurationFile) { + private Optional<TlsConfiguration> readHttpsConfiguration(PropertiesConfiguration configurationFile) { boolean enabled = configurationFile.getBoolean("https.enabled", DEFAULT_HTTPS_DISABLED); if (enabled) { - return TlsConfiguration.builder() - .enabled() + return Optional.of(TlsConfiguration.builder() .raw(configurationFile.getString("https.keystore", DEFAULT_NO_KEYSTORE), configurationFile.getString("https.password", DEFAULT_NO_PASSWORD), configurationFile.getString("https.trust.keystore", DEFAULT_NO_TRUST_KEYSTORE), configurationFile.getString("https.trust.password", DEFAULT_NO_TRUST_PASSWORD)) - .build(); + .build()); } - return TlsConfiguration.DEFAULT_DISABLE; + return Optional.empty(); } @Singleton http://git-wip-us.apache.org/repos/asf/james-project/blob/ca04fd6e/server/protocols/webadmin/src/main/java/org/apache/james/webadmin/TlsConfiguration.java ---------------------------------------------------------------------- diff --git a/server/protocols/webadmin/src/main/java/org/apache/james/webadmin/TlsConfiguration.java b/server/protocols/webadmin/src/main/java/org/apache/james/webadmin/TlsConfiguration.java index 81ecb66..d549ea3 100644 --- a/server/protocols/webadmin/src/main/java/org/apache/james/webadmin/TlsConfiguration.java +++ b/server/protocols/webadmin/src/main/java/org/apache/james/webadmin/TlsConfiguration.java @@ -20,41 +20,22 @@ package org.apache.james.webadmin; import java.util.Objects; -import java.util.Optional; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; public class TlsConfiguration { - public static final TlsConfiguration DEFAULT_DISABLE = TlsConfiguration.builder() - .disabled() - .build(); - public static Builder builder() { return new Builder(); } public static class Builder { - private Optional<Boolean> enabled = Optional.empty(); private String keystoreFilePath; private String keystorePassword; private String truststoreFilePath; private String truststorePassword; - public Builder enable(boolean isEnabled) { - this.enabled = Optional.of(isEnabled); - return this; - } - - public Builder enabled() { - return enable(true); - } - - public Builder disabled() { - return enable(false); - } - public Builder raw(String keystoreFilePath, String keystorePassword, String truststoreFilePath, @@ -73,17 +54,15 @@ public class TlsConfiguration { Preconditions.checkNotNull(keystoreFilePath); Preconditions.checkNotNull(keystorePassword); - this.enabled = Optional.of(true); this.keystoreFilePath = keystoreFilePath; this.keystorePassword = keystorePassword; return this; } public TlsConfiguration build() { - Preconditions.checkState(enabled.isPresent(), "You need to specify if https is enabled"); - Preconditions.checkState(!enabled.get() || hasKeystoreInformation(), "If enabled, you need to provide keystore information"); + Preconditions.checkState(hasKeystoreInformation(), "If enabled, you need to provide keystore information"); Preconditions.checkState(optionalHasTrustStoreInformation(), "You need to provide both information about trustStore"); - return new TlsConfiguration(enabled.get(), keystoreFilePath, keystorePassword, truststoreFilePath, truststorePassword); + return new TlsConfiguration(keystoreFilePath, keystorePassword, truststoreFilePath, truststorePassword); } private boolean optionalHasTrustStoreInformation() { @@ -96,25 +75,19 @@ public class TlsConfiguration { } - private final boolean enabled; private final String keystoreFilePath; private final String keystorePassword; private final String truststoreFilePath; private final String truststorePassword; @VisibleForTesting - TlsConfiguration(boolean enabled, String keystoreFilePath, String keystorePassword, String truststoreFilePath, String truststorePassword) { - this.enabled = enabled; + TlsConfiguration(String keystoreFilePath, String keystorePassword, String truststoreFilePath, String truststorePassword) { this.keystoreFilePath = keystoreFilePath; this.keystorePassword = keystorePassword; this.truststoreFilePath = truststoreFilePath; this.truststorePassword = truststorePassword; } - public boolean isEnabled() { - return enabled; - } - public String getKeystoreFilePath() { return keystoreFilePath; } @@ -136,8 +109,7 @@ public class TlsConfiguration { if (o instanceof TlsConfiguration) { TlsConfiguration that = (TlsConfiguration) o; - return Objects.equals(this.enabled, that.enabled) - && Objects.equals(this.keystoreFilePath, that.keystoreFilePath) + return Objects.equals(this.keystoreFilePath, that.keystoreFilePath) && Objects.equals(this.keystorePassword, that.keystorePassword) && Objects.equals(this.truststoreFilePath, that.truststoreFilePath) && Objects.equals(this.truststorePassword, that.truststorePassword); @@ -147,6 +119,6 @@ public class TlsConfiguration { @Override public final int hashCode() { - return Objects.hash(enabled, keystoreFilePath, keystorePassword, truststoreFilePath, truststorePassword); + return Objects.hash(keystoreFilePath, keystorePassword, truststoreFilePath, truststorePassword); } } http://git-wip-us.apache.org/repos/asf/james-project/blob/ca04fd6e/server/protocols/webadmin/src/main/java/org/apache/james/webadmin/WebAdminConfiguration.java ---------------------------------------------------------------------- diff --git a/server/protocols/webadmin/src/main/java/org/apache/james/webadmin/WebAdminConfiguration.java b/server/protocols/webadmin/src/main/java/org/apache/james/webadmin/WebAdminConfiguration.java index c61ae86..4099fdc 100644 --- a/server/protocols/webadmin/src/main/java/org/apache/james/webadmin/WebAdminConfiguration.java +++ b/server/protocols/webadmin/src/main/java/org/apache/james/webadmin/WebAdminConfiguration.java @@ -47,11 +47,16 @@ public class WebAdminConfiguration { private Optional<Boolean> enabled = Optional.empty(); private Optional<Port> port = Optional.empty(); private Optional<Boolean> enableCORS = Optional.empty(); - private Optional<TlsConfiguration> httpsConfiguration = Optional.empty(); + private Optional<TlsConfiguration> tlsConfiguration = Optional.empty(); private Optional<String> urlCORSOrigin = Optional.empty(); - public Builder https(TlsConfiguration tlsConfiguration) { - this.httpsConfiguration = Optional.of(tlsConfiguration); + public Builder tls(TlsConfiguration tlsConfiguration) { + this.tlsConfiguration = Optional.of(tlsConfiguration); + return this; + } + + public Builder tls(Optional<TlsConfiguration> tlsConfiguration) { + this.tlsConfiguration = tlsConfiguration; return this; } @@ -95,10 +100,7 @@ public class WebAdminConfiguration { Preconditions.checkState(!enabled.get() || port.isPresent(), "You need to specify a port for WebAdminConfiguration"); return new WebAdminConfiguration(enabled.get(), port, - httpsConfiguration.orElse( - TlsConfiguration.builder() - .disabled() - .build()), + tlsConfiguration, enableCORS.orElse(DEFAULT_CORS_DISABLED), urlCORSOrigin.orElse(CORS_ALL_ORIGINS)); } @@ -106,12 +108,12 @@ public class WebAdminConfiguration { private final boolean enabled; private final Optional<Port> port; - private final TlsConfiguration tlsConfiguration; + private final Optional<TlsConfiguration> tlsConfiguration; private final boolean enableCORS; private final String urlCORSOrigin; @VisibleForTesting - WebAdminConfiguration(boolean enabled, Optional<Port> port, TlsConfiguration tlsConfiguration, boolean enableCORS, String urlCORSOrigin) { + WebAdminConfiguration(boolean enabled, Optional<Port> port, Optional<TlsConfiguration> tlsConfiguration, boolean enableCORS, String urlCORSOrigin) { this.enabled = enabled; this.port = port; this.tlsConfiguration = tlsConfiguration; @@ -132,13 +134,17 @@ public class WebAdminConfiguration { } public TlsConfiguration getTlsConfiguration() { - return tlsConfiguration; + return tlsConfiguration.orElseThrow(() -> new IllegalStateException("No tls configuration")); } public boolean isEnableCORS() { return enableCORS; } + public boolean isTlsEnabled() { + return tlsConfiguration.isPresent(); + } + @Override public final boolean equals(Object o) { if (o instanceof WebAdminConfiguration) { http://git-wip-us.apache.org/repos/asf/james-project/blob/ca04fd6e/server/protocols/webadmin/src/main/java/org/apache/james/webadmin/WebAdminServer.java ---------------------------------------------------------------------- diff --git a/server/protocols/webadmin/src/main/java/org/apache/james/webadmin/WebAdminServer.java b/server/protocols/webadmin/src/main/java/org/apache/james/webadmin/WebAdminServer.java index 7c16045..c5bc5fd 100644 --- a/server/protocols/webadmin/src/main/java/org/apache/james/webadmin/WebAdminServer.java +++ b/server/protocols/webadmin/src/main/java/org/apache/james/webadmin/WebAdminServer.java @@ -92,8 +92,8 @@ public class WebAdminServer implements Configurable { } private void configureHTTPS() { - TlsConfiguration tlsConfiguration = configuration.getTlsConfiguration(); - if (tlsConfiguration.isEnabled()) { + if (configuration.isTlsEnabled()) { + TlsConfiguration tlsConfiguration = configuration.getTlsConfiguration(); service.secure(tlsConfiguration.getKeystoreFilePath(), tlsConfiguration.getKeystorePassword(), tlsConfiguration.getTruststoreFilePath(), http://git-wip-us.apache.org/repos/asf/james-project/blob/ca04fd6e/server/protocols/webadmin/src/test/java/org/apache/james/webadmin/TlsConfigurationTest.java ---------------------------------------------------------------------- diff --git a/server/protocols/webadmin/src/test/java/org/apache/james/webadmin/TlsConfigurationTest.java b/server/protocols/webadmin/src/test/java/org/apache/james/webadmin/TlsConfigurationTest.java index c21ddb7..2d93282 100644 --- a/server/protocols/webadmin/src/test/java/org/apache/james/webadmin/TlsConfigurationTest.java +++ b/server/protocols/webadmin/src/test/java/org/apache/james/webadmin/TlsConfigurationTest.java @@ -43,7 +43,7 @@ public class TlsConfigurationTest { public void buildShouldThrowWhenEnableWithoutKeystore() { expectedException.expect(IllegalStateException.class); - TlsConfiguration.builder().enabled().build(); + TlsConfiguration.builder().build(); } @Test @@ -51,7 +51,6 @@ public class TlsConfigurationTest { expectedException.expect(NullPointerException.class); TlsConfiguration.builder() - .enabled() .selfSigned(null, "abc"); } @@ -60,37 +59,25 @@ public class TlsConfigurationTest { expectedException.expect(NullPointerException.class); TlsConfiguration.builder() - .enabled() .selfSigned("abc", null); } @Test - public void buildShouldWorkOnDisabledHttps() { - assertThat( - TlsConfiguration.builder() - .disabled() - .build()) - .isEqualTo(new TlsConfiguration(false, null, null, null, null)); - } - - @Test public void buildShouldWorkOnSelfSignedHttps() { assertThat( TlsConfiguration.builder() - .enabled() .selfSigned("abcd", "efgh") .build()) - .isEqualTo(new TlsConfiguration(true, "abcd", "efgh", null, null)); + .isEqualTo(new TlsConfiguration("abcd", "efgh", null, null)); } @Test public void buildShouldWorkOnTrustedHttps() { assertThat( TlsConfiguration.builder() - .enabled() .raw("a", "b", "c", "d") .build()) - .isEqualTo(new TlsConfiguration(true, "a", "b", "c", "d")); + .isEqualTo(new TlsConfiguration("a", "b", "c", "d")); } @Test http://git-wip-us.apache.org/repos/asf/james-project/blob/ca04fd6e/server/protocols/webadmin/src/test/java/org/apache/james/webadmin/WebAdminConfigurationTest.java ---------------------------------------------------------------------- diff --git a/server/protocols/webadmin/src/test/java/org/apache/james/webadmin/WebAdminConfigurationTest.java b/server/protocols/webadmin/src/test/java/org/apache/james/webadmin/WebAdminConfigurationTest.java index dd46f34..1030dcf 100644 --- a/server/protocols/webadmin/src/test/java/org/apache/james/webadmin/WebAdminConfigurationTest.java +++ b/server/protocols/webadmin/src/test/java/org/apache/james/webadmin/WebAdminConfigurationTest.java @@ -83,14 +83,13 @@ public class WebAdminConfigurationTest { @Test public void builderShouldAcceptHttps() { TlsConfiguration tlsConfiguration = TlsConfiguration.builder() - .enable(true) .selfSigned("abcd", "efgh") .build(); assertThat( WebAdminConfiguration.builder() .enabled() - .https(tlsConfiguration) + .tls(tlsConfiguration) .port(PORT) .build()) .extracting(WebAdminConfiguration::getTlsConfiguration) @@ -98,6 +97,33 @@ public class WebAdminConfigurationTest { } @Test + public void builderShouldReturnTlsEnableWhenTlsConfiguration() { + TlsConfiguration tlsConfiguration = TlsConfiguration.builder() + .selfSigned("abcd", "efgh") + .build(); + + assertThat( + WebAdminConfiguration.builder() + .enabled() + .tls(tlsConfiguration) + .port(PORT) + .build()) + .extracting(WebAdminConfiguration::getTlsConfiguration) + .containsExactly(tlsConfiguration); + } + + @Test + public void builderShouldReturnTlsDisableWhenNoTlsConfiguration() { + assertThat( + WebAdminConfiguration.builder() + .enabled() + .port(PORT) + .build()) + .extracting(WebAdminConfiguration::isTlsEnabled) + .containsExactly(false); + } + + @Test public void builderShouldCORSEnabled() { assertThat( WebAdminConfiguration.builder() @@ -109,7 +135,6 @@ public class WebAdminConfigurationTest { .containsExactly(true); } - @Test public void builderShouldAcceptAllOriginsByDefault() { assertThat( --------------------------------------------------------------------- To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org
