Azurehound enhancement - Fetch Managed Devices from Intune #136
Conversation
…etron-azurehound-enhancement-metronlabs Files are causing a conflict because newly created files were added but the deleted files were not removed.
# Test the current implementation Build the app using: go build -ldflags="-s -w -X github.com/bloodhoundad/azurehound/v2/constants.Version=dev-intune" Then test using added JWT token after these commands: ./azurehound list intune-devices --jwt JWT_TOKEN ./azurehound list intune-data --jwt JWT_TOKEN ./azurehound list intune-compliance --jwt JWT_TOKEN
Add 1. client id 2. client secret & 3. tenant id for the script to work.
…the deployed script instead of mock data
WalkthroughThis change introduces comprehensive Intune integration, adding new commands to list Intune managed devices and their compliance states. It defines Intune-specific enums and data models, extends the Azure client interface with Intune methods, and implements asynchronous, concurrent data retrieval for Intune devices, compliance, and configuration states. The Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant AzureClient
participant IntuneAPI
User->>CLI: Run "intune-devices" or "intune-compliance"
CLI->>AzureClient: Connect and authenticate
CLI->>AzureClient: ListIntuneManagedDevices(params)
AzureClient->>IntuneAPI: GET /managedDevices
IntuneAPI-->>AzureClient: Stream paginated device data
AzureClient-->>CLI: Stream ManagedDevice objects
alt intune-compliance (detailed)
CLI->>AzureClient: GetIntuneDeviceCompliance(deviceId)
AzureClient->>IntuneAPI: GET /managedDevices/{id}/compliance
IntuneAPI-->>AzureClient: Compliance data
AzureClient-->>CLI: Stream ComplianceState objects
end
CLI-->>User: Output device/compliance data
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (6)
cmd/list-intune-devices.go (1)
49-51: Consider making the OS filter configurable.The hard-coded Windows filter limits flexibility. Consider making this configurable via command-line flags to support other operating systems.
+var ( + osFilter string +) + +func init() { + listIntuneDevicesCmd.Flags().StringVar(&osFilter, "os", "Windows", "Filter by operating system (Windows, iOS, Android, macOS)") + listRootCmd.AddCommand(listIntuneDevicesCmd) +} func listIntuneDevices(ctx context.Context, client client.AzureClient) <-chan interface{} { var ( out = make(chan interface{}) params = query.GraphParams{ - Filter: "operatingSystem eq 'Windows'", // Focus on Windows devices for BloodHound + Filter: fmt.Sprintf("operatingSystem eq '%s'", osFilter), } )client/intune_devices.go (1)
18-18: Consider making the default Top value configurable.The hard-coded value of 999 may not be optimal for all scenarios. Consider making this configurable or using a more reasonable default.
- params.Top = 999 + params.Top = 100 // More reasonable default, or make configurablemodels/intune/models.go (3)
13-13: Inconsistent field naming: use "ID" instead of "Id" for Go conventions.Go convention is to use "ID" for identifier fields, not "Id". This applies to multiple fields in the structs.
- Id string `json:"id"` + ID string `json:"id"`- AzureADDeviceId string `json:"azureADDeviceId"` + AzureADDeviceID string `json:"azureADDeviceId"`Apply similar changes to other structs in this file.
Also applies to: 21-21
28-36: Consider consolidating similar struct patterns.The
ComplianceStateandConfigurationStatestructs share very similar field patterns (ID, DeviceID, DeviceName, State, Version, SettingStates). Consider creating a base struct or using composition to reduce duplication.+// BaseDeviceState represents common fields for device state structures +type BaseDeviceState struct { + ID string `json:"id"` + DeviceID string `json:"deviceId"` + DeviceName string `json:"deviceName"` + State string `json:"state"` + Version int `json:"version"` +} + // ComplianceState represents device compliance information type ComplianceState struct { + BaseDeviceState - Id string `json:"id"` - DeviceId string `json:"deviceId"` - DeviceName string `json:"deviceName"` ComplianceGracePeriodExpirationDateTime time.Time `json:"complianceGracePeriodExpirationDateTime"` - State string `json:"state"` - Version int `json:"version"` SettingStates []ComplianceSettingState `json:"settingStates"` }Also applies to: 46-54
12-25: Add struct field documentation for better maintainability.The struct fields would benefit from inline documentation explaining their purpose, especially for fields like
EnrollmentType,ManagementAgent, andJoinTypewhich may have specific meanings in the Intune context.// ManagedDevice represents an Intune managed device type ManagedDevice struct { - Id string `json:"id"` - DeviceName string `json:"deviceName"` - OperatingSystem string `json:"operatingSystem"` - OSVersion string `json:"osVersion"` - ComplianceState string `json:"complianceState"` - LastSyncDateTime time.Time `json:"lastSyncDateTime"` - EnrollmentType string `json:"enrollmentType"` - ManagementAgent string `json:"managementAgent"` - AzureADDeviceId string `json:"azureADDeviceId"` - UserPrincipalName string `json:"userPrincipalName"` - DeviceEnrollmentType string `json:"deviceEnrollmentType"` - JoinType string `json:"joinType"` + ID string `json:"id"` // Unique identifier for the device + DeviceName string `json:"deviceName"` // Display name of the device + OperatingSystem string `json:"operatingSystem"` // Operating system type (Windows, iOS, etc.) + OSVersion string `json:"osVersion"` // Operating system version + ComplianceState string `json:"complianceState"` // Current compliance status + LastSyncDateTime time.Time `json:"lastSyncDateTime"` // Last time device synced with Intune + EnrollmentType string `json:"enrollmentType"` // How the device was enrolled + ManagementAgent string `json:"managementAgent"` // Agent managing the device + AzureADDeviceID string `json:"azureADDeviceId"` // Azure AD device identifier + UserPrincipalName string `json:"userPrincipalName"` // UPN of the device user + DeviceEnrollmentType string `json:"deviceEnrollmentType"` // Type of device enrollment + JoinType string `json:"joinType"` // How device is joined to Azure AD }cmd/list-intune-compliance.go (1)
101-103: Hard-coded Windows filter limits functionality.The function hard-codes filtering to Windows devices only, which limits the tool's flexibility for organizations with mixed device environments.
func getComplianceTargetDevices(ctx context.Context, client client.AzureClient) <-chan intune.ManagedDevice { var ( out = make(chan intune.ManagedDevice) params = query.GraphParams{ - Filter: "operatingSystem eq 'Windows'", + // Allow filtering by OS through command line parameter if needed + // Filter: "operatingSystem eq 'Windows'", } )Consider adding a command-line flag for OS filtering:
var targetOS string func init() { listIntuneComplianceCmd.Flags().StringVar(&targetOS, "os", "Windows", "Filter by operating system") }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.gitignore(1 hunks)client/client.go(2 hunks)client/intune_devices.go(1 hunks)cmd/list-intune-compliance.go(1 hunks)cmd/list-intune-devices.go(1 hunks)enums/intune.go(1 hunks)get_token.ps1(1 hunks)models/intune/models.go(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
client/client.go (2)
client/query/params.go (1)
GraphParams(109-120)models/intune/models.go (3)
ManagedDevice(12-25)ComplianceState(28-36)ConfigurationState(46-54)
client/intune_devices.go (4)
client/query/params.go (2)
GraphParams(109-120)Top(42-42)client/client.go (1)
AzureResult(95-98)models/intune/models.go (3)
ManagedDevice(12-25)ComplianceState(28-36)ConfigurationState(46-54)constants/misc.go (1)
GraphApiVersion(43-43)
cmd/list-intune-devices.go (5)
panicrecovery/panic_recovery.go (2)
HandleBubbledPanic(14-26)PanicRecovery(29-33)client/client.go (1)
AzureClient(218-231)client/query/params.go (1)
GraphParams(109-120)cmd/utils.go (1)
NewAzureWrapper(437-442)enums/intune.go (1)
KindAZIntuneDevice(5-5)
enums/intune.go (2)
enums/kind.go (1)
Kind(20-20)models/intune/models.go (1)
ComplianceState(28-36)
🔇 Additional comments (10)
.gitignore (1)
235-238: Good security practice: Excluding sensitive files.Properly excluding the token script and GitHub workflows from version control helps prevent accidental commits of sensitive configuration.
client/client.go (2)
35-35: LGTM: Proper import addition for Intune models.The import for Intune models is correctly added to support the new interface methods.
226-230: LGTM: Well-designed interface extensions.The new Intune methods follow consistent patterns with existing interface methods:
- Accept context and query parameters
- Return channels for async streaming
- Use clear, descriptive naming
- Maintain type safety with generics
cmd/list-intune-devices.go (2)
31-44: LGTM: Proper command structure and error handling.The command implementation follows established patterns with:
- Proper context cancellation setup
- Signal handling for graceful shutdown
- Appropriate logging and timing
- Panic recovery handling
66-66: To verify whereNewAzureWrapperis defined and whether an import is actually required, let’s locate its declaration and check the packages:#!/bin/bash # 1. Find all occurrences of NewAzureWrapper (definition and calls) rg -n "NewAzureWrapper" # 2. Confirm package declarations for the two files grep -R "package " -n cmd/list-intune-devices.go cmd/utils.goclient/intune_devices.go (1)
24-34: LGTM: Well-structured API method implementation.The
ListIntuneManagedDevicesmethod follows established patterns with proper endpoint construction and async channel-based results.enums/intune.go (4)
4-9: Verify Kind constant naming consistency with existing patterns.The Kind constants follow a specific naming pattern with "AZ" prefix. Ensure this aligns with existing Kind enumerations in the codebase.
#!/bin/bash # Description: Check existing Kind constant patterns in the codebase # Expected: Find similar naming patterns to ensure consistency # Search for existing Kind constants to verify naming pattern ast-grep --pattern 'const ( $$$ Kind$_ Kind = $_ $$$ )' # Also check for any existing AZ-prefixed Kind constants rg -A 10 -B 2 'Kind.*=.*"AZ'
14-21: Validate ComplianceState enum values against Microsoft Graph API.The compliance state values should match exactly what Microsoft Graph API returns to avoid mapping issues.
What are the valid compliance state values for Microsoft Intune devices in Microsoft Graph API?
47-52: Verify OperatingSystem enum values and consider adding missing platforms.The enum includes major platforms but might be missing some that Intune supports (like Linux distributions).
What operating system values does Microsoft Graph API return for Intune managed devices?
26-32: Consider adding WindowsBulkEnrollment and other enrollment types.The EnrollmentType enum might be missing some common Intune enrollment types. Verify completeness against the Microsoft Graph API documentation.
What are all the possible enrollment types for Microsoft Intune devices according to Microsoft Graph API deviceEnrollmentType property?
|
CLA Assistant Lite bot: I have read the CLA Document and I hereby sign the CLA You can retrigger this bot by commenting recheck in this Pull Request |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
cmd/list-intune-compliance.go (1)
117-117: Consider making the operating system filter configurable.The filter is hardcoded to only Windows devices, which may be too restrictive. Consider adding a flag to allow filtering by different operating systems or removing the OS filter entirely to support all managed devices.
- params = query.GraphParams{ - Filter: "operatingSystem eq 'Windows'", - } + params = query.GraphParams{} + + // Add OS filter if needed - could be made configurable + if operatingSystem := "Windows"; operatingSystem != "" { + params.Filter = fmt.Sprintf("operatingSystem eq '%s'", operatingSystem) + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
client/intune_devices.go(1 hunks)cmd/list-intune-compliance.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- client/intune_devices.go
🧰 Additional context used
🧬 Code Graph Analysis (1)
cmd/list-intune-compliance.go (7)
models/intune/models.go (1)
ManagedDevice(12-25)panicrecovery/panic_recovery.go (2)
HandleBubbledPanic(14-26)PanicRecovery(29-33)client/client.go (1)
AzureClient(218-231)pipeline/pipeline.go (1)
Demux(109-141)config/config.go (1)
ColStreamCount(308-317)cmd/utils.go (1)
NewAzureWrapper(437-442)enums/intune.go (1)
KindAZIntuneCompliance(8-8)
🔇 Additional comments (5)
cmd/list-intune-compliance.go (5)
164-192: Well-implemented concurrent processing pattern.The use of
pipeline.Demuxto split device processing across multiple streams with proper synchronization usingsync.WaitGroupis excellent. The panic recovery and context cancellation handling are also correctly implemented.
194-234: Excellent error handling and fallback mechanism.The improved error handling addresses previous concerns perfectly. The
hasErrorflag with immediate break on first error, followed by fallback logic after the loop, ensures robust error recovery while preventing unnecessary processing.
25-33: ID collision prevention successfully implemented.The use of
time.Now().UnixNano()in the ID generation effectively prevents the collision issues identified in previous reviews while maintaining uniqueness across multiple invocations.
46-59: Robust input validation implementation.The
PreRunEvalidation function effectively prevents invalid compliance state values from being processed, addressing the security concerns from previous reviews.
123-138: Security validation successfully implemented.The validation map approach effectively prevents injection attacks while maintaining clean filter construction. The early return on invalid state prevents further processing with malicious input.
2 participants
Summary by CodeRabbit
New Features
Chores