Don't overwrite target field with SetSecurityUserProcessor (#51454)

* 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 * Unused imports removed
elastic · Jan 27, 2020 · 8f87bb3 · 8f87bb3
1 parent 6c9ca44
commit 8f87bb3
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
@@ -14,9 +14,9 @@
 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;
 import java.util.Map;
 
 import static org.hamcrest.Matchers.equalTo;
@@ -37,9 +37,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));
@@ -93,9 +91,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 {
@@ -147,4 +143,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"));
+    }
+
 }