From 4fe7bcabf0ee76942ea061ea68e86203d39d88c5 Mon Sep 17 00:00:00 2001 From: rhanton Date: Mon, 20 Aug 2018 11:09:14 -0700 Subject: [PATCH 1/3] Make decryption code fail-fast AFTER logging the error. --- .../encrypt/EnvironmentDecryptApplicationInitializer.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/encrypt/EnvironmentDecryptApplicationInitializer.java b/spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/encrypt/EnvironmentDecryptApplicationInitializer.java index ec67ef5c..40560fce 100644 --- a/spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/encrypt/EnvironmentDecryptApplicationInitializer.java +++ b/spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/encrypt/EnvironmentDecryptApplicationInitializer.java @@ -198,15 +198,15 @@ public class EnvironmentDecryptApplicationInitializer implements } catch (Exception e) { String message = "Cannot decrypt: key=" + key; - if (this.failOnError) { - throw new IllegalStateException(message, e); - } if (logger.isDebugEnabled()) { logger.warn(message, e); } else { logger.warn(message); } + if (this.failOnError) { + throw new IllegalStateException(message, e); + } // Set value to empty to avoid making a password out of the // cipher text value = ""; From a9fa0c3d772aceb84e122ec87928679a12b220a7 Mon Sep 17 00:00:00 2001 From: rhanton Date: Tue, 21 Aug 2018 10:45:13 -0700 Subject: [PATCH 2/3] Adding tests of EnvironmentDecryptApplicationInitializer change --- ...entDecryptApplicationInitializerTests.java | 26 ++++++++++++++++--- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/spring-cloud-context/src/test/java/org/springframework/cloud/bootstrap/encrypt/EnvironmentDecryptApplicationInitializerTests.java b/spring-cloud-context/src/test/java/org/springframework/cloud/bootstrap/encrypt/EnvironmentDecryptApplicationInitializerTests.java index 60889642..79ea7fc8 100644 --- a/spring-cloud-context/src/test/java/org/springframework/cloud/bootstrap/encrypt/EnvironmentDecryptApplicationInitializerTests.java +++ b/spring-cloud-context/src/test/java/org/springframework/cloud/bootstrap/encrypt/EnvironmentDecryptApplicationInitializerTests.java @@ -18,8 +18,9 @@ package org.springframework.cloud.bootstrap.encrypt; import java.util.Collections; import java.util.Map; +import org.junit.Rule; import org.junit.Test; - +import org.springframework.boot.test.rule.OutputCapture; import org.springframework.boot.test.util.TestPropertyValues; import org.springframework.boot.test.util.TestPropertyValues.Type; import org.springframework.context.ApplicationContext; @@ -31,9 +32,11 @@ import org.springframework.core.env.MutablePropertySources; import org.springframework.core.env.PropertySource; import org.springframework.security.crypto.encrypt.Encryptors; +import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.Matchers.is; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import static org.springframework.cloud.bootstrap.encrypt.EnvironmentDecryptApplicationInitializer.DECRYPTED_PROPERTY_SOURCE_NAME; @@ -46,6 +49,9 @@ public class EnvironmentDecryptApplicationInitializerTests { private EnvironmentDecryptApplicationInitializer listener = new EnvironmentDecryptApplicationInitializer( Encryptors.noOpText()); + + @Rule + public OutputCapture outputCapture = new OutputCapture(); @Test public void decryptCipherKey() { @@ -75,14 +81,22 @@ public class EnvironmentDecryptApplicationInitializerTests { assertEquals("spam", context.getEnvironment().getProperty("foo")); } - @Test(expected = IllegalStateException.class) + @Test public void errorOnDecrypt() { this.listener = new EnvironmentDecryptApplicationInitializer( Encryptors.text("deadbeef", "AFFE37")); ConfigurableApplicationContext context = new AnnotationConfigApplicationContext(); TestPropertyValues.of("foo: {cipher}bar").applyTo(context); - this.listener.initialize(context); - assertEquals("bar", context.getEnvironment().getProperty("foo")); + // catch IllegalStateException and verify + try { + this.listener.initialize(context); + } catch (Exception e) { + assertTrue(e instanceof IllegalStateException); + } + // Assert logs contain warning even when exception thrown + String sysOutput = this.outputCapture.toString(); + assertThat("Decryption error log message missing", sysOutput, containsString( + "EnvironmentDecryptApplicationInitializer - Cannot decrypt: key=foo")); } @Test @@ -93,6 +107,10 @@ public class EnvironmentDecryptApplicationInitializerTests { ConfigurableApplicationContext context = new AnnotationConfigApplicationContext(); TestPropertyValues.of("foo: {cipher}bar").applyTo(context); this.listener.initialize(context); + // Assert logs contain warning + String sysOutput = this.outputCapture.toString(); + assertThat("Decryption error log message missing", sysOutput, containsString( + "EnvironmentDecryptApplicationInitializer - Cannot decrypt: key=foo")); // Empty is safest fallback for undecryptable cipher assertEquals("", context.getEnvironment().getProperty("foo")); } From d8bc895f5e4e5a931f00af4a87c9408035b4f385 Mon Sep 17 00:00:00 2001 From: rhanton Date: Tue, 21 Aug 2018 13:23:39 -0700 Subject: [PATCH 3/3] Try to fix circleci error with log message checking. --- .../EnvironmentDecryptApplicationInitializerTests.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spring-cloud-context/src/test/java/org/springframework/cloud/bootstrap/encrypt/EnvironmentDecryptApplicationInitializerTests.java b/spring-cloud-context/src/test/java/org/springframework/cloud/bootstrap/encrypt/EnvironmentDecryptApplicationInitializerTests.java index 79ea7fc8..49fb6e0a 100644 --- a/spring-cloud-context/src/test/java/org/springframework/cloud/bootstrap/encrypt/EnvironmentDecryptApplicationInitializerTests.java +++ b/spring-cloud-context/src/test/java/org/springframework/cloud/bootstrap/encrypt/EnvironmentDecryptApplicationInitializerTests.java @@ -96,7 +96,7 @@ public class EnvironmentDecryptApplicationInitializerTests { // Assert logs contain warning even when exception thrown String sysOutput = this.outputCapture.toString(); assertThat("Decryption error log message missing", sysOutput, containsString( - "EnvironmentDecryptApplicationInitializer - Cannot decrypt: key=foo")); + "Cannot decrypt: key=foo")); } @Test @@ -110,7 +110,7 @@ public class EnvironmentDecryptApplicationInitializerTests { // Assert logs contain warning String sysOutput = this.outputCapture.toString(); assertThat("Decryption error log message missing", sysOutput, containsString( - "EnvironmentDecryptApplicationInitializer - Cannot decrypt: key=foo")); + "Cannot decrypt: key=foo")); // Empty is safest fallback for undecryptable cipher assertEquals("", context.getEnvironment().getProperty("foo")); }