Fix ingest decoder bug (#462)

* fix: allocate struct properly in decoders to prevent re-use of underlying map * test: add tests for property assertions
SpecterOps · Mar 5, 2024 · 9ae711a · 9ae711a
1 parent 5cc5174
commit 9ae711a
Show file tree

Hide file tree

Showing 4 changed files with 47 additions and 3 deletions.
diff --git a/cmd/api/src/api/v2/file_uploads_integration_test.go b/cmd/api/src/api/v2/file_uploads_integration_test.go
@@ -179,6 +179,7 @@ func Test_FileUploadWorkFlowVersion6(t *testing.T) {
 	//Assert that we created stuff we expected
 	testCtx.AssertIngest(fixtures.IngestAssertions)
 	testCtx.AssertIngest(fixtures.IngestAssertionsv6)
+	testCtx.AssertIngest(fixtures.PropertyAssertions)
 }
 
 func Test_FileUploadVersion6AllOptionADCS(t *testing.T) {
@@ -219,6 +220,7 @@ func Test_CompressedFileUploadWorkFlowVersion5(t *testing.T) {
 
 	//Assert that we created stuff we expected
 	testCtx.AssertIngest(fixtures.IngestAssertions)
+	testCtx.AssertIngest(fixtures.PropertyAssertions)
 }
 
 func Test_CompressedFileUploadWorkFlowVersion6(t *testing.T) {
@@ -239,4 +241,5 @@ func Test_CompressedFileUploadWorkFlowVersion6(t *testing.T) {
 	//Assert that we created stuff we expected
 	testCtx.AssertIngest(fixtures.IngestAssertions)
 	testCtx.AssertIngest(fixtures.IngestAssertionsv6)
+	testCtx.AssertIngest(fixtures.PropertyAssertions)
 }
diff --git a/cmd/api/src/api/v2/integration/ingest.go b/cmd/api/src/api/v2/integration/ingest.go
@@ -148,3 +148,13 @@ func (s *Context) AssertIngest(assertion IngestAssertion) {
 		return nil
 	}), "Unexpected database error during reconciliation assertion")
 }
+
+func (s *Context) AssertIngestProperties(assertion IngestAssertion) {
+	graphDB := integration.OpenGraphDB(s.TestCtrl)
+	defer graphDB.Close(s.ctx)
+
+	require.Nil(s.TestCtrl, graphDB.ReadTransaction(s.ctx, func(tx graph.Transaction) error {
+		assertion(s.TestCtrl, tx)
+		return nil
+	}), "Unexpected database error during reconciliation assertion")
+}
diff --git a/cmd/api/src/daemons/datapipe/decoders.go b/cmd/api/src/daemons/datapipe/decoders.go
@@ -73,11 +73,11 @@ func decodeGroupData(batch graph.Batch, reader io.ReadSeeker) error {
 
 	var (
 		convertedData = ConvertedGroupData{}
-		group         ein.Group
 		count         = 0
 	)
 
 	for decoder.More() {
+		var group ein.Group
 		if err := decoder.Decode(&group); err != nil {
 			log.Errorf("Error decoding group object: %v", err)
 		} else {
@@ -106,10 +106,10 @@ func decodeSessionData(batch graph.Batch, reader io.ReadSeeker) error {
 
 	var (
 		convertedData = ConvertedSessionData{}
-		session       ein.Session
 		count         = 0
 	)
 	for decoder.More() {
+		var session ein.Session
 		if err := decoder.Decode(&session); err != nil {
 			log.Errorf("Error decoding session object: %v", err)
 		} else {
@@ -138,11 +138,11 @@ func decodeAzureData(batch graph.Batch, reader io.ReadSeeker) error {
 
 	var (
 		convertedData = ConvertedAzureData{}
-		data          AzureBase
 		count         = 0
 	)
 
 	for decoder.More() {
+		var data AzureBase
 		if err := decoder.Decode(&data); err != nil {
 			log.Errorf("Error decoding azure object: %v", err)
 		} else {

diff --git a/cmd/api/src/test/fixtures/fixtures/expected_ingest.go b/cmd/api/src/test/fixtures/fixtures/expected_ingest.go
@@ -28,6 +28,12 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+type PropertyAssertion struct {
+	ObjectID      string
+	Property      string
+	ExpectedValue any
+}
+
 var (
 	ingestRelationshipAssertionCriteria = []graph.Criteria{
 		//// DOMAINS
@@ -252,6 +258,19 @@ var (
 			query.Kind(query.End(), ad.Domain),
 			query.Equals(query.EndProperty(common.ObjectID.String()), "S-1-5-21-3130019616-2776909439-2417379446")),
 	}
+
+	propertyAssertionCriteria = []PropertyAssertion{
+		{
+			ObjectID:      "TESTLAB.LOCAL-S-1-5-32-550",
+			Property:      common.Name.String(),
+			ExpectedValue: "PRINT OPERATORS@TESTLAB.LOCAL",
+		},
+		{
+			ObjectID:      "S-1-5-21-3130019616-2776909439-2417379446-521",
+			Property:      common.Name.String(),
+			ExpectedValue: "READ-ONLY DOMAIN CONTROLLERS@TESTLAB.LOCAL",
+		},
+	}
 )
 
 func FormatQueryComponent(criteria graph.Criteria) string {
@@ -280,3 +299,15 @@ func IngestAssertionsv6(testCtrl test.Controller, tx graph.Transaction) {
 		require.Nilf(testCtrl, err, "Unable to find an expected relationship: %s", FormatQueryComponent(assertionCriteria))
 	}
 }
+
+func PropertyAssertions(testCtrl test.Controller, tx graph.Transaction) {
+	for _, assertionCriteria := range propertyAssertionCriteria {
+		node, err := tx.Nodes().Filterf(func() graph.Criteria {
+			return query.Equals(query.NodeProperty(common.ObjectID.String()), assertionCriteria.ObjectID)
+		}).First()
+		require.Nilf(testCtrl, err, "Unable to find expected node %s", assertionCriteria.ObjectID)
+		prop := node.Properties.Get(assertionCriteria.Property).Any()
+		require.Equal(testCtrl, assertionCriteria.ExpectedValue, prop)
+		require.Nilf(testCtrl, err, "Unable to find an expected relationship: %s", FormatQueryComponent(assertionCriteria))
+	}
+}