Don't overwrite target field with SetSecurityUserProcessor (elastic#5…

…1454) * 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 elastic#51428 * Unused imports removed
probakowski · Jan 27, 2020 · b5e80fd · b5e80fd
1 parent ab5a9a9
commit b5e80fd
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 8 deletions.
diff --git a/...urity/src/main/java/org/elasticsearch/xpack/security/ingest/SetSecurityUserProcessor.java b/...urity/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<String, Object> userObject = new HashMap<>();
+        Object fieldValue = ingestDocument.getFieldValue(field, Object.class, true);
+
+        @SuppressWarnings("unchecked")
+        Map<String, Object> userObject = fieldValue instanceof Map ? (Map<String, Object>) fieldValue : new HashMap<>();
+
         for (Property property : properties) {
             switch (property) {
                 case USERNAME:

diff --git a/.../src/test/java/org/elasticsearch/xpack/security/ingest/SetSecurityUserProcessorTests.java b/.../src/test/java/org/elasticsearch/xpack/security/ingest/SetSecurityUserProcessorTests.java
@@ -15,9 +15,9 @@
 import org.elasticsearch.xpack.security.ingest.SetSecurityUserProcessor.Property;
 
 import java.util.Collections;
+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;
@@ -38,9 +38,7 @@ public void testProcessor() throws Exception {
         Map<String, Object> 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));
@@ -94,9 +92,7 @@ public void testRolesProperties() throws Exception {
         @SuppressWarnings("unchecked")
         Map<String, Object> 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 {
@@ -148,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<String, Object> 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<String, Object> result2 = ingestDocument.getFieldValue("_field", Map.class);
+        assertThat(result2.size(), equalTo(2));
+        assertThat(result2.get("username"), equalTo("_username"));
+        assertThat(result2.get("other"), equalTo("test"));
+    }
+
 }