From 5bb9a3259c3993132f438c2d82c92eb6af870d81 Mon Sep 17 00:00:00 2001 From: Alexander Reelsen Date: Mon, 2 Jul 2018 11:29:03 +0200 Subject: [PATCH 1/3] Watcher: Fix chain input toXcontent serialization The xcontent parameters were not passed to the xcontent serialization of the chain input. This could lead to wrongly stored watches, which did not contain passwords but only their redacted counterparts. --- .../watcher/get_watch/30_with_chain_input.yml | 51 +++++++++++++++++++ .../xpack/watcher/input/chain/ChainInput.java | 2 +- .../watcher/input/chain/ChainInputTests.java | 35 +++++++++++++ 3 files changed, 87 insertions(+), 1 deletion(-) create mode 100644 x-pack/plugin/src/test/resources/rest-api-spec/test/watcher/get_watch/30_with_chain_input.yml diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/watcher/get_watch/30_with_chain_input.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/watcher/get_watch/30_with_chain_input.yml new file mode 100644 index 0000000000000..81a12fe6f7ddb --- /dev/null +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/watcher/get_watch/30_with_chain_input.yml @@ -0,0 +1,51 @@ +--- +"Test get watch api with chained input and basic auth": + - do: + cluster.health: + wait_for_status: yellow + + - do: + xpack.watcher.put_watch: + id: "my_watch" + body: > + { + "trigger": { + "schedule": { + "cron": "0 0 0 1 * ? 2099" + } + }, + "input": { + "chain": { + "inputs": [ + { + "http": { + "http": { + "request": { + "url" : "http://localhost/", + "auth": { + "basic": { + "username": "Username123", + "password": "Password123" + } + } + } + } + } + } + ] + } + }, + "actions": { + "logging": { + "logging": { + "text": "logging statement here" + } + } + } + } + + - do: + xpack.watcher.get_watch: + id: "my_watch" + - match: { found : true} + - match: { _id: "my_watch" } diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/input/chain/ChainInput.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/input/chain/ChainInput.java index 3c62f4d1066d2..1599531429bf5 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/input/chain/ChainInput.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/input/chain/ChainInput.java @@ -41,7 +41,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws builder.startArray(INPUTS.getPreferredName()); for (Tuple tuple : inputs) { builder.startObject().startObject(tuple.v1()); - builder.field(tuple.v2().type(), tuple.v2()); + builder.field(tuple.v2().type(), tuple.v2(), params); builder.endObject().endObject(); } builder.endArray(); diff --git a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/input/chain/ChainInputTests.java b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/input/chain/ChainInputTests.java index e654452779ab8..b34870421aca5 100644 --- a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/input/chain/ChainInputTests.java +++ b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/input/chain/ChainInputTests.java @@ -9,6 +9,7 @@ import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.common.Strings; import org.elasticsearch.common.bytes.BytesReference; +import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; @@ -18,6 +19,7 @@ import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.SecuritySettingsSourceField; import org.elasticsearch.xpack.core.watcher.execution.WatchExecutionContext; +import org.elasticsearch.xpack.core.watcher.input.Input; import org.elasticsearch.xpack.core.watcher.watch.Payload; import org.elasticsearch.xpack.watcher.common.http.HttpRequestTemplate; import org.elasticsearch.xpack.watcher.common.http.auth.basic.BasicAuth; @@ -29,6 +31,8 @@ import org.elasticsearch.xpack.watcher.input.simple.SimpleInputFactory; import org.elasticsearch.xpack.watcher.test.WatcherTestUtils; +import java.io.IOException; +import java.util.Collections; import java.util.HashMap; import java.util.Map; @@ -46,6 +50,7 @@ import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.sameInstance; public class ChainInputTests extends ESTestCase { @@ -220,4 +225,34 @@ public void testParsingShouldBeStrictWhenStartingInputs() throws Exception { expectThrows(ElasticsearchParseException.class, () -> chainInputFactory.parseInput("test", parser)); assertThat(e.getMessage(), containsString("Expected starting JSON object after [first] in watch [test]")); } + + public void testThatXContentParametersArePassedToInputs() throws Exception { + ToXContent.Params params = new ToXContent.MapParams(Collections.singletonMap(randomAlphaOfLength(5), randomAlphaOfLength(5))); + ChainInput chainInput = new ChainInput(Collections.singletonList(Tuple.tuple("second", new TestInput(params)))); + + try (XContentBuilder builder = jsonBuilder()) { + chainInput.toXContent(builder, params); + } + } + + // a test input that checks the xcontent parameters to ensure the correct params have been passed + private static final class TestInput implements Input { + + private ToXContent.Params expectedParams; + + TestInput(ToXContent.Params expectedParams) { + this.expectedParams = expectedParams; + } + + @Override + public String type() { + return "test"; + } + + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + assertThat(params, sameInstance(expectedParams)); + return builder; + } + } } From 015dd3a0094d404ab409df58e6948084b5cd70d1 Mon Sep 17 00:00:00 2001 From: Alexander Reelsen Date: Mon, 2 Jul 2018 11:41:14 +0200 Subject: [PATCH 2/3] shorten test --- .../watcher/input/chain/ChainInputTests.java | 38 +++++++------------ 1 file changed, 14 insertions(+), 24 deletions(-) diff --git a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/input/chain/ChainInputTests.java b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/input/chain/ChainInputTests.java index b34870421aca5..b42daa3c21546 100644 --- a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/input/chain/ChainInputTests.java +++ b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/input/chain/ChainInputTests.java @@ -227,32 +227,22 @@ public void testParsingShouldBeStrictWhenStartingInputs() throws Exception { } public void testThatXContentParametersArePassedToInputs() throws Exception { - ToXContent.Params params = new ToXContent.MapParams(Collections.singletonMap(randomAlphaOfLength(5), randomAlphaOfLength(5))); - ChainInput chainInput = new ChainInput(Collections.singletonList(Tuple.tuple("second", new TestInput(params)))); + ToXContent.Params randomParams = new ToXContent.MapParams(Collections.singletonMap(randomAlphaOfLength(5), randomAlphaOfLength(5))); + ChainInput chainInput = new ChainInput(Collections.singletonList(Tuple.tuple("second", new Input() { + @Override + public String type() { + return "test"; + } + + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + assertThat(params, sameInstance(randomParams)); + return builder; + } + }))); try (XContentBuilder builder = jsonBuilder()) { - chainInput.toXContent(builder, params); - } - } - - // a test input that checks the xcontent parameters to ensure the correct params have been passed - private static final class TestInput implements Input { - - private ToXContent.Params expectedParams; - - TestInput(ToXContent.Params expectedParams) { - this.expectedParams = expectedParams; - } - - @Override - public String type() { - return "test"; - } - - @Override - public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - assertThat(params, sameInstance(expectedParams)); - return builder; + chainInput.toXContent(builder, randomParams); } } } From 915e2b4ea877c4a895cc20022d7dd59413fdb1b8 Mon Sep 17 00:00:00 2001 From: Alexander Reelsen Date: Mon, 2 Jul 2018 11:42:29 +0200 Subject: [PATCH 3/3] minor changes --- .../xpack/watcher/input/chain/ChainInputTests.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/input/chain/ChainInputTests.java b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/input/chain/ChainInputTests.java index b42daa3c21546..cc19cef7b4768 100644 --- a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/input/chain/ChainInputTests.java +++ b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/input/chain/ChainInputTests.java @@ -31,7 +31,6 @@ import org.elasticsearch.xpack.watcher.input.simple.SimpleInputFactory; import org.elasticsearch.xpack.watcher.test.WatcherTestUtils; -import java.io.IOException; import java.util.Collections; import java.util.HashMap; import java.util.Map; @@ -228,14 +227,14 @@ public void testParsingShouldBeStrictWhenStartingInputs() throws Exception { public void testThatXContentParametersArePassedToInputs() throws Exception { ToXContent.Params randomParams = new ToXContent.MapParams(Collections.singletonMap(randomAlphaOfLength(5), randomAlphaOfLength(5))); - ChainInput chainInput = new ChainInput(Collections.singletonList(Tuple.tuple("second", new Input() { + ChainInput chainInput = new ChainInput(Collections.singletonList(Tuple.tuple("whatever", new Input() { @Override public String type() { return "test"; } @Override - public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + public XContentBuilder toXContent(XContentBuilder builder, Params params) { assertThat(params, sameInstance(randomParams)); return builder; }