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.

Sign up for GitHub

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

[BUG] Fix duplicate policy creation bug & dynamic group compliance #5536

Open
wants to merge 10 commits into
base: Dev
Choose a base branch
from
Open

[BUG] Fix duplicate policy creation bug & dynamic group compliance #5536

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
cdcbbd5
Fix policy creation bug
Dec 11, 2024
e9b867e
Fix dynamic group compliance check & added DOCS
Dec 11, 2024
ce951f9
Merge branch 'Dev' into Dev
ykuijs Dec 11, 2024
67dd4bb
Update Modules/Microsoft365DSC/DSCResources/MSFT_IntuneAccountProtect…
AlfredSchreuder Dec 11, 2024
d21970e
Update MSFT_IntuneSettingCatalogASRRulesPolicyWindows10.psm1
AlfredSchreuder Dec 11, 2024
e91cf86
Update MSFT_IntuneAccountProtectionLocalAdministratorPasswordSolution…
AlfredSchreuder Dec 11, 2024
075a188
Update MSFT_IntuneAccountProtectionPolicyWindows10.psm1
AlfredSchreuder Dec 11, 2024
8cfff93
Update MSFT_IntuneAntivirusPolicyLinux.psm1
AlfredSchreuder Dec 11, 2024
6d8a9be
Capital I
Dec 11, 2024
d2649e5
Update verbose messages
Dec 12, 2024
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
29 changes: 29 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,38 @@
* AADFeatureRolloutPolicy
* Fixed policy retrieval
FIXES [#5521](https://github.com/microsoft/Microsoft365DSC/issues/5521)
* AADGroup
* Only get Members & GroupAsMembers when a static group is defined.
* IntuneAccountProtectionLocalAdministratorPasswordSolutionPolicy
* Fixed creation of policy while it was found by name, now it updates existing policies correctly.
* IntuneAccountProtectionPolicyWindows10
* Fixed creation of policy while it was found by name, now it updates existing policies correctly.
* IntuneAntivirusPolicyLinux
* Fixed creation of policy while it was found by name, now it updates existing policies correctly.
* IntuneAntivirusPolicyMacOS
* Fixed creation of policy while it was found by name, now it updates existing policies correctly.
* IntuneAntivirusPolicyWindows10SettingCatalog
* Fixed creation of policy while it was found by name, now it updates existing policies correctly.
* IntuneAppAndBrowserIsolationPolicyWindows10
* Fixed creation of policy while it was found by name, now it updates existing policies correctly.
* IntuneDeviceControlPolicyWindows10
* Fixed creation of policy while it was found by name, now it updates existing policies correctly.
* IntuneDiskEncryptionMacOS
* Fixed creation of policy while it was found by name, now it updates existing policies correctly.
* IntuneDiskEncryptionWindows10
* Fixed creation of policy while it was found by name, now it updates existing policies correctly.
* IntuneEndpointDetectionAndResponsePolicyLinux
* Fixed creation of policy while it was found by name, now it updates existing policies correctly.
* IntuneEndpointDetectionAndResponsePolicyMacOS
* Fixed creation of policy while it was found by name, now it updates existing policies correctly.
* IntuneEndpointDetectionAndResponsePolicyWindows10
* Fixed creation of policy while it was found by name, now it updates existing policies correctly.
* IntuneSettingCatalogASRRulesPolicyWindows10
* Fixed creation of policy while it was found by name, now it updates existing policies correctly.
* IntuneDeviceManagementAndroidDeviceOwnerEnrollmentProfile
* Fixing issue with the way the QrCodeImage property was exported and handled.
* IntuneFirewallPolicyWindows10
* Fixed creation of policy while it was found by name, now it updates existing policies correctly.
* Fix export of properties that appear multiple times in subsections.
* M365DSCDRGUtil
* Improve settings catalog handling for nested objects.
Expand Down
9 changes: 5 additions & 4 deletions Modules/Microsoft365DSC/DSCResources/MSFT_AADGroup/MSFT_AADGroup.psm1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,7 @@ function Get-TargetResource
}

$MembersValues = $null
$result = @{}
if ($Group.MembershipRuleProcessingState -ne 'On')
{
# Members
Expand All @@ -237,6 +238,8 @@ function Get-TargetResource
$GroupAsMembersValues += $member.AdditionalProperties.displayName
}
}
$result.Add('Members', $MembersValues)
$result.Add('GroupAsMembers', $GroupAsMembersValues)
}

