This is an automated email from the ASF dual-hosted git repository. btellier pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/james-project.git
commit d3c90a9e2551e43b79ae8e7605cfa232dc2f9628 Author: Tran Tien Duc <[email protected]> AuthorDate: Thu Jun 6 10:56:41 2019 +0700 JAMES-2146 StartUpCheck for Jmap security configuration --- .../apache/james/MemoryJmapJamesServerTest.java | 93 ++++++++++++++++++--- .../src/test/resources/badAliasKeystore | Bin 0 -> 2246 bytes .../org/apache/james/jmap/JMAPCommonModule.java | 5 ++ .../james/jmap/JMAPConfigurationStartUpCheck.java | 63 ++++++++++++++ .../apache/james/modules/TestJMAPServerModule.java | 17 ++-- 5 files changed, 160 insertions(+), 18 deletions(-) diff --git a/server/container/guice/memory-guice/src/test/java/org/apache/james/MemoryJmapJamesServerTest.java b/server/container/guice/memory-guice/src/test/java/org/apache/james/MemoryJmapJamesServerTest.java index 47afa30..2b0469a 100644 --- a/server/container/guice/memory-guice/src/test/java/org/apache/james/MemoryJmapJamesServerTest.java +++ b/server/container/guice/memory-guice/src/test/java/org/apache/james/MemoryJmapJamesServerTest.java @@ -19,20 +19,91 @@ package org.apache.james; -import org.apache.james.mailbox.extractor.TextExtractor; -import org.apache.james.mailbox.store.search.PDFTextExtractor; +import static org.apache.james.JmapJamesServerContract.DOMAIN_LIST_CONFIGURATION_MODULE; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import org.apache.james.jmap.JMAPConfiguration; +import org.apache.james.jmap.JMAPConfigurationStartUpCheck; import org.apache.james.modules.TestJMAPServerModule; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; -class MemoryJmapJamesServerTest implements JmapJamesServerContract { +class MemoryJmapJamesServerTest { + private static final int LIMIT_TO_10_MESSAGES = 10; - @RegisterExtension - static JamesServerExtension jamesServerExtension = new JamesServerBuilder() - .server(configuration -> GuiceJamesServer.forConfiguration(configuration) - .combineWith(MemoryJamesServerMain.IN_MEMORY_SERVER_AGGREGATE_MODULE) - .overrideWith(new TestJMAPServerModule(LIMIT_TO_10_MESSAGES)) - .overrideWith(binder -> binder.bind(TextExtractor.class).to(PDFTextExtractor.class)) - .overrideWith(DOMAIN_LIST_CONFIGURATION_MODULE)) - .build(); + @Nested + class JmapJamesServerTest implements JmapJamesServerContract { + + @RegisterExtension + JamesServerExtension jamesServerExtension = new JamesServerBuilder() + .server(configuration -> GuiceJamesServer.forConfiguration(configuration) + .combineWith(MemoryJamesServerMain.IN_MEMORY_SERVER_AGGREGATE_MODULE) + .overrideWith(new TestJMAPServerModule(LIMIT_TO_10_MESSAGES)) + .overrideWith(DOMAIN_LIST_CONFIGURATION_MODULE)) + .build(); + } + + @Nested + class JamesServerJmapConfigurationTest { + + @Nested + class BadAliasKeyStore { + + @RegisterExtension + JamesServerExtension jamesServerExtension = new JamesServerBuilder() + .server(configuration -> GuiceJamesServer.forConfiguration(configuration) + .combineWith(MemoryJamesServerMain.IN_MEMORY_SERVER_AGGREGATE_MODULE) + .overrideWith(new TestJMAPServerModule(LIMIT_TO_10_MESSAGES)) + .overrideWith(DOMAIN_LIST_CONFIGURATION_MODULE) + .overrideWith(binder -> binder.bind(JMAPConfiguration.class) + .toInstance(TestJMAPServerModule + .jmapConfigurationBuilder() + .keystore("badAliasKeystore") + .secret("password") + .build()))) + .disableAutoStart() + .build(); + + @Test + void jamesShouldNotStartWhenBadAliasKeyStore(GuiceJamesServer server) { + assertThatThrownBy(server::start) + .isInstanceOfSatisfying( + StartUpChecksPerformer.StartUpChecksException.class, + ex -> assertThat(ex.badCheckNames()) + .containsOnly(JMAPConfigurationStartUpCheck.CHECK_NAME)) + .hasMessageContaining("Alias 'james' keystore can't be found"); + } + } + + @Nested + class BadSecretKeyStore { + + @RegisterExtension + JamesServerExtension jamesServerExtension = new JamesServerBuilder() + .server(configuration -> GuiceJamesServer.forConfiguration(configuration) + .combineWith(MemoryJamesServerMain.IN_MEMORY_SERVER_AGGREGATE_MODULE) + .overrideWith(new TestJMAPServerModule(LIMIT_TO_10_MESSAGES)) + .overrideWith(DOMAIN_LIST_CONFIGURATION_MODULE) + .overrideWith(binder -> binder.bind(JMAPConfiguration.class) + .toInstance(TestJMAPServerModule + .jmapConfigurationBuilder() + .secret("WrongSecret") + .build()))) + .disableAutoStart() + .build(); + + @Test + void jamesShouldNotStartWhenBadSecret(GuiceJamesServer server) { + assertThatThrownBy(server::start) + .isInstanceOfSatisfying( + StartUpChecksPerformer.StartUpChecksException.class, + ex -> assertThat(ex.badCheckNames()) + .containsOnly(JMAPConfigurationStartUpCheck.CHECK_NAME)) + .hasMessageContaining("Keystore was tampered with, or password was incorrect"); + } + } + } } diff --git a/server/container/guice/memory-guice/src/test/resources/badAliasKeystore b/server/container/guice/memory-guice/src/test/resources/badAliasKeystore new file mode 100644 index 0000000..0a4de22 Binary files /dev/null and b/server/container/guice/memory-guice/src/test/resources/badAliasKeystore differ diff --git a/server/container/guice/protocols/jmap/src/main/java/org/apache/james/jmap/JMAPCommonModule.java b/server/container/guice/protocols/jmap/src/main/java/org/apache/james/jmap/JMAPCommonModule.java index 9a44d3a..792b1e3 100644 --- a/server/container/guice/protocols/jmap/src/main/java/org/apache/james/jmap/JMAPCommonModule.java +++ b/server/container/guice/protocols/jmap/src/main/java/org/apache/james/jmap/JMAPCommonModule.java @@ -36,6 +36,7 @@ import org.apache.james.jmap.model.MessageFactory; import org.apache.james.jmap.model.MessagePreviewGenerator; import org.apache.james.jmap.send.MailSpool; import org.apache.james.jmap.utils.HeadersAuthenticationExtractor; +import org.apache.james.lifecycle.api.StartUpCheck; import org.apache.james.util.date.DefaultZonedDateTimeProvider; import org.apache.james.util.date.ZonedDateTimeProvider; import org.apache.james.util.mime.MessageContentExtractor; @@ -46,6 +47,7 @@ import com.google.common.collect.ImmutableList; import com.google.inject.AbstractModule; import com.google.inject.Provides; import com.google.inject.Scopes; +import com.google.inject.multibindings.Multibinder; import com.google.inject.name.Names; public class JMAPCommonModule extends AbstractModule { @@ -75,6 +77,9 @@ public class JMAPCommonModule extends AbstractModule { bindConstant().annotatedWith(Names.named(AccessTokenRepository.TOKEN_EXPIRATION_IN_MS)).to(DEFAULT_TOKEN_EXPIRATION_IN_MS); bind(AccessTokenManager.class).to(AccessTokenManagerImpl.class); + + Multibinder.newSetBinder(binder(), StartUpCheck.class) + .addBinding().to(JMAPConfigurationStartUpCheck.class); } @Provides diff --git a/server/container/guice/protocols/jmap/src/main/java/org/apache/james/jmap/JMAPConfigurationStartUpCheck.java b/server/container/guice/protocols/jmap/src/main/java/org/apache/james/jmap/JMAPConfigurationStartUpCheck.java new file mode 100644 index 0000000..158d4fc --- /dev/null +++ b/server/container/guice/protocols/jmap/src/main/java/org/apache/james/jmap/JMAPConfigurationStartUpCheck.java @@ -0,0 +1,63 @@ +/**************************************************************** + * Licensed to the Apache Software Foundation (ASF) under one * + * or more contributor license agreements. See the NOTICE file * + * distributed with this work for additional information * + * regarding copyright ownership. The ASF licenses this file * + * to you under the Apache License, Version 2.0 (the * + * "License"); you may not use this file except in compliance * + * with the License. You may obtain a copy of the License at * + * * + * http://www.apache.org/licenses/LICENSE-2.0 * + * * + * Unless required by applicable law or agreed to in writing, * + * software distributed under the License is distributed on an * + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY * + * KIND, either express or implied. See the License for the * + * specific language governing permissions and limitations * + * under the License. * + ****************************************************************/ + +package org.apache.james.jmap; + +import javax.inject.Inject; + +import org.apache.james.jmap.crypto.SecurityKeyLoader; +import org.apache.james.lifecycle.api.StartUpCheck; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class JMAPConfigurationStartUpCheck implements StartUpCheck { + + private static final Logger LOGGER = LoggerFactory.getLogger(JMAPConfigurationStartUpCheck.class); + public static final String CHECK_NAME = "JMAPConfigurationStartUpCheck"; + + private final SecurityKeyLoader securityKeyLoader; + + @Inject + JMAPConfigurationStartUpCheck(SecurityKeyLoader securityKeyLoader) { + this.securityKeyLoader = securityKeyLoader; + } + + @Override + public CheckResult check() { + try { + securityKeyLoader.load(); + return CheckResult.builder() + .checkName(checkName()) + .resultType(ResultType.GOOD) + .build(); + } catch (Exception e) { + LOGGER.error("Cannot load security key from jmap configuration", e); + return CheckResult.builder() + .checkName(checkName()) + .resultType(ResultType.BAD) + .description(e.getMessage()) + .build(); + } + } + + @Override + public String checkName() { + return CHECK_NAME; + } +} diff --git a/server/container/guice/protocols/jmap/src/test/java/org/apache/james/modules/TestJMAPServerModule.java b/server/container/guice/protocols/jmap/src/test/java/org/apache/james/modules/TestJMAPServerModule.java index a2f9313..ba78607 100644 --- a/server/container/guice/protocols/jmap/src/test/java/org/apache/james/modules/TestJMAPServerModule.java +++ b/server/container/guice/protocols/jmap/src/test/java/org/apache/james/modules/TestJMAPServerModule.java @@ -45,6 +45,15 @@ public class TestJMAPServerModule extends AbstractModule { + "kwIDAQAB\n" + "-----END PUBLIC KEY-----"; + public static JMAPConfiguration.Builder jmapConfigurationBuilder() { + return JMAPConfiguration.builder() + .enable() + .keystore("keystore") + .secret("james72laBalle") + .jwtPublicKeyPem(Optional.of(PUBLIC_PEM_KEY)) + .randomPort(); + } + private final long maximumLimit; public TestJMAPServerModule(long maximumLimit) { @@ -59,12 +68,6 @@ public class TestJMAPServerModule extends AbstractModule { @Provides @Singleton JMAPConfiguration provideConfiguration() throws FileNotFoundException, ConfigurationException { - return JMAPConfiguration.builder() - .enable() - .keystore("keystore") - .secret("james72laBalle") - .jwtPublicKeyPem(Optional.of(PUBLIC_PEM_KEY)) - .randomPort() - .build(); + return jmapConfigurationBuilder().build(); } } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
