Skip to content

Commit

Permalink
chore: remove user labelling on unknown nodes (#776)
Browse files Browse the repository at this point in the history
fix: update tests to account for this behavior
  • Loading branch information
superlinkx authored Aug 6, 2024
1 parent d32fedb commit 3319444
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 19 deletions.
7 changes: 3 additions & 4 deletions cmd/api/src/analysis/hybrid/hybrid_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,7 @@ func TestHybridAttackPaths(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 upgraded to a user node and used for the path. SyncedToADUser and SyncedToEntraUser
// edges should be created and linked to new ADUser node.
// The existing node should be used to create SyncedToADUser and SyncedToEntraUser edges.
testContext.DatabaseTestWithSetup(
func(harness *integration.HarnessDetails) error {
adUserObjectID := ""
Expand Down Expand Up @@ -202,7 +201,7 @@ func verifyHybridPaths(t *testing.T, db graph.Database, harness integration.Harn

// Ensure we got the correct node types
assert.True(t, start.Kinds.ContainsOneOf(azure.User))
assert.True(t, end.Kinds.ContainsOneOf(ad.User))
assert.True(t, end.Kinds.ContainsOneOf(ad.User, ad.Entity))

// Verify the AZUser is the first node
assert.Equal(t, harness.HybridAttackPaths.AZUserObjectID, startObjectID)
Expand Down Expand Up @@ -249,7 +248,7 @@ func verifyHybridPaths(t *testing.T, db graph.Database, harness integration.Harn
assert.Nil(t, err)

// Ensure we got the correct node types
assert.True(t, start.Kinds.ContainsOneOf(ad.User))
assert.True(t, start.Kinds.ContainsOneOf(ad.User, ad.Entity))
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
Expand Down
19 changes: 4 additions & 15 deletions packages/go/analysis/hybrid/hybrid.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,28 +174,17 @@ func createMissingADUser(ctx context.Context, db graph.Database, objectID string
common.ObjectID.String(): objectID,
})

// Using a switch to make the complex error handling logic more clear
err = db.WriteTransaction(ctx, func(tx graph.Transaction) error {
newNode, err = analysis.FetchNodeByObjectID(tx, objectID)
switch {
// No node found, so it's safe to create a new AD User
case errors.Is(err, graph.ErrNoResultsFound):
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
}
// Node was found, so we need to update it to an AD User
case err == nil:
newNode.AddKinds(adSchema.User)
if err := tx.UpdateNode(newNode); err != nil {
return fmt.Errorf("update missing ad user label: %w", err)
} else {
return nil
}
// Database error while checking for node
default:
} else if err != nil {
return fmt.Errorf("create missing ad user precheck: %w", err)
} else {
return nil
}
})

Expand Down

0 comments on commit 3319444

Please sign in to comment.