diff --git a/changelogs/fragments/130-group-member.yml b/changelogs/fragments/130-group-member.yml new file mode 100644 index 0000000..486f619 --- /dev/null +++ b/changelogs/fragments/130-group-member.yml @@ -0,0 +1,4 @@ +bugfixes: + - >- + microsoft.ad.group - Fix setting group members of Builtin groups of a domain controller - + https://github.com/ansible-collections/microsoft.ad/issues/130 diff --git a/plugins/module_utils/_ADObject.psm1 b/plugins/module_utils/_ADObject.psm1 index 70868c3..cfb8a2e 100644 --- a/plugins/module_utils/_ADObject.psm1 +++ b/plugins/module_utils/_ADObject.psm1 @@ -769,6 +769,7 @@ Function Invoke-AnsibleADObject { StateRequired - Set to 'present' or 'absent' if this needs to be defined for either state DNLookup - Whether each value needs to be looked up to get the DN IsRawAttribute - Whether the attribute is a raw LDAP attribute name and not a parameter name + SupportsReplace - Whether the IsRawAttribute supports setting through -Replace or needs -Add/-Remove New - Called when the option is to be set on the New-AD* cmdlet splat Set - Called when the option is to be set on the Set-AD* cmdlet splat @@ -998,6 +999,11 @@ Function Invoke-AnsibleADObject { $option.type = 'raw' } + if (-not $propInfo.PSObject.Properties.Match('SupportsReplace')) { + $propInfo.PSObject.Properties.Add( + [System.Management.Automation.PSNoteProperty]::new('SupportsReplace', $true)) + } + $spec.options[$ansibleOption] = $option if ($propInfo.Attribute) { @@ -1362,10 +1368,25 @@ Function Invoke-AnsibleADObject { if ($res.Changed) { if ($propInfo.IsRawAttribute) { if ($newValue) { - if (-not $setParams.ContainsKey('Replace')) { - $setParams['Replace'] = @{} + $toSet = @( + if ($propInfo.SupportsReplace) { + @{ Prop = 'Replace'; Value = $newValue } + } + else { + @{ Prop = 'Add'; Value = $res.ToAdd } + @{ Prop = 'Remove'; Value = $res.ToRemove } + } + ) + + foreach ($setInfo in $toSet) { + if (-not $setInfo.Value) { + continue + } + if (-not $setParams.ContainsKey($setInfo.Prop)) { + $setParams[$setInfo.Prop] = @{} + } + $setParams[$setInfo.Prop][$propInfo.Attribute] = $setInfo.Value } - $setParams['Replace'][$propInfo.Attribute] = $newValue } else { if (-not $setParams.ContainsKey('Clear')) { diff --git a/plugins/modules/group.ps1 b/plugins/modules/group.ps1 index ed4a521..dffc57d 100644 --- a/plugins/modules/group.ps1 +++ b/plugins/modules/group.ps1 @@ -34,6 +34,11 @@ $setParams = @{ Attribute = 'member' DNLookup = $true IsRawAttribute = $true + # If the group is part of the CN=Builtin groups, it cannot + # use -Replace. This ensures it always uses -Add/-Remove when + # setting a changed value to handle this. + # https://github.com/ansible-collections/microsoft.ad/issues/130 + SupportsReplace = $false } [PSCustomObject]@{ Name = 'sam_account_name' diff --git a/tests/integration/targets/group/tasks/tests.yml b/tests/integration/targets/group/tasks/tests.yml index 958398a..c9cb7d8 100644 --- a/tests/integration/targets/group/tasks/tests.yml +++ b/tests/integration/targets/group/tasks/tests.yml @@ -67,6 +67,50 @@ that: - not create_group_again is changed +- name: add member to builtin group + group: + name: Administrators + path: CN=Builtin,{{ setup_domain_info.output[0].defaultNamingContext }} + members: + add: + - '{{ create_group.distinguished_name }}' + register: add_member_to_builtin + +- name: get result of add member to builtin group + object_info: + identity: '{{ add_member_to_builtin.object_guid }}' + properties: + - member + register: add_member_to_builtin_actual + +- name: assert add member to builtin group + assert: + that: + - add_member_to_builtin is changed + - create_group.distinguished_name in add_member_to_builtin_actual.objects[0].member + +- name: remove member on builtin group + group: + name: Administrators + path: CN=Builtin,{{ setup_domain_info.output[0].defaultNamingContext }} + members: + remove: + - '{{ create_group.distinguished_name }}' + register: remove_member_from_builtin + +- name: get result of add member to builtin group + object_info: + identity: '{{ add_member_to_builtin.object_guid }}' + properties: + - member + register: remove_member_from_builtin_actual + +- name: assert remove member to builtin group + assert: + that: + - remove_member_from_builtin is changed + - create_group.distinguished_name not in remove_member_from_builtin_actual.objects[0].member + - name: create ou to store group members ou: name: MyOU