From e13764db66969007c3459415d1101b36538958ec Mon Sep 17 00:00:00 2001 From: Spencer Gibb Date: Fri, 9 Sep 2016 14:23:19 -0600 Subject: [PATCH] Fixes indexed properties merging problem. Given the following in one property source ``` mine[0].someValue=Foo mine[0].someKey={cipher}XXXX mine[1].someValue=Bar mine[1].someKey={cipher}XXXX yours[0].anotherKey=Baz ``` only `mine[*].someKey` were added to "decrypted" property source. Due to house spring boot manages indexed properties, they can not be merged across property source. This change includes all indexed properties in the "decrypted" property source if any one of them is encrypted. In the above example, `yours[*].*` is included even though unrelated. fixes gh-466 --- ...ironmentDecryptApplicationInitializer.java | 27 ++++++++++-- ...entDecryptApplicationInitializerTests.java | 43 ++++++++++++++++--- 2 files changed, 61 insertions(+), 9 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 17788e42..179b3a8a 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 @@ -21,6 +21,7 @@ import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.regex.Pattern; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -155,8 +156,8 @@ public class EnvironmentDecryptApplicationInitializer implements } public Map decrypt(PropertySources propertySources) { - Map overrides = new LinkedHashMap(); - List> sources = new ArrayList>(); + Map overrides = new LinkedHashMap<>(); + List> sources = new ArrayList<>(); for (PropertySource source : propertySources) { sources.add(0, source); } @@ -167,14 +168,19 @@ public class EnvironmentDecryptApplicationInitializer implements } private Map decrypt(PropertySource source) { - Map overrides = new LinkedHashMap(); + Map overrides = new LinkedHashMap<>(); decrypt(source, overrides); return overrides; } + private static final Pattern COLLECTION_PROPERTY = Pattern + .compile("(\\S+)?\\[(\\d+)\\](\\.\\S+)?"); + private void decrypt(PropertySource source, Map overrides) { if (source instanceof EnumerablePropertySource) { + Map otherCollectionProperties = new LinkedHashMap<>(); + boolean sourceHasDecryptedCollection = false; EnumerablePropertySource enumerable = (EnumerablePropertySource) source; for (String key : enumerable.getPropertyNames()) { @@ -205,9 +211,19 @@ public class EnvironmentDecryptApplicationInitializer implements value = ""; } overrides.put(key, value); + if (COLLECTION_PROPERTY.matcher(key).matches()) { + sourceHasDecryptedCollection = true; + } + } else if (COLLECTION_PROPERTY.matcher(key).matches()){ + // put non-ecrypted properties so merging of index properties happens correctly + otherCollectionProperties.put(key, value); } } } + // copy all indexed properties even if not encrypted + if (sourceHasDecryptedCollection && !otherCollectionProperties.isEmpty()) { + overrides.putAll(otherCollectionProperties); + } } else if (source instanceof CompositePropertySource) { @@ -221,4 +237,9 @@ public class EnvironmentDecryptApplicationInitializer implements } + class DecryptResult { + Map properties = new LinkedHashMap<>(); + boolean hasEncryptedProperties = false; + } + } 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 208203fc..8fe9ec86 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 @@ -16,15 +16,21 @@ package org.springframework.cloud.bootstrap.encrypt; import java.util.Collections; +import java.util.Map; import org.junit.Test; -import org.springframework.boot.test.EnvironmentTestUtils; import org.springframework.context.ConfigurableApplicationContext; import org.springframework.context.annotation.AnnotationConfigApplicationContext; import org.springframework.core.env.MapPropertySource; +import org.springframework.core.env.MutablePropertySources; +import org.springframework.core.env.PropertySource; import org.springframework.security.crypto.encrypt.Encryptors; +import static org.hamcrest.Matchers.is; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThat; +import static org.springframework.boot.test.EnvironmentTestUtils.addEnvironment; +import static org.springframework.cloud.bootstrap.encrypt.EnvironmentDecryptApplicationInitializer.DECRYPTED_PROPERTY_SOURCE_NAME; /** * @author Dave Syer @@ -38,7 +44,7 @@ public class EnvironmentDecryptApplicationInitializerTests { @Test public void decryptCipherKey() { ConfigurableApplicationContext context = new AnnotationConfigApplicationContext(); - EnvironmentTestUtils.addEnvironment(context, "foo: {cipher}bar"); + addEnvironment(context, "foo: {cipher}bar"); this.listener.initialize(context); assertEquals("bar", context.getEnvironment().getProperty("foo")); } @@ -46,7 +52,7 @@ public class EnvironmentDecryptApplicationInitializerTests { @Test public void relaxedBinding() { ConfigurableApplicationContext context = new AnnotationConfigApplicationContext(); - EnvironmentTestUtils.addEnvironment(context, "FOO_TEXT: {cipher}bar"); + addEnvironment(context, "FOO_TEXT: {cipher}bar"); this.listener.initialize(context); assertEquals("bar", context.getEnvironment().getProperty("foo.text")); } @@ -54,7 +60,7 @@ public class EnvironmentDecryptApplicationInitializerTests { @Test public void propertySourcesOrderedCorrectly() { ConfigurableApplicationContext context = new AnnotationConfigApplicationContext(); - EnvironmentTestUtils.addEnvironment(context, "foo: {cipher}bar"); + addEnvironment(context, "foo: {cipher}bar"); context.getEnvironment().getPropertySources().addFirst(new MapPropertySource( "test_override", Collections. singletonMap("foo", "{cipher}spam"))); @@ -67,7 +73,7 @@ public class EnvironmentDecryptApplicationInitializerTests { this.listener = new EnvironmentDecryptApplicationInitializer( Encryptors.text("deadbeef", "AFFE37")); ConfigurableApplicationContext context = new AnnotationConfigApplicationContext(); - EnvironmentTestUtils.addEnvironment(context, "foo: {cipher}bar"); + addEnvironment(context, "foo: {cipher}bar"); this.listener.initialize(context); assertEquals("bar", context.getEnvironment().getProperty("foo")); } @@ -78,10 +84,35 @@ public class EnvironmentDecryptApplicationInitializerTests { Encryptors.text("deadbeef", "AFFE37")); this.listener.setFailOnError(false); ConfigurableApplicationContext context = new AnnotationConfigApplicationContext(); - EnvironmentTestUtils.addEnvironment(context, "foo: {cipher}bar"); + addEnvironment(context, "foo: {cipher}bar"); this.listener.initialize(context); // Empty is safest fallback for undecryptable cipher assertEquals("", context.getEnvironment().getProperty("foo")); } + @Test + @SuppressWarnings("unchecked") + public void indexedPropertiesCopied() { + ConfigurableApplicationContext context = new AnnotationConfigApplicationContext(); + // tests that collections in another property source don't get copied into "decrypted" property source + addEnvironment(context, "yours[0].someValue: yourFoo", "yours[1].someValue: yourBar"); + + // collection with some encrypted keys and some not encrypted + addEnvironment("combinedTest", context.getEnvironment(), "mine[0].someValue: Foo", + "mine[0].someKey: {cipher}Foo0", "mine[1].someValue: Bar", + "mine[1].someKey: {cipher}Bar1", "nonindexed: nonindexval"); + this.listener.initialize(context); + + assertEquals("Foo", context.getEnvironment().getProperty("mine[0].someValue")); + assertEquals("Foo0", context.getEnvironment().getProperty("mine[0].someKey")); + assertEquals("Bar", context.getEnvironment().getProperty("mine[1].someValue")); + assertEquals("Bar1", context.getEnvironment().getProperty("mine[1].someKey")); + assertEquals("yourFoo", context.getEnvironment().getProperty("yours[0].someValue")); + assertEquals("yourBar", context.getEnvironment().getProperty("yours[1].someValue")); + + MutablePropertySources propertySources = context.getEnvironment().getPropertySources(); + PropertySource decrypted = (PropertySource) propertySources.get(DECRYPTED_PROPERTY_SOURCE_NAME); + assertThat("decrypted property source had wrong size", decrypted.getSource().size(), is(4)); + } + }