Skip to content

Add Azure test for User Access Administrator #803

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 13 commits into from
May 4, 2025

Conversation

Oppedijk
Copy link
Contributor

No description provided.

@Oppedijk Oppedijk requested review from a team as code owners April 14, 2025 08:59
Copy link
Contributor

@moorereason moorereason left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting some minor changes from a quick proofread.

@merill
Copy link
Contributor

merill commented Apr 15, 2025

@Oppedijk This is a good test to include. Thanks for the PR.

In addition to the formatting details above, there are some changes we need to make to best align with the existing tests.

  • Move Test-MtUserAccessAdmin.ps1 and Test-MtUserAccessAdmin.md to /powershell/public/maester/azure (we'll need to create a new folder called azure)
  • Switch to using the Rest API instead of the cli (this will reduce dependencies on additional az modules for commands that have a rest api). We already include Az.Accounts module in Maester so the Invoke-AzRestMethod should work.
Invoke-AzRestMethod -Path "/providers/Microsoft.Authorization/roleDefinitions?api-version=2022-04-01&`$filter=roleName+eq+'User Access Administrator'"

Thanks!

@Oppedijk
Copy link
Contributor Author

@Oppedijk This is a good test to include. Thanks for the PR.

In addition to the formatting details above, there are some changes we need to make to best align with the existing tests.

  • Move Test-MtUserAccessAdmin.ps1 and Test-MtUserAccessAdmin.md to /powershell/public/maester/azure (we'll need to create a new folder called azure)
  • Switch to using the Rest API instead of the cli (this will reduce dependencies on additional az modules for commands that have a rest api). We already include Az.Accounts module in Maester so the Invoke-AzRestMethod should work.
Invoke-AzRestMethod -Path "/providers/Microsoft.Authorization/roleDefinitions?api-version=2022-04-01&`$filter=roleName+eq+'User Access Administrator'"

Thanks!

I will take a look, however that will complicate things, as it will be harder to detect the permission denied, as this will list all roles on all levels?
If someone has no permission on the root scope, this will probably return an empty array?

I will do some testing and let you know.

@Oppedijk
Copy link
Contributor Author

@merill can you take a look at the current setup, I've used the Get-AzRoleAssignment, as I need to look up the assigned permissions at the root scope.

Copy link
Contributor

@moorereason moorereason left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't resolve the conversations related to my previous review, but it looks like those issues are fixed.

@merill
Copy link
Contributor

merill commented May 4, 2025

@Oppedijk I had some time and was able to find the right filter and made the switch.

Can you please check and verify?

@merill
Copy link
Contributor

merill commented May 4, 2025

Thanks @Oppedijk for adding this new test and @moorereason + @SamErde for helping improve!

@merill merill merged commit e22fce0 into maester365:main May 4, 2025
6 of 7 checks passed
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.

4 participants