From baccaad8b5717a022daf158a0383839cd4cf3ed4 Mon Sep 17 00:00:00 2001
From: Andrew Kroh <andrew.kroh@elastic.co>
Date: Tue, 9 Jun 2020 12:36:55 -0400
Subject: [PATCH] Fix translate_sid's empty target field handling (#18991)

The translate_sid processor only requires one of the three target fields to be configured. It should work properly when some of the targets are not set, but it doesn't check if the are empty strings. So it ends up adding target fields that are empty strings to the event (e.g. "": "Group").

Closes #18990
---
 CHANGELOG.next.asciidoc                       |  1 +
 .../processors/translate_sid/translatesid.go  | 18 ++++++----
 .../translate_sid/translatesid_test.go        | 36 +++++++++++++++++++
 3 files changed, 49 insertions(+), 6 deletions(-)

diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc
index 1a582ba08a2..e0b88067f46 100644
--- a/CHANGELOG.next.asciidoc
+++ b/CHANGELOG.next.asciidoc
@@ -119,6 +119,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d
 - Fix an issue where error messages are not accurate in mapstriface. {issue}18662[18662] {pull}18663[18663]
 - Fix regression in `add_kubernetes_metadata`, so configured `indexers` and `matchers` are used if defaults are not disabled. {issue}18481[18481] {pull}18818[18818]
 - Fix potential race condition in fingerprint processor. {pull}18738[18738]
+- Fix the `translate_sid` processor's handling of unconfigured target fields. {issue}18990[18990] {pull}18991[18991]
 - Fixed a service restart failure under Windows. {issue}18914[18914] {pull}18916[18916]
 - The `monitoring.elasticsearch.api_key` value is correctly base64-encoded before being sent to the monitoring Elasticsearch cluster. {issue}18939[18939] {pull}18945[18945]
 
diff --git a/libbeat/processors/translate_sid/translatesid.go b/libbeat/processors/translate_sid/translatesid.go
index 491ed66859f..f794019d78e 100644
--- a/libbeat/processors/translate_sid/translatesid.go
+++ b/libbeat/processors/translate_sid/translatesid.go
@@ -111,14 +111,20 @@ func (p *processor) translateSID(event *beat.Event) error {
 
 	// Do all operations even if one fails.
 	var errs []error
-	if _, err = event.PutValue(p.AccountNameTarget, account); err != nil {
-		errs = append(errs, err)
+	if p.AccountNameTarget != "" {
+		if _, err = event.PutValue(p.AccountNameTarget, account); err != nil {
+			errs = append(errs, err)
+		}
 	}
-	if _, err = event.PutValue(p.AccountTypeTarget, sys.SIDType(accountType).String()); err != nil {
-		errs = append(errs, err)
+	if p.AccountTypeTarget != "" {
+		if _, err = event.PutValue(p.AccountTypeTarget, sys.SIDType(accountType).String()); err != nil {
+			errs = append(errs, err)
+		}
 	}
-	if _, err = event.PutValue(p.DomainTarget, domain); err != nil {
-		errs = append(errs, err)
+	if p.DomainTarget != "" {
+		if _, err = event.PutValue(p.DomainTarget, domain); err != nil {
+			errs = append(errs, err)
+		}
 	}
 	return multierr.Combine(errs...)
 }
diff --git a/libbeat/processors/translate_sid/translatesid_test.go b/libbeat/processors/translate_sid/translatesid_test.go
index ad1fbc6a4b8..529f90b065f 100644
--- a/libbeat/processors/translate_sid/translatesid_test.go
+++ b/libbeat/processors/translate_sid/translatesid_test.go
@@ -25,6 +25,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	"golang.org/x/sys/windows"
 
 	"github.com/elastic/beats/v7/libbeat/beat"
@@ -83,6 +84,41 @@ func TestTranslateSID(t *testing.T) {
 	}
 }
 
+func TestTranslateSIDEmptyTarget(t *testing.T) {
+	c := defaultConfig()
+	c.Field = "sid"
+
+	evt := &beat.Event{Fields: map[string]interface{}{
+		"sid": "S-1-5-32-544",
+	}}
+
+	tests := []struct {
+		Name   string
+		Config config
+	}{
+		{"account_name", config{Field: "sid", AccountNameTarget: "account_name"}},
+		{"account_type", config{Field: "sid", AccountTypeTarget: "account_type"}},
+		{"domain", config{Field: "sid", DomainTarget: "domain"}},
+	}
+
+	for _, tc := range tests {
+		tc := tc
+		t.Run(tc.Name, func(t *testing.T) {
+			p, err := newFromConfig(tc.Config)
+			require.NoError(t, err)
+
+			evt, err := p.Run(&beat.Event{Fields: evt.Fields.Clone()})
+			require.NoError(t, err)
+
+			// Verify that only the configured target field is set.
+			// And ensure that no empty string keys are used.
+			assert.Len(t, evt.Fields, 2)
+			assert.Contains(t, evt.Fields, tc.Name)
+			assert.NotContains(t, evt.Fields, "")
+		})
+	}
+}
+
 func BenchmarkProcessor_Run(b *testing.B) {
 	p, err := newFromConfig(config{
 		Field:             "sid",