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

ADGroup: Throws incorrect error message when MembersToInclude syntax is wrong #166

Closed
johlju opened this issue Dec 8, 2017 · 7 comments · Fixed by #497
Closed

ADGroup: Throws incorrect error message when MembersToInclude syntax is wrong #166

johlju opened this issue Dec 8, 2017 · 7 comments · Fixed by #497
Assignees
Labels
bug The issue is a bug.

Comments

@johlju
Copy link
Member

johlju commented Dec 8, 2017

When adding a member to a group using the SamAccountName.

xADGroup 'AddClusterNetworkObjectToActiveDirectoryCreateClusterVirtualComputerObjects'
{
    GroupName        = 'Active Directory Create Cluster Virtual Computer Objects'
    Path             = 'OU=Groups,OU=Europe,OU=Regions,DC=companylab,DC=se'
    MembersToInclude = 'TESTCLU01'

    Credential       = $DomainAdministratorCredential

    DependsOn        = '[xADComputer]TESTCLU01'
}

It throws an error indicating it tried to create the group again.

VERBOSE: [sql01]: LCM:  [ Start  Test     ]  [[xADGroup]AddClusterNetowrkObjectToActiveDirectoryCreateClusterVirtualComputerObjects::[SqlAlwaysOnCluster]-::[SqlAlwaysOnCluster]-]
VERBOSE: [sql01]:                            [[xADGroup]AddClusterNetowrkObjectToActiveDirectoryCreateClusterVirtualComputerObjects::[SqlAlwaysOnCluster]-::[SqlAlwaysOnCluster]-] Retrieving group membership based on 'SamAccountName' property.
VERBOSE: [sql01]:                            [[xADGroup]AddClusterNetowrkObjectToActiveDirectoryCreateClusterVirtualComputerObjects::[SqlAlwaysOnCluster]-::[SqlAlwaysOnCluster]-] Checking for 'Included' members.
VERBOSE: [sql01]:                            [[xADGroup]AddClusterNetowrkObjectToActiveDirectoryCreateClusterVirtualComputerObjects::[SqlAlwaysOnCluster]-::[SqlAlwaysOnCluster]-] Member 'testclu01' is not in the desired state.
VERBOSE: [sql01]:                            [[xADGroup]AddClusterNetowrkObjectToActiveDirectoryCreateClusterVirtualComputerObjects::[SqlAlwaysOnCluster]-::[SqlAlwaysOnCluster]-] Group membership is NOT in the desired state.
VERBOSE: [sql01]: LCM:  [ End    Test     ]  [[xADGroup]AddClusterNetowrkObjectToActiveDirectoryCreateClusterVirtualComputerObjects::[SqlAlwaysOnCluster]-::[SqlAlwaysOnCluster]-]  in 0.1400 seconds.
VERBOSE: [sql01]: LCM:  [ Start  Set      ]  [[xADGroup]AddClusterNetowrkObjectToActiveDirectoryCreateClusterVirtualComputerObjects::[SqlAlwaysOnCluster]-::[SqlAlwaysOnCluster]-]
VERBOSE: [sql01]:                            [[xADGroup]AddClusterNetowrkObjectToActiveDirectoryCreateClusterVirtualComputerObjects::[SqlAlwaysOnCluster]-::[SqlAlwaysOnCluster]-] Updating AD Group 'Active Directory Create Cluster Virtual Computer Objects'
VERBOSE: [sql01]:                            [[xADGroup]AddClusterNetowrkObjectToActiveDirectoryCreateClusterVirtualComputerObjects::[SqlAlwaysOnCluster]-::[SqlAlwaysOnCluster]-] Retrieving group membership based on 'SamAccountName' property.
VERBOSE: [sql01]:                            [[xADGroup]AddClusterNetowrkObjectToActiveDirectoryCreateClusterVirtualComputerObjects::[SqlAlwaysOnCluster]-::[SqlAlwaysOnCluster]-] Checking for 'Explicit' members.
VERBOSE: [sql01]:                            [[xADGroup]AddClusterNetowrkObjectToActiveDirectoryCreateClusterVirtualComputerObjects::[SqlAlwaysOnCluster]-::[SqlAlwaysOnCluster]-] Checking for 'Included' members.
VERBOSE: [sql01]:                            [[xADGroup]AddClusterNetowrkObjectToActiveDirectoryCreateClusterVirtualComputerObjects::[SqlAlwaysOnCluster]-::[SqlAlwaysOnCluster]-] Member 'testclu01' is not in the desired state.
VERBOSE: [sql01]:                            [[xADGroup]AddClusterNetowrkObjectToActiveDirectoryCreateClusterVirtualComputerObjects::[SqlAlwaysOnCluster]-::[SqlAlwaysOnCluster]-] Adding '1' member(s) to AD group 'Active Directory Create Cluster Virtual Computer Objects'.
VERBOSE: [sql01]:                            [[xADGroup]AddClusterNetowrkObjectToActiveDirectoryCreateClusterVirtualComputerObjects::[SqlAlwaysOnCluster]-::[SqlAlwaysOnCluster]-] AD Group 'Active Directory Create Cluster Virtual Computer Objects' was not found
VERBOSE: [sql01]:                            [[xADGroup]AddClusterNetowrkObjectToActiveDirectoryCreateClusterVirtualComputerObjects::[SqlAlwaysOnCluster]-::[SqlAlwaysOnCluster]-] Adding AD Group 'Active Directory Create Cluster Virtual Computer Objects'
The specified group already exists
    + CategoryInfo          : NotSpecified: (CN=Active Direc...ompanylab,DC=se:) [], CimException
    + FullyQualifiedErrorId : ActiveDirectoryServer:1318,Microsoft.ActiveDirectory.Management.Commands.NewADGroup
    + PSComputerName        : sql01

