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

Fix bug with msaad52v1 only admins consent to apps #1043

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions PowerShell/ScubaGear/Rego/AADConfig.rego
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
tkol2022 marked this conversation as resolved.
Show resolved Hide resolved

# 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",
Expand Down
43 changes: 31 additions & 12 deletions PowerShell/ScubaGear/Testing/Unit/Rego/AAD/AADConfig_05_test.rego
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
]
Expand All @@ -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"
}
Expand All @@ -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
Expand Down
27 changes: 25 additions & 2 deletions Testing/Functional/Products/TestPlans/aad.testplan.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion Testing/RunUnitTests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -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
tkol2022 marked this conversation as resolved.
Show resolved Hide resolved
}
else {
Get-ErrorMsg FileIOError, $Filename
Expand Down