# MemberOf
Expand Down Expand Up @@ -273,15 +276,12 @@ function Get-TargetResource
if ($assignedLicensesRequest.value.Length -gt 0)
{
$assignedLicensesValues = Get-M365DSCAzureADGroupLicenses -AssignedLicenses $assignedLicensesRequest.value

}

$result = @{
$policySettings = @{
DisplayName = $Group.DisplayName
Id = $Group.Id
Owners = $OwnersValues
Members = $MembersValues
GroupAsMembers = $GroupAsMembersValues
MemberOf = $MemberOfValues
Description = $Group.Description
GroupTypes = [System.String[]]$Group.GroupTypes
Expand All @@ -303,6 +303,7 @@ function Get-TargetResource
Managedidentity = $ManagedIdentity.IsPresent
AccessTokens = $AccessTokens
}
$result += $policySettings

return $result
}
Expand Down
49 changes: 31 additions & 18 deletions ...dSolutionPolicy/MSFT_IntuneAccountProtectionLocalAdministratorPasswordSolutionPolicy.psm1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,34 +129,47 @@ function Get-TargetResource
Add-M365DSCTelemetryEvent -Data $data
#endregion

$nullResult = $PSBoundParameters
$nullResult.Ensure = 'Absent'

#Retrieve policy general settings
$templateReferenceId = 'adc46e5a-f4aa-4ff6-aeff-4f27bc525796_1'

# Retrieve policy general settings
$policy = $null
$policy = Get-MgBetaDeviceManagementConfigurationPolicy -DeviceManagementConfigurationPolicyId $Identity -ErrorAction SilentlyContinue