If changing the configuration to use the distinguished name for the member to add, it correctly adds the member and does not try to create the group.

xADGroup 'AddClusterNetowrkObjectToActiveDirectoryCreateClusterVirtualComputerObjects'
{
    GroupName        = 'Active Directory Create Cluster Virtual Computer Objects'
    Path             = 'OU=Groups,OU=Europe,OU=Regions,DC=companylab,DC=se'
    MembersToInclude = 'CN=TESTCLU01,OU=Cluster Computer Objects,OU=Europe,OU=Regions,DC=companylab,DC=se'

    Credential       = $DomainAdministratorCredential

    DependsOn        = '[xADComputer]TESTCLU01'
}
VERBOSE: [sql01]: LCM:  [ Start  Test     ]  [[xADGroup]AddClusterNetowrkObjectToActiveDirectoryCreateClusterVirtualComputerObjects::[SqlAlwaysOnCluster]-::[SqlAlwaysOnCluster]-]
VERBOSE: [sql01]:                            [[xADGroup]AddClusterNetowrkObjectToActiveDirectoryCreateClusterVirtualComputerObjects::[SqlAlwaysOnCluster]-::[SqlAlwaysOnCluster]-] Retrieving group membership based on 'SamAccountName' property.
VERBOSE: [sql01]:                            [[xADGroup]AddClusterNetowrkObjectToActiveDirectoryCreateClusterVirtualComputerObjects::[SqlAlwaysOnCluster]-::[SqlAlwaysOnCluster]-] Checking for 'Included' members.
VERBOSE: [sql01]:                            [[xADGroup]AddClusterNetowrkObjectToActiveDirectoryCreateClusterVirtualComputerObjects::[SqlAlwaysOnCluster]-::[SqlAlwaysOnCluster]-] Member 'cn=testclu01,ou=cluster computer objects,ou=europe,ou=regions,dc=companylab,dc=se' is not in the desired state.
VERBOSE: [sql01]:                            [[xADGroup]AddClusterNetowrkObjectToActiveDirectoryCreateClusterVirtualComputerObjects::[SqlAlwaysOnCluster]-::[SqlAlwaysOnCluster]-] Group membership is NOT in the desired state.
VERBOSE: [sql01]: LCM:  [ End    Test     ]  [[xADGroup]AddClusterNetowrkObjectToActiveDirectoryCreateClusterVirtualComputerObjects::[SqlAlwaysOnCluster]-::[SqlAlwaysOnCluster]-]  in 0.2190 seconds.
VERBOSE: [sql01]: LCM:  [ Start  Set      ]  [[xADGroup]AddClusterNetowrkObjectToActiveDirectoryCreateClusterVirtualComputerObjects::[SqlAlwaysOnCluster]-::[SqlAlwaysOnCluster]-]
VERBOSE: [sql01]:                            [[xADGroup]AddClusterNetowrkObjectToActiveDirectoryCreateClusterVirtualComputerObjects::[SqlAlwaysOnCluster]-::[SqlAlwaysOnCluster]-] Updating AD Group 'Active Directory Create Cluster Virtual Computer Objects'
VERBOSE: [sql01]:                            [[xADGroup]AddClusterNetowrkObjectToActiveDirectoryCreateClusterVirtualComputerObjects::[SqlAlwaysOnCluster]-::[SqlAlwaysOnCluster]-] Retrieving group membership based on 'SamAccountName' property.
VERBOSE: [sql01]:                            [[xADGroup]AddClusterNetowrkObjectToActiveDirectoryCreateClusterVirtualComputerObjects::[SqlAlwaysOnCluster]-::[SqlAlwaysOnCluster]-] Checking for 'Explicit' members.
VERBOSE: [sql01]:                            [[xADGroup]AddClusterNetowrkObjectToActiveDirectoryCreateClusterVirtualComputerObjects::[SqlAlwaysOnCluster]-::[SqlAlwaysOnCluster]-] Checking for 'Included' members.
VERBOSE: [sql01]:                            [[xADGroup]AddClusterNetowrkObjectToActiveDirectoryCreateClusterVirtualComputerObjects::[SqlAlwaysOnCluster]-::[SqlAlwaysOnCluster]-] Member 'cn=testclu01,ou=cluster computer objects,ou=europe,ou=regions,dc=companylab,dc=se' is not in the desired state.
VERBOSE: [sql01]:                            [[xADGroup]AddClusterNetowrkObjectToActiveDirectoryCreateClusterVirtualComputerObjects::[SqlAlwaysOnCluster]-::[SqlAlwaysOnCluster]-] Adding '1' member(s) to AD group 'Active Directory Create Cluster Virtual Computer Objects'.
VERBOSE: [sql01]: LCM:  [ End    Set      ]  [[xADGroup]AddClusterNetowrkObjectToActiveDirectoryCreateClusterVirtualComputerObjects::[SqlAlwaysOnCluster]-::[SqlAlwaysOnCluster]-]  in 0.2030 seconds.
  1. I would expect the resource to not try to create the group again if it encounters an error. If possible.
  2. The documentation should be updated to reflect that the distinguished name should be used.
