Skip to content
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

Offload user filtering to Azure API to avoid resource issues in large AD #227

Merged

Conversation

trickyearlobe
Copy link
Contributor

Description

Suggested fix for #198 where we get failures testing for guest accounts in large AD because we run out of memory (after waiting a LONG time for the whole AD to be downloaded)

This PR could allow a wrapper profile (such as CIS Azure) to offload filtering to Azure instead of pulling all the accounts back to the Inspec machine to perform local filtering. This is especially useful in AD with more than 500k accounts.

This PR would allow the CIS Azure profile to test in a similar way to this:-

control "guest-users" do
  impact 0.7
  title "There should be no guest users"
  desc "Guest users are bad"
  describe azurerm_ad_users(filter: "userType eq 'Guest'") do
    its('display_names') { should eq [] }
    its('count') {should eq 0}
  end
end

…arge AD

 - Modify Azure::Graph.users to pass a filter parameter to Azure REST API
 - Modify AzurermAdUsers to pass a filter parameter to Azure::Graph.users

Signed-off-by: Richard Nixon <richard.nixon@btinternet.com>
@trickyearlobe trickyearlobe requested a review from a team December 16, 2019 19:32
Copy link

@skpaterson skpaterson left a comment

Choose a reason for hiding this comment

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

@skpaterson skpaterson added the Version: Bump Minor Used by github.minor_bump_labels to bump the Minor version number. label Dec 17, 2019
@skpaterson skpaterson merged commit c266b59 into inspec:master Dec 17, 2019
@trickyearlobe trickyearlobe deleted the azurerm_ad_users_pass_filter_to_api branch January 25, 2020 15:42
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.

2 participants