feat: collect Microsoft Entra Domain Services resources - BED-9245 - #206
feat: collect Microsoft Entra Domain Services resources - BED-9245#206martinsohn wants to merge 4 commits into
Conversation
WalkthroughThe change adds Azure Domain Services models, Resource Manager retrieval, CLI commands, role-assignment collection, Azure RM pipeline integration, and related tests. ChangesAzure Domain Services
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant DomainServicesCommand
participant SubscriptionStream
participant AzureResourceManagerClient
participant DomainServiceStream
participant OutputStream
DomainServicesCommand->>SubscriptionStream: enumerate subscription IDs
SubscriptionStream->>AzureResourceManagerClient: ListAzureDomainServices(subscription ID)
AzureResourceManagerClient-->>DomainServiceStream: stream domain-service results
DomainServiceStream-->>OutputStream: emit DomainService records
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@models/azure/domain_service.go`:
- Around line 40-49: Add a []DomainServiceReplicaSet field named ReplicaSets to
DomainServiceProperties with the appropriate JSON tag replicaSets, so
ListAzureDomainServices preserves API-returned replica set entries including
location and subnetId during unmarshalling.
In `@models/domain-service_test.go`:
- Around line 34-40: The domain service test fixture currently exercises only
untyped extra fields, so it does not validate typed LDAPS serialization. Update
the typed model setup used by the domain-service test to populate pfxCertificate
and pfxCertificatePassword through DomainServiceLDAPSSettings or the dedicated
API DTO, then assert that MarshalJSON omits both fields while preserving the
existing ldaps and externalAccess serialization.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 93784177-286b-4056-92b4-1c8bb9e77b1a
📒 Files selected for processing (12)
client/client.goclient/domain_services.goclient/mocks/client.gocmd/list-azure-rm.gocmd/list-domain-service-role-assignments.gocmd/list-domain-service-role-assignments_test.gocmd/list-domain-services.gocmd/list-domain-services_test.goenums/kind.gomodels/azure/domain_service.gomodels/domain-service.gomodels/domain-service_test.go
| type DomainServiceProperties struct { | ||
| TenantID string `json:"tenantId,omitempty"` | ||
| DomainName string `json:"domainName,omitempty"` | ||
| DomainConfigurationType string `json:"domainConfigurationType,omitempty"` | ||
| FilteredSync string `json:"filteredSync,omitempty"` | ||
| SyncScope string `json:"syncScope,omitempty"` | ||
| SyncApplicationID string `json:"syncApplicationId,omitempty"` | ||
| DomainSecuritySettings DomainServiceSecuritySettings `json:"domainSecuritySettings,omitempty"` | ||
| LDAPSSettings DomainServiceLDAPSSettings `json:"ldapsSettings,omitempty"` | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- candidate files ---'
fd -i 'domain_service.go|domain.*service|azure.*domain' . || true
printf '%s\n' '--- symbols and references ---'
rg -n --glob '*.go' 'DomainServiceProperties|ListAzureDomainServices|replicaSets|ReplicaSets' . || true
printf '%s\n' '--- model outline ---'
if [ -f models/azure/domain_service.go ]; then
ast-grep outline models/azure/domain_service.go
printf '%s\n' '--- model ---'
cat -n models/azure/domain_service.go
fiRepository: SpecterOps/AzureHound
Length of output: 6259
🌐 Web query:
Microsoft Azure ARM template Microsoft.AAD/domainServices 2025-06-01 replicaSets location subnetId schema
💡 Result:
In the Microsoft.AAD/domainServices ARM template API version 2025-06-01, the replicaSets property is a required configuration for defining the deployment locations and associated network subnets [1][2]. Key points regarding the replicaSets schema in this version include: Structure: The replicaSets property accepts an array of objects [1]. Each object within this array must define the following: location: A string specifying the Azure region (e.g., "eastus") where the replica set is deployed [1][2]. subnetId: A string representing the resource ID of the subnet where Domain Services is to be deployed [1][2]. This should be formatted as a full Azure resource ID (e.g., /subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Network/virtualNetworks/{vnetName}/subnets/{subnetName}) [1][2]. Evolution of the Schema: Important Note: Starting with the 2025-06-01 API version, the property subnetId was removed from the top-level DomainServiceProperties [3]. Instead, the configuration for networking and location is now handled exclusively through the replicaSets array [1][3]. This change reflects a move toward a multi-region capable deployment model for Azure Active Directory Domain Services, requiring all subnet associations to be defined within the replicaSets configuration rather than as standalone top-level properties [1][3].
Citations:
- 1: https://learn.microsoft.com/en-us/azure/templates/microsoft.aad/2025-06-01/domainservices
- 2: https://learn.microsoft.com/en-us/azure/templates/Microsoft.AAD/2025-06-01/domainServices
- 3: https://learn.microsoft.com/en-us/azure/templates/microsoft.aad/change-log/domainservices
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- client/domain_services.go ---'
cat -n client/domain_services.go
printf '%s\n' '--- list-domain-services.go ---'
cat -n cmd/list-domain-services.go
printf '%s\n' '--- domain-service output model ---'
cat -n models/domain-service.go
printf '%s\n' '--- related tests ---'
cat -n cmd/list-domain-services_test.go
cat -n models/domain-service_test.go
printf '%s\n' '--- official schema excerpt ---'
curl -LfsS 'https://learn.microsoft.com/en-us/azure/templates/microsoft.aad/2025-06-01/domainservices' |
rg -n -i -C 3 'replicaSets|subnetId|location' | head -120 || trueRepository: SpecterOps/AzureHound
Length of output: 18410
🏁 Script executed:
#!/bin/bash
set -e
python3 - <<'PY'
import json
payload = {
"properties": {
"tenantId": "tenant",
"replicaSets": [
{"location": "eastus", "subnetId": "/subscriptions/s/resourceGroups/rg/providers/Microsoft.Network/virtualNetworks/v/subnets/sn"}
]
}
}
# Equivalent shape of the current Go model: only declared JSON keys are retained.
current_keys = {
"tenantId", "domainName", "domainConfigurationType", "filteredSync",
"syncScope", "syncApplicationId", "domainSecuritySettings", "ldapsSettings"
}
current = {k: v for k, v in payload["properties"].items() if k in current_keys}
# Candidate shape from the review comment.
candidate = {
k: v for k, v in payload["properties"].items()
if k in current_keys or k == "replicaSets"
}
print("current replicaSets:", current.get("replicaSets"))
print("candidate replicaSets:", candidate.get("replicaSets"))
assert "replicaSets" not in current
assert candidate["replicaSets"][0]["location"] == "eastus"
assert candidate["replicaSets"][0]["subnetId"].endswith("/subnets/sn")
PYRepository: SpecterOps/AzureHound
Length of output: 340
Preserve replicaSets in DomainServiceProperties.
ListAzureDomainServices uses the 2025-06-01 API, which returns replicaSets entries with location and subnetId. Add a []DomainServiceReplicaSet field so JSON unmarshalling retains this configuration.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@models/azure/domain_service.go` around lines 40 - 49, Add a
[]DomainServiceReplicaSet field named ReplicaSets to DomainServiceProperties
with the appropriate JSON tag replicaSets, so ListAzureDomainServices preserves
API-returned replica set entries including location and subnetId during
unmarshalling.
| "ldapsSettings":{ | ||
| "ldaps":"Enabled", | ||
| "externalAccess":"Disabled", | ||
| "publicCertificate":"certificate-data", | ||
| "certificateNotAfter":"2030-01-01T00:00:00Z", | ||
| "certificateThumbprint":"thumbprint" | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- candidate files ---'
git ls-files | rg '(^|/)models/domain-service_test\.go$|domain.?service|domainservices' || true
printf '%s\n' '--- test file outline ---'
ast-grep outline models/domain-service_test.go 2>/dev/null || true
printf '%s\n' '--- relevant test sections ---'
sed -n '1,120p' models/domain-service_test.go
printf '%s\n' '--- related model definitions and field names ---'
rg -n -C 3 'ldapsSettings|pfxCertificate|pfxCertificatePassword|publicCertificate|certificateNotAfter|certificateThumbprint' . -g '*.go' -g '*.json' -g '*.yaml' -g '*.yml'Repository: SpecterOps/AzureHound
Length of output: 6182
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- model definitions ---'
sed -n '1,180p' models/azure/domain_service.go
printf '%s\n' '--- domain service marshal implementation ---'
sed -n '1,220p' models/domain-service.go
printf '%s\n' '--- all LDAPS-related declarations and marshal filters ---'
rg -n -C 5 'DomainServiceLDAPSSettings|LDAPSSettings|MarshalJSON|publicCertificate|certificateNotAfter|certificateThumbprint|pfxCertificate|pfxCertificatePassword' models client cmd
printf '%s\n' '--- API schema references ---'
rg -n -i -C 3 'Microsoft\.AAD|2025-06-01|ldapsSettings|pfxCertificate' . -g '!vendor/**' -g '!node_modules/**'Repository: SpecterOps/AzureHound
Length of output: 50378
🌐 Web query:
Microsoft.AAD 2025-06-01 domainServices ldapsSettings pfxCertificate pfxCertificatePassword
💡 Result:
In the Microsoft.AAD API version 2025-06-01 for Domain Services, the LdapsSettings object is used to configure Secure LDAP (LDAPS) [1][2]. This object includes two key properties related to certificate management: pfxCertificate: This property requires a base64-encoded representation of the certificate PFX file used to configure Secure LDAP [1][2]. When using the Azure CLI, this parameter can accept either the file path to the PFX file or its base64-encoded string [3]. pfxCertificatePassword: This property requires the password used to decrypt the provided Secure LDAP certificate PFX file [1][2][4]. These settings are part of the broader domainServices resource configuration, which also allows you to toggle the enabled or disabled status of Secure LDAP and external access via the ldaps and externalAccess flags, respectively [1][2].
Citations:
- 1: https://learn.microsoft.com/en-us/azure/templates/microsoft.aad/2025-06-01/domainservices
- 2: https://learn.microsoft.com/en-us/azure/templates/Microsoft.AAD/2025-06-01/domainServices
- 3: https://learn.microsoft.com/en-us/cli/azure/ad/ds?view=azure-cli-latest
- 4: https://learn.microsoft.com/en-us/dotnet/api/microsoft.azure.powershell.cmdlets.addomainservices.models.api202001.ldapssettings.pfxcertificatepassword?view=az-ps-12
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
import json
raw = {
"properties": {
"ldapsSettings": {
"ldaps": "Enabled",
"externalAccess": "Disabled",
"pfxCertificate": "certificate-data",
"pfxCertificatePassword": "certificate-password",
"publicCertificate": "legacy-certificate",
"certificateNotAfter": "2030-01-01T00:00:00Z",
"certificateThumbprint": "thumbprint",
}
}
}
# Model the exact JSON surface of azure.DomainServiceLDAPSSettings.
known = {"ldaps", "externalAccess"}
settings = {
key: value
for key, value in raw["properties"]["ldapsSettings"].items()
if key in known
}
encoded = {"properties": {"ldapsSettings": settings}}
print("input fields: ", sorted(raw["properties"]["ldapsSettings"]))
print("output fields: ", sorted(encoded["properties"]["ldapsSettings"]))
print("sensitive output:", {
key: encoded["properties"]["ldapsSettings"].get(key)
for key in ("pfxCertificate", "pfxCertificatePassword")
})
PYRepository: SpecterOps/AzureHound
Length of output: 437
Exercise the typed LDAPS serialization path.
pfxCertificate and pfxCertificatePassword are fields in Microsoft.AAD/domainServices 2025-06-01. DomainServiceLDAPSSettings declares only ldaps and externalAccess, so json.Unmarshal discards these fields before MarshalJSON runs. Adding them to this fixture and exclusion list alone will not test their serialization. Populate them through the typed model or a dedicated API DTO, then assert that MarshalJSON omits both fields.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@models/domain-service_test.go` around lines 34 - 40, The domain service test
fixture currently exercises only untyped extra fields, so it does not validate
typed LDAPS serialization. Update the typed model setup used by the
domain-service test to populate pfxCertificate and pfxCertificatePassword
through DomainServiceLDAPSSettings or the dedicated API DTO, then assert that
MarshalJSON omits both fields while preserving the existing ldaps and
externalAccess serialization.
1 participant
Description
Adds Microsoft Entra Domain Services resource collection to
az-rm, including accessibleMicrosoft.AAD/domainServicesresources and their direct resource-scope role assignments.Changes include:
AZEntraDSandAZEntraDSRoleAssignmentoutput kinds and models.Motivation and Context
BloodHound requires Azure resource, configuration, and authorization data to anchor the Entra DS graph and model hybrid paths across Azure Resource Manager, Microsoft Entra ID, and the managed Active Directory domain.
This PR is part of: BED-9245
How Has This Been Tested?
go test ./...az-rmagainst the lab tenant and confirm the expected records.Screenshots (if appropriate):
N/A
Types of changes
Summary by CodeRabbit
New Features
Bug Fixes
Tests