From 978144f11851618cc737ae0f1a7bcbbc244eafa6 Mon Sep 17 00:00:00 2001 From: Przemko Robakowski Date: Sat, 25 Jan 2020 00:06:31 +0100 Subject: [PATCH 1/2] Don't overwrite target field with SetSecurityUserProcessor This change fix problem with `SetSecurityUserProcessor` which was overwriting whole target field and not only fields really filled by the processor. Closes #51428 --- .../ingest/SetSecurityUserProcessor.java | 6 ++- .../ingest/SetSecurityUserProcessorTests.java | 38 ++++++++++++++++--- 2 files changed, 37 insertions(+), 7 deletions(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/ingest/SetSecurityUserProcessor.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/ingest/SetSecurityUserProcessor.java index 0c30af1879cc9..791e700ff6937 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/ingest/SetSecurityUserProcessor.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/ingest/SetSecurityUserProcessor.java @@ -53,7 +53,11 @@ public IngestDocument execute(IngestDocument ingestDocument) throws Exception { throw new IllegalStateException("No user for authentication"); } - Map userObject = new HashMap<>(); + Object fieldValue = ingestDocument.getFieldValue(field, Object.class, true); + + @SuppressWarnings("unchecked") + Map userObject = fieldValue instanceof Map ? (Map) fieldValue : new HashMap<>(); + for (Property property : properties) { switch (property) { case USERNAME: diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/ingest/SetSecurityUserProcessorTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/ingest/SetSecurityUserProcessorTests.java index 77d1ba667d1b9..65f0863fe3166 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/ingest/SetSecurityUserProcessorTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/ingest/SetSecurityUserProcessorTests.java @@ -14,6 +14,7 @@ import org.elasticsearch.xpack.core.security.user.User; import org.elasticsearch.xpack.security.ingest.SetSecurityUserProcessor.Property; +import java.util.Arrays; import java.util.EnumSet; import java.util.HashMap; import java.util.List; @@ -37,9 +38,7 @@ public void testProcessor() throws Exception { Map result = ingestDocument.getFieldValue("_field", Map.class); assertThat(result.size(), equalTo(5)); assertThat(result.get("username"), equalTo("_username")); - assertThat(((List) result.get("roles")).size(), equalTo(2)); - assertThat(((List) result.get("roles")).get(0), equalTo("role1")); - assertThat(((List) result.get("roles")).get(1), equalTo("role2")); + assertThat(result.get("roles"), equalTo(Arrays.asList("role1", "role2"))); assertThat(result.get("full_name"), equalTo("firstname lastname")); assertThat(result.get("email"), equalTo("_email")); assertThat(((Map) result.get("metadata")).size(), equalTo(1)); @@ -93,9 +92,7 @@ public void testRolesProperties() throws Exception { @SuppressWarnings("unchecked") Map result = ingestDocument.getFieldValue("_field", Map.class); assertThat(result.size(), equalTo(1)); - assertThat(((List) result.get("roles")).size(), equalTo(2)); - assertThat(((List) result.get("roles")).get(0), equalTo("role1")); - assertThat(((List) result.get("roles")).get(1), equalTo("role2")); + assertThat(result.get("roles"), equalTo(Arrays.asList("role1", "role2"))); } public void testFullNameProperties() throws Exception { @@ -147,4 +144,33 @@ public void testMetadataProperties() throws Exception { assertThat(((Map) result.get("metadata")).get("key"), equalTo("value")); } + public void testOverwriteExistingField() throws Exception { + ThreadContext threadContext = new ThreadContext(Settings.EMPTY); + User user = new User("_username", null, null); + Authentication.RealmRef realmRef = new Authentication.RealmRef("_name", "_type", "_node_name"); + threadContext.putTransient(AuthenticationField.AUTHENTICATION_KEY, new Authentication(user, realmRef, null)); + + SetSecurityUserProcessor processor = new SetSecurityUserProcessor("_tag", threadContext, "_field", EnumSet.of(Property.USERNAME)); + + IngestDocument ingestDocument = new IngestDocument(new HashMap<>(), new HashMap<>()); + ingestDocument.setFieldValue("_field", "test"); + processor.execute(ingestDocument); + + @SuppressWarnings("unchecked") + Map result = ingestDocument.getFieldValue("_field", Map.class); + assertThat(result.size(), equalTo(1)); + assertThat(result.get("username"), equalTo("_username")); + + ingestDocument = new IngestDocument(new HashMap<>(), new HashMap<>()); + ingestDocument.setFieldValue("_field.other", "test"); + ingestDocument.setFieldValue("_field.username", "test"); + processor.execute(ingestDocument); + + @SuppressWarnings("unchecked") + Map result2 = ingestDocument.getFieldValue("_field", Map.class); + assertThat(result2.size(), equalTo(2)); + assertThat(result2.get("username"), equalTo("_username")); + assertThat(result2.get("other"), equalTo("test")); + } + } From d33f2e3192550a78055da8c5fc6ef27e9e19f668 Mon Sep 17 00:00:00 2001 From: Przemko Robakowski Date: Sat, 25 Jan 2020 01:20:39 +0100 Subject: [PATCH 2/2] Unused imports removed --- .../xpack/security/ingest/SetSecurityUserProcessorTests.java | 1 - 1 file changed, 1 deletion(-) diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/ingest/SetSecurityUserProcessorTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/ingest/SetSecurityUserProcessorTests.java index 65f0863fe3166..3c82da113aeea 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/ingest/SetSecurityUserProcessorTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/ingest/SetSecurityUserProcessorTests.java @@ -17,7 +17,6 @@ import java.util.Arrays; import java.util.EnumSet; import java.util.HashMap; -import java.util.List; import java.util.Map; import static org.hamcrest.Matchers.equalTo;