From 0395ad06cb39b64243707e0043fbdab2de1c8c21 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Wed, 1 Aug 2018 19:46:21 +0200 Subject: [PATCH] Omit secret backend URL encoding for Vault. (#1098) We now construct the URL template from the base URL, API version prefix and secret backend upfront and apply templating only for the secret key. This bypasses URL encoding for the secret backend which allows for special characters such as slashes to be passed thru directly. Fixes gh-1094. --- .../VaultKvAccessStrategyFactory.java | 4 +- .../VaultKvAccessStrategySupport.java | 12 ++- .../VaultEnvironmentRepositoryTests.java | 84 +++++++++++++++---- 3 files changed, 76 insertions(+), 24 deletions(-) diff --git a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/VaultKvAccessStrategyFactory.java b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/VaultKvAccessStrategyFactory.java index 1cb9679e..f4f36ce5 100644 --- a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/VaultKvAccessStrategyFactory.java +++ b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/VaultKvAccessStrategyFactory.java @@ -60,7 +60,7 @@ public class VaultKvAccessStrategyFactory { @Override public String getPath() { - return "/v1/{backend}/{key}"; + return "{key}"; } @Override @@ -80,7 +80,7 @@ public class VaultKvAccessStrategyFactory { @Override public String getPath() { - return "/v1/{backend}/data/{key}"; + return "data/{key}"; } @Override diff --git a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/VaultKvAccessStrategySupport.java b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/VaultKvAccessStrategySupport.java index 692d4c87..2841ebab 100644 --- a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/VaultKvAccessStrategySupport.java +++ b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/VaultKvAccessStrategySupport.java @@ -39,7 +39,9 @@ abstract class VaultKvAccessStrategySupport implements VaultKvAccessStrategy { } /** - * @return the context path to append to the {@code baseUrl}. + * @return the relative context path template within a secret backend to append to a + * URL containing the {@code baseURL}, API version segment and secret backend. May be + * templated with {@literal {key}} to as template variable for the secret key. */ abstract String getPath(); @@ -60,9 +62,11 @@ abstract class VaultKvAccessStrategySupport implements VaultKvAccessStrategy { @Override public String getData(HttpHeaders headers, String backend, String key) { try { - ResponseEntity response = rest.exchange(baseUrl + getPath(), - HttpMethod.GET, new HttpEntity<>(headers), VaultResponse.class, - backend, key); + + String urlTemplate = String.format("%s/v1/%s/%s", baseUrl, backend, getPath()); + + ResponseEntity response = rest.exchange(urlTemplate, HttpMethod.GET, + new HttpEntity<>(headers), VaultResponse.class, key); HttpStatus status = response.getStatusCode(); if (status == HttpStatus.OK) { return extractDataFromBody(response.getBody()); diff --git a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/environment/VaultEnvironmentRepositoryTests.java b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/environment/VaultEnvironmentRepositoryTests.java index bffb0abf..f5c75a14 100644 --- a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/environment/VaultEnvironmentRepositoryTests.java +++ b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/environment/VaultEnvironmentRepositoryTests.java @@ -43,6 +43,7 @@ import static org.mockito.Mockito.when; * @author Spencer Gibb * @author Ryan Baxter * @author Haroun Pacquee + * @author Mark Paluch */ public class VaultEnvironmentRepositoryTests { @@ -73,18 +74,18 @@ public class VaultEnvironmentRepositoryTests { VaultResponse myAppVaultResp = mock(VaultResponse.class); when(myAppVaultResp.getData()).thenReturn("{\"foo\":\"bar\"}"); when(myAppResp.getBody()).thenReturn(myAppVaultResp); - when(rest.exchange(eq("http://127.0.0.1:8200/v1/{backend}/{key}"), + when(rest.exchange(eq("http://127.0.0.1:8200/v1/secret/{key}"), eq(HttpMethod.GET), any(HttpEntity.class), eq(VaultResponse.class), - eq("secret"), eq("myapp"))).thenReturn(myAppResp); + eq("myapp"))).thenReturn(myAppResp); ResponseEntity appResp = mock(ResponseEntity.class); when(appResp.getStatusCode()).thenReturn(HttpStatus.OK); VaultResponse appVaultResp = mock(VaultResponse.class); when(appVaultResp.getData()).thenReturn("{\"def-foo\":\"def-bar\"}"); when(appResp.getBody()).thenReturn(appVaultResp); - when(rest.exchange(eq("http://127.0.0.1:8200/v1/{backend}/{key}"), + when(rest.exchange(eq("http://127.0.0.1:8200/v1/secret/{key}"), eq(HttpMethod.GET), any(HttpEntity.class), eq(VaultResponse.class), - eq("secret"), eq("application"))).thenReturn(appResp); + eq("application"))).thenReturn(appResp); VaultEnvironmentRepository repo = new VaultEnvironmentRepository(mockProvide(configRequest), new EnvironmentWatch.Default(), rest, new VaultEnvironmentProperties()); @@ -104,6 +105,53 @@ public class VaultEnvironmentRepositoryTests { secondResult, e.getPropertySources().get(1).getSource()); } + @Test + @SuppressWarnings("unchecked") + public void testBackendWithSlashes() { + MockHttpServletRequest configRequest = new MockHttpServletRequest(); + configRequest.addHeader("X-CONFIG-TOKEN", "mytoken"); + + RestTemplate rest = mock(RestTemplate.class); + + ResponseEntity myAppResp = mock(ResponseEntity.class); + when(myAppResp.getStatusCode()).thenReturn(HttpStatus.OK); + VaultResponse myAppVaultResp = mock(VaultResponse.class); + when(myAppVaultResp.getData()).thenReturn("{\"foo\":\"bar\"}"); + when(myAppResp.getBody()).thenReturn(myAppVaultResp); + when(rest.exchange(eq("http://127.0.0.1:8200/v1/foo/bar/secret/{key}"), + eq(HttpMethod.GET), any(HttpEntity.class), eq(VaultResponse.class), + eq("myapp"))).thenReturn(myAppResp); + + ResponseEntity appResp = mock(ResponseEntity.class); + when(appResp.getStatusCode()).thenReturn(HttpStatus.OK); + VaultResponse appVaultResp = mock(VaultResponse.class); + when(appVaultResp.getData()).thenReturn("{\"def-foo\":\"def-bar\"}"); + when(appResp.getBody()).thenReturn(appVaultResp); + when(rest.exchange(eq("http://127.0.0.1:8200/v1/foo/bar/secret/{key}"), + eq(HttpMethod.GET), any(HttpEntity.class), eq(VaultResponse.class), + eq("application"))).thenReturn(appResp); + + VaultEnvironmentProperties properties = new VaultEnvironmentProperties(); + properties.setBackend("foo/bar/secret"); + + VaultEnvironmentRepository repo = new VaultEnvironmentRepository(mockProvide(configRequest), + new EnvironmentWatch.Default(), rest, properties); + + Environment e = repo.findOne("myapp", null, null); + assertEquals("Name should be the same as the application argument", "myapp", e.getName()); + assertEquals("Properties for specified application and default application with key 'application' should be returned", + 2, e.getPropertySources().size()); + Map firstResult = new HashMap(); + firstResult.put("foo", "bar"); + assertEquals("Properties for specified application should be returned in priority position", + firstResult, e.getPropertySources().get(0).getSource()); + + Map secondResult = new HashMap(); + secondResult.put("def-foo", "def-bar"); + assertEquals("Properties for default application with key 'application' should be returned in second position", + secondResult, e.getPropertySources().get(1).getSource()); + } + @Test @SuppressWarnings("unchecked") public void testFindOneDefaultKeySetAndDifferentToApplication() { @@ -116,18 +164,18 @@ public class VaultEnvironmentRepositoryTests { VaultResponse myAppVaultResp = mock(VaultResponse.class); when(myAppVaultResp.getData()).thenReturn("{\"foo\":\"bar\"}"); when(myAppResp.getBody()).thenReturn(myAppVaultResp); - when(rest.exchange(eq("http://127.0.0.1:8200/v1/{backend}/{key}"), + when(rest.exchange(eq("http://127.0.0.1:8200/v1/secret/{key}"), eq(HttpMethod.GET), any(HttpEntity.class), eq(VaultResponse.class), - eq("secret"), eq("myapp"))).thenReturn(myAppResp); + eq("myapp"))).thenReturn(myAppResp); ResponseEntity myDefaultKeyResp = mock(ResponseEntity.class); when(myDefaultKeyResp.getStatusCode()).thenReturn(HttpStatus.OK); VaultResponse myDefaultKeyVaultResp = mock(VaultResponse.class); when(myDefaultKeyVaultResp.getData()).thenReturn("{\"def-foo\":\"def-bar\"}"); when(myDefaultKeyResp.getBody()).thenReturn(myDefaultKeyVaultResp); - when(rest.exchange(eq("http://127.0.0.1:8200/v1/{backend}/{key}"), + when(rest.exchange(eq("http://127.0.0.1:8200/v1/secret/{key}"), eq(HttpMethod.GET), any(HttpEntity.class), eq(VaultResponse.class), - eq("secret"), eq("mydefaultkey"))).thenReturn(myDefaultKeyResp); + eq("mydefaultkey"))).thenReturn(myDefaultKeyResp); VaultEnvironmentRepository repo = new VaultEnvironmentRepository(mockProvide(configRequest), new EnvironmentWatch.Default(), rest, new VaultEnvironmentProperties()); @@ -161,18 +209,18 @@ public class VaultEnvironmentRepositoryTests { VaultResponse myAppVaultResp = mock(VaultResponse.class); when(myAppVaultResp.getData()).thenReturn("{\"foo\":\"bar\"}"); when(myAppResp.getBody()).thenReturn(myAppVaultResp); - when(rest.exchange(eq("http://127.0.0.1:8200/v1/{backend}/{key}"), + when(rest.exchange(eq("http://127.0.0.1:8200/v1/secret/{key}"), eq(HttpMethod.GET), any(HttpEntity.class), eq(VaultResponse.class), - eq("secret"), eq("myapp"))).thenReturn(myAppResp); + eq("myapp"))).thenReturn(myAppResp); ResponseEntity appResp = mock(ResponseEntity.class); when(appResp.getStatusCode()).thenReturn(HttpStatus.OK); VaultResponse appVaultResp = mock(VaultResponse.class); when(appVaultResp.getData()).thenReturn("{\"def-foo\":\"def-bar\"}"); when(appResp.getBody()).thenReturn(appVaultResp); - when(rest.exchange(eq("http://127.0.0.1:8200/v1/{backend}/{key}"), + when(rest.exchange(eq("http://127.0.0.1:8200/v1/secret/{key}"), eq(HttpMethod.GET), any(HttpEntity.class), eq(VaultResponse.class), - eq("secret"), eq("application"))).thenReturn(appResp); + eq("application"))).thenReturn(appResp); VaultEnvironmentRepository repo = new VaultEnvironmentRepository(mockProvide(configRequest), new EnvironmentWatch.Default(), rest, new VaultEnvironmentProperties()); @@ -200,9 +248,9 @@ public class VaultEnvironmentRepositoryTests { VaultResponse myAppVaultResp = mock(VaultResponse.class); when(myAppVaultResp.getData()).thenReturn("{\"foo\":\"bar\"}"); when(myAppResp.getBody()).thenReturn(myAppVaultResp); - when(rest.exchange(eq("http://127.0.0.1:8200/v1/{backend}/{key}"), + when(rest.exchange(eq("http://127.0.0.1:8200/v1/secret/{key}"), eq(HttpMethod.GET), any(HttpEntity.class), eq(VaultResponse.class), - eq("secret"), eq("myapp"))).thenReturn(myAppResp); + eq("myapp"))).thenReturn(myAppResp); VaultEnvironmentRepository repo = new VaultEnvironmentRepository(mockProvide(configRequest), new EnvironmentWatch.Default(), rest, new VaultEnvironmentProperties()); repo.findOne("myapp", null, null); @@ -232,17 +280,17 @@ public class VaultEnvironmentRepositoryTests { when(myAppResp.getStatusCode()).thenReturn(HttpStatus.OK); VaultResponse myAppVaultResp = getVaultResponse("{\"data\": {\"data\": {\"foo\": \"bar\"}}}"); when(myAppResp.getBody()).thenReturn(myAppVaultResp); - when(rest.exchange(eq("http://127.0.0.1:8200/v1/{backend}/data/{key}"), + when(rest.exchange(eq("http://127.0.0.1:8200/v1/secret/data/{key}"), eq(HttpMethod.GET), any(HttpEntity.class), eq(VaultResponse.class), - eq("secret"), eq("myapp"))).thenReturn(myAppResp); + eq("myapp"))).thenReturn(myAppResp); ResponseEntity appResp = mock(ResponseEntity.class); when(appResp.getStatusCode()).thenReturn(HttpStatus.OK); VaultResponse appVaultResp = getVaultResponse("{\"data\": {\"data\": {\"def-foo\":\"def-bar\"}}}"); when(appResp.getBody()).thenReturn(appVaultResp); - when(rest.exchange(eq("http://127.0.0.1:8200/v1/{backend}/data/{key}"), + when(rest.exchange(eq("http://127.0.0.1:8200/v1/secret/data/{key}"), eq(HttpMethod.GET), any(HttpEntity.class), eq(VaultResponse.class), - eq("secret"), eq("application"))).thenReturn(appResp); + eq("application"))).thenReturn(appResp); final VaultEnvironmentProperties vaultEnvironmentProperties = new VaultEnvironmentProperties(); vaultEnvironmentProperties.setKvVersion(2);