BED-7634: Azurehound - Fix listResourceGroupUserAccessAdmins filtering by wrong role ID

[StranDutton]

Ticket:

BED-7634

Summary:

Fixes the role ID filter in listResourceGroupUserAccessAdmins() — it was using constants.OwnerRoleID instead of constants.UserAccessAdminRoleID, causing Resource Group User Access Admin edges in BloodHound to reflect Owner relationships.

As a result of this, I made another ticket to go and update all the other tests that follow this same pattern since most of them have the same flaw.

Changes:

cmd/list-resource-group-user-access-admins.go

• Changed filter from OwnerRoleID to UserAccessAdminRoleID to match name of function

cmd/list-resource-group-user-access-admins_test.go

• Rewrote test to send both a matching and non-matching role assignment and assert on the output data, not just channel activity

Resolves BED-7634

Customer Impact

The incorrectly filtered nodes will remain incorrect for up to 7 days in customer envs without adding an additional DB migration. BHE runs auto-pruning operations and deletes all relationship edges whose lastSeen timestamp is older than BaseTTL (which is 7 days) so when these old (incorrect) edges are left in the env for 7 days and get replaced by the new (valid) nodes, the old ones will be cleaned up. No migration needed.

Testing Notes

Refer to the ticket

Demo

• Before this change, querying ResourceGroupUserAccessAdmins and ResourceGroupOwners would yield the same results, which was incorrect.
• After running a fresh AzureHound collection with the new changes, importing the output into BHE (local) now provides accurate Resource Group User Access Admin edges:

Before:

• Cypher query for UAAs and Owners of a ResourceGroup returned the same results.

After:

• The cypher query for to find UAAs of a ResourceGroup now resolve accurately:

Summary by CodeRabbit

• Bug Fixes

  • Listing of resource group user access administrators now returns only user-access-admin entries, improving accuracy of administrator results.

• Tests

  • Tests updated to include multiple role assignments, validate resource group identity, confirm a single user-access-admin is returned, and fix assertion/error message wording.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 74bb135b-6c9a-4db6-8350-fb9cbf87be1e

📥 Commits

Reviewing files that changed from the base of the PR and between 32af8e9 and ca42246.

📒 Files selected for processing (2)

• cmd/list-resource-group-user-access-admins.go
• cmd/list-resource-group-user-access-admins_test.go

🚧 Files skipped from review as they are similar to previous changes (2)

• cmd/list-resource-group-user-access-admins.go
• cmd/list-resource-group-user-access-admins_test.go

---

Walkthrough

The role filter in the resource-group user access admin listing was changed from OwnerRoleID to UserAccessAdminRoleID. Corresponding test data and assertions were updated to include both owner and user-access-admin assignments and to validate the returned wrapper and admin entry.

Changes

Cohort / File(s)

Summary

Role filter & tests cmd/list-resource-group-user-access-admins.go, cmd/list-resource-group-user-access-admins_test.go

Replaced rgRoleAssignmentFilter(constants.OwnerRoleID) with rgRoleAssignmentFilter(constants.UserAccessAdminRoleID). Tests now supply two role assignments (user-access-admin and owner), read channel with tuple-style receive and ok check, type-assert the emitted azureWrapper[models.ResourceGroupUserAccessAdmins], validate ResourceGroupId, ensure one UserAccessAdmin present, assert UserAccessAdmin.Name == "uaa-assignment", and corrected a typo in an error message.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐇 I hopped through lines both short and long,
Swapped a filter where it didn't belong,
Tests now shout the admin's name,
One small change, yet not the same,
A tiny hop — the build stays strong ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% 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
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically describes the main fix: correcting the role ID filter in listResourceGroupUserAccessAdmins from OwnerRoleID to UserAccessAdminRoleID.

_{✏️ 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-7634-user-access-admin-filter-fix

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

🧹 Nitpick comments (1)

cmd/list-resource-group-user-access-admins_test.go (1)

41-45: Unused mockClient and mockTenant can be removed.

mockClient is instantiated and configured with a TenantInfo() expectation, but it's never passed to listResourceGroupUserAccessAdmins(ctx, mockRoleAssignmentsChannel) on line 46. This appears to be leftover from a template or previous implementation.

🧹 Suggested cleanup

 func TestListResourceGroupUserAccessAdmins(t *testing.T) {
 	ctrl := gomock.NewController(t)
 	defer ctrl.Finish()
 	ctx := context.Background()

-	mockClient := mocks.NewMockAzureClient(ctrl)
-
 	mockRoleAssignmentsChannel := make(chan azureWrapper[models.ResourceGroupRoleAssignments])
-	mockTenant := azure.Tenant{}
-	mockClient.EXPECT().TenantInfo().Return(mockTenant).AnyTimes()
 	channel := listResourceGroupUserAccessAdmins(ctx, mockRoleAssignmentsChannel)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@cmd/list-resource-group-user-access-admins_test.go` around lines 41 - 45,
Remove the unused test setup for mockClient and mockTenant: delete the
NewMockAzureClient(ctrl) creation, the mockTenant variable, and the
mockClient.EXPECT().TenantInfo().Return(mockTenant).AnyTimes() call since
mockClient is not passed into listResourceGroupUserAccessAdmins(ctx,
mockRoleAssignmentsChannel); ensure mockRoleAssignmentsChannel remains and run
tests to confirm no other references to mockClient or mockTenant exist.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@cmd/list-resource-group-user-access-admins_test.go`:
- Around line 41-45: Remove the unused test setup for mockClient and mockTenant:
delete the NewMockAzureClient(ctrl) creation, the mockTenant variable, and the
mockClient.EXPECT().TenantInfo().Return(mockTenant).AnyTimes() call since
mockClient is not passed into listResourceGroupUserAccessAdmins(ctx,
mockRoleAssignmentsChannel); ensure mockRoleAssignmentsChannel remains and run
tests to confirm no other references to mockClient or mockTenant exist.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d924cfeb-3f53-4bc3-a3a4-cfe6c848e7e8

📥 Commits

Reviewing files that changed from the base of the PR and between d71bba0 and 32af8e9.

📒 Files selected for processing (2)

• cmd/list-resource-group-user-access-admins.go
• cmd/list-resource-group-user-access-admins_test.go

🚧 Files skipped from review as they are similar to previous changes (1)

• cmd/list-resource-group-user-access-admins.go