Skip to content

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

Open
StephenHinck wants to merge 4 commits intomainfrom
BED-7710
Open

feat: Cease creation of AD stub users during Azure ingest, post-processing - BED-7710#2530
StephenHinck wants to merge 4 commits intomainfrom
BED-7710

Conversation

@StephenHinck
Copy link
Contributor

@StephenHinck StephenHinck commented Mar 20, 2026

Description

AI was utilized to support this PR. All output was manually reviewed and adjusted. Commits utilizing AI have been labeled accordingly.

Azure ingest currently inserts AD user stub nodes. These nodes are subsequently deleted in BHE environments, and then additional matched or recreated during Hybrid post-processing. This creates unnecessary churn on the database, and may create nodes for which no context exists for the risk they may generate. This change set:

  • Will no longer create stub nodes during Azure ingest or hybrid post-processing
  • Will only introduce hybrid edges for AZUser and User objects which have been collected in full (i.e. AzureHound and SharpHound collections).

Motivation and Context

Resolves BED-7710

Why is this change required? What problem does it solve?

How Has This Been Tested?

Validated locally, including utilizing multiple test datasets, and attempting to ingest new data over top to ensure consistent outcomes.

Screenshots (optional):

Types of changes

  • New feature (non-breaking change which adds functionality)

Checklist:

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Resolved incorrect generation of placeholder on-premises nodes in hybrid attack path analysis. The system now only includes actual synchronized Azure users and groups in attack paths, improving analysis accuracy and eliminating spurious entities.

@StephenHinck StephenHinck self-assigned this Mar 20, 2026
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 20, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: b01dd614-fe50-4375-a12b-10285b6b790a

📥 Commits

Reviewing files that changed from the base of the PR and between 827822b and d0dc73e.

📒 Files selected for processing (6)
  • cmd/api/src/analysis/hybrid/hybrid_integration_test.go
  • cmd/api/src/services/graphify/azure_convertors.go
  • cmd/api/src/services/graphify/ingest.go
  • cmd/api/src/services/graphify/models.go
  • packages/go/analysis/hybrid/hybrid.go
  • packages/go/ein/azure.go
💤 Files with no reviewable changes (1)
  • cmd/api/src/services/graphify/ingest.go

📝 Walkthrough

Walkthrough

This PR removes on-premises node generation from Azure data conversion pipelines. Azure groups and users are no longer converted to on-premises (AD) node representations. The createMissingADUser placeholder logic and related data structures are deleted, and on-premises node handling is removed from ingest, conversion, and hybrid analysis workflows.

Changes

Cohort / File(s) Summary
Core conversion logic
packages/go/ein/azure.go, cmd/api/src/services/graphify/azure_convertors.go
Removed on-premises node generation: deleted ConvertAzureGroupToOnPremisesNode function and updated ConvertAzureUser signature to return only (IngestibleNode, IngestibleRelationship) instead of including on-premises node. On-premises extractions removed from convertAzureGroup and convertAzureUser implementations.
Data model updates
cmd/api/src/services/graphify/models.go
Removed OnPremNodes field from ConvertedAzureData struct; Clear() method now only resets NodeProps and RelProps.
Ingest pipeline
cmd/api/src/services/graphify/ingest.go
Removed call to IngestNodes for on-premises nodes in IngestAzureData, eliminating processing of converted.OnPremNodes.
Hybrid analysis refactoring
packages/go/analysis/hybrid/hybrid.go
Removed intermediate entraObjIDMap mapping and deleted createMissingADUser function entirely. Simplified relationship emission loop to use existing entraToADMap entries directly without placeholder node creation.
Test updates
cmd/api/src/analysis/hybrid/hybrid_integration_test.go
Simplified verifyHybridPaths signature from (shouldHaveEdges, shouldHaveUserNode) to (shouldHaveEdges). Updated test cases to assert that SyncedToADUser and SyncedToEntraUser edges are not created in previously placeholder-dependent scenarios; removed conditional node assertions.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 27.27% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: ceasing creation of AD stub users during Azure ingest and post-processing, with reference to the associated ticket BED-7710.
Description check ✅ Passed The description covers the key sections: changes made, motivation (resolves BED-7710), testing approach, type of change, and completed checklist items, though 'How Has This Been Tested' section is somewhat brief.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch BED-7710

Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant