[4ndr3w6]

Collect group isCloudManaged (cloud source-of-authority) from onPremisesSyncBehavior

Summary

Collects a group's cloud source-of-authority flag, isCloudManaged, so downstream tooling can tell when an Entra group is cloud-managed and has its membership written back to its SID-matched on-premises AD group by Entra Cloud Sync group provisioning.

• New model azure.OnPremisesSyncBehavior and Group.IsCloudManaged *bool.
• New client method GetAzureADGroupOnPremisesSyncBehavior — GET /beta/groups/{id}/onPremisesSyncBehavior.
• list groups resolves the flag per-group, only for groups that carry an on-premises SID (OnPremisesSecurityIdentifier != ""), since only those can be a writeback target. Groups without an on-prem SID incur no extra call.
• Resolution is best-effort: an error (e.g. the beta endpoint 404s, or the token lacks the permission) is logged at V(2) and the group is still emitted with IsCloudManaged unresolved (nil).

Why

This is the collection half of the BloodHound AZCloudSyncGroupWriteback edge (SpecterOps/BloodHound#3524), which models a cloud-managed Entra group → the existing on-prem AD group its membership is written back to. That edge joins on the on-prem SID (already collected via onPremisesSecurityIdentifier → OnPremID) and gates on isCloudManaged; this PR populates the flag. Consuming it in BloodHound additionally requires bumping BloodHound's azurehound dependency to a release containing this field.

Implementation notes

• onPremisesSyncBehavior is a navigation property that is not returned on the group resource and does not support $expand, so it requires a per-group GET. The SID gate keeps that cost off the vast majority of (cloud-only) groups.
• IsCloudManaged is a *bool: nil = not resolved (no on-prem SID, or resolution errored); true/false = the group's resolved source-of-authority state.
• The mock (client/mocks/client.go) was regenerated with go generate ./client/....

Tests

cmd/list-groups_test.go:

• a cloud-managed group with an on-prem SID resolves IsCloudManaged == true;
• a group with no on-prem SID is not queried (strict mock controller fails on any unexpected call) and stays nil;
• a resolution error is tolerated — the group is still emitted with IsCloudManaged == nil.

go build ./..., go vet, and the cmd / client / models package tests all pass.

Open question for maintainers

The per-group call is gated to SID-bearing groups, but in forests with many synced groups it still adds one GET each. Happy to put it behind a collection flag if that's preferred over default-on.

Summary by CodeRabbit

• New Features

  • Group listings now include whether eligible groups are cloud-managed.
  • Cloud-management status is resolved for groups with an on-premises security identifier.
  • Groups without sufficient synchronization data retain an unknown status.

• Bug Fixes

  • Issues retrieving synchronization status no longer prevent groups from appearing in listings.

[github-actions]

All contributors have signed the CLA ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: c082fcce-e515-4557-a98c-2202c467bd21

📥 Commits

Reviewing files that changed from the base of the PR and between b02d457 and a721edc.

📒 Files selected for processing: 6

• client/client.go
• client/groups.go
• client/mocks/client.go
• cmd/list-groups.go
• cmd/list-groups_test.go
• models/azure/group.go

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

Walkthrough

The client now retrieves each group’s on-premises sync behavior. Group listing uses the returned IsCloudManaged value for groups with an on-premises security identifier. Lookup errors are logged, and group processing continues.

Changes

Group Sync Behavior

Layer / File(s)	Summary
Sync behavior contract and client models/azure/group.go, client/client.go, client/groups.go, client/mocks/client.go	Adds OnPremisesSyncBehavior and nullable Group.IsCloudManaged. The client interface and implementation expose a per-group beta Graph request, and the mock supports the method.
Group listing resolution cmd/list-groups.go, cmd/list-groups_test.go	For groups with an on-premises security identifier, group listing requests sync behavior and copies IsCloudManaged when returned. Tests cover successful resolution, skipped lookups, and tolerated errors.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant listGroups
  participant AzureGraphClient
  participant Graph as Microsoft Graph beta endpoint
  listGroups->>AzureGraphClient: GetAzureADGroupOnPremisesSyncBehavior(ctx, objectId)
  AzureGraphClient->>Graph: GET onPremisesSyncBehavior, select id and isCloudManaged
  Graph-->>AzureGraphClient: Sync behavior response
  AzureGraphClient-->>listGroups: OnPremisesSyncBehavior or error
  listGroups->>listGroups: Copy IsCloudManaged when behavior is returned

Loading

Suggested reviewers: codemonkey2841

Merge Risk: ⚪ Minimal · up to a721e

Group collection now records whether on-premises-synced groups are cloud-managed. It fetches this through an extra lookup per synced group, and a failed lookup leaves the value empty without stopping collection. No concrete defect was found in the request, the response handling, or the fallback behavior, so the change looks ready to merge.

Security architecture risk: 🟡 Moderate · up to a721e

An optional metadata lookup becomes a serial dependency for group collection without an end-to-end timeout. A stalled response can prevent the remaining security inventory from being emitted. The change otherwise remains read-only and preserves unresolved values rather than treating them as false.

Retained concerns

• Medium · reliability · inferred: The best-effort lookup is tolerant of returned errors but not bounded in duration. It executes synchronously before emitting each eligible group, outside the existing page-request deadline. A response that stalls after headers can therefore prevent all subsequent group output until cancellation or transport failure, weakening failure containment and security-inventory completeness. This newly extends a pre-existing REST limitation into the per-group collection path; it is not an established attacker-controlled privilege boundary bypass.

Security review details

Security Blast Radius

• inferred — The added requests apply to SID-bearing groups within the existing security-enabled group enumeration and configured client identity. Although eligibility limits request volume, a single stalled eligible lookup can delay every later group in that collection stream.

Security Findings and Attack Paths

• inferred — The supported failure path is loss of inventory progress through a stalled Graph response. The inspected path does not establish that an unprivileged group actor can cause such a response; exploitability must not be inferred merely from the added request.

Trust Boundaries and Controls

• observed — The listing path supplies the enumerated group's identifier, and REST request construction resolves it as a path against the configured Graph origin. Requests pass through the existing authentication mechanism and caller context; the new method does not accept a separate destination URL or credential.

Resilience and Maintainability Implications

• observed — The existing page-fetch deadline ends before group enrichment begins. Cancellation can interrupt active request I/O and output sends, but existing retry sleeps are not cancellation-aware. Consequently, error tolerance and manual interruption are weaker guarantees than a bounded best-effort enrichment stage.

Hardening Proposals

• proposed — Give optional enrichment an end-to-end time budget, including cancellation-aware retry waits, and preserve unresolved state when that budget expires. This would keep cloud-management metadata from becoming an unbounded prerequisite for the remaining security inventory.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 6 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: collecting the group isCloudManaged value from onPremisesSyncBehavior.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

• Fix all pre-merge checks with AI

✨ Finishing Touches: 💡 1

🛠️ Fix failing CI checks: 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{A rabbit checks the group fields bright
And asks Graph for sync insight
A cloud flag hops into view
Errors pass; the group comes through
Tests watch each lookup path
Then thump their paws at the result!}

_{Comment @coderabbitai help to get the list of available commands.}

[4ndr3w6]

I have read the CLA Document and I hereby sign the CLA