Fix bug with msaad52v1 only admins consent to apps (cisagov#1043)

* uploading for Cassey debugging session * modified Rego policy 5.2 and unit tests to account for Microsoft updates to tenant output data * modified 5.2 functional tests based on Microsoft changes to JSON output
LedgerLand · Apr 2, 2024 · 4527eed · 4527eed
1 parent 3343bfb
commit 4527eed
Show file tree

Hide file tree

Showing 4 changed files with 66 additions and 20 deletions.
diff --git a/PowerShell/ScubaGear/Rego/AADConfig.rego b/PowerShell/ScubaGear/Rego/AADConfig.rego
@@ -520,22 +520,26 @@ tests contains {
 # MS.AAD.5.2v1
 #--
 
-# Save the policy Id of any user allowed to consent to third
-# party applications
+# Return the Id if non-compliant user consent policies
 BadDefaultGrantPolicies contains Policy.Id if {
     some Policy in input.authorization_policies
-    count(Policy.PermissionGrantPolicyIdsAssignedToDefaultUserRole) != 0
+    "ManagePermissionGrantsForSelf.microsoft-user-default-legacy" in Policy.PermissionGrantPolicyIdsAssignedToDefaultUserRole
 }
 
-# Get all policy Ids
+BadDefaultGrantPolicies contains Policy.Id if {
+    some Policy in input.authorization_policies
+    "ManagePermissionGrantsForSelf.microsoft-user-default-low" in Policy.PermissionGrantPolicyIdsAssignedToDefaultUserRole
+}
+
+# Return all policy Ids
 AllDefaultGrantPolicies contains {
     "DefaultUser_DefaultGrantPolicy": Policy.PermissionGrantPolicyIdsAssignedToDefaultUserRole,
     "PolicyId": Policy.Id
 } if {
     some Policy in input.authorization_policies
 }
 
-# If there is a policy that allows user to cconsent to third party apps, fail
+# If there is a policy that allows user to consent to third party apps, fail
 tests contains {
     "PolicyId": "MS.AAD.5.2v1",
     "Criticality": "Shall",

diff --git a/PowerShell/ScubaGear/Testing/Unit/Rego/AAD/AADConfig_05_test.rego b/PowerShell/ScubaGear/Testing/Unit/Rego/AAD/AADConfig_05_test.rego
@@ -75,11 +75,30 @@ test_AllowedToCreateApps_Incorrect_V2 if {
 #
 # Policy MS.AAD.5.2v1
 #--
-test_PermissionGrantPolicyIdsAssignedToDefaultUserRole_Correct if {
+test_UserConsentNotAllowed_Correct if {
     Output := aad.tests with input as {
         "authorization_policies": [
             {
-                "PermissionGrantPolicyIdsAssignedToDefaultUserRole": [],
+                "PermissionGrantPolicyIdsAssignedToDefaultUserRole": [
+                    "ManagePermissionGrantsForOwnedResource.microsoft-dynamically-managed-permissions-for-chat",
+                    "ManagePermissionGrantsForOwnedResource.microsoft-dynamically-managed-permissions-for-team"
+                ],
+                "Id": "authorizationPolicy"
+            }
+        ]
+    }
+
+    ReportDetailStr :=
+        "0 authorization policies found that allow non-admin users to consent to third-party applications"
+    TestResult("MS.AAD.5.2v1", Output, ReportDetailStr, true) == true
+}
+
+test_UserConsentNotAllowedEmptyDefaultUserArray_Correct if {
+    Output := aad.tests with input as {
+        "authorization_policies": [
+            {
+                "PermissionGrantPolicyIdsAssignedToDefaultUserRole": [
+                ],
                 "Id": "authorizationPolicy"
             }
         ]
@@ -90,12 +109,14 @@ test_PermissionGrantPolicyIdsAssignedToDefaultUserRole_Correct if {
     TestResult("MS.AAD.5.2v1", Output, ReportDetailStr, true) == true
 }
 
-test_PermissionGrantPolicyIdsAssignedToDefaultUserRole_Incorrect_V1 if {
+test_UserConsentFromVerifiedPublishersAllowed_Incorrect if {
     Output := aad.tests with input as {
         "authorization_policies": [
             {
                 "PermissionGrantPolicyIdsAssignedToDefaultUserRole": [
-                    "Test user"
+                    "ManagePermissionGrantsForOwnedResource.microsoft-dynamically-managed-permissions-for-chat",
+                    "ManagePermissionGrantsForOwnedResource.microsoft-dynamically-managed-permissions-for-team",
+                    "ManagePermissionGrantsForSelf.microsoft-user-default-legacy"
                 ],
                 "Id": "authorizationPolicy"
             }
@@ -110,25 +131,23 @@ test_PermissionGrantPolicyIdsAssignedToDefaultUserRole_Incorrect_V1 if {
     TestResult("MS.AAD.5.2v1", Output, ReportDetailStr, false) == true
 }
 
-test_PermissionGrantPolicyIdsAssignedToDefaultUserRole_Incorrect_V2 if {
+test_UserConsentAllowed_Incorrect if {
     Output := aad.tests with input as {
         "authorization_policies": [
-            {
-                "PermissionGrantPolicyIdsAssignedToDefaultUserRole": [],
-                "Id": "Good policy"
-            },
             {
                 "PermissionGrantPolicyIdsAssignedToDefaultUserRole": [
-                    "Test user"
+                    "ManagePermissionGrantsForOwnedResource.microsoft-dynamically-managed-permissions-for-chat",
+                    "ManagePermissionGrantsForOwnedResource.microsoft-dynamically-managed-permissions-for-team",
+                    "ManagePermissionGrantsForSelf.microsoft-user-default-low"
                 ],
-                "Id": "Bad policy"
+                "Id": "authorizationPolicy"
             }
         ]
     }
 
     ReportDetailStr := concat("", [
         "1 authorization policies found that allow non-admin users to consent to third-party applications:",
-        "<br/>Bad policy"
+        "<br/>authorizationPolicy"
     ])
 
     TestResult("MS.AAD.5.2v1", Output, ReportDetailStr, false) == true

diff --git a/Testing/Functional/Products/TestPlans/aad.testplan.yaml b/Testing/Functional/Products/TestPlans/aad.testplan.yaml
@@ -577,16 +577,29 @@ TestPlan:
   - PolicyId: MS.AAD.5.2v1
     TestDriver: RunCached
     Tests:
-      - TestDescription: MS.AAD.5.2v1 Non-Compliant case - Allow users to consent to apps
+      - TestDescription: MS.AAD.5.2v1 Non-Compliant case - Allow user to consent to apps
         Preconditions:
           - Command: UpdateProviderExport
             Splat:
               updates:
                 authorization_policies[0].PermissionGrantPolicyIdsAssignedToDefaultUserRole:
                   - ManagePermissionGrantsForSelf.microsoft-user-default-legacy
+                  - ManagePermissionGrantsForOwnedResource.microsoft-dynamically-managed-permissions-for-chat
+                  - ManagePermissionGrantsForOwnedResource.microsoft-dynamically-managed-permissions-for-team
         Postconditions: []
         ExpectedResult: false
-      - TestDescription: MS.AAD.5.2v1 Compliant case - Do NOT allow users to consent to apps
+      - TestDescription: MS.AAD.5.2v1 Non-Compliant case - Allow user to consent to verified apps
+        Preconditions:
+          - Command: UpdateProviderExport
+            Splat:
+              updates:
+                authorization_policies[0].PermissionGrantPolicyIdsAssignedToDefaultUserRole:
+                  - ManagePermissionGrantsForSelf.microsoft-user-default-low
+                  - ManagePermissionGrantsForOwnedResource.microsoft-dynamically-managed-permissions-for-chat
+                  - ManagePermissionGrantsForOwnedResource.microsoft-dynamically-managed-permissions-for-team
+        Postconditions: []
+        ExpectedResult: false
+      - TestDescription: MS.AAD.5.2v1 Compliant case - Do NOT allow users to consent to apps - empty grant policy
         Preconditions:
           - Command: UpdateProviderExport
             Splat:
@@ -595,6 +608,16 @@ TestPlan:
                   []
         Postconditions: []
         ExpectedResult: true
+      - TestDescription: MS.AAD.5.2v1 Compliant case - Do NOT allow users to consent to apps - chat teams grant policy
+        Preconditions:
+          - Command: UpdateProviderExport
+            Splat:
+              updates:
+                authorization_policies[0].PermissionGrantPolicyIdsAssignedToDefaultUserRole:
+                  - ManagePermissionGrantsForOwnedResource.microsoft-dynamically-managed-permissions-for-chat
+                  - ManagePermissionGrantsForOwnedResource.microsoft-dynamically-managed-permissions-for-team
+        Postconditions: []
+        ExpectedResult: true
 
   - PolicyId: MS.AAD.5.3v1
     TestDriver: RunCached

diff --git a/Testing/RunUnitTests.ps1 b/Testing/RunUnitTests.ps1
@@ -156,7 +156,7 @@ function Invoke-ControlGroupItem {
 
             elseif(Test-Path -Path $Filename.Fullname -PathType Leaf) {
                 Write-Output "`nTesting Control Group $ControlGroup"
-                ..\opa_windows_amd64.exe test $RegoPolicyPath .\$($Filename.Fullname) $Flag
+                & $OPAExe test $RegoPolicyPath .\$($Filename.Fullname) $Flag
             }
             else {
                 Get-ErrorMsg FileIOError, $Filename