You are viewing a plain text version of this content. The canonical link for it is here.
Posted to server-dev@james.apache.org by bt...@apache.org on 2017/03/15 10:02:04 UTC
[18/22] james-project git commit: JAMES-1958 Improve
WebAdminConfiguration and TlsConfiguration
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 <bt...@linagora.com>
Authored: Tue Mar 14 15:53:29 2017 +0700
Committer: benwa <bt...@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-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org