feat: Cease creation of AD stub users during Azure ingest, post-processing - BED-7710

cmd/api/src/analysis/hybrid/hybrid_integration_test.go

@@ -55,7 +55,7 @@ func TestHybridAttackPaths(t *testing.T) {
 				}
 				operation.Done()
 
-				verifyHybridPaths(t, db, harness, true, true)
+				verifyHybridPaths(t, db, harness, true)
 			},
 		)
 	})
@@ -80,7 +80,7 @@ func TestHybridAttackPaths(t *testing.T) {
 				}
 				operation.Done()
 
-				verifyHybridPaths(t, db, harness, false, true)
+				verifyHybridPaths(t, db, harness, false)
 			},
 		)
 	})
@@ -104,14 +104,14 @@ func TestHybridAttackPaths(t *testing.T) {
 				}
 				operation.Done()
 
-				verifyHybridPaths(t, db, harness, false, true)
+				verifyHybridPaths(t, db, harness, false)
 			},
 		)
 	})
 
-	t.Run("SyncedEdgesCreatedWithPlaceholderADNode", func(t *testing.T) {
+	t.Run("SyncedEdgesNotCreatedWithoutMatchingADUser", func(t *testing.T) {
 		// ADUser does not exist. AZUser has OnPremID and OnPremSyncEnabled=true
-		// A new ADUser node should be created. SyncedToADUser and SyncedToEntraUser edges should be created and linked to new ADUser node.
+		// No ADUser node should be created. SyncedToADUser and SyncedToEntraUser edges should not be created.
 		testContext := integration.NewGraphTestContext(t, graphschema.DefaultGraphSchema())
 		testContext.DatabaseTestWithSetup(
 			func(harness *integration.HarnessDetails) error {
@@ -128,15 +128,15 @@ func TestHybridAttackPaths(t *testing.T) {
 				}
 				operation.Done()
 
-				verifyHybridPaths(t, db, harness, true, true)
+				verifyHybridPaths(t, db, harness, false)
 			},
 		)
 	})
 
-	t.Run("CreateSyncedEdges NonUserADNode", func(t *testing.T) {
+	t.Run("SyncedEdgesNotCreatedForUnknownADEntity", func(t *testing.T) {
 		// ADUser does not exist, but the objectid from a selected AZUser exists in the graph. Selected AZUser has OnPremID and
 		// OnPremSyncEnabled=true
-		// The existing node should be used to create SyncedToADUser and SyncedToEntraUser edges.
+		// Hybrid post-processing only links existing AD user nodes, so no synced edges should be created.
 		testContext := integration.NewGraphTestContext(t, graphschema.DefaultGraphSchema())
 		testContext.DatabaseTestWithSetup(
 			func(harness *integration.HarnessDetails) error {
@@ -153,14 +153,14 @@ func TestHybridAttackPaths(t *testing.T) {
 				}
 				operation.Done()
 
-				verifyHybridPaths(t, db, harness, true, false)
+				verifyHybridPaths(t, db, harness, false)
 			},
 		)
 	})
 
-	t.Run("CreateSyncedEdges ObjectID No Match OnPremID", func(t *testing.T) {
+	t.Run("SyncedEdgesNotCreatedWhenADObjectIDDoesNotMatchOnPremID", func(t *testing.T) {
 		// ADUser.ObjectID does NOT match AZUser.OnPremID, AZUser.OnPremSyncEnabled is true
-		// SyncedToEntraUser and SyncedToADUser edges should be created, but a new ADUser node should be created with ObjectID that matches AZUser.OnPremID
+		// SyncedToEntraUser and SyncedToADUser edges should not be created.
 		testContext := integration.NewGraphTestContext(t, graphschema.DefaultGraphSchema())
 		testContext.DatabaseTestWithSetup(
 			func(harness *integration.HarnessDetails) error {
@@ -177,13 +177,13 @@ func TestHybridAttackPaths(t *testing.T) {
 				}
 				operation.Done()
 
-				verifyHybridPaths(t, db, harness, true, true)
+				verifyHybridPaths(t, db, harness, false)
 			},
 		)
 	})
 }
 
-func verifyHybridPaths(t *testing.T, db graph.Database, harness integration.HarnessDetails, shouldHaveEdges bool, shouldHaveUserNode bool) {
+func verifyHybridPaths(t *testing.T, db graph.Database, harness integration.HarnessDetails, shouldHaveEdges bool) {
 	expectedEdgeCount := 1
 	if !shouldHaveEdges {
 		expectedEdgeCount = 0
@@ -218,24 +218,15 @@ func verifyHybridPaths(t *testing.T, db graph.Database, harness integration.Harn
 			assert.Nil(t, err)
 
 			// Ensure we got the correct node types
-			if shouldHaveUserNode {
-				assert.True(t, end.Kinds.ContainsOneOf(ad.User))
-			} else {
-				assert.True(t, end.Kinds.ContainsOneOf(ad.Entity))
-			}
+			assert.True(t, end.Kinds.ContainsOneOf(ad.User))
 			assert.True(t, start.Kinds.ContainsOneOf(azure.User))
 
-			// Verify the AZUser is the first node
+			// Verify the AZUser is the first node, User is the second
 			assert.Equal(t, harness.HybridAttackPaths.AZUserObjectID, startObjectID)
+			assert.Equal(t, harness.HybridAttackPaths.ADUserObjectID, endObjectID)
 
-			// Verify the ADUser, but we have to handle the case where the ADUser node is created by the post-processing logic
-			if harness.HybridAttackPaths.ADUserObjectID != startObjectOnPremId {
-				// Node was created during post-processing. Pull AZUser.OnPremID from the node itself.
-				assert.Equal(t, startObjectOnPremId, endObjectID)
-			} else {
-				// Node existed prior to post-processing. Use the node configured during setup.
-				assert.Equal(t, harness.HybridAttackPaths.ADUserObjectID, endObjectID)
-			}
+			// Verify the AZUser OnPremID property matches the User's ObjectID
+			assert.Equal(t, startObjectOnPremId, endObjectID)
 		}
 
 		return nil
@@ -270,22 +261,15 @@ func verifyHybridPaths(t *testing.T, db graph.Database, harness integration.Harn
 			assert.Nil(t, err)
 
 			// Ensure we got the correct node types
-			if shouldHaveUserNode {
-				assert.True(t, start.Kinds.ContainsOneOf(ad.User))
-			} else {
-				assert.True(t, start.Kinds.ContainsOneOf(ad.Entity))
-			}
+			assert.True(t, start.Kinds.ContainsOneOf(ad.User))
 			assert.True(t, end.Kinds.ContainsOneOf(azure.User))
 
-			// Verify the ADUser, but we have to handle the case where the ADUser node is created by the post-processing logic
-			if harness.HybridAttackPaths.ADUserObjectID != endObjectOnPremId {
-				// Node was created during post-processing. Pull AZUser.OnPremID from the node itself.
-				assert.Equal(t, endObjectOnPremId, startObjectID)
-			} else {
-				// Node existed prior to post-processing. Use the node configured during setup.
-				assert.Equal(t, harness.HybridAttackPaths.ADUserObjectID, startObjectID)
-			}
+			// Verify the User is the first node, AZUser is the second
+			assert.Equal(t, harness.HybridAttackPaths.ADUserObjectID, startObjectID)
 			assert.Equal(t, harness.HybridAttackPaths.AZUserObjectID, endObjectID)
+
+			// Verify the User's ObjectID matches the AZUser's OnPremID property
+			assert.Equal(t, endObjectOnPremId, startObjectID)
 		}
 
 		return nil

cmd/api/src/services/graphify/azure_convertors.go

@@ -279,9 +279,6 @@ func convertAzureGroup(raw json.RawMessage, converted *ConvertedAzureData, inges
 		slog.Error(fmt.Sprintf(SerialError, "azure group", err))
 	} else {
 		converted.NodeProps = append(converted.NodeProps, ein.ConvertAzureGroupToNode(data, ingestTime))
-		if onPremNode := ein.ConvertAzureGroupToOnPremisesNode(data); onPremNode.IsValid() {
-			converted.OnPremNodes = append(converted.OnPremNodes, onPremNode)
-		}
 		converted.RelProps = append(converted.RelProps, ein.ConvertAzureGroupToRel(data))
 	}
 }
@@ -535,11 +532,8 @@ func convertAzureUser(raw json.RawMessage, converted *ConvertedAzureData, ingest
 	if err := json.Unmarshal(raw, &data); err != nil {
 		slog.Error(fmt.Sprintf(SerialError, "azure user", err))
 	} else {
-		node, onPremNode, rel := ein.ConvertAzureUser(data, ingestTime)
+		node, rel := ein.ConvertAzureUser(data, ingestTime)
 		converted.NodeProps = append(converted.NodeProps, node)
-		if onPremNode.IsValid() {
-			converted.OnPremNodes = append(converted.OnPremNodes, onPremNode)
-		}
 		converted.RelProps = append(converted.RelProps, rel)
 	}
 }

cmd/api/src/services/graphify/ingest.go

@@ -277,10 +277,6 @@ func IngestAzureData(batch *IngestContext, converted ConvertedAzureData) error {
 		errs.Add(err)
 	}
 
-	if err := IngestNodes(batch, ad.Entity, converted.OnPremNodes); err != nil {
-		errs.Add(err)
-	}
-
 	if err := IngestRelationships(batch, azure.Entity, converted.RelProps); err != nil {
 		errs.Add(err)
 	}

cmd/api/src/services/graphify/models.go

@@ -59,13 +59,11 @@ type AzureBase struct {
 }
 
 type ConvertedAzureData struct {
-	NodeProps   []ein.IngestibleNode
-	RelProps    []ein.IngestibleRelationship
-	OnPremNodes []ein.IngestibleNode
+	NodeProps []ein.IngestibleNode
+	RelProps  []ein.IngestibleRelationship
 }
 
 func (s *ConvertedAzureData) Clear() {
 	s.NodeProps = s.NodeProps[:0]
 	s.RelProps = s.RelProps[:0]
-	s.OnPremNodes = s.OnPremNodes[:0]
 }

packages/go/analysis/hybrid/hybrid.go

@@ -56,8 +56,6 @@ func PostHybrid(ctx context.Context, db graph.Database) (*analysis.AtomicPostPro
 
 	err = db.ReadTransaction(ctx, func(tx graph.Transaction) error {
 		var (
-			// entraObjIDMap is used to index AD user objectids by Entra node ids
-			entraObjIDMap = make(map[graph.ID]string, 1024)
 			// adObjIDMap is used as a reverse mapping of a list of Entra node ids indexed by the AD user objectids
 			adObjIDMap = make(map[string][]graph.ID, 1024)
 			// entraToADMap is the final mapping between an Entra user node id to an AD user node id
@@ -81,13 +79,8 @@ func PostHybrid(ctx context.Context, db graph.Database) (*analysis.AtomicPostPro
 					} else if err != nil {
 						return err
 					} else {
-						// We know this user has an onPrem counterpart, so add the node id and onPremID to our three maps
+						// We know this user has an onPrem counterpart, so add the node id and onPremID to our mapping inputs.
 						adObjIDMap[onPremID] = append(adObjIDMap[onPremID], tenantUser.ID)
-						entraObjIDMap[tenantUser.ID] = onPremID
-
-						// Initialize the current user id as an index in the entraToADMap, but use 0 as the nodeid for AD since we
-						// currently don't know it and 0 is never going to be a valid user node id
-						entraToADMap[tenantUser.ID] = 0
 					}
 				}
 			}
@@ -104,7 +97,7 @@ func PostHybrid(ctx context.Context, db graph.Database) (*analysis.AtomicPostPro
 				if objectID, err := adUser.Properties.Get(common.ObjectID.String()).String(); err != nil {
 					return err
 				} else if azUsers, ok := adObjIDMap[objectID]; !ok {
-					// Skip adding this relationship if we've already seen it before as that implies it will be created
+					// Skip AD users that do not correspond to any synced Entra users.
 					continue
 				} else {
 					// Because there could theoretically be more than one Entra user mapped to this objectid, we want to loop through all when adding our current id to the final map
@@ -116,19 +109,7 @@ func PostHybrid(ctx context.Context, db graph.Database) (*analysis.AtomicPostPro
 		}
 
 		if err := operation.Operation.SubmitReader(func(ctx context.Context, tx graph.Transaction, outC chan<- analysis.CreatePostRelationshipJob) error {
-			for azUser, potentialADUser := range entraToADMap {
-				var adUser = potentialADUser
-
-				// The 0 value should never be a valid id for an AD user node, just by the nature of the graph, so we're cheating
-				// by checking if we set it to 0 as a flag that this node was never actually found, meaning it needs to be created first
-				if potentialADUser == 0 {
-					if adUserNode, err := createMissingADUser(ctx, db, entraObjIDMap[azUser]); err != nil {
-						return err
-					} else {
-						adUser = adUserNode.ID
-					}
-				}
-
+			for azUser, adUser := range entraToADMap {
 				SyncedToEntraUserRelationship := analysis.CreatePostRelationshipJob{
 					FromID: adUser,
 					ToID:   azUser,
@@ -183,35 +164,6 @@ func hasOnPremUser(node *graph.Node) (string, bool, error) {
 	}
 }
 
-// createMissingADUser will create a new standalone AD User node with the required objectID for displaying in hybrid graphs
-func createMissingADUser(ctx context.Context, db graph.Database, objectID string) (*graph.Node, error) {
-	var (
-		err     error
-		newNode *graph.Node
-	)
-
-	slog.DebugContext(ctx, fmt.Sprintf("Matching AD User node with objectID %s not found, creating a new one", objectID))
-	properties := graph.AsProperties(map[string]any{
-		common.ObjectID.String(): objectID,
-	})
-
-	err = db.WriteTransaction(ctx, func(tx graph.Transaction) error {
-		if newNode, err = analysis.FetchNodeByObjectID(tx, objectID); errors.Is(err, graph.ErrNoResultsFound) {
-			if newNode, err = tx.CreateNode(properties, adSchema.Entity, adSchema.User); err != nil {
-				return fmt.Errorf("create missing ad user: %w", err)
-			} else {
-				return nil
-			}
-		} else if err != nil {
-			return fmt.Errorf("create missing ad user precheck: %w", err)
-		} else {
-			return nil
-		}
-	})
-
-	return newNode, err
-}
-
 // fetchEntraUsers fetches all the Entra users for a given root node (generally the tenant node)
 func fetchEntraUsers(tx graph.Transaction, root *graph.Node) (graph.NodeSet, error) {
 	return ops.FetchEndNodes(tx.Relationships().Filterf(func() graph.Criteria {

packages/go/ein/azure.go

@@ -30,7 +30,6 @@ import (
 	"github.com/bloodhoundad/azurehound/v2/enums"
 	"github.com/bloodhoundad/azurehound/v2/models"
 	azure2 "github.com/bloodhoundad/azurehound/v2/models/azure"
-	"github.com/specterops/bloodhound/packages/go/graphschema/ad"
 	"github.com/specterops/bloodhound/packages/go/graphschema/azure"
 	"github.com/specterops/bloodhound/packages/go/graphschema/common"
 	"github.com/specterops/dawgs/graph"
@@ -448,22 +447,6 @@ func ConvertAzureGroupToNode(data models.Group, ingestTime time.Time) Ingestible
 	}
 }
 
-func ConvertAzureGroupToOnPremisesNode(data models.Group) IngestibleNode {
-	if data.OnPremisesSecurityIdentifier != "" {
-		return IngestibleNode{
-			ObjectID:    strings.ToUpper(data.OnPremisesSecurityIdentifier),
-			PropertyMap: map[string]any{},
-			Labels:      []graph.Kind{ad.Group},
-		}
-	}
-
-	return IngestibleNode{
-		ObjectID:    "",
-		PropertyMap: nil,
-		Labels:      nil,
-	}
-}
-
 func ConvertAzureGroupToRel(data models.Group) IngestibleRelationship {
 	return NewIngestibleRelationship(
 		IngestibleEndpoint{
@@ -1238,17 +1221,8 @@ func ConvertAzureTenantToNode(data models.Tenant, ingestTime time.Time) Ingestib
 	return node
 }
 
-// ConvertAzureUser returns the basic node, the on prem node and then the ingestible contains relationship
-func ConvertAzureUser(data models.User, ingestTime time.Time) (IngestibleNode, IngestibleNode, IngestibleRelationship) {
-	onPremNode := IngestibleNode{}
-	if data.OnPremisesSecurityIdentifier != "" {
-		onPremNode = IngestibleNode{
-			ObjectID:    strings.ToUpper(data.OnPremisesSecurityIdentifier),
-			PropertyMap: map[string]any{},
-			Labels:      []graph.Kind{ad.User},
-		}
-	}
-
+// ConvertAzureUser returns the basic node and the ingestible contains relationship.
+func ConvertAzureUser(data models.User, ingestTime time.Time) (IngestibleNode, IngestibleRelationship) {
 	properties := map[string]any{
 		common.Name.String():             strings.ToUpper(data.UserPrincipalName),
 		common.Enabled.String():          data.AccountEnabled,
@@ -1273,7 +1247,7 @@ func ConvertAzureUser(data models.User, ingestTime time.Time) (IngestibleNode, I
 			ObjectID:    strings.ToUpper(data.Id),
 			PropertyMap: properties,
 			Labels:      []graph.Kind{azure.User},
-		}, onPremNode, NewIngestibleRelationship(
+		}, NewIngestibleRelationship(
 			IngestibleEndpoint{
 				Value: strings.ToUpper(data.TenantId),
 				Kind:  azure.Tenant,