@matt6697
Copy link

Having the same issue when adding computer accounts to "Terminal Server License Servers" built-in group. After a few research, I found the answer on #90 thanks to @iainbrighton .

No more error when adding a $ at the end of the computer account.

The sAMAccountName attribute of a computer object is the NetBIOS name of the computer with a trailing dollar sign, "$", appended. Besides flagging the object as a computer (which has class user), it also helps ensure uniqueness. The sAMAccountName value must be unique in the domain. Note, the Common Name of computer objects (the value of the cn attribute) does not have a trailing "$ https://social.technet.microsoft.com/Forums/windowsserver/en-US/68e20176-60af-42d4-b32a-4d563ff798e9/why-does-a-machine-name-in-the-domain-have-a-dollar-sign-behind-it

$invokeParams = @{
  Name          = 'xADGroup'
  Method        = 'set'
  Property      = @{
    groupname = 'Terminal Server License Servers'
    groupscope = 'DomainLocal'
    path = 'CN=Builtin,DC=domain-test,DC=com'
    ensure = 'present'
    memberstoinclude = @('srv-lic01$')
  }

However, would it be possible to correct the xadgroup code to return the right error (member account not existing, not group creation error message) ?

@johlju
Copy link
Member Author

johlju commented Dec 22, 2017

I also see that in the README.md it says that if MembershipAttribute is not specified it defaults to use SamAccountName. This is not the case since I needed to change to DN in the above example.

MembershipAttribute: Defines the AD object attribute that is used to determine group membership (optional).
Valid values are 'SamAccountName', 'DistinguishedName', 'ObjectGUID' and 'SID'.
If not specified, it defaults to 'SamAccountName'.
You cannot mix multiple attribute types.

@matt6697
Copy link

Have you tried with the following syntax (add a $ at the end of TESTCLU01 if TESTCLU01 is a computer account) ?

xADGroup 'AddClusterNetowrkObjectToActiveDirectoryCreateClusterVirtualComputerObjects'
{
    GroupName        = 'Active Directory Create Cluster Virtual Computer Objects'
    Path             = 'OU=Groups,OU=Europe,OU=Regions,DC=companylab,DC=se'
    MembersToInclude = 'TESTCLU01$'

    Credential       = $DomainAdministratorCredential

    DependsOn        = '[xADComputer]TESTCLU01'
}

@johlju
Copy link
Member Author

johlju commented Dec 23, 2017

@matt6697 My bad. That does work. As you said in your comment above, we should just fix so the resource return a correct error message. It wouldn't hurt to update the README.md that SamAccountName of computer objects should be suffixed with '$'.

@johlju johlju added bug The issue is a bug. help wanted The issue is up for grabs for anyone in the community. labels May 8, 2018
@SteveL-MSFT SteveL-MSFT added this to Help Wanted in powershell/dscresources May 14, 2019
@johlju johlju added this to To do in All issues and PR's May 23, 2019
@devopsjesus
Copy link
Contributor

I've dug into this and found a couple things:

  1. In the case described above, where a member that does not exist is listed as a member in the group resource, the resource will ultimately execute Add-ADCommonGroupMember, line 1334, which then throws an ADIdentityNotFoundException because the member object being added could not be found in the domain. This ADIdentityNotFoundException is then caught by error handling that is intended to create new groups when they are not found. This explains the group creation error message when a member not found exception would be more appropriate. I'm working on implementing a fix...

  2. The calling line in the resource described above (the Add-ADCommonGroupMember function) uses an incorrect parameter name when calling the function (the call refers to 'Parameter' but the function defines a 'Parameters' parameter instead), but the call seems to work regardless. I'm not sure if maybe I'm missing something in the heat of the troubleshooting, but this has struck me as very odd.

@johlju
Copy link
Member Author

johlju commented Jul 11, 2019

Thanks for fixing this @devopsjesus!

In regards to the wrong parameter name, it is not correct that it has the wrong parameter name, it is a typo (can be resolved in either end). It works since parameter names can be written so much as they are unique, e.g Get-ChildItem -Pa c:\temp works since 'Path` is the only parameter starting with 'Pa'.

@devopsjesus devopsjesus added in progress The issue is being actively worked on by someone. and removed help wanted The issue is up for grabs for anyone in the community. labels Jul 11, 2019
@devopsjesus
Copy link
Contributor

devopsjesus commented Jul 11, 2019

ah yeah...parameter name resolution, duh! I'll update the resource with the correct param name!

Oh, and I'm fixing/updating the unit tests as well, along with modifying the stubs.

devopsjesus pushed a commit to devopsjesus/xActiveDirectory that referenced this issue Jul 11, 2019
@johlju johlju changed the title xADGroup: Throws incorrect error message when MembersToInclude syntax is wrong ADGroup: Throws incorrect error message when MembersToInclude syntax is wrong Jul 28, 2019
@johlju johlju moved this from To do to In progress in All issues and PR's Aug 3, 2019
All issues and PR's automation moved this from In progress to Done Sep 2, 2019
johlju added a commit that referenced this issue Sep 2, 2019
- Changes to ActiveDirectoryDsc.Common
  - Update helper function `Add-ADCommonGroupMember` to reduce duplicated
    code, and add an evaluation if `Members` is empty.
  - Updated helper function `Restore-ADCommonObject` to write out a verbose
    message when no object was found in the recycle bin.
  - Updated helper function `Assert-MemberParameters` to not throw an error
    if the parameter `Members` is en empty array.
- Changes to ADGroup
  - Added a read-only property `DistinguishedName`.
  - Refactor the function `Set-TargetResource` to use the function
    `Get-TargetResource` so that `Set-TargetResource` can correctly throw
    an error when something goes wrong (issue #151, issue #166, issue #493).
  - It is now possible to enforce a group with no members by using
    `Members = @()` in a configuration (issue #189).
@johlju johlju removed the in progress The issue is being actively worked on by someone. label Sep 2, 2019
@SteveL-MSFT SteveL-MSFT removed this from Help Wanted in powershell/dscresources Nov 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug The issue is a bug.
Projects
3 participants