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

AADGroup - fixes issues with code that is never executed and incorrect removal of assigned licenses #5129

Merged
merged 12 commits into from
Oct 2, 2024
Merged
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
# Change log for Microsoft365DSC

# UNRELEASED
* AADGroup
* Fixes issue with code that is never executed
FIXES [#5001](https://github.com/microsoft/Microsoft365DSC/issues/5001)
* Fixes issue with incorrect removal of assigned license(s)
FIXES [#5128](https://github.com/microsoft/Microsoft365DSC/issues/5128)


* IntuneAppCategory
* Initial release.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -591,10 +591,6 @@ function Set-TargetResource
-Source $MyInvocation.MyCommand.ModuleName
}
}
if ($assignedLicensesGUIDs.Length -gt 0)
{
Set-MgGroupLicense -GroupId $currentGroup.Id -AddLicenses $licensesToAdd -RemoveLicenses @()
}
}
if ($Ensure -eq 'Present')
{
Expand All @@ -620,7 +616,7 @@ function Set-TargetResource
Update-MgGroup @currentParameters | Out-Null
}

if (($licensesToAdd.Length -gt 0 -or $licensesToRemove.Length -gt 0) -and $AssignedLicenses -ne $null)
if (($licensesToAdd.Length -gt 0 -or $licensesToRemove.Length -gt 0) -and $PSBoundParameters.ContainsKey('AssignedLicenses'))
{
try
{
Expand Down Expand Up @@ -1088,9 +1084,9 @@ function Test-TargetResource
try
{
if ($null -ne $CurrentValues.AssignedLicenses -and $CurrentValues.AssignedLicenses.Length -gt 0 -and `
$null -eq $AssignedLicenses)
($PSBoundParameters.ContainsKey('AssignedLicenses') -and $null -eq $AssignedLicenses))
{
Write-Verbose -Message "The group currently has licenses assigned but it shouldn't"
Write-verbose -Message "The group {$DisplayName} currently has licenses assigned but it shouldn't"
Write-Verbose -Message "Test-TargetResource returned $false"
$EventMessage = "Assigned Licenses for Azure AD Group {$DisplayName} were not in the desired state.`r`nThe group should not have any licenses assigned but instead contained {$($CurrentValues.AssignedLicenses.SkuId)}"
Add-M365DSCEvent -Message $EventMessage -EntryType 'Warning' `
Expand All @@ -1101,7 +1097,7 @@ function Test-TargetResource
elseif ($null -eq $CurrentValues.AssignedLicenses -and $null -ne $AssignedLicenses -and `
$AssignedLicenses.Length -gt 0)
{
Write-Verbose -Message "The group currently doesn't have licenses assigned but it should"
Write-verbose -Message "The group {$DisplayName} currently doesn't have licenses assigned but it should"
Write-Verbose -Message "Test-TargetResource returned $false"
$EventMessage = "Assigned Licenses for Azure AD Group {$DisplayName} were not in the desired state.`r`nThe group doesn't not have any licenses assigned but should have {$($CurrentValues.AssignedLicenses.SkuId)}"
Add-M365DSCEvent -Message $EventMessage -EntryType 'Warning' `
Expand All @@ -1111,11 +1107,11 @@ function Test-TargetResource
}
elseif ($CurrentValues.AssignedLicenses.Length -gt 0 -and $AssignedLicenses.Length -gt 0)
{
Write-Verbose -Message "Current assigned licenses and desired assigned licenses are not null"
Write-verbose -Message "Current assigned licenses and desired assigned licenses for group {$DisplayName} are not null"
$licensesDiff = Compare-Object -ReferenceObject ($CurrentValues.AssignedLicenses.SkuId) -DifferenceObject ($AssignedLicenses.SkuId)
if ($null -ne $licensesDiff)
{
Write-Verbose -Message "AssignedLicenses differ: $($licensesDiff | Out-String)"
Write-verbose -Message "AssignedLicenses differ for group {$DisplayName}: $($licensesDiff | Out-String)"
Write-Verbose -Message "Test-TargetResource returned $false"
$EventMessage = "Assigned Licenses for Azure AD Group {$DisplayName} were not in the desired state.`r`nThey should contain {$($AssignedLicenses.SkuId)} but instead contained {$($CurrentValues.AssignedLicenses.SkuId)}"
Add-M365DSCEvent -Message $EventMessage -EntryType 'Warning' `
Expand All @@ -1125,35 +1121,66 @@ function Test-TargetResource
}
else
{
Write-Verbose -Message 'AssignedLicenses for Azure AD Group are the same'
Write-verbose -Message "AssignedLicenses for Azure AD Group {$DisplayName} are the same"
}

# Disabled Plans
$licensesDiff = Compare-Object -ReferenceObject ($CurrentValues.AssignedLicenses.DisabledPlans) -DifferenceObject ($AssignedLicenses.DisabledPlans)
if ($null -ne $licensesDiff)
#Compare DisabledPlans for each SkuId - all SkuId's are processed regardless of result
$result = $true
foreach ($assignedLicense in $AssignedLicenses)
{
Write-Verbose -Message "DisabledPlans differ: $($licensesDiff | Out-String)"
Write-Verbose -Message "Test-TargetResource returned $false"
$EventMessage = "Disabled Plans for Azure AD Group Licenses {$DisplayName} were not in the desired state.`r`n" + `
"They should contain {$($AssignedLicenses.DisabledPlans)} but instead contained {$($CurrentValues.AssignedLicenses.DisabledPlans)}"
Add-M365DSCEvent -Message $EventMessage -EntryType 'Warning' `
-EventID 1 -Source $($MyInvocation.MyCommand.Source)

return $false
write-verbose "Compare DisabledPlans for SkuId $($assignedLicense.SkuId) in group {$DisplayName}"
$currentLicense = $CurrentValues.AssignedLicenses | Where-Object -FilterScript {$_.SkuId -eq $assignedLicense.SkuId}
if ($assignedLicense.DisabledPlans.Count -ne 0 -or $currentLicense.DisabledPlans.Count -ne 0)
{
try {
$licensesDiff = Compare-Object -ReferenceObject $assignedLicense.DisabledPlans -DifferenceObject $currentLicense.DisabledPlans
if ($null -ne $licensesDiff)
{
Write-verbose -Message "DisabledPlans for SkuId $($assignedLicense.SkuId) differ: $($licensesDiff | Out-String)"
Write-Verbose -Message "Test-TargetResource returned $false"
$EventMessage = "Disabled Plans for Azure AD Group Licenses {$DisplayName} SkuId $($assignedLicense.SkuId) were not in the desired state.`r`n" + `
"They should contain {$($assignedLicense.DisabledPlans -join ',')} but instead contained {$($currentLicense.DisabledPlans -join ',')}"
Add-M365DSCEvent -Message $EventMessage -EntryType 'Warning' `
-EventID 1 -Source $($MyInvocation.MyCommand.Source)

$result = $false
}
else
{
Write-verbose -Message "DisabledPlans for SkuId $($assignedLicense.SkuId) are the same"
}
}
catch
{
Write-verbose -Message "Test-TargetResource returned `$false (DisabledPlans: $($_.Exception.Message))"
$result = $false
}
}
}
else
if ($true -ne $result)
{
Write-Verbose -Message 'DisabledPlans for Azure AD Group Licensing are the same'
return $result
}
}
elseif ($PSBoundParameters.ContainsKey('AssignedLicenses'))
{
Write-verbose -Message "The group {$DisplayName} currently has licenses assigned but it shouldn't have"
Write-Verbose -Message "Test-TargetResource returned $false"
$EventMessage = "Assigned Licenses for Azure AD Group {$DisplayName} were not in the desired state.`r`nThe group has licenses assigned but shouldn't have {$($CurrentValues.AssignedLicenses.SkuId)}"
Add-M365DSCEvent -Message $EventMessage -EntryType 'Warning' `
-EventID 1 -Source $($MyInvocation.MyCommand.Source)

return $false
}
else
{
Write-Verbose -Message "Both the current and desired assigned licenses lists are empty."
Write-verbose -Message "Both the current and desired assigned licenses lists for group {$DisplayName} are empty or not specified."
}
}
catch
{
Write-Verbose -Message "Error evaluating the AssignedLicenses: $_"
Write-Verbose -Message "Error evaluating the AssignedLicenses for group {$DisplayName}: $_"
Write-Verbose -Message "Test-TargetResource returned $false"
return $false
}
Expand Down
43 changes: 43 additions & 0 deletions Modules/Microsoft365DSC/Examples/Resources/AADGroup/4-License.ps1
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
<#
This example is used to test new resources and showcase the usage of new resources being worked on.
It is not meant to use as a production baseline.
#>

Configuration Example
{
param(
[Parameter()]
[System.String]
$ApplicationId,

[Parameter()]
[System.String]
$TenantId,

[Parameter()]
[System.String]
$CertificateThumbprint
)
Import-DscResource -ModuleName Microsoft365DSC
node localhost
{
AADGroup 'MyGroups'
{
DisplayName = "DSCGroup"
Description = "Microsoft DSC Group with assigned license" # Updated Property
SecurityEnabled = $True
MailEnabled = $False
MailNickname = "M365DSC"
AssignedLicenses = @(
MSFT_AADGroupLicense -Property @{
SkuId = 'AAD_PREMIUM_P2'
}
)

Ensure = "Present"
ApplicationId = $ApplicationId
TenantId = $TenantId
CertificateThumbprint = $CertificateThumbprint
}
}
}
Loading
Loading