maester icon indicating copy to clipboard operation
maester copied to clipboard

Proposal to add conditional access gap test

Open RobbeVandenDaele opened this issue 1 year ago • 3 comments

Hi all!

I created a new Maester test to look for conditional access policy gaps. The approach is to look for objects which are excluded in conditional access policies, but not specifically included in at least one other conditional access policy. By doing this, I try to find possibly overlooked exclusions which do not have a fallback.

I am open for any feedback you have regarding the test 😃 .

Kind regards Robbe

RobbeVandenDaele avatar Aug 13 '24 20:08 RobbeVandenDaele

Thanks a lot for submitting this as a PR.

I 💛 the idea. I think this will be a good one to have as a ⚠️ Warning test. Since this is more to highlight accidental issues.Having said that we haven't added the Severity flag to Maester yet.

@f-bader @Snozzberries and @Cloud-Architekt thoughts? Are there suggestions you have to make this better?

merill avatar Aug 16 '24 10:08 merill

I do like this, and it has been something I have considered in the past, so glad to see this.

I think splitting it into a few functions and tests would be helpful. Move the helper functions Related and Differences to their own cmdlets, then split user, group, role, app, spn, location, and platform into individual cmdlets or switches and tests.

Having a tag for severity or impact would be valuable too. We could likely add that to the tags as a placeholder until we have a plan to surface in the report.

soulemike avatar Aug 19 '24 02:08 soulemike

Hi @merill @Snozzberries

Thank you for the feedback! I added cmdlets to the two helper functions 'Get-ObjectDifferences' and 'Get-RalatedPolicies', and I added the 'Warning' tag to MT.1036 in 'Test-ConditionalAccessBaseline.Tests.ps1'. image I hope this is what you meant by adding the Warning tag as a placeholder, if not please let me know 😄 .

@Snozzberries I am not sure what you mean by splitting user, groups, roles, etc. into individual cmdlets or tests. Although I do see the value (being able to choose which objects you want to test), I currently fail to see how an end user will be able to choose these flags when using the 'Invoke-Maester' command.

I am happy to discuss this further. Let me known what is expected from me to help get this test production ready.

RobbeVandenDaele avatar Aug 20 '24 20:08 RobbeVandenDaele

Looks good. I'll merge it in

merill avatar Aug 21 '24 23:08 merill

That is what I had in mind with the Warning.

Here is an example of what I meant for the tests. The idea being it lets the user use the report to focus on the specific exclusions. Not major, but likely would be easier to work through if I have multiple types of exclusions, or want to ignore group exclusions, etc...

It "MT.1036 ...
  Test-MtCaGaps -Users | Should ...

Returns this result section:

        # Add user objects to results
        if ($differencesUsers.Count -ne 0) {
            $testResult = "The following user objects did not have a fallback:`n`n"
            $differencesUsers | ForEach-Object {
                $testResult += "    - $_`n`n"
                $testResult += Get-RalatedPolicies -Arr $mappingArray -ObjName $_
            }
        }

It "MT.1037 ...
  Test-MtCaGaps -Groups | Should ...

Returns this result section:

        # Add group objects to results
        if ($differencesGroups.Count -ne 0) {
            $testResult += "The following group objects did not have a fallback:`n`n"
            $differencesGroups | ForEach-Object {
                $testResult += "    - $_`n`n"
                $testResult += Get-RalatedPolicies -Arr $mappingArray -ObjName $_
            }
        }

soulemike avatar Aug 22 '24 01:08 soulemike

Thank you for elaborating @Snozzberries! I will try to rework this somewhere in September and create a new PR if done.

Thank you for merging @merill

RobbeVandenDaele avatar Aug 22 '24 08:08 RobbeVandenDaele

@merill @Snozzberries, I see that the documentation at https://maester.dev/docs/tests/maester is not yet updated with the new MT1036 test. I did add the .md file to the website/docs/tests/maester folder. Is there anything else I still need to do?

RobbeVandenDaele avatar Aug 23 '24 06:08 RobbeVandenDaele