if ($null -eq $policy)
if ($PSBoundParameters.ContainsKey('Identity'))
{
Write-Verbose -Message "No Account Protection LAPS Policy with Id {$Identity} was found"

if (-not [System.String]::IsNullOrEmpty($DisplayName))
Write-Verbose -Message 'PolicyID was specified'
try
{
$policy = Get-MgBetaDeviceManagementConfigurationPolicy `
-Filter "Name eq '$DisplayName' and templateReference/TemplateId eq '$templateReferenceId'" `
-ErrorAction SilentlyContinue
$policy = Get-MgBetaDeviceManagementConfigurationPolicy -DeviceManagementConfigurationPolicyId $Identity -ErrorAction Stop
}
catch
{
Write-Verbose -Message "Couldn't find existing policy by ID {$Identity}"
$policy = Get-MgBetaDeviceManagementConfigurationPolicy -All -Filter "Name eq '$DisplayName' and templateReference/TemplateId eq '$templateReferenceId'"
if ($policy.Length -gt 1)
{
throw "Duplicate Account Protection LAPS Policyy named $DisplayName exist in tenant"
AlfredSchreuder marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
else
{
Write-Verbose -Message 'Id was NOT specified'
## Can retreive multiple Intune Policies since displayname is not unique
$policy = Get-MgBetaDeviceManagementConfigurationPolicy -All -Filter "Name eq '$DisplayName' and templateReference/TemplateId eq '$templateReferenceId'"
if ($policy.Length -gt 1)
{
throw "Duplicate Account Protection LAPS Policy named $DisplayName exist in tenant"
}
}

if ($null -eq $policy)
if ([String]::IsNullOrEmpty($policy.id))
{
Write-Verbose -Message "No Account Protection LAPS Policy with Name {$DisplayName} was found"
return $nullResult
Write-Verbose -Message "No existing Policy with name {$DisplayName} were found"
$currentValues = $PSBoundParameters
$currentValues.Ensure = 'Absent'
return $currentValues
}

$Identity = $policy.Id
Write-Verbose "Found Account Protection LAPS Policy with Id {$Identity} and Name {$($policy.Name)}"
Write-Verbose -Message "An Account Protection LAPS Policy with Id {$Identity} and Name {$DisplayName} was found"

[array]$settings = Get-MgBetaDeviceManagementConfigurationPolicySetting `
-DeviceManagementConfigurationPolicyId $Identity `
Expand Down
49 changes: 30 additions & 19 deletions ...T_IntuneAccountProtectionPolicyWindows10/MSFT_IntuneAccountProtectionPolicyWindows10.psm1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,31 +85,42 @@ function Get-TargetResource
Add-M365DSCTelemetryEvent -Data $data
#endregion

$nullResult = $PSBoundParameters
$nullResult.Ensure = 'Absent'

$getValue = $null
#region resource generator code
$getValue = Get-MgBetaDeviceManagementConfigurationPolicy -DeviceManagementConfigurationPolicyId $Id -ErrorAction SilentlyContinue

if ($null -eq $getValue)
if ($PSBoundParameters.ContainsKey('Id'))
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this even necessary? Now there is two times the same block:

$getValue = Get-MgBetaDeviceManagementConfigurationPolicy -All -Filter "Name eq '$DisplayName'"
if ($getValue.Length -gt 1)
{
    throw "Duplicate Intune Account Protection Policy for Windows10 named $DisplayName exist in tenant"
}

Copy link
Author

Choose a reason for hiding this comment

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

As cases described in the below comment, I believe it is usefull because it are 2 different cases. The code used to result in described cases is integrated from the MSFT_AADConditionalAccessPolicy which had the behaviour that this policies lacked.

Copy link
Contributor

Choose a reason for hiding this comment

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

I still believe that only the if check should be added to the modules and not really other changes. I'll check some things and report back.

Copy link
Author

Choose a reason for hiding this comment

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

Actually this block that is present 2 times handles the cases where multiple policies are present with the same Name. This was not present in the old code. Maybe the If statement was enough to handle the other cases, I don't know, I copied/used/integrated working code to make these policies show the expected behaviour.

{
Write-Verbose -Message "Could not find an Intune Account Protection Policy for Windows10 with Id {$Id}"
AlfredSchreuder marked this conversation as resolved.
Show resolved Hide resolved

if (-not [System.String]::IsNullOrEmpty($DisplayName))
Write-Verbose -Message 'PolicyID was specified'
try
{
$getValue = Get-MgBetaDeviceManagementConfigurationPolicy -DeviceManagementConfigurationPolicyId $Id -ErrorAction Stop
}
catch
{
Write-Verbose -Message "Couldn't find existing policy by ID {$Id}"
$getValue = Get-MgBetaDeviceManagementConfigurationPolicy -All -Filter "Name eq '$DisplayName'"
if ($getValue.Length -gt 1)
{
throw "Duplicate Intune Account Protection Policy for Windows10 named $DisplayName exist in tenant"
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm also not fond of switching the ErrorAction to stop and wrapping it in a try/catch block. Try/catch adds massive underlying logic and degrades performance (even though it might not impact us that hard), whereas the previous SilentlyContinue action with a $null result was perfectly enough to indicate that a policy was not found.

Copy link
Author

Choose a reason for hiding this comment

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

The previous code did not check compliance of the existing policy found by Name when the Id was not specified. It always resulted in false and created a new policy. I felt like the SilentlyContinue ignored alot of Verbose logging and did not tell me what went right and what went wrong. After implementing this code I allways got the right Verbose logging to tell me what the script did find or did not find.

Copy link
Author

Choose a reason for hiding this comment

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

Example of old code with no Id specified:
image

Example of old code with wrong Id specified:
image

These examples were applicable for all the files changed. And with this fix it also found the policies in both cases and check compliancy or updated the existing policies.

Copy link
Contributor

Choose a reason for hiding this comment

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

This seems more like an internal bug because the log output tells us that there was a policy found but it still created a new policy, which is incorrect. This should not be the case.

Copy link
Author

Choose a reason for hiding this comment

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

It should not, but it does, even when compliant. I tested this multiple times on all different policies. After the changes compliant policies actually returned True or when imcompliant changed the existing policies found by name. Feel free to test it yourself.

}
else
{
Write-Verbose -Message 'Id was NOT specified'
## Can retreive multiple Intune Policies since displayname is not unique
$getValue = Get-MgBetaDeviceManagementConfigurationPolicy -All -Filter "Name eq '$DisplayName'"
if ($getValue.Length -gt 1)
{
$getValue = Get-MgBetaDeviceManagementConfigurationPolicy `
-All `
-Filter "Name eq '$DisplayName'" `
-ErrorAction SilentlyContinue
throw "Duplicate Intune Account Protection Policy for Windows10 named $DisplayName exist in tenant"
}
}
#endregion
if ($null -eq $getValue)

if ([String]::IsNullOrEmpty($getValue.id))
AlfredSchreuder marked this conversation as resolved.
Show resolved Hide resolved
{
Write-Verbose -Message "Could not find an Intune Account Protection Policy for Windows10 with Name {$DisplayName}."
return $nullResult
Write-Verbose -Message "No existing Policy with name {$DisplayName} were found"
$currentValues = $PSBoundParameters
$currentValues.Ensure = 'Absent'
return $currentValues
}

$Id = $getValue.Id
Write-Verbose -Message "An Intune Account Protection Policy for Windows10 with Id {$Id} and Name {$DisplayName} was found"

Expand Down
49 changes: 30 additions & 19 deletions ...t365DSC/DSCResources/MSFT_IntuneAntivirusPolicyLinux/MSFT_IntuneAntivirusPolicyLinux.psm1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,31 +192,42 @@ function Get-TargetResource
Add-M365DSCTelemetryEvent -Data $data
#endregion

$nullResult = $PSBoundParameters
$nullResult.Ensure = 'Absent'

$getValue = $null
#region resource generator code
$getValue = Get-MgBetaDeviceManagementConfigurationPolicy -DeviceManagementConfigurationPolicyId $Id -ErrorAction SilentlyContinue

if ($null -eq $getValue)
if ($PSBoundParameters.ContainsKey('Id'))
{
Write-Verbose -Message "Could not find an Intune Antivirus Policy for Linux with Id {$Id}"

if (-not [System.String]::IsNullOrEmpty($DisplayName))
Write-Verbose -Message 'PolicyID was specified'
try
{
$getValue = Get-MgBetaDeviceManagementConfigurationPolicy -DeviceManagementConfigurationPolicyId $Id -ErrorAction Stop
}
catch
{
Write-Verbose -Message "Couldn't find existing policy by ID {$Id}"
$getValue = Get-MgBetaDeviceManagementConfigurationPolicy -All -Filter "Name eq '$DisplayName'"
if ($getValue.Length -gt 1)
{
throw "Duplicate Intune Antivirus Policy for Linux named $DisplayName exist in tenant"
}
}
}
else
{
Write-Verbose -Message 'Id was NOT specified'
## Can retreive multiple Intune Policies since displayname is not unique
$getValue = Get-MgBetaDeviceManagementConfigurationPolicy -All -Filter "Name eq '$DisplayName'"
if ($getValue.Length -gt 1)
{
$getValue = Get-MgBetaDeviceManagementConfigurationPolicy `
-All `
-Filter "Name eq '$DisplayName'" `
-ErrorAction SilentlyContinue
throw "Duplicate Intune Antivirus Policy for Linux named $DisplayName exist in tenant"
}
}
#endregion
if ($null -eq $getValue)

if ([String]::IsNullOrEmpty($getValue.id))
{
Write-Verbose -Message "Could not find an Intune Antivirus Policy for Linux with Name {$DisplayName}."
return $nullResult
Write-Verbose -Message "No existing Policy with name {$DisplayName} were found"
$currentValues = $PSBoundParameters
$currentValues.Ensure = 'Absent'
return $currentValues
}

$Id = $getValue.Id
Write-Verbose -Message "An Intune Antivirus Policy for Linux with Id {$Id} and Name {$DisplayName} was found"

Expand Down
49 changes: 30 additions & 19 deletions ...t365DSC/DSCResources/MSFT_IntuneAntivirusPolicyMacOS/MSFT_IntuneAntivirusPolicyMacOS.psm1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,31 +195,42 @@ function Get-TargetResource
Add-M365DSCTelemetryEvent -Data $data
#endregion

$nullResult = $PSBoundParameters
$nullResult.Ensure = 'Absent'

$getValue = $null
#region resource generator code
$getValue = Get-MgBetaDeviceManagementConfigurationPolicy -DeviceManagementConfigurationPolicyId $Id -ErrorAction SilentlyContinue

if ($null -eq $getValue)
if ($PSBoundParameters.ContainsKey('Id'))
{
Write-Verbose -Message "Could not find an Intune Antivirus Policy for macOS with Id {$Id}"

if (-not [System.String]::IsNullOrEmpty($DisplayName))
Write-Verbose -Message 'PolicyID was specified'
try
{
$getValue = Get-MgBetaDeviceManagementConfigurationPolicy -DeviceManagementConfigurationPolicyId $Id -ErrorAction Stop
}
catch
{
Write-Verbose -Message "Couldn't find existing policy by ID {$Id}"
$getValue = Get-MgBetaDeviceManagementConfigurationPolicy -All -Filter "Name eq '$DisplayName'"
if ($getValue.Length -gt 1)
{
throw "Duplicate Intune Antivirus Policy for macOS named $DisplayName exist in tenant"
}
}
}
else
{
Write-Verbose -Message 'Id was NOT specified'
## Can retreive multiple Intune Policies since displayname is not unique
$getValue = Get-MgBetaDeviceManagementConfigurationPolicy -All -Filter "Name eq '$DisplayName'"
if ($getValue.Length -gt 1)
{
$getValue = Get-MgBetaDeviceManagementConfigurationPolicy `
-Filter "Name eq '$DisplayName'" `
-All `
-ErrorAction SilentlyContinue
throw "Duplicate Intune Antivirus Policy for macOS named $DisplayName exist in tenant"
}
}
#endregion
if ($null -eq $getValue)

if ([String]::IsNullOrEmpty($getValue.id))
{
Write-Verbose -Message "Could not find an Intune Antivirus Policy for macOS with Name {$DisplayName}."
return $nullResult
Write-Verbose -Message "No existing Policy with name {$DisplayName} were found"
$currentValues = $PSBoundParameters
$currentValues.Ensure = 'Absent'
return $currentValues
}

$Id = $getValue.Id
Write-Verbose -Message "An Intune Antivirus Policy for macOS with Id {$Id} and Name {$DisplayName} was found"

Expand Down
49 changes: 29 additions & 20 deletions ...virusPolicyWindows10SettingCatalog/MSFT_IntuneAntivirusPolicyWindows10SettingCatalog.psm1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -439,37 +439,46 @@ function Get-TargetResource
Add-M365DSCTelemetryEvent -Data $data
#endregion

$nullResult = $PSBoundParameters
$nullResult.Ensure = 'Absent'

$templateReferences = 'd948ff9b-99cb-4ee0-8012-1fbc09685377_1', 'e3f74c5a-a6de-411d-aef6-eb15628f3a0a_1', '45fea5e9-280d-4da1-9792-fb5736da0ca9_1', '804339ad-1553-4478-a742-138fb5807418_1'

#Retrieve policy general settings
$policy = Get-MgBetaDeviceManagementConfigurationPolicy -DeviceManagementConfigurationPolicyId $Identity -ErrorAction SilentlyContinue

if ($null -eq $policy)
if ($PSBoundParameters.ContainsKey('Identity'))
{
Write-Verbose -Message "Could not find an Intune Antivirus Policy for Windows10 Setting Catalog with Id {$Identity}"

if (-not [System.String]::IsNullOrEmpty($DisplayName))
Write-Verbose -Message 'PolicyID was specified'
try
{
$policy = Get-MgBetaDeviceManagementConfigurationPolicy `
-All `
-Filter "Name eq '$DisplayName'" `
-ErrorAction SilentlyContinue | Where-Object `
-FilterScript {
$_.TemplateReference.TemplateId -in $templateReferences
$policy = Get-MgBetaDeviceManagementConfigurationPolicy -DeviceManagementConfigurationPolicyId $Identity -ErrorAction Stop
}
catch
{
Write-Verbose -Message "Couldn't find existing policy by ID {$Identity}"
$policy = Get-MgBetaDeviceManagementConfigurationPolicy -All -Filter "Name eq '$DisplayName'" | Where-Object -FilterScript {$_.TemplateReference.TemplateId -in $templateReferences}
if ($policy.Length -gt 1)
{
throw "Duplicate Intune Antivirus Policy for Windows10 Setting Catalog named $DisplayName exist in tenant"
}
}
}
else
{
Write-Verbose -Message 'Id was NOT specified'
## Can retreive multiple Intune Policies since displayname is not unique
$policy = Get-MgBetaDeviceManagementConfigurationPolicy -All -Filter "Name eq '$DisplayName'"
if ($policy.Length -gt 1)
{
throw "Duplicate Intune Antivirus Policy for Windows10 Setting Catalog named $DisplayName exist in tenant"
}
}

if ($null -eq $policy)
if ([String]::IsNullOrEmpty($policy.Id))
{
Write-Verbose -Message "Could not find an Intune Antivirus Policy for Windows10 Setting Catalog with Name {$DisplayName}"
return $nullResult
Write-Verbose -Message "No existing Policy with name {$DisplayName} were found"
$currentValues = $PSBoundParameters
$currentValues.Ensure = 'Absent'
return $currentValues
}

$Identity = $policy.Id
Write-Verbose -Message "An Intune Antivirus Policy for Windows10 Setting Catalog with Id {$Identity} and Name {$DisplayName} was found."
Write-Verbose -Message "An Intune Antivirus Policy for Windows10 Setting Catalog with Id {$Identity} and Name {$DisplayName} was found"

#Retrieve policy specific settings
[array]$settings = Get-MgBetaDeviceManagementConfigurationPolicySetting `
Expand Down
Loading