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 2019/06/17 11:04:36 UTC

[james-project] 04/09: JAMES-2146 StartUpCheck for Jmap security configuration

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 <dt...@linagora.com>
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